feat(pdgroup): migrate to task v3 (#6011)

* feat(pdgroup): migrate to task v3 Signed-off-by: liubo02 <[email protected]> * add more uts and fix comments Signed-off-by: liubo02 <[email protected]> * remove fmt.Print Signed-off-by: liubo02 <[email protected]> --------- Signed-off-by: liubo02 <[email protected]>
pingcap · Dec 27, 2024 · 90a3a2a · 90a3a2a
1 parent 5244f35
commit 90a3a2a
Show file tree

Hide file tree

Showing 36 changed files with 2,290 additions and 499 deletions.
diff --git a/apis/core/v1alpha1/common_types.go b/apis/core/v1alpha1/common_types.go
@@ -288,15 +288,15 @@ type GroupStatus struct {
 	Replicas int32 `json:"replicas"`
 
 	// ReadyReplicas is the number of Instances created for this ComponentGroup with a Ready Condition.
-	ReadyReplicas int32 `json:"readyReplicas,omitempty"`
+	ReadyReplicas int32 `json:"readyReplicas"`
 
 	// CurrentReplicas is the number of Instances created by the Group controller from the Group version
 	// indicated by currentRevision.
-	CurrentReplicas int32 `json:"currentReplicas,omitempty"`
+	CurrentReplicas int32 `json:"currentReplicas"`
 
 	// UpdatedReplicas is the number of Instances created by the Group controller from the Group version
 	// indicated by updateRevision.
-	UpdatedReplicas int32 `json:"updatedReplicas,omitempty"`
+	UpdatedReplicas int32 `json:"updatedReplicas"`
 }
 
 type UpdateStrategy struct {

diff --git a/apis/core/v1alpha1/pd_types.go b/apis/core/v1alpha1/pd_types.go
@@ -230,7 +230,7 @@ func (in *PD) CollisionCount() *int32 {
 }
 
 func (in *PD) IsHealthy() bool {
-	return meta.IsStatusConditionTrue(in.Status.Conditions, PDCondInitialized) && meta.IsStatusConditionTrue(in.Status.Conditions, PDCondHealth) && in.DeletionTimestamp.IsZero()
+	return meta.IsStatusConditionTrue(in.Status.Conditions, PDCondHealth) && in.DeletionTimestamp.IsZero()
 }
 
 func (in *PD) GetClientPort() int32 {

diff --git a/manifests/crd/core.pingcap.com_pdgroups.yaml b/manifests/crd/core.pingcap.com_pdgroups.yaml
@@ -399,7 +399,10 @@ spec:
                   It will be same as the `spec.version` only when all instances are upgraded to the desired version.
                 type: string
             required:
+            - currentReplicas
+            - readyReplicas
             - replicas
+            - updatedReplicas
             type: object
         type: object
     served: true

diff --git a/manifests/crd/core.pingcap.com_tidbgroups.yaml b/manifests/crd/core.pingcap.com_tidbgroups.yaml
@@ -538,7 +538,10 @@ spec:
                   It will be same as the `spec.version` only when all instances are upgraded to the desired version.
                 type: string
             required:
+            - currentReplicas
+            - readyReplicas
             - replicas
+            - updatedReplicas
             type: object
         type: object
     served: true

diff --git a/manifests/crd/core.pingcap.com_tiflashgroups.yaml b/manifests/crd/core.pingcap.com_tiflashgroups.yaml
@@ -435,7 +435,10 @@ spec:
                   It will be same as the `spec.version` only when all instances are upgraded to the desired version.
                 type: string
             required:
+            - currentReplicas
+            - readyReplicas
             - replicas
+            - updatedReplicas
             type: object
         type: object
     served: true

diff --git a/manifests/crd/core.pingcap.com_tikvgroups.yaml b/manifests/crd/core.pingcap.com_tikvgroups.yaml
@@ -400,7 +400,10 @@ spec:
                   It will be same as the `spec.version` only when all instances are upgraded to the desired version.
                 type: string
             required:
+            - currentReplicas
+            - readyReplicas
             - replicas
+            - updatedReplicas
             type: object
         type: object
     served: true

diff --git a/pkg/client/fake.go b/pkg/client/fake.go
@@ -89,6 +89,9 @@ var _ client.WithWatch = &fakeUnderlayClient{}
 func newFakeUnderlayClient(objs ...client.Object) *fakeUnderlayClient {
 	t := testing.NewObjectTracker(scheme.Scheme, scheme.Codecs.UniversalDecoder())
 	for _, obj := range objs {
+		if obj == nil {
+			continue
+		}
 		if err := t.Add(obj); err != nil {
 			panic(err)
 		}
@@ -432,7 +435,40 @@ func (*SubResourceClient) Create(_ context.Context, _, _ client.Object, _ ...cli
 	return nil
 }
 
-func (*SubResourceClient) Update(_ context.Context, _ client.Object, _ ...client.SubResourceUpdateOption) error {
+func (c *SubResourceClient) Update(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
+	gvk, err := apiutil.GVKForObject(obj, c.scheme)
+	if err != nil {
+		return err
+	}
+
+	namespaced, err := apiutil.IsGVKNamespaced(gvk, c.restMapper)
+	if err != nil {
+		return err
+	}
+
+	mapping, err := c.restMapper.RESTMapping(gvk.GroupKind(), gvk.Version)
+	if err != nil {
+		return err
+	}
+
+	var action testing.UpdateAction
+	if namespaced {
+		action = testing.NewUpdateSubresourceAction(mapping.Resource, c.subResource, obj.GetNamespace(), obj)
+	} else {
+		action = testing.NewRootUpdateSubresourceAction(mapping.Resource, c.subResource, obj)
+	}
+	newObj, err := c.Invokes(action, nil)
+	if err != nil {
+		return err
+	}
+	if newObj == nil {
+		return fmt.Errorf("obj is not handled")
+	}
+
+	nv := reflect.ValueOf(newObj).Elem()
+	v := reflect.ValueOf(obj).Elem()
+	v.Set(nv)
+
 	return nil
 }
 

diff --git a/pkg/controllers/common/cond.go b/pkg/controllers/common/cond.go
@@ -39,3 +39,15 @@ func CondClusterIsPaused(ctx ClusterState) task.Condition {
 		return ctx.Cluster().ShouldPauseReconcile()
 	})
 }
+
+func CondPDGroupHasBeenDeleted(ctx PDGroupState) task.Condition {
+	return task.CondFunc(func() bool {
+		return ctx.PDGroup() == nil
+	})
+}
+
+func CondPDGroupIsDeleting(ctx PDGroupState) task.Condition {
+	return task.CondFunc(func() bool {
+		return !ctx.PDGroup().GetDeletionTimestamp().IsZero()
+	})
+}
diff --git a/pkg/controllers/common/interfaces.go b/pkg/controllers/common/interfaces.go
@@ -32,12 +32,13 @@ type ObjectList[T any] interface {
 }
 
 type (
-	PDInitializer = ResourceInitializer[*v1alpha1.PD]
+	PDInitializer = ResourceInitializer[v1alpha1.PD]
 
-	ClusterInitializer = ResourceInitializer[*v1alpha1.Cluster]
+	ClusterInitializer = ResourceInitializer[v1alpha1.Cluster]
 
-	PodInitializer     = ResourceInitializer[*corev1.Pod]
-	PDSliceInitializer = ResourceSliceInitializer[*v1alpha1.PD]
+	PodInitializer     = ResourceInitializer[corev1.Pod]
+	PDSliceInitializer = ResourceSliceInitializer[v1alpha1.PD]
+	PDGroupInitializer = ResourceInitializer[v1alpha1.PDGroup]
 )
 
 type PDStateInitializer interface {
@@ -64,6 +65,14 @@ type PodState interface {
 	Pod() *corev1.Pod
 }
 
+type PDGroupStateInitializer interface {
+	PDGroupInitializer() PDGroupInitializer
+}
+
+type PDGroupState interface {
+	PDGroup() *v1alpha1.PDGroup
+}
+
 type PDSliceStateInitializer interface {
 	PDSliceInitializer() PDSliceInitializer
 }

diff --git a/pkg/controllers/common/interfaces_test.go b/pkg/controllers/common/interfaces_test.go
@@ -30,7 +30,7 @@ func (f *fakeState[T]) Object() *T {
 	return f.obj
 }
 
-func (f *fakeState[T]) Initializer() ResourceInitializer[*T] {
+func (f *fakeState[T]) Initializer() ResourceInitializer[T] {
 	return NewResource(func(obj *T) { f.obj = obj }).
 		WithNamespace(Namespace(f.ns)).
 		WithName(Name(f.name)).
@@ -47,7 +47,7 @@ func (f *fakeSliceState[T]) Slice() []*T {
 	return f.objs
 }
 
-func (f *fakeSliceState[T]) Initializer() ResourceSliceInitializer[*T] {
+func (f *fakeSliceState[T]) Initializer() ResourceSliceInitializer[T] {
 	return NewResourceSlice(func(objs []*T) { f.objs = objs }).
 		WithNamespace(Namespace(f.ns)).
 		WithLabels(Labels(f.labels)).

diff --git a/pkg/controllers/common/resource.go b/pkg/controllers/common/resource.go
@@ -82,7 +82,7 @@ func (f SetFunc[T]) Set(obj T) {
 
 type ResourceInitializer[T any] interface {
 	GetOptions
-	Setter[T]
+	Setter[*T]
 }
 
 type Resource[T any] interface {
@@ -91,19 +91,19 @@ type Resource[T any] interface {
 	Initializer() ResourceInitializer[T]
 }
 
-func NewResource[T any](setter SetFunc[T]) Resource[T] {
+func NewResource[T any](setter SetFunc[*T]) Resource[T] {
 	return &resource[T]{
 		setter: setter,
 	}
 }
 
 type resource[T any] struct {
-	setter Setter[T]
+	setter Setter[*T]
 	ns     NamespaceOption
 	name   NameOption
 }
 
-func (r *resource[T]) Set(obj T) {
+func (r *resource[T]) Set(obj *T) {
 	r.setter.Set(obj)
 }
 
@@ -131,7 +131,7 @@ func (r *resource[T]) Initializer() ResourceInitializer[T] {
 
 type ResourceSliceInitializer[T any] interface {
 	ListOptions
-	Setter[[]T]
+	Setter[[]*T]
 }
 
 type ResourceSlice[T any] interface {
@@ -140,7 +140,7 @@ type ResourceSlice[T any] interface {
 	Initializer() ResourceSliceInitializer[T]
 }
 
-func NewResourceSlice[T any](setter SetFunc[[]T]) ResourceSlice[T] {
+func NewResourceSlice[T any](setter SetFunc[[]*T]) ResourceSlice[T] {
 	return &resourceSlice[T]{
 		setter: setter,
 	}
@@ -149,7 +149,7 @@ func NewResourceSlice[T any](setter SetFunc[[]T]) ResourceSlice[T] {
 type resourceSlice[T any] struct {
 	ns     NamespaceOption
 	labels LabelsOption
-	setter Setter[[]T]
+	setter Setter[[]*T]
 }
 
 func (r *resourceSlice[T]) Namespace() string {
@@ -160,7 +160,7 @@ func (r *resourceSlice[T]) Labels() map[string]string {
 	return r.labels.Labels()
 }
 
-func (r *resourceSlice[T]) Set(objs []T) {
+func (r *resourceSlice[T]) Set(objs []*T) {
 	r.setter.Set(objs)
 }
 

diff --git a/pkg/controllers/common/resource_test.go b/pkg/controllers/common/resource_test.go
@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"k8s.io/utils/ptr"
 )
 
 func TestResource(t *testing.T) {
@@ -56,14 +57,14 @@ func TestResource(t *testing.T) {
 			tt.Parallel()
 
 			var obj int
-			r := NewResource(func(t int) {
-				obj = t
+			r := NewResource(func(t *int) {
+				obj = *t
 			}).
 				WithNamespace(c.ns).
 				WithName(c.name).
 				Initializer()
 
-			r.Set(c.obj)
+			r.Set(&c.obj)
 
 			assert.Equal(tt, c.expectedNs, r.Namespace(), c.desc)
 			assert.Equal(tt, c.expectedName, r.Name(), c.desc)
@@ -77,32 +78,32 @@ func TestResourceSlice(t *testing.T) {
 		desc           string
 		ns             NamespaceOption
 		labels         LabelsOption
-		objs           []int
+		objs           []*int
 		expectedNs     string
 		expectedLabels map[string]string
-		expectedObjs   []int
+		expectedObjs   []*int
 	}{
 		{
 			desc:       "normal",
 			ns:         Namespace("aaa"),
 			labels:     Labels(map[string]string{"xxx": "yyy"}),
-			objs:       []int{42},
+			objs:       []*int{ptr.To(42)},
 			expectedNs: "aaa",
 			expectedLabels: map[string]string{
 				"xxx": "yyy",
 			},
-			expectedObjs: []int{42},
+			expectedObjs: []*int{ptr.To(42)},
 		},
 		{
 			desc:       "use func",
 			ns:         NameFunc(func() string { return "aaa" }),
 			labels:     LabelsFunc(func() map[string]string { return map[string]string{"xxx": "yyy"} }),
-			objs:       []int{42},
+			objs:       []*int{ptr.To(42)},
 			expectedNs: "aaa",
 			expectedLabels: map[string]string{
 				"xxx": "yyy",
 			},
-			expectedObjs: []int{42},
+			expectedObjs: []*int{ptr.To(42)},
 		},
 	}
 
@@ -111,8 +112,8 @@ func TestResourceSlice(t *testing.T) {
 		t.Run(c.desc, func(tt *testing.T) {
 			tt.Parallel()
 
-			var objs []int
-			r := NewResourceSlice(func(t []int) {
+			var objs []*int
+			r := NewResourceSlice(func(t []*int) {
 				objs = t
 			}).
 				WithNamespace(c.ns).

diff --git a/pkg/controllers/common/task.go b/pkg/controllers/common/task.go
@@ -31,7 +31,7 @@ import (
 	"github.com/pingcap/tidb-operator/pkg/utils/task/v3"
 )
 
-func taskContextResource[T any, PT Object[T]](name string, w ResourceInitializer[PT], c client.Client, shouldExist bool) task.Task {
+func taskContextResource[T any, PT Object[T]](name string, w ResourceInitializer[T], c client.Client, shouldExist bool) task.Task {
 	return task.NameTaskFunc("Context"+name, func(ctx context.Context) task.Result {
 		var obj PT = new(T)
 		key := types.NamespacedName{
@@ -56,7 +56,7 @@ func taskContextResource[T any, PT Object[T]](name string, w ResourceInitializer
 
 func taskContextResourceSlice[T any, L any, PT Object[T], PL ObjectList[L]](
 	name string,
-	w ResourceSliceInitializer[PT],
+	w ResourceSliceInitializer[T],
 	c client.Client,
 ) task.Task {
 	return task.NameTaskFunc("Context"+name, func(ctx context.Context) task.Result {
@@ -68,7 +68,7 @@ func taskContextResourceSlice[T any, L any, PT Object[T], PL ObjectList[L]](
 			return task.Fail().With("cannot list objs: %v", err)
 		}
 
-		objs := make([]PT, 0, meta.LenList(l))
+		objs := make([]*T, 0, meta.LenList(l))
 		if err := meta.EachListItem(l, func(item kuberuntime.Object) error {
 			obj, ok := item.(PT)
 			if !ok {
@@ -82,8 +82,9 @@ func taskContextResourceSlice[T any, L any, PT Object[T], PL ObjectList[L]](
 			return task.Fail().With("cannot extract list objs: %v", err)
 		}
 
-		slices.SortFunc(objs, func(a, b PT) int {
-			return cmp.Compare(a.GetName(), b.GetName())
+		slices.SortFunc(objs, func(a, b *T) int {
+			var pa, pb PT = a, b
+			return cmp.Compare(pa.GetName(), pb.GetName())
 		})
 
 		w.Set(objs)
@@ -107,6 +108,11 @@ func TaskContextPod(state PodStateInitializer, c client.Client) task.Task {
 	return taskContextResource("Pod", w, c, false)
 }
 
+func TaskContextPDGroup(state PDGroupStateInitializer, c client.Client) task.Task {
+	w := state.PDGroupInitializer()
+	return taskContextResource("PDGroup", w, c, false)
+}
+
 func TaskContextPDSlice(state PDSliceStateInitializer, c client.Client) task.Task {
 	w := state.PDSliceInitializer()
 	return taskContextResourceSlice[v1alpha1.PD, v1alpha1.PDList]("PDSlice", w, c)