From 7c883a214178dc0baac0227af634c223ce46e762 Mon Sep 17 00:00:00 2001 From: zhujian Date: Thu, 23 Nov 2023 07:55:33 +0000 Subject: [PATCH] :bug: create migration cr after the cluster manager cr is applied Signed-off-by: zhujian --- ...r-managedclustersetbindings-migration.yaml | 2 +- ...-manager-managedclustersets-migration.yaml | 2 +- .../clustermanager_controller.go | 13 ++--- .../clustermanager_crd_reconcile.go | 5 +- .../migration_controller.go | 11 +++- .../migration_controller_test.go | 51 ++++++++++--------- 6 files changed, 47 insertions(+), 37 deletions(-) diff --git a/manifests/cluster-manager/cluster-manager-managedclustersetbindings-migration.yaml b/manifests/cluster-manager/cluster-manager-managedclustersetbindings-migration.yaml index 1277ea86215..547ace63102 100644 --- 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 index 0d955666cb0..4dbabbca89f 100644 --- 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/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go b/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go index 7a178197c5f..9ffa15e6e0d 100644 --- a/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go +++ b/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go @@ -205,7 +205,7 @@ 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 } @@ -213,7 +213,7 @@ func (n *clusterManagerController) sync(ctx context.Context, controllerContext f 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/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_crd_reconcile.go b/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_crd_reconcile.go index 2832010d0d1..44291c2119c 100644 --- a/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_crd_reconcile.go +++ b/pkg/operators/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 index 564606bc604..39aed4b32db 100644 --- 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 index 5b32815983e..209a5375aa6 100644 --- a/pkg/operators/clustermanager/controllers/migrationcontroller/migration_controller_test.go +++ b/pkg/operators/clustermanager/controllers/migrationcontroller/migration_controller_test.go @@ -80,9 +80,9 @@ 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) }, }, { @@ -90,7 +90,7 @@ func TestApplyStorageVersionMigrations(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{ @@ -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{