diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 21603f0a5..09d0ae06a 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -246,3 +246,47 @@ func FindTaintByKey(managedCluster *clusterv1.ManagedCluster, key string) *clust } return nil } + +func IsTaintEqual(taint1, taint2 clusterv1.Taint) bool { + // Ignore the comparison of time + return taint1.Key == taint2.Key && taint1.Value == taint2.Value && taint1.Effect == taint2.Effect +} + +// AddTaints add taints to the specified slice, if it did not already exist. +// Return a boolean indicating whether the slice has been updated. +func AddTaints(taints *[]clusterv1.Taint, taint clusterv1.Taint) bool { + if taints == nil || *taints == nil { + *taints = make([]clusterv1.Taint, 0) + } + if FindTaint(*taints, taint) != nil { + return false + } + *taints = append(*taints, taint) + return true +} + +func RemoveTaints(taints *[]clusterv1.Taint, targets ...clusterv1.Taint) (updated bool) { + if taints == nil || len(*taints) == 0 || len(targets) == 0 { + return false + } + + newTaints := make([]clusterv1.Taint, 0) + for _, v := range *taints { + if FindTaint(targets, v) == nil { + newTaints = append(newTaints, v) + } + } + updated = len(*taints) != len(newTaints) + *taints = newTaints + return updated +} + +func FindTaint(taints []clusterv1.Taint, taint clusterv1.Taint) *clusterv1.Taint { + for i := range taints { + if IsTaintEqual(taints[i], taint) { + return &taints[i] + } + } + + return nil +} diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index d3bf95baf..30bfb1ac6 100644 --- a/pkg/helpers/helpers_test.go +++ b/pkg/helpers/helpers_test.go @@ -383,3 +383,146 @@ func getApplyFileNames(applyFiles map[string]runtime.Object) []string { } return keys } + +var ( + UnavailableTaint = clusterv1.Taint{ + Key: clusterv1.ManagedClusterTaintUnavailable, + Effect: clusterv1.TaintEffectNoSelect, + } + + UnreachableTaint = clusterv1.Taint{ + Key: clusterv1.ManagedClusterTaintUnreachable, + Effect: clusterv1.TaintEffectNoSelect, + } +) + +func TestIsTaintsEqual(t *testing.T) { + cases := []struct { + name string + taints1 []clusterv1.Taint + taints2 []clusterv1.Taint + expect bool + }{ + { + name: "two empty taints", + taints1: []clusterv1.Taint{}, + taints2: []clusterv1.Taint{}, + expect: true, + }, + { + name: "two nil taints", + taints1: nil, + taints2: nil, + expect: true, + }, + { + name: "len(taints1) = 1, len(taints2) = 0", + taints1: []clusterv1.Taint{UnavailableTaint}, + taints2: []clusterv1.Taint{}, + expect: false, + }, + { + name: "taints1 is the same as taints", + taints1: []clusterv1.Taint{UnreachableTaint}, + taints2: []clusterv1.Taint{UnreachableTaint}, + expect: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + actual := reflect.DeepEqual(c.taints1, c.taints2) + if actual != c.expect { + t.Errorf("expected %t, but %t", c.expect, actual) + } + }) + } +} + +func TestAddTaints(t *testing.T) { + cases := []struct { + name string + taints []clusterv1.Taint + addTaint clusterv1.Taint + resTaints []clusterv1.Taint + expectUpdated bool + }{ + { + name: "add taint success", + taints: []clusterv1.Taint{}, + addTaint: UnreachableTaint, + expectUpdated: true, + resTaints: []clusterv1.Taint{UnreachableTaint}, + }, + { + name: "add taint fail, taint already exists", + taints: []clusterv1.Taint{UnreachableTaint}, + addTaint: UnreachableTaint, + expectUpdated: false, + resTaints: []clusterv1.Taint{UnreachableTaint}, + }, + { + name: "nil pointer judgment", + taints: nil, + addTaint: UnreachableTaint, + expectUpdated: true, + resTaints: []clusterv1.Taint{UnreachableTaint}, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + updated := AddTaints(&c.taints, c.addTaint) + if updated != c.expectUpdated { + t.Errorf("updated expected %t, but %t", c.expectUpdated, updated) + } + if !reflect.DeepEqual(c.taints, c.resTaints) { + t.Errorf("taints expected %+v, but %+v", c.taints, c.resTaints) + } + }) + } +} + +func TestRemoveTaints(t *testing.T) { + cases := []struct { + name string + taints []clusterv1.Taint + removeTaint clusterv1.Taint + resTaints []clusterv1.Taint + expectUpdated bool + }{ + { + name: "nil pointer judgment", + taints: nil, + removeTaint: UnreachableTaint, + expectUpdated: false, + resTaints: nil, + }, + { + name: "remove success", + taints: []clusterv1.Taint{UnreachableTaint}, + removeTaint: UnreachableTaint, + expectUpdated: true, + resTaints: []clusterv1.Taint{}, + }, + { + name: "remove taint failed, taint not exists", + taints: []clusterv1.Taint{UnreachableTaint}, + removeTaint: UnavailableTaint, + expectUpdated: false, + resTaints: []clusterv1.Taint{UnreachableTaint}, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + updated := RemoveTaints(&c.taints, c.removeTaint) + if updated != c.expectUpdated { + t.Errorf("updated expected %t, but %t", c.expectUpdated, updated) + } + if !reflect.DeepEqual(c.taints, c.resTaints) { + t.Errorf("taints expected %+v, but %+v", c.taints, c.resTaints) + } + }) + } +} diff --git a/pkg/helpers/testing/testinghelpers.go b/pkg/helpers/testing/testinghelpers.go index 0214cc387..6d2a37da0 100644 --- a/pkg/helpers/testing/testinghelpers.go +++ b/pkg/helpers/testing/testinghelpers.go @@ -90,14 +90,27 @@ func NewAcceptedManagedCluster() *clusterv1.ManagedCluster { func NewAvailableManagedCluster() *clusterv1.ManagedCluster { managedCluster := NewAcceptedManagedCluster() - availableCondtion := NewManagedClusterCondition( + availableCondition := NewManagedClusterCondition( clusterv1.ManagedClusterConditionAvailable, "True", "ManagedClusterAvailable", "Managed cluster is available", nil, ) - managedCluster.Status.Conditions = append(managedCluster.Status.Conditions, availableCondtion) + managedCluster.Status.Conditions = append(managedCluster.Status.Conditions, availableCondition) + return managedCluster +} + +func NewUnAvailableManagedCluster() *clusterv1.ManagedCluster { + managedCluster := NewAcceptedManagedCluster() + condition := NewManagedClusterCondition( + clusterv1.ManagedClusterConditionAvailable, + "False", + "ManagedClusterUnAvailable", + "Managed cluster is Unavailable", + nil, + ) + managedCluster.Status.Conditions = append(managedCluster.Status.Conditions, condition) return managedCluster } diff --git a/pkg/hub/manager.go b/pkg/hub/manager.go index 67833d41a..0335ad242 100644 --- a/pkg/hub/manager.go +++ b/pkg/hub/manager.go @@ -2,6 +2,7 @@ package hub import ( "context" + "open-cluster-management.io/registration/pkg/hub/taint" "time" addonclient "open-cluster-management.io/api/client/addon/clientset/versioned" @@ -25,6 +26,8 @@ import ( "k8s.io/client-go/rest" ) +var ResyncInterval = 5 * time.Minute + // RunControllerManager starts the controllers on hub to manage spoke cluster registration. func RunControllerManager(ctx context.Context, controllerContext *controllercmd.ControllerContext) error { // If qps in kubconfig is not set, increase the qps and burst to enhance the ability of kube client to handle @@ -68,6 +71,12 @@ func RunControllerManager(ctx context.Context, controllerContext *controllercmd. controllerContext.EventRecorder, ) + taintController := taint.NewTaintController( + clusterClient, + clusterInformers.Cluster().V1().ManagedClusters(), + controllerContext.EventRecorder, + ) + csrController := csr.NewCSRApprovingController( kubeClient, kubeInfomers.Certificates().V1().CertificateSigningRequests(), @@ -79,7 +88,7 @@ func RunControllerManager(ctx context.Context, controllerContext *controllercmd. clusterClient, clusterInformers.Cluster().V1().ManagedClusters(), kubeInfomers.Coordination().V1().Leases(), - 5*time.Minute, //TODO: this interval time should be allowed to change from outside + ResyncInterval, //TODO: this interval time should be allowed to change from outside controllerContext.EventRecorder, ) @@ -127,6 +136,7 @@ func RunControllerManager(ctx context.Context, controllerContext *controllercmd. go addOnInformers.Start(ctx.Done()) go managedClusterController.Run(ctx, 1) + go taintController.Run(ctx, 1) go csrController.Run(ctx, 1) go leaseController.Run(ctx, 1) go rbacFinalizerController.Run(ctx, 1) diff --git a/pkg/hub/taint/controller.go b/pkg/hub/taint/controller.go new file mode 100644 index 000000000..1e596ec39 --- /dev/null +++ b/pkg/hub/taint/controller.go @@ -0,0 +1,96 @@ +package taint + +import ( + "context" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/klog/v2" + clientset "open-cluster-management.io/api/client/cluster/clientset/versioned" + informerv1 "open-cluster-management.io/api/client/cluster/informers/externalversions/cluster/v1" + listerv1 "open-cluster-management.io/api/client/cluster/listers/cluster/v1" + v1 "open-cluster-management.io/api/cluster/v1" + "open-cluster-management.io/registration/pkg/helpers" +) + +var ( + UnavailableTaint = v1.Taint{ + Key: v1.ManagedClusterTaintUnavailable, + Effect: v1.TaintEffectNoSelect, + } + + UnreachableTaint = v1.Taint{ + Key: v1.ManagedClusterTaintUnreachable, + Effect: v1.TaintEffectNoSelect, + } +) + +// taintController +type taintController struct { + clusterClient clientset.Interface + clusterLister listerv1.ManagedClusterLister + eventRecorder events.Recorder +} + +// NewTaintController creates a new taint controller +func NewTaintController( + clusterClient clientset.Interface, + clusterInformer informerv1.ManagedClusterInformer, + recorder events.Recorder) factory.Controller { + c := &taintController{ + clusterClient: clusterClient, + clusterLister: clusterInformer.Lister(), + eventRecorder: recorder.WithComponentSuffix("taint-controller"), + } + return factory.New(). + WithInformersQueueKeyFunc(func(obj runtime.Object) string { + accessor, _ := meta.Accessor(obj) + return accessor.GetName() + }, clusterInformer.Informer()). + WithSync(c.sync). + ToController("taintController", recorder) +} + +func (c *taintController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + managedClusterName := syncCtx.QueueKey() + klog.V(4).Infof("Reconciling ManagedCluster %s", managedClusterName) + managedCluster, err := c.clusterLister.Get(managedClusterName) + if errors.IsNotFound(err) { + // Spoke cluster not found, could have been deleted, do nothing. + return nil + } + if err != nil { + return err + } + if !managedCluster.DeletionTimestamp.IsZero() { + return nil + } + + managedCluster = managedCluster.DeepCopy() + newTaints := managedCluster.Spec.Taints + cond := meta.FindStatusCondition(managedCluster.Status.Conditions, v1.ManagedClusterConditionAvailable) + var updated bool + + switch { + case cond == nil || cond.Status == metav1.ConditionUnknown: + updated = helpers.RemoveTaints(&newTaints, UnavailableTaint) + updated = helpers.AddTaints(&newTaints, UnreachableTaint) || updated + case cond.Status == metav1.ConditionFalse: + updated = helpers.RemoveTaints(&newTaints, UnreachableTaint) + updated = helpers.AddTaints(&newTaints, UnavailableTaint) || updated + case cond.Status == metav1.ConditionTrue: + updated = helpers.RemoveTaints(&newTaints, UnavailableTaint, UnreachableTaint) + } + + if updated { + managedCluster.Spec.Taints = newTaints + if _, err = c.clusterClient.ClusterV1().ManagedClusters().Update(ctx, managedCluster, metav1.UpdateOptions{}); err != nil { + return err + } + c.eventRecorder.Eventf("ManagedClusterConditionAvailableUpdated", "Update the original taints to the %+v", newTaints) + } + return nil +} diff --git a/pkg/hub/taint/controller_test.go b/pkg/hub/taint/controller_test.go new file mode 100644 index 000000000..d822c5efa --- /dev/null +++ b/pkg/hub/taint/controller_test.go @@ -0,0 +1,96 @@ +package taint + +import ( + "context" + v1 "open-cluster-management.io/api/cluster/v1" + "reflect" + "testing" + "time" + + clusterfake "open-cluster-management.io/api/client/cluster/clientset/versioned/fake" + clusterinformers "open-cluster-management.io/api/client/cluster/informers/externalversions" + testinghelpers "open-cluster-management.io/registration/pkg/helpers/testing" + + "github.com/openshift/library-go/pkg/operator/events/eventstesting" + + "k8s.io/apimachinery/pkg/runtime" + clienttesting "k8s.io/client-go/testing" +) + +func TestSyncTaintCluster(t *testing.T) { + cases := []struct { + name string + startingObjects []runtime.Object + validateActions func(t *testing.T, actions []clienttesting.Action) + }{ + { + name: "ManagedClusterConditionAvailable conditionStatus is True", + startingObjects: []runtime.Object{testinghelpers.NewAvailableManagedCluster()}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertNoActions(t, actions) + }, + }, + { + name: "ManagedClusterConditionAvailable conditionStatus is False", + startingObjects: []runtime.Object{testinghelpers.NewUnAvailableManagedCluster()}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertActions(t, actions, "update") + managedCluster := (actions[0].(clienttesting.UpdateActionImpl).Object).(*v1.ManagedCluster) + taints := []v1.Taint{UnavailableTaint} + if !reflect.DeepEqual(managedCluster.Spec.Taints, taints) { + t.Errorf("expected taint %#v, but actualTaints: %#v", taints, managedCluster.Spec.Taints) + } + }, + }, + { + name: "There is no ManagedClusterConditionAvailable", + startingObjects: []runtime.Object{testinghelpers.NewManagedCluster()}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertActions(t, actions, "update") + managedCluster := (actions[0].(clienttesting.UpdateActionImpl).Object).(*v1.ManagedCluster) + taints := []v1.Taint{UnreachableTaint} + if !reflect.DeepEqual(managedCluster.Spec.Taints, taints) { + t.Errorf("expected taint %#v, but actualTaints: %#v", taints, managedCluster.Spec.Taints) + } + }, + }, + { + name: "ManagedClusterConditionAvailable conditionStatus is Unknown", + startingObjects: []runtime.Object{testinghelpers.NewUnknownManagedCluster()}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertActions(t, actions, "update") + managedCluster := (actions[0].(clienttesting.UpdateActionImpl).Object).(*v1.ManagedCluster) + taints := []v1.Taint{UnreachableTaint} + if !reflect.DeepEqual(managedCluster.Spec.Taints, taints) { + t.Errorf("expected taint %#v, but actualTaints: %#v", taints, managedCluster.Spec.Taints) + } + }, + }, + { + name: "sync a deleted spoke cluster", + startingObjects: []runtime.Object{}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertNoActions(t, actions) + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + clusterClient := clusterfake.NewSimpleClientset(c.startingObjects...) + clusterInformerFactory := clusterinformers.NewSharedInformerFactory(clusterClient, time.Minute*10) + clusterStore := clusterInformerFactory.Cluster().V1().ManagedClusters().Informer().GetStore() + for _, cluster := range c.startingObjects { + clusterStore.Add(cluster) + } + + ctrl := taintController{clusterClient, clusterInformerFactory.Cluster().V1().ManagedClusters().Lister(), eventstesting.NewTestingEventRecorder(t)} + syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, testinghelpers.TestManagedClusterName)) + if syncErr != nil { + t.Errorf("unexpected err: %v", syncErr) + } + + c.validateActions(t, clusterClient.Actions()) + }) + } +} diff --git a/pkg/hub/taint/doc.go b/pkg/hub/taint/doc.go new file mode 100644 index 000000000..c0856ea8a --- /dev/null +++ b/pkg/hub/taint/doc.go @@ -0,0 +1 @@ +package taint diff --git a/test/integration/integration_suite_test.go b/test/integration/integration_suite_test.go index a176683df..62eeae9c3 100644 --- a/test/integration/integration_suite_test.go +++ b/test/integration/integration_suite_test.go @@ -74,6 +74,7 @@ var _ = ginkgo.BeforeSuite(func(done ginkgo.Done) { transport.CertCallbackRefreshDuration = 5 * time.Second clientcert.ControllerResyncInterval = 5 * time.Second managedcluster.CreatingControllerSyncInterval = 1 * time.Second + hub.ResyncInterval = 5 * time.Second // crank up the addon lease sync and udpate speed spoke.AddOnLeaseControllerSyncInterval = 5 * time.Second diff --git a/test/integration/taint_add_test.go b/test/integration/taint_add_test.go new file mode 100644 index 000000000..38eaadeba --- /dev/null +++ b/test/integration/taint_add_test.go @@ -0,0 +1,148 @@ +package integration_test + +import ( + "context" + "fmt" + "github.com/onsi/ginkgo" + "github.com/onsi/gomega" + "github.com/openshift/library-go/pkg/controller/controllercmd" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/rand" + v1 "open-cluster-management.io/api/cluster/v1" + "open-cluster-management.io/registration/pkg/helpers" + "open-cluster-management.io/registration/pkg/hub/taint" + "open-cluster-management.io/registration/pkg/spoke" + "open-cluster-management.io/registration/test/integration/util" + "path" + "time" +) + +var _ = ginkgo.Describe("ManagedCluster Taints Update", func() { + var managedClusterName string + var hubKubeconfigSecret string + var hubKubeconfigDir string + + ginkgo.BeforeEach(func() { + managedClusterName = fmt.Sprintf("managedcluster-%s", rand.String(6)) + hubKubeconfigSecret = fmt.Sprintf("%s-secret", managedClusterName) + hubKubeconfigDir = path.Join(util.TestDir, "leasetest", fmt.Sprintf("%s-config", managedClusterName)) + }) + + ginkgo.It("ManagedCluster taint should be updated automatically", func() { + ctx, stop := context.WithCancel(context.Background()) + // run registration agent + go func() { + agentOptions := spoke.SpokeAgentOptions{ + ClusterName: managedClusterName, + BootstrapKubeconfig: bootstrapKubeConfigFile, + HubKubeconfigSecret: hubKubeconfigSecret, + HubKubeconfigDir: hubKubeconfigDir, + ClusterHealthCheckPeriod: 1 * time.Minute, + } + err := agentOptions.RunSpokeAgent(ctx, &controllercmd.ControllerContext{ + KubeConfig: spokeCfg, + EventRecorder: util.NewIntegrationTestEventRecorder("cluster-tainttest"), + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }() + + gomega.Eventually(func() error { + spokeCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return err + } + spokeCluster.Spec.HubAcceptsClient = true + _, err = clusterClient.ClusterV1().ManagedClusters().Update(context.TODO(), spokeCluster, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + + gomega.Eventually(func() error { + managedCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return err + } + if len(managedCluster.Spec.Taints) != 1 { + return fmt.Errorf("managedCluster taints len is not 1") + } + if !helpers.IsTaintEqual(managedCluster.Spec.Taints[0], taint.UnreachableTaint) { + return fmt.Errorf("the %+v is not equal to UnreachableTaint", managedCluster.Spec.Taints[0]) + } + return nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil()) + + gomega.Eventually(func() error { + if err := authn.ApproveSpokeClusterCSR(kubeClient, managedClusterName, time.Hour*24); err != nil { + return err + } + return nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil()) + + // The managed cluster is available, so taint is expected to be empty + gomega.Eventually(func() bool { + managedCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return false + } + if len(managedCluster.Spec.Taints) != 0 { + return false + } + return true + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + stop() + + gomega.Eventually(func() error { + managedCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return err + } + condition := metav1.Condition{ + Type: v1.ManagedClusterConditionAvailable, + Status: metav1.ConditionFalse, + Reason: "ForTest", + } + meta.SetStatusCondition(&(managedCluster.Status.Conditions), condition) + _, err = clusterClient.ClusterV1().ManagedClusters().UpdateStatus(context.Background(), managedCluster, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil()) + + gomega.Eventually(func() error { + managedCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return err + } + if len(managedCluster.Spec.Taints) != 1 { + return fmt.Errorf("managedCluster taints len is not 1") + } + if !helpers.IsTaintEqual(managedCluster.Spec.Taints[0], taint.UnavailableTaint) { + return fmt.Errorf("the %+v is not equal to UnavailableTaint", managedCluster.Spec.Taints[0]) + } + return nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil()) + + gomega.Eventually(func() error { + spokeCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return err + } + spokeCluster.Spec.LeaseDurationSeconds = 1 + _, err = clusterClient.ClusterV1().ManagedClusters().Update(context.TODO(), spokeCluster, metav1.UpdateOptions{}) + return err + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + + gomega.Eventually(func() error { + managedCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return err + } + if len(managedCluster.Spec.Taints) != 1 { + return fmt.Errorf("managedCluster taints len is not 1") + } + if !helpers.IsTaintEqual(managedCluster.Spec.Taints[0], taint.UnreachableTaint) { + return fmt.Errorf("the %+v is not equal to UnreachableTaint", managedCluster.Spec.Taints[0]) + } + return nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil()) + }) +})