Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛 create migration cr after the cluster manager cr is applied #389

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+134 −50
Open

🐛 create migration cr after the cluster manager cr is applied #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions ...perators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,11 @@ func (n *clusterManagerController) sync(ctx context.Context, controllerContext f
clusterManager.Status.ObservedGeneration = clusterManager.Generation
if len(errs) == 0 {
meta.SetStatusCondition(&clusterManager.Status.Conditions, 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 {
// When appliedCondition is false, we should not update related resources and resource generations
Expand Down
17 changes: 16 additions & 1 deletion ...operator/operators/clustermanager/controllers/migrationcontroller/migration_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ 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 {
err = clusterManagerAppliedSuccessfully(clusterManager)
if err != nil {
klog.Info(err.Error())
controllerContext.Queue().AddRateLimited(clusterManagerName)
return nil
}
Expand Down Expand Up @@ -206,6 +208,19 @@ func (c *crdMigrationController) sync(ctx context.Context, controllerContext fac
return nil
}

func clusterManagerAppliedSuccessfully(clusterManager *operatorapiv1.ClusterManager) error {
clusterManagerAppliedCondition := meta.FindStatusCondition(clusterManager.Status.Conditions, clusterManagerApplied)
if clusterManagerAppliedCondition == nil || clusterManagerAppliedCondition.Status != metav1.ConditionTrue {
return fmt.Errorf("applied condition %v is not true, wait for cluster manager to be applied",
clusterManagerAppliedCondition)
}
if clusterManagerAppliedCondition.ObservedGeneration != clusterManager.Generation {
return fmt.Errorf("observedGeneration %v not match generation: %v, wait for cluster manager to be applied",
clusterManagerAppliedCondition.ObservedGeneration, clusterManager.Generation)
}
return nil
}

// supportStorageVersionMigration returns ture if StorageVersionMigration CRD exists; otherwise returns false.
func supportStorageVersionMigration(ctx context.Context, apiExtensionClient apiextensionsclient.Interface) (bool, error) {
_, err := apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, migrationRequestCRDName, metav1.GetOptions{})
Expand Down
158 changes: 113 additions & 45 deletions ...tor/operators/clustermanager/controllers/migrationcontroller/migration_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,48 +459,110 @@ func TestSyncStorageVersionMigrationsCondition(t *testing.T) {
}
}

func TestSync(t *testing.T) {
clusterManager := newClusterManager("testhub")
tc, client := newTestController(t, clusterManager)

syncContext := testingcommon.NewFakeSyncContext(t, "testhub")
//Do not support migration
err := tc.sync(context.Background(), syncContext)
if err != nil {
t.Fatalf("Expected no error when sync, %v", err)
}

clusterManager, err = client.OperatorV1().ClusterManagers().Get(context.Background(), "testhub", metav1.GetOptions{})
if err != nil {
t.Fatalf("Expected no error when sync, %v", err)
}

if notsucceeded := meta.IsStatusConditionFalse(clusterManager.Status.Conditions, MigrationSucceeded); !notsucceeded {
t.Errorf("Error to sync clusterManager.Status.Conditions %v", clusterManager.Status.Conditions)
}
// all resources applied
clusterManager.Status.Conditions = []metav1.Condition{
func TestSyncNew(t *testing.T) {
cases := []struct {
name string
clusterManager *operatorapiv1.ClusterManager
crds []runtime.Object
validate func(t *testing.T, clusterManager *operatorapiv1.ClusterManager)
}{
{
Type: clusterManagerApplied,
Status: metav1.ConditionTrue,
name: "no crd",
clusterManager: newClusterManager("testhub"),
validate: func(t *testing.T, clusterManager *operatorapiv1.ClusterManager) {
mc := meta.FindStatusCondition(clusterManager.Status.Conditions, MigrationSucceeded)
if mc == nil {
t.Fatalf("Migration condition should exist in conditions %v", clusterManager.Status.Conditions)
}
if mc.Status != metav1.ConditionFalse {
t.Errorf("Migration condition should be false %v", mc)
}
if mc.Message != "Do not support StorageVersionMigration" {
t.Errorf("Migration condition message not correct %v", mc)
}
},
},
{
name: "migration cr created",
clusterManager: newClusterManager("testhub", metav1.Condition{
Type: clusterManagerApplied,
Status: metav1.ConditionTrue,
ObservedGeneration: 2,
}),
crds: []runtime.Object{
newFakeCRD(migrationRequestCRDName, "v1", "v1"),
newFakeCRD("foos.cluster.open-cluster-management.io", "v1beta2", "v1beta1", "v1beta2"),
newFakeCRD("bars.cluster.open-cluster-management.io", "v1beta2", "v1beta1", "v1beta2"),
},
validate: func(t *testing.T, clusterManager *operatorapiv1.ClusterManager) {
mc := meta.FindStatusCondition(clusterManager.Status.Conditions, MigrationSucceeded)
if mc == nil {
t.Fatalf("Migration condition should exist in conditions %v", clusterManager.Status.Conditions)
}
if mc.Status != metav1.ConditionFalse {
t.Errorf("Migration condition should be false %v", mc)
}
if mc.Message != "Wait StorageVersionMigration foo succeed." {
t.Errorf("Migration condition message not correct %v", mc)
}
},
},
{
name: "cluster manager applied condition generation not match",
clusterManager: newClusterManager("testhub", metav1.Condition{
Type: clusterManagerApplied,
Status: metav1.ConditionTrue,
ObservedGeneration: 1,
}),
crds: []runtime.Object{
newFakeCRD(migrationRequestCRDName, "v1", "v1"),
newFakeCRD("foos.cluster.open-cluster-management.io", "v1beta2", "v1beta1", "v1beta2"),
newFakeCRD("bars.cluster.open-cluster-management.io", "v1beta2", "v1beta1", "v1beta2"),
},
validate: func(t *testing.T, clusterManager *operatorapiv1.ClusterManager) {
mc := meta.FindStatusCondition(clusterManager.Status.Conditions, MigrationSucceeded)
if mc != nil {
t.Fatalf("Migration condition should not exist in conditions %v", clusterManager.Status.Conditions)
}
},
},
{
name: "cluster manager not applied",
clusterManager: newClusterManager("testhub", metav1.Condition{
Type: clusterManagerApplied,
Status: metav1.ConditionFalse,
ObservedGeneration: 1,
}),
crds: []runtime.Object{
newFakeCRD(migrationRequestCRDName, "v1", "v1"),
newFakeCRD("foos.cluster.open-cluster-management.io", "v1beta2", "v1beta1", "v1beta2"),
newFakeCRD("bars.cluster.open-cluster-management.io", "v1beta2", "v1beta1", "v1beta2"),
},
validate: func(t *testing.T, clusterManager *operatorapiv1.ClusterManager) {
mc := meta.FindStatusCondition(clusterManager.Status.Conditions, MigrationSucceeded)
if mc != nil {
t.Fatalf("Migration condition should not exist in conditions %v", clusterManager.Status.Conditions)
}
},
},
}

tc, client = newTestController(t, clusterManager,
newFakeCRD(migrationRequestCRDName, "v1", "v1"),
newFakeCRD("foos.cluster.open-cluster-management.io", "v1beta2", "v1beta1", "v1beta2"),
newFakeCRD("bars.cluster.open-cluster-management.io", "v1beta2", "v1beta1", "v1beta2"))
err = tc.sync(context.Background(), syncContext)
if err != nil {
t.Fatalf("Expected no error when sync, %v", err)
}
clusterManager, err = client.OperatorV1().ClusterManagers().Get(context.Background(), "testhub", metav1.GetOptions{})
if err != nil {
t.Fatalf("Expected no error when sync, %v", err)
}
if notsucceeded := meta.IsStatusConditionFalse(clusterManager.Status.Conditions, MigrationSucceeded); !notsucceeded {
t.Errorf("Error to sync clusterManager.Status.Conditions %v", clusterManager.Status.Conditions)
for _, c := range cases {
tc, client := newTestController(t, c.clusterManager, c.crds...)
syncContext := testingcommon.NewFakeSyncContext(t, "testhub")
//Do not support migration
err := tc.sync(context.Background(), syncContext)
if err != nil {
t.Fatalf("Expected no error when sync, %v", err)
}
cm, err := client.OperatorV1().ClusterManagers().Get(context.Background(), "testhub", metav1.GetOptions{})
if err != nil {
t.Fatalf("Get cluster manager failed, %v", err)
}
if c.validate != nil {
c.validate(t, cm)
}
}

}

func newTestController(
Expand All @@ -509,9 +571,14 @@ func newTestController(
crds ...runtime.Object) (*crdMigrationController, *fakeoperatorlient.Clientset) {
fakeOperatorClient := fakeoperatorlient.NewSimpleClientset(clustermanager)
operatorInformers := operatorinformers.NewSharedInformerFactory(fakeOperatorClient, 5*time.Minute)

fakeAPIExtensionClient := fakeapiextensions.NewSimpleClientset(crds...)
fakeMigrationClient := fakemigrationclient.NewSimpleClientset()

store := operatorInformers.Operator().V1().ClusterManagers().Informer().GetStore()
if err := store.Add(clustermanager); err != nil {
t.Fatal(err)
}
crdMigrationController := &crdMigrationController{
clusterManagerLister: operatorInformers.Operator().V1().ClusterManagers().Lister(),
recorder: eventstesting.NewTestingEventRecorder(t),
Expand All @@ -529,18 +596,15 @@ func newTestController(
newFakeMigration("bar", "cluster.open-cluster-management.io", "bars", "v1beta1"),
}, nil
}
store := operatorInformers.Operator().V1().ClusterManagers().Informer().GetStore()
if err := store.Add(clustermanager); err != nil {
t.Fatal(err)
}

return crdMigrationController, fakeOperatorClient
}

func newClusterManager(name string) *operatorapiv1.ClusterManager {
return &operatorapiv1.ClusterManager{
func newClusterManager(name string, conditions ...metav1.Condition) *operatorapiv1.ClusterManager {
cm := &operatorapiv1.ClusterManager{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Name: name,
Generation: 2,
},
Spec: operatorapiv1.ClusterManagerSpec{
RegistrationImagePullSpec: "testregistration",
Expand All @@ -549,4 +613,8 @@ func newClusterManager(name string) *operatorapiv1.ClusterManager {
},
},
}
for _, condition := range conditions {
meta.SetStatusCondition(&cm.Status.Conditions, condition)
}
return cm
}
Loading