diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index f0dbdceba..0cd2f02ee 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -173,6 +173,17 @@ func UpdateKlusterletConditionFn(conds ...metav1.Condition) UpdateKlusterletStat } } +// TODO: just for upgrading, we need to remove this in 0.11.0 +func ReplaceKlusterletConditionFn(removeConditionType string, conds ...metav1.Condition) UpdateKlusterletStatusFunc { + return func(oldStatus *operatorapiv1.KlusterletStatus) error { + meta.RemoveStatusCondition(&oldStatus.Conditions, removeConditionType) + for _, cond := range conds { + meta.SetStatusCondition(&oldStatus.Conditions, cond) + } + return nil + } +} + func CleanUpStaticObject( ctx context.Context, client kubernetes.Interface, diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index 17e6faece..9dd21e1c1 100644 --- a/pkg/helpers/helpers_test.go +++ b/pkg/helpers/helpers_test.go @@ -161,6 +161,99 @@ func TestUpdateStatusCondition(t *testing.T) { } } +func TestReplaceKlusterletConditionFn(t *testing.T) { + cases := []struct { + name string + startingConditions []metav1.Condition + newCondition metav1.Condition + removeType string + expectedUpdated bool + expectedConditions []metav1.Condition + }{ + { + name: "replace empty", + startingConditions: []metav1.Condition{ + newCondition("two", "True", "my-reason", "my-message", nil), + }, + newCondition: newCondition("one", "True", "my-reason", "my-message", nil), + expectedUpdated: true, + expectedConditions: []metav1.Condition{ + newCondition("two", "True", "my-reason", "my-message", nil), + newCondition("one", "True", "my-reason", "my-message", nil), + }, + }, + { + name: "replace an existing type", + startingConditions: []metav1.Condition{ + newCondition("two", "True", "my-reason", "my-message", nil), + }, + newCondition: newCondition("one", "True", "my-reason", "my-message", nil), + removeType: "two", + expectedUpdated: true, + expectedConditions: []metav1.Condition{ + newCondition("one", "True", "my-reason", "my-message", nil), + }, + }, + { + name: "replace a non-existing type", + startingConditions: []metav1.Condition{ + newCondition("two", "True", "my-reason", "my-message", nil), + }, + newCondition: newCondition("one", "True", "my-reason", "my-message", nil), + removeType: "three", + expectedUpdated: true, + expectedConditions: []metav1.Condition{ + newCondition("two", "True", "my-reason", "my-message", nil), + newCondition("one", "True", "my-reason", "my-message", nil), + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + fakeOperatorClient := opereatorfake.NewSimpleClientset( + &operatorapiv1.ClusterManager{ + ObjectMeta: metav1.ObjectMeta{Name: "testmanagedcluster"}, + Status: operatorapiv1.ClusterManagerStatus{ + Conditions: c.startingConditions, + }, + }, + &operatorapiv1.Klusterlet{ + ObjectMeta: metav1.ObjectMeta{Name: "testmanagedcluster"}, + Status: operatorapiv1.KlusterletStatus{ + Conditions: c.startingConditions, + }, + }, + ) + + klusterletstatus, updated, err := UpdateKlusterletStatus( + context.TODO(), + fakeOperatorClient.OperatorV1().Klusterlets(), + "testmanagedcluster", + ReplaceKlusterletConditionFn(c.removeType, c.newCondition), + ) + if err != nil { + t.Errorf("unexpected err: %v", err) + } + if updated != c.expectedUpdated { + t.Errorf("expected %t, but %t", c.expectedUpdated, updated) + } + + for i := range c.expectedConditions { + expected := c.expectedConditions[i] + + klusterletactual := klusterletstatus.Conditions[i] + if expected.LastTransitionTime == (metav1.Time{}) { + klusterletactual.LastTransitionTime = metav1.Time{} + } + if !equality.Semantic.DeepEqual(expected, klusterletactual) { + t.Errorf(diff.ObjectDiff(expected, klusterletactual)) + } + } + }) + } +} + func newCondition(name, status, reason, message string, lastTransition *metav1.Time) metav1.Condition { ret := metav1.Condition{ Type: name, diff --git a/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go b/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go index d6a5b7ec1..ebbbcf3a1 100644 --- a/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go +++ b/pkg/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller.go @@ -270,7 +270,7 @@ func (n *clusterManagerController) sync(ctx context.Context, controllerContext f conditions := &clusterManager.Status.Conditions // If there are some invalid feature gates of registration, will output - // condition `InvalidRegistrationFeatureGates` in ClusterManager. + // condition `ValidRegistrationFeatureGates` False in ClusterManager. if clusterManager.Spec.RegistrationConfiguration != nil { featureGates, condition, err := n.checkFeatureGate(ctx, clusterManagerName, clusterManager.Spec.RegistrationConfiguration.FeatureGates, "registration") @@ -281,7 +281,7 @@ func (n *clusterManagerController) sync(ctx context.Context, controllerContext f meta.SetStatusCondition(conditions, condition) } // If there are some invalid feature gates of work, will output - // condition `InvalidWorkFeatureGates` in ClusterManager. + // condition `ValidWorkFeatureGates` False in ClusterManager. if clusterManager.Spec.WorkConfiguration != nil { featureGates, condition, err := n.checkFeatureGate(ctx, clusterManagerName, clusterManager.Spec.WorkConfiguration.FeatureGates, "work") diff --git a/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go b/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go index 3ad62c0d7..3c5d27359 100644 --- a/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go +++ b/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go @@ -49,6 +49,7 @@ const ( appliedManifestWorkFinalizer = "cluster.open-cluster-management.io/applied-manifest-work-cleanup" spokeRegistrationFeatureGatesInvalid = "InvalidRegistrationFeatureGates" + spokeRegistrationFeatureGatesValid = "ValidRegistrationFeatureGates" ) var ( @@ -239,22 +240,28 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto appliedManifestWorkClient: n.appliedManifestWorkClient, } - // If there are some invalid feature gates of registration, will output condition `InvalidRegistrationFeatureGates` in Klusterlet. + // If there are some invalid feature gates of registration, will output condition `ValidRegistrationFeatureGates` False in Klusterlet. if klusterlet.Spec.RegistrationConfiguration != nil && len(klusterlet.Spec.RegistrationConfiguration.FeatureGates) > 0 { featureGateArgs, invalidFeatureGates := helpers.FeatureGatesArgs( klusterlet.Spec.RegistrationConfiguration.FeatureGates, helpers.ComponentSpokeRegistrationKey) if len(invalidFeatureGates) == 0 { config.RegistrationFeatureGates = featureGateArgs - _, _, _ = helpers.UpdateKlusterletStatus(ctx, n.klusterletClient, klusterletName, helpers.UpdateKlusterletConditionFn(metav1.Condition{ - Type: spokeRegistrationFeatureGatesInvalid, Status: metav1.ConditionTrue, Reason: "FeatureGatesAllValid", - Message: "Registration feature gates of klusterlet are all valid", - })) + // TODO: after the 0.10.0 is released, change back to UpdateKlusterletConditionFn + _, _, _ = helpers.UpdateKlusterletStatus(ctx, n.klusterletClient, klusterletName, helpers.ReplaceKlusterletConditionFn( + spokeRegistrationFeatureGatesInvalid, + metav1.Condition{ + Type: spokeRegistrationFeatureGatesValid, Status: metav1.ConditionTrue, Reason: "FeatureGatesAllValid", + Message: "Registration feature gates of klusterlet are all valid", + })) } else { invalidFGErr := fmt.Errorf("there are some invalid feature gates of registration: %v ", invalidFeatureGates) - _, _, _ = helpers.UpdateKlusterletStatus(ctx, n.klusterletClient, klusterletName, helpers.UpdateKlusterletConditionFn(metav1.Condition{ - Type: spokeRegistrationFeatureGatesInvalid, Status: metav1.ConditionFalse, Reason: "InvalidFeatureGatesExisting", - Message: invalidFGErr.Error(), - })) + // TODO: after the 0.10.0 is released, change back to UpdateKlusterletConditionFn + _, _, _ = helpers.UpdateKlusterletStatus(ctx, n.klusterletClient, klusterletName, helpers.ReplaceKlusterletConditionFn( + spokeRegistrationFeatureGatesInvalid, + metav1.Condition{ + Type: spokeRegistrationFeatureGatesValid, Status: metav1.ConditionFalse, Reason: "InvalidFeatureGatesExisting", + Message: invalidFGErr.Error(), + })) return invalidFGErr } } diff --git a/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go b/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go index a98e1db34..be83f19b9 100644 --- a/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go +++ b/pkg/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go @@ -516,7 +516,7 @@ func TestSyncDeploy(t *testing.T) { testinghelper.AssertOnlyConditions( t, operatorAction[3].(clienttesting.UpdateActionImpl).Object, testinghelper.NamedCondition(klusterletApplied, "KlusterletApplied", metav1.ConditionTrue), - testinghelper.NamedCondition(spokeRegistrationFeatureGatesInvalid, "FeatureGatesAllValid", metav1.ConditionTrue)) + testinghelper.NamedCondition(spokeRegistrationFeatureGatesValid, "FeatureGatesAllValid", metav1.ConditionTrue)) } // TestSyncDeployHosted test deployment of klusterlet components in hosted mode @@ -619,7 +619,7 @@ func TestSyncDeployHosted(t *testing.T) { conditionReady := testinghelper.NamedCondition(klusterletReadyToApply, "KlusterletPrepared", metav1.ConditionTrue) conditionApplied := testinghelper.NamedCondition(klusterletApplied, "KlusterletApplied", metav1.ConditionTrue) - conditionFeaturesValid := testinghelper.NamedCondition(spokeRegistrationFeatureGatesInvalid, "FeatureGatesAllValid", metav1.ConditionTrue) + conditionFeaturesValid := testinghelper.NamedCondition(spokeRegistrationFeatureGatesValid, "FeatureGatesAllValid", metav1.ConditionTrue) testinghelper.AssertOnlyConditions( t, operatorAction[1].(clienttesting.UpdateActionImpl).Object, conditionReady, conditionFeaturesValid) testinghelper.AssertOnlyConditions( @@ -936,7 +936,7 @@ func TestDeployOnKube111(t *testing.T) { testinghelper.AssertOnlyConditions( t, operatorAction[3].(clienttesting.UpdateActionImpl).Object, testinghelper.NamedCondition(klusterletApplied, "KlusterletApplied", metav1.ConditionTrue), - testinghelper.NamedCondition(spokeRegistrationFeatureGatesInvalid, "FeatureGatesAllValid", metav1.ConditionTrue)) + testinghelper.NamedCondition(spokeRegistrationFeatureGatesValid, "FeatureGatesAllValid", metav1.ConditionTrue)) // Delete the klusterlet now := metav1.Now()