🐛 create migration cr after the cluster manager cr is applied

Signed-off-by: zhujian <[email protected]>
stolostron · Nov 23, 2023 · 7c883a2 · 7c883a2
1 parent c8a7814
commit 7c883a2
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 37 deletions.
diff --git a/manifests/cluster-manager/cluster-manager-managedclustersetbindings-migration.yaml b/manifests/cluster-manager/cluster-manager-managedclustersetbindings-migration.yaml
@@ -1,7 +1,7 @@
 apiVersion: migration.k8s.io/v1alpha1
 kind: StorageVersionMigration
 metadata:
-  name: managedclustersetbindings.cluster.open-cluster-management.io
+  name: managedclustersetbindings.v1beta1.cluster.open-cluster-management.io
 spec:
   resource:
     group: cluster.open-cluster-management.io

diff --git a/manifests/cluster-manager/cluster-manager-managedclustersets-migration.yaml b/manifests/cluster-manager/cluster-manager-managedclustersets-migration.yaml
@@ -1,7 +1,7 @@
 apiVersion: migration.k8s.io/v1alpha1
 kind: StorageVersionMigration
 metadata:
-  name: managedclustersets.cluster.open-cluster-management.io
+  name: managedclustersets.v1beta1.cluster.open-cluster-management.io
 spec:
   resource:
     group: cluster.open-cluster-management.io

diff --git a/...perators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go b/...perators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go
@@ -205,15 +205,15 @@ func (n *clusterManagerController) sync(ctx context.Context, controllerContext f
 	if err != nil {
 		return err
 	}
-	hubClient, hubApiExtensionClient, hubApiRegistrationClient, hubMigrationClient, err := n.generateHubClusterClients(hubKubeConfig)
+	hubClient, hubApiExtensionClient, hubApiRegistrationClient, _, err := n.generateHubClusterClients(hubKubeConfig)
 	if err != nil {
 		return err
 	}
 	managementClient := n.operatorKubeClient // We assume that operator is always running on the management cluster.
 
 	var errs []error
 	reconcilers := []clusterManagerReconcile{
-		&crdReconcile{cache: n.cache, recorder: n.recorder, hubAPIExtensionClient: hubApiExtensionClient, hubMigrationClient: hubMigrationClient, skipRemoveCRDs: n.skipRemoveCRDs},
+		&crdReconcile{cache: n.cache, recorder: n.recorder, hubAPIExtensionClient: hubApiExtensionClient, skipRemoveCRDs: n.skipRemoveCRDs},
 		&hubReoncile{cache: n.cache, recorder: n.recorder, hubKubeClient: hubClient, hubAPIRegistrationClient: hubApiRegistrationClient},
 		&runtimeReconcile{cache: n.cache, recorder: n.recorder, hubKubeConfig: hubKubeConfig, hubKubeClient: hubClient, kubeClient: managementClient, ensureSAKubeconfigs: n.ensureSAKubeconfigs},
 		&webhookReconcile{cache: n.cache, recorder: n.recorder, hubKubeClient: hubClient, kubeClient: managementClient},
@@ -271,10 +271,11 @@ func (n *clusterManagerController) sync(ctx context.Context, controllerContext f
 
 	if len(errs) == 0 {
 		conds = append(conds, metav1.Condition{
-			Type:    clusterManagerApplied,
-			Status:  metav1.ConditionTrue,
-			Reason:  "ClusterManagerApplied",
-			Message: "Components of cluster manager are applied",
+			Type:               clusterManagerApplied,
+			Status:             metav1.ConditionTrue,
+			Reason:             "ClusterManagerApplied",
+			Message:            "Components of cluster manager are applied",
+			ObservedGeneration: clusterManager.Generation,
 		})
 	} else {
 		if cond := meta.FindStatusCondition(clusterManager.Status.Conditions, clusterManagerApplied); cond != nil {

diff --git a/...ators/clustermanager/controllers/clustermanagercontroller/clustermanager_crd_reconcile.go b/...ators/clustermanager/controllers/clustermanagercontroller/clustermanager_crd_reconcile.go
@@ -19,7 +19,6 @@ import (
 	"open-cluster-management.io/registration-operator/manifests"
 	"open-cluster-management.io/registration-operator/pkg/helpers"
 	"open-cluster-management.io/registration-operator/pkg/operators/crdmanager"
-	migrationclient "sigs.k8s.io/kube-storage-version-migrator/pkg/clients/clientset/typed/migration/v1alpha1"
 )
 
 var (
@@ -51,8 +50,8 @@ var (
 
 type crdReconcile struct {
 	hubAPIExtensionClient apiextensionsclient.Interface
-	hubMigrationClient    migrationclient.StorageVersionMigrationsGetter
-	skipRemoveCRDs        bool
+	// hubMigrationClient    migrationclient.StorageVersionMigrationsGetter
+	skipRemoveCRDs bool
 
 	cache    resourceapply.ResourceCache
 	recorder events.Recorder

diff --git a/pkg/operators/clustermanager/controllers/migrationcontroller/migration_controller.go b/pkg/operators/clustermanager/controllers/migrationcontroller/migration_controller.go
@@ -142,8 +142,17 @@ func (c *crdMigrationController) sync(ctx context.Context, controllerContext fac
 	}
 
 	// do not apply storage version migrations until other resources are applied
-	if applied := meta.IsStatusConditionTrue(clusterManager.Status.Conditions, clusterManagerApplied); !applied {
+	clusterManagerAppliedCondition := meta.FindStatusCondition(clusterManager.Status.Conditions, clusterManagerApplied)
+	if clusterManagerAppliedCondition == nil || clusterManagerAppliedCondition.Status != metav1.ConditionTrue {
 		controllerContext.Queue().AddRateLimited(clusterManagerName)
+		klog.Info("Applied condition false, wait for cluster manager to be applied")
+		return nil
+	}
+	if clusterManagerAppliedCondition.ObservedGeneration != clusterManager.Generation {
+		controllerContext.Queue().AddRateLimited(clusterManagerName)
+		klog.InfoS("ObservedGeneration not match,wait for cluster manager to be applied",
+			"ObservedGeneration", clusterManagerAppliedCondition.ObservedGeneration,
+			"Generation", clusterManager.Generation)
 		return nil
 	}
 

diff --git a/pkg/operators/clustermanager/controllers/migrationcontroller/migration_controller_test.go b/pkg/operators/clustermanager/controllers/migrationcontroller/migration_controller_test.go
@@ -80,17 +80,17 @@ func TestApplyStorageVersionMigrations(t *testing.T) {
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
 				assertActions(t, actions, "get", "create", "get", "create")
 				actual := actions[1].(clienttesting.CreateActionImpl).Object
-				assertStorageVersionMigration(t, "managedclustersets.cluster.open-cluster-management.io", actual)
+				assertStorageVersionMigration(t, "managedclustersets.v1beta1.cluster.open-cluster-management.io", actual)
 				actual = actions[3].(clienttesting.CreateActionImpl).Object
-				assertStorageVersionMigration(t, "managedclustersetbindings.cluster.open-cluster-management.io", actual)
+				assertStorageVersionMigration(t, "managedclustersetbindings.v1beta1.cluster.open-cluster-management.io", actual)
 			},
 		},
 		{
 			name: "created and updated",
 			existingObjects: []runtime.Object{
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersetbindings.cluster.open-cluster-management.io",
+						Name: "managedclustersetbindings.v1beta1.cluster.open-cluster-management.io",
 					},
 				},
 				&migrationv1alpha1.StorageVersionMigration{
@@ -102,9 +102,9 @@ func TestApplyStorageVersionMigrations(t *testing.T) {
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
 				assertActions(t, actions, "get", "create", "get", "update")
 				actual := actions[1].(clienttesting.CreateActionImpl).Object
-				assertStorageVersionMigration(t, "managedclustersets.cluster.open-cluster-management.io", actual)
+				assertStorageVersionMigration(t, "managedclustersets.v1beta1.cluster.open-cluster-management.io", actual)
 				actual = actions[3].(clienttesting.UpdateActionImpl).Object
-				assertStorageVersionMigration(t, "managedclustersetbindings.cluster.open-cluster-management.io", actual)
+				assertStorageVersionMigration(t, "managedclustersetbindings.v1beta1.cluster.open-cluster-management.io", actual)
 			},
 		},
 	}
@@ -118,7 +118,8 @@ func TestApplyStorageVersionMigrations(t *testing.T) {
 		t.Run(c.name, func(t *testing.T) {
 			fakeMigrationClient := fakemigrationclient.NewSimpleClientset(c.existingObjects...)
 
-			err := applyStorageVersionMigrations(context.TODO(), fakeMigrationClient.MigrationV1alpha1(), eventstesting.NewTestingEventRecorder(t))
+			err := applyStorageVersionMigrations(context.TODO(),
+				fakeMigrationClient.MigrationV1alpha1(), eventstesting.NewTestingEventRecorder(t))
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
@@ -129,10 +130,8 @@ func TestApplyStorageVersionMigrations(t *testing.T) {
 
 func TestRemoveStorageVersionMigrations(t *testing.T) {
 	names := []string{
-		"managedclustersets.cluster.open-cluster-management.io",
-		"managedclustersetbindings.cluster.open-cluster-management.io",
-		"placements.cluster.open-cluster-management.io",
-		"placementdecisions.cluster.open-cluster-management.io",
+		"managedclustersets.v1beta1.cluster.open-cluster-management.io",
+		"managedclustersetbindings.v1beta1.cluster.open-cluster-management.io",
 	}
 	cases := []struct {
 		name            string
@@ -150,6 +149,11 @@ func TestRemoveStorageVersionMigrations(t *testing.T) {
 						Name: "managedclustersetbindings.cluster.open-cluster-management.io",
 					},
 				},
+				&migrationv1alpha1.StorageVersionMigration{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "managedclustersetbindings.v1beta1.cluster.open-cluster-management.io",
+					},
+				},
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
 						Name: "placementdecisions.cluster.open-cluster-management.io",
@@ -169,11 +173,8 @@ func TestRemoveStorageVersionMigrations(t *testing.T) {
 
 			for _, name := range names {
 				_, err := fakeMigrationClient.MigrationV1alpha1().StorageVersionMigrations().Get(context.TODO(), name, metav1.GetOptions{})
-				if errors.IsNotFound(err) {
-					continue
-				}
-				if err != nil {
-					t.Fatalf("unexpected error: %v", err)
+				if !errors.IsNotFound(err) {
+					t.Fatalf("expected not found error for %s but got: %v", name, err)
 				}
 			}
 		})
@@ -215,12 +216,12 @@ func Test_syncStorageVersionMigrationsCondition(t *testing.T) {
 			existingObjects: []runtime.Object{
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersetbindings.cluster.open-cluster-management.io",
+						Name: "managedclustersetbindings.v1beta1.cluster.open-cluster-management.io",
 					},
 				},
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersets.cluster.open-cluster-management.io",
+						Name: "managedclustersets.v1beta1.cluster.open-cluster-management.io",
 					},
 				},
 			},
@@ -235,7 +236,7 @@ func Test_syncStorageVersionMigrationsCondition(t *testing.T) {
 			existingObjects: []runtime.Object{
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersetbindings.cluster.open-cluster-management.io",
+						Name: "managedclustersetbindings.v1beta1.cluster.open-cluster-management.io",
 					},
 					Status: migrationv1alpha1.StorageVersionMigrationStatus{
 						Conditions: []migrationv1alpha1.MigrationCondition{
@@ -248,7 +249,7 @@ func Test_syncStorageVersionMigrationsCondition(t *testing.T) {
 				},
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersets.cluster.open-cluster-management.io",
+						Name: "managedclustersets.v1beta1.cluster.open-cluster-management.io",
 					},
 					Status: migrationv1alpha1.StorageVersionMigrationStatus{
 						Conditions: []migrationv1alpha1.MigrationCondition{
@@ -271,7 +272,7 @@ func Test_syncStorageVersionMigrationsCondition(t *testing.T) {
 			existingObjects: []runtime.Object{
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersetbindings.cluster.open-cluster-management.io",
+						Name: "managedclustersetbindings.v1beta1.cluster.open-cluster-management.io",
 					},
 					Status: migrationv1alpha1.StorageVersionMigrationStatus{
 						Conditions: []migrationv1alpha1.MigrationCondition{
@@ -284,7 +285,7 @@ func Test_syncStorageVersionMigrationsCondition(t *testing.T) {
 				},
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersets.cluster.open-cluster-management.io",
+						Name: "managedclustersets.v1beta1.cluster.open-cluster-management.io",
 					},
 					Status: migrationv1alpha1.StorageVersionMigrationStatus{
 						Conditions: []migrationv1alpha1.MigrationCondition{
@@ -307,7 +308,7 @@ func Test_syncStorageVersionMigrationsCondition(t *testing.T) {
 			existingObjects: []runtime.Object{
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersetbindings.cluster.open-cluster-management.io",
+						Name: "managedclustersetbindings.v1beta1.cluster.open-cluster-management.io",
 					},
 					Status: migrationv1alpha1.StorageVersionMigrationStatus{
 						Conditions: []migrationv1alpha1.MigrationCondition{
@@ -320,7 +321,7 @@ func Test_syncStorageVersionMigrationsCondition(t *testing.T) {
 				},
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersets.cluster.open-cluster-management.io",
+						Name: "managedclustersets.v1beta1.cluster.open-cluster-management.io",
 					},
 					Status: migrationv1alpha1.StorageVersionMigrationStatus{
 						Conditions: []migrationv1alpha1.MigrationCondition{
@@ -343,7 +344,7 @@ func Test_syncStorageVersionMigrationsCondition(t *testing.T) {
 			existingObjects: []runtime.Object{
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersetbindings.cluster.open-cluster-management.io",
+						Name: "managedclustersetbindings.v1beta1.cluster.open-cluster-management.io",
 					},
 					Status: migrationv1alpha1.StorageVersionMigrationStatus{
 						Conditions: []migrationv1alpha1.MigrationCondition{
@@ -356,7 +357,7 @@ func Test_syncStorageVersionMigrationsCondition(t *testing.T) {
 				},
 				&migrationv1alpha1.StorageVersionMigration{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "managedclustersets.cluster.open-cluster-management.io",
+						Name: "managedclustersets.v1beta1.cluster.open-cluster-management.io",
 					},
 					Status: migrationv1alpha1.StorageVersionMigrationStatus{
 						Conditions: []migrationv1alpha1.MigrationCondition{