fix(group): fix missing component in controller revision name (#6060)

Author: liubog2008

pkg/controllers/common/revision.go

@@ -115,13 +115,13 @@ func TaskRevision[
 	var gt GT
 	w := state.RevisionInitializer()
 	return task.NameTaskFunc("ContextRevision", func(ctx context.Context) task.Result {
-		historyCli := history.NewClient(c)
 		parent := w.Parent()
+		historyCli := history.NewClient(c, parent.Component())
 
 		lbs := w.Labels()
 		selector := labels.SelectorFromSet(labels.Set(lbs))
 
-		revisions, err := historyCli.ListControllerRevisions(gt.To(parent), selector)
+		revisions, err := historyCli.ListControllerRevisions(ctx, gt.To(parent), selector)
 		if err != nil {
 			return task.Fail().With("cannot list controller revisions: %w", err)
 		}

pkg/controllers/common/revision_test.go

@@ -46,8 +46,8 @@ func (f *fakeRevisionStateInitializer[G]) RevisionInitializer() RevisionInitiali
 }
 
 const (
-	fakeOldRevision = "aaa-77554fbd78"
-	fakeNewRevision = "aaa-77554fbd79"
+	fakeOldRevision = "aaa-pd-77554fbd78"
+	fakeNewRevision = "aaa-pd-77554fbd79"
 )
 
 func TestTaskRevision(t *testing.T) {
@@ -169,7 +169,7 @@ func TestTaskRevision(t *testing.T) {
 				fake.FakeObj(fakeOldRevision, fake.Label[appsv1.ControllerRevision]("ccc", "ddd")),
 				fake.FakeObj(fakeNewRevision, fake.Label[appsv1.ControllerRevision]("ccc", "ddd")),
 			},
-			expectedUpdateRevision:  "aaa-77554fbd76",
+			expectedUpdateRevision:  "aaa-pd-77554fbd76",
 			expectedCurrentRevision: fakeOldRevision,
 			expectedCollisionCount:  2,
 		},

pkg/controllers/tidbgroup/tasks/state_test.go

@@ -79,8 +79,8 @@ func TestState(t *testing.T) {
 						return obj
 					}),
 				},
-				updateRevision:  "aaa-dfb684447",
-				currentRevision: "aaa-dfb684447",
+				updateRevision:  "aaa-tidb-dfb684447",
+				currentRevision: "aaa-tidb-dfb684447",
 			},
 		},
 	}

pkg/controllers/tiflashgroup/tasks/state_test.go

@@ -79,8 +79,8 @@ func TestState(t *testing.T) {
 						return obj
 					}),
 				},
-				updateRevision:  "aaa-77554fbd78",
-				currentRevision: "aaa-77554fbd78",
+				updateRevision:  "aaa-tiflash-77554fbd78",
+				currentRevision: "aaa-tiflash-77554fbd78",
 			},
 		},
 	}

pkg/utils/k8s/revision/controller_revision.go

@@ -47,7 +47,7 @@ var encoderMap = map[schema.GroupVersion]kuberuntime.Encoder{}
 // a new revision, or modify the Revision of an existing revision if an update to ComponentGroup is detected.
 // This method expects that revisions is sorted when supplied.
 func GetCurrentAndUpdate(
-	_ context.Context,
+	ctx context.Context,
 	group client.Object,
 	component string,
 	labels map[string]string,
@@ -86,13 +86,13 @@ func GetCurrentAndUpdate(
 	} else if equalCount > 0 {
 		// if the equivalent revision is not immediately prior we will roll back by incrementing the
 		// Revision of the equivalent revision
-		updateRevision, err = cli.UpdateControllerRevision(equalRevisions[equalCount-1], updateRevision.Revision)
+		updateRevision, err = cli.UpdateControllerRevision(ctx, equalRevisions[equalCount-1], updateRevision.Revision)
 		if err != nil {
 			return nil, nil, collisionCount, fmt.Errorf("failed to update a revision: %w", err)
 		}
 	} else {
 		// if there is no equivalent revision we create a new one
-		updateRevision, err = cli.CreateControllerRevision(group, updateRevision, &collisionCount)
+		updateRevision, err = cli.CreateControllerRevision(ctx, group, updateRevision, &collisionCount)
 		if err != nil {
 			return nil, nil, collisionCount, fmt.Errorf("failed to create a revision: %w", err)
 		}
@@ -180,6 +180,7 @@ func getPatch(obj client.Object, gvk schema.GroupVersionKind) ([]byte, error) {
 // until only RevisionHistoryLimit revisions remain.
 // This method expects that revisions is sorted when supplied.
 func TruncateHistory[T runtime.Instance](
+	ctx context.Context,
 	cli history.Interface,
 	instances []T,
 	revisions []*appsv1.ControllerRevision,
@@ -224,7 +225,7 @@ func TruncateHistory[T runtime.Instance](
 	// delete any non-live history to maintain the revision limit.
 	hist = hist[:(historyLen - historyLimit)]
 	for i := 0; i < len(hist); i++ {
-		if err := cli.DeleteControllerRevision(hist[i]); err != nil {
+		if err := cli.DeleteControllerRevision(ctx, hist[i]); err != nil {
 			return fmt.Errorf("failed to delete controller revision %s: %w", hist[i].Name, err)
 		}
 	}

pkg/utils/k8s/revision/controller_revision_test.go

@@ -38,7 +38,7 @@ type FakeHistoryClient struct {
 	CreateFunc func(parent client.Object, revision *appsv1.ControllerRevision, collisionCount *int32) (*appsv1.ControllerRevision, error)
 }
 
-func (f *FakeHistoryClient) CreateControllerRevision(parent client.Object, revision *appsv1.ControllerRevision, collisionCount *int32) (*appsv1.ControllerRevision, error) {
+func (f *FakeHistoryClient) CreateControllerRevision(_ context.Context, parent client.Object, revision *appsv1.ControllerRevision, collisionCount *int32) (*appsv1.ControllerRevision, error) {
 	if f.CreateFunc != nil {
 		rev, err := f.CreateFunc(parent, revision, collisionCount)
 		if err != nil {
@@ -49,7 +49,7 @@ func (f *FakeHistoryClient) CreateControllerRevision(parent client.Object, revis
 	return nil, nil
 }
 
-func (f *FakeHistoryClient) DeleteControllerRevision(revision *appsv1.ControllerRevision) error {
+func (f *FakeHistoryClient) DeleteControllerRevision(_ context.Context, revision *appsv1.ControllerRevision) error {
 	for i, r := range f.Revisions {
 		if r.Name == revision.Name {
 			f.Revisions = append(f.Revisions[:i], f.Revisions[i+1:]...)
@@ -59,11 +59,11 @@ func (f *FakeHistoryClient) DeleteControllerRevision(revision *appsv1.Controller
 	return nil
 }
 
-func (f *FakeHistoryClient) ListControllerRevisions(_ client.Object, _ labels.Selector) ([]*appsv1.ControllerRevision, error) {
+func (f *FakeHistoryClient) ListControllerRevisions(_ context.Context, _ client.Object, _ labels.Selector) ([]*appsv1.ControllerRevision, error) {
 	return f.Revisions, nil
 }
 
-func (f *FakeHistoryClient) UpdateControllerRevision(revision *appsv1.ControllerRevision, newRevision int64) (*appsv1.ControllerRevision, error) {
+func (f *FakeHistoryClient) UpdateControllerRevision(_ context.Context, revision *appsv1.ControllerRevision, newRevision int64) (*appsv1.ControllerRevision, error) {
 	for _, r := range f.Revisions {
 		if r.Name == revision.Name {
 			r.Revision = newRevision
@@ -176,10 +176,11 @@ func TestTruncateHistory(t *testing.T) {
 			cli := &FakeHistoryClient{
 				Revisions: tt.revisions,
 			}
-			err := TruncateHistory(cli, runtime.FromTiDBSlice(tt.instances), tt.revisions, tt.current.Name, tt.update.Name, tt.limit)
+			ctx := context.TODO()
+			err := TruncateHistory(ctx, cli, runtime.FromTiDBSlice(tt.instances), tt.revisions, tt.current.Name, tt.update.Name, tt.limit)
 			require.NoError(t, err)
 
-			remainingRevisions, err := cli.ListControllerRevisions(nil, labels.Everything())
+			remainingRevisions, err := cli.ListControllerRevisions(ctx, nil, labels.Everything())
 			require.NoError(t, err)
 			assert.Equal(t, len(tt.expected), len(remainingRevisions))
 			m := make(map[string]struct{}, len(remainingRevisions))

third_party/kubernetes/pkg/controller/history/controller_history.go

@@ -173,37 +173,41 @@ type Interface interface {
 	// ListControllerRevisions lists all ControllerRevisions matching selector and owned by parent or no other
 	// controller. If the returned error is nil the returned slice of ControllerRevisions is valid. If the
 	// returned error is not nil, the returned slice is not valid.
-	ListControllerRevisions(parent client.Object, selector labels.Selector) ([]*appsv1.ControllerRevision, error)
+	ListControllerRevisions(ctx context.Context, parent client.Object, selector labels.Selector) ([]*appsv1.ControllerRevision, error)
 	// CreateControllerRevision attempts to create the revision as owned by parent via a ControllerRef. If name
 	// collision occurs, collisionCount (incremented each time collision occurs except for the first time) is
 	// added to the hash of the revision, and it is renamed using ControllerRevisionName. Implementations may
 	// cease to attempt to retry creation after some number of attempts and return an error. If the returned
 	// error is not nil, creation failed. If the returned error is nil, the returned ControllerRevision has been
 	// created.
 	// Callers must make sure that collisionCount is not nil. An error is returned if it is.
-	CreateControllerRevision(parent client.Object, revision *appsv1.ControllerRevision, collisionCount *int32) (*appsv1.ControllerRevision, error)
+	CreateControllerRevision(ctx context.Context, parent client.Object, revision *appsv1.ControllerRevision, collisionCount *int32) (*appsv1.ControllerRevision, error)
 	// DeleteControllerRevision attempts to delete revision. If the returned error is not nil, deletion has failed.
-	DeleteControllerRevision(revision *appsv1.ControllerRevision) error
+	DeleteControllerRevision(ctx context.Context, revision *appsv1.ControllerRevision) error
 	// UpdateControllerRevision updates revision such that its Revision is equal to newRevision. Implementations
 	// may retry on conflict. If the returned error is nil, the update was successful and returned ControllerRevision
 	// is valid. If the returned error is not nil, the update failed and the returned ControllerRevision is invalid.
-	UpdateControllerRevision(revision *appsv1.ControllerRevision, newRevision int64) (*appsv1.ControllerRevision, error)
+	UpdateControllerRevision(ctx context.Context, revision *appsv1.ControllerRevision, newRevision int64) (*appsv1.ControllerRevision, error)
 }
 
 // NewClient returns an instance of Interface that uses client to communicate with the API Server and lister to list
 // ControllerRevisions. This method should be used to create an Interface for all scenarios other than testing.
-func NewClient(cli client.Client) Interface {
-	return &realHistory{cli: cli}
+func NewClient(cli client.Client, component string) Interface {
+	return &realHistory{
+		cli:       cli,
+		component: component,
+	}
 }
 
 type realHistory struct {
-	cli client.Client
+	cli       client.Client
+	component string
 }
 
-func (rh *realHistory) ListControllerRevisions(parent client.Object, selector labels.Selector) ([]*appsv1.ControllerRevision, error) {
+func (rh *realHistory) ListControllerRevisions(ctx context.Context, parent client.Object, selector labels.Selector) ([]*appsv1.ControllerRevision, error) {
 	// List all revisions in the namespace that match the selector
 	var list appsv1.ControllerRevisionList
-	if err := rh.cli.List(context.TODO(), &list, &client.ListOptions{
+	if err := rh.cli.List(ctx, &list, &client.ListOptions{
 		Namespace:     parent.GetNamespace(),
 		LabelSelector: selector,
 	}); err != nil {
@@ -220,21 +224,22 @@ func (rh *realHistory) ListControllerRevisions(parent client.Object, selector la
 	return owned, nil
 }
 
-func (rh *realHistory) CreateControllerRevision(parent client.Object, revision *appsv1.ControllerRevision, collisionCount *int32) (*appsv1.ControllerRevision, error) {
+func (rh *realHistory) CreateControllerRevision(ctx context.Context, parent client.Object, revision *appsv1.ControllerRevision, collisionCount *int32) (*appsv1.ControllerRevision, error) {
 	if collisionCount == nil {
 		return nil, fmt.Errorf("collisionCount should not be nil")
 	}
 
 	// Clone the input
 	clone := revision.DeepCopy()
 
+	namePrefix := parent.GetName() + "-" + rh.component
 	// Continue to attempt to create the revision updating the name with a new hash on each iteration
 	for {
 		hash := HashControllerRevision(revision, collisionCount)
 		// Update the revisions name
-		clone.Name = ControllerRevisionName(parent.GetName(), hash)
+		clone.Name = ControllerRevisionName(namePrefix, hash)
 		clone.Namespace = parent.GetNamespace()
-		err := rh.cli.Create(context.TODO(), clone)
+		err := rh.cli.Create(ctx, clone)
 		if errors.IsAlreadyExists(err) {
 			var exists appsv1.ControllerRevision
 			if err := rh.cli.Get(context.TODO(), client.ObjectKey{
@@ -253,19 +258,19 @@ func (rh *realHistory) CreateControllerRevision(parent client.Object, revision *
 	}
 }
 
-func (rh *realHistory) UpdateControllerRevision(revision *appsv1.ControllerRevision, newRevision int64) (*appsv1.ControllerRevision, error) {
+func (rh *realHistory) UpdateControllerRevision(ctx context.Context, revision *appsv1.ControllerRevision, newRevision int64) (*appsv1.ControllerRevision, error) {
 	clone := revision.DeepCopy()
 	err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
 		if clone.Revision == newRevision {
 			return nil
 		}
 		clone.Revision = newRevision
-		updateErr := rh.cli.Update(context.TODO(), clone)
+		updateErr := rh.cli.Update(ctx, clone)
 		if updateErr == nil {
 			return nil
 		}
 		var updated appsv1.ControllerRevision
-		if err := rh.cli.Get(context.TODO(), client.ObjectKey{
+		if err := rh.cli.Get(ctx, client.ObjectKey{
 			Namespace: clone.Namespace,
 			Name:      clone.Name,
 		}, &updated); err == nil {
@@ -277,6 +282,6 @@ func (rh *realHistory) UpdateControllerRevision(revision *appsv1.ControllerRevis
 	return clone, err
 }
 
-func (rh *realHistory) DeleteControllerRevision(revision *appsv1.ControllerRevision) error {
-	return rh.cli.Delete(context.TODO(), revision)
+func (rh *realHistory) DeleteControllerRevision(ctx context.Context, revision *appsv1.ControllerRevision) error {
+	return rh.cli.Delete(ctx, revision)
 }