diff --git a/pkg/controllers/scheduling/cluster_event_handler.go b/pkg/controllers/scheduling/cluster_event_handler.go index 5ae4f6c36..e5eee2e86 100644 --- a/pkg/controllers/scheduling/cluster_event_handler.go +++ b/pkg/controllers/scheduling/cluster_event_handler.go @@ -2,14 +2,15 @@ package scheduling import ( "fmt" + "reflect" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" cache "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" clusterlisterv1beta1 "open-cluster-management.io/api/client/cluster/listers/cluster/v1beta1" clusterapiv1 "open-cluster-management.io/api/cluster/v1" + clusterapiv1beta1 "open-cluster-management.io/api/cluster/v1beta1" ) type clusterEventHandler struct { @@ -38,8 +39,8 @@ func (h *clusterEventHandler) OnUpdate(oldObj, newObj interface{}) { return } - // if the clusterset of the cluster changes, process the original clusterset - if newCluster.Labels[clusterSetLabel] != oldCluster.Labels[clusterSetLabel] { + // if the cluster labels changes, process the original clusterset + if !reflect.DeepEqual(newCluster.Labels, oldCluster.Labels) { h.onChange(oldCluster) } } @@ -62,46 +63,43 @@ func (h *clusterEventHandler) onChange(obj interface{}) { return } - clusterSetName, err := h.getClusterSetName(cluster) + clusterSetNames, err := h.getClusterSetNames(cluster) if err != nil { klog.V(4).Infof("Unable to get clusterset of cluster %q: %v", cluster.GetName(), err) return } // skip cluster belongs to no clusterset - if len(clusterSetName) == 0 { + if len(clusterSetNames) == 0 { return } - // enqueue placements which might be impacted - err = enqueuePlacementsByClusterSet( - clusterSetName, - h.clusterSetBindingLister, - h.placementLister, - h.enqueuePlacementFunc, - ) - if err != nil { - klog.Errorf("Unable to enqueue placements with access to clusterset %q: %v", clusterSetName, err) + for _, clusterSetName := range clusterSetNames { + // enqueue placements which might be impacted + err = enqueuePlacementsByClusterSet( + clusterSetName, + h.clusterSetBindingLister, + h.placementLister, + h.enqueuePlacementFunc, + ) + if err != nil { + klog.Errorf("Unable to enqueue placements with access to clusterset %q: %v", clusterSetName, err) + } } } // getClusterSetName returns the name of the clusterset the cluster belongs to. It also checks the existence // of the clusterset. -func (h *clusterEventHandler) getClusterSetName(cluster metav1.Object) (string, error) { - // skip cluster belongs to no clusterset - labels := cluster.GetLabels() - clusterSetName, ok := labels[clusterSetLabel] - if !ok { - return "", nil - } - _, err := h.clusterSetLister.Get(clusterSetName) - // skip if the clusterset does not exist - if errors.IsNotFound(err) { - return "", nil - } +func (h *clusterEventHandler) getClusterSetNames(cluster metav1.Object) ([]string, error) { + clusterSetNames := []string{} + clusterSets, err := clusterapiv1beta1.GetClusterSetsOfCluster(cluster.(*clusterapiv1.ManagedCluster), h.clusterSetLister) if err != nil { - return "", err + return clusterSetNames, err } - return clusterSetName, nil + for _, cs := range clusterSets { + clusterSetNames = append(clusterSetNames, cs.Name) + } + + return clusterSetNames, nil } diff --git a/pkg/controllers/scheduling/cluster_event_handler_test.go b/pkg/controllers/scheduling/cluster_event_handler_test.go index 100f1fb6f..e8e757828 100644 --- a/pkg/controllers/scheduling/cluster_event_handler_test.go +++ b/pkg/controllers/scheduling/cluster_event_handler_test.go @@ -5,11 +5,13 @@ import ( "strings" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/cache" clusterfake "open-cluster-management.io/api/client/cluster/clientset/versioned/fake" + clusterapiv1beta1 "open-cluster-management.io/api/cluster/v1beta1" testinghelpers "open-cluster-management.io/placement/pkg/helpers/testing" ) @@ -55,6 +57,36 @@ func TestOnClusterChange(t *testing.T) { "ns2/placement2", }, }, + { + name: "cluster blongs to multiple clusterset", + obj: testinghelpers.NewManagedCluster("cluster1").WithLabel("cloud", "Amazon").WithLabel(clusterSetLabel, "clusterset2").Build(), + initObjs: []runtime.Object{ + testinghelpers.NewClusterSet("global").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{}, + }).Build(), + testinghelpers.NewClusterSet("clusterset1").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cloud": "Amazon", + }, + }, + }).Build(), + testinghelpers.NewClusterSet("clusterset2").Build(), + testinghelpers.NewClusterSetBinding("ns1", "global"), + testinghelpers.NewClusterSetBinding("ns2", "clusterset1"), + testinghelpers.NewClusterSetBinding("ns3", "clusterset2"), + testinghelpers.NewPlacement("ns1", "placement1").Build(), + testinghelpers.NewPlacement("ns2", "placement2").Build(), + testinghelpers.NewPlacement("ns3", "placement3").Build(), + }, + queuedKeys: []string{ + "ns1/placement1", + "ns2/placement2", + "ns3/placement3", + }, + }, } for _, c := range cases { @@ -93,6 +125,42 @@ func TestOnClusterUpdate(t *testing.T) { name: "cluster belongs to no clusterset", newObj: testinghelpers.NewManagedCluster("cluster1").WithLabel("cloud", "Amazon").Build(), oldObj: testinghelpers.NewManagedCluster("cluster1").WithLabel("cloud", "Google").Build(), + initObjs: []runtime.Object{ + testinghelpers.NewClusterSet("clusterset1").Build(), + testinghelpers.NewClusterSetBinding("ns1", "clusterset1"), + testinghelpers.NewPlacement("ns1", "placement1").Build(), + }, + }, + { + name: "cluster blongs to multiple clusterset", + newObj: testinghelpers.NewManagedCluster("cluster1").WithLabel(clusterSetLabel, "clusterset2").WithLabel("cloud", "Google").Build(), + oldObj: testinghelpers.NewManagedCluster("cluster1").WithLabel(clusterSetLabel, "clusterset2").WithLabel("cloud", "Amazon").Build(), + initObjs: []runtime.Object{ + testinghelpers.NewClusterSet("global").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{}, + }).Build(), + testinghelpers.NewClusterSet("clusterset1").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cloud": "Amazon", + }, + }, + }).Build(), + testinghelpers.NewClusterSet("clusterset2").Build(), + testinghelpers.NewClusterSetBinding("ns1", "global"), + testinghelpers.NewClusterSetBinding("ns2", "clusterset1"), + testinghelpers.NewClusterSetBinding("ns3", "clusterset2"), + testinghelpers.NewPlacement("ns1", "placement1").Build(), + testinghelpers.NewPlacement("ns2", "placement2").Build(), + testinghelpers.NewPlacement("ns3", "placement3").Build(), + }, + queuedKeys: []string{ + "ns1/placement1", + "ns2/placement2", + "ns3/placement3", + }, }, { name: "assign a cluster to a clusterset", @@ -110,16 +178,33 @@ func TestOnClusterUpdate(t *testing.T) { }, { name: "remove cluster from a clusterset", - newObj: testinghelpers.NewManagedCluster("cluster1").WithLabel("cloud", "Amazon").Build(), - oldObj: testinghelpers.NewManagedCluster("cluster1"). - WithLabel(clusterSetLabel, "clusterset1").WithLabel("cloud", "Amazon").Build(), + newObj: testinghelpers.NewManagedCluster("cluster1").Build(), + oldObj: testinghelpers.NewManagedCluster("cluster1").WithLabel(clusterSetLabel, "clusterset2").WithLabel("cloud", "Amazon").Build(), initObjs: []runtime.Object{ - testinghelpers.NewClusterSet("clusterset1").Build(), - testinghelpers.NewClusterSetBinding("ns1", "clusterset1"), + testinghelpers.NewClusterSet("global").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{}, + }).Build(), + testinghelpers.NewClusterSet("clusterset1").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cloud": "Amazon", + }, + }, + }).Build(), + testinghelpers.NewClusterSet("clusterset2").Build(), + testinghelpers.NewClusterSetBinding("ns1", "global"), + testinghelpers.NewClusterSetBinding("ns2", "clusterset1"), + testinghelpers.NewClusterSetBinding("ns3", "clusterset2"), testinghelpers.NewPlacement("ns1", "placement1").Build(), + testinghelpers.NewPlacement("ns2", "placement2").Build(), + testinghelpers.NewPlacement("ns3", "placement3").Build(), }, queuedKeys: []string{ "ns1/placement1", + "ns2/placement2", + "ns3/placement3", }, }, { @@ -195,14 +280,32 @@ func TestOnClusterDelete(t *testing.T) { }, { name: "cluster", - obj: testinghelpers.NewManagedCluster("cluster1").WithLabel(clusterSetLabel, "clusterset1").Build(), + obj: testinghelpers.NewManagedCluster("cluster1").WithLabel(clusterSetLabel, "clusterset2").WithLabel("cloud", "Amazon").Build(), initObjs: []runtime.Object{ - testinghelpers.NewClusterSet("clusterset1").Build(), - testinghelpers.NewClusterSetBinding("ns1", "clusterset1"), + testinghelpers.NewClusterSet("global").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{}, + }).Build(), + testinghelpers.NewClusterSet("clusterset1").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cloud": "Amazon", + }, + }, + }).Build(), + testinghelpers.NewClusterSet("clusterset2").Build(), + testinghelpers.NewClusterSetBinding("ns1", "global"), + testinghelpers.NewClusterSetBinding("ns2", "clusterset1"), + testinghelpers.NewClusterSetBinding("ns3", "clusterset2"), testinghelpers.NewPlacement("ns1", "placement1").Build(), + testinghelpers.NewPlacement("ns2", "placement2").Build(), + testinghelpers.NewPlacement("ns3", "placement3").Build(), }, queuedKeys: []string{ "ns1/placement1", + "ns2/placement2", + "ns3/placement3", }, }, { diff --git a/pkg/controllers/scheduling/clusterset_event_handler_test.go b/pkg/controllers/scheduling/clusterset_event_handler_test.go index 536d0a45a..deec8d0ff 100644 --- a/pkg/controllers/scheduling/clusterset_event_handler_test.go +++ b/pkg/controllers/scheduling/clusterset_event_handler_test.go @@ -5,11 +5,13 @@ import ( "strings" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" clusterfake "open-cluster-management.io/api/client/cluster/clientset/versioned/fake" + clusterapiv1beta1 "open-cluster-management.io/api/cluster/v1beta1" testinghelpers "open-cluster-management.io/placement/pkg/helpers/testing" ) @@ -75,7 +77,7 @@ func TestOnClusterSetAdd(t *testing.T) { obj: "invalid object type", }, { - name: "clusterset", + name: "clusterset selector type is LegacyClusterSetLabel", obj: testinghelpers.NewClusterSet("clusterset1").Build(), initObjs: []runtime.Object{ testinghelpers.NewClusterSetBinding("ns1", "clusterset1"), @@ -85,6 +87,24 @@ func TestOnClusterSetAdd(t *testing.T) { "ns1/placement1", }, }, + { + name: "clusterset selector type is LabelSelector", + obj: testinghelpers.NewClusterSet("clusterset1").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cloud": "Amazon", + }, + }, + }).Build(), + initObjs: []runtime.Object{ + testinghelpers.NewClusterSetBinding("ns1", "clusterset1"), + testinghelpers.NewPlacement("ns1", "placement1").Build(), + }, + queuedKeys: []string{ + "ns1/placement1", + }, + }, } for _, c := range cases { @@ -122,7 +142,7 @@ func TestOnClusterSetDelete(t *testing.T) { obj: "invalid object type", }, { - name: "clusterset", + name: "clusterset selector type is LegacyClusterSetLabel", obj: testinghelpers.NewClusterSet("clusterset1").Build(), initObjs: []runtime.Object{ testinghelpers.NewClusterSetBinding("ns1", "clusterset1"), @@ -132,6 +152,24 @@ func TestOnClusterSetDelete(t *testing.T) { "ns1/placement1", }, }, + { + name: "clusterset selector type is LabelSelector", + obj: testinghelpers.NewClusterSet("clusterset1").WithClusterSelector(clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "cloud": "Amazon", + }, + }, + }).Build(), + initObjs: []runtime.Object{ + testinghelpers.NewClusterSetBinding("ns1", "clusterset1"), + testinghelpers.NewPlacement("ns1", "placement1").Build(), + }, + queuedKeys: []string{ + "ns1/placement1", + }, + }, { name: "tombstone", obj: cache.DeletedFinalStateUnknown{ diff --git a/test/e2e/placement_test.go b/test/e2e/placement_test.go index 808dc125e..ae0ec30ee 100644 --- a/test/e2e/placement_test.go +++ b/test/e2e/placement_test.go @@ -24,6 +24,7 @@ var _ = ginkgo.Describe("Placement", func() { var namespace string var placementName string var clusterSet1Name string + var clusterSetGlobal string var suffix string var err error @@ -32,6 +33,7 @@ var _ = ginkgo.Describe("Placement", func() { namespace = fmt.Sprintf("ns-%s", suffix) placementName = fmt.Sprintf("placement-%s", suffix) clusterSet1Name = fmt.Sprintf("clusterset-%s", suffix) + clusterSetGlobal = "global" // create testing namespace ns := &corev1.Namespace{ @@ -46,6 +48,7 @@ var _ = ginkgo.Describe("Placement", func() { ginkgo.AfterEach(func() { ginkgo.By("Delete managedclusterset") clusterClient.ClusterV1beta1().ManagedClusterSets().Delete(context.Background(), clusterSet1Name, metav1.DeleteOptions{}) + clusterClient.ClusterV1beta1().ManagedClusterSets().Delete(context.Background(), clusterSetGlobal, metav1.DeleteOptions{}) ginkgo.By("Delete managedclusters") clusterClient.ClusterV1().ManagedClusters().DeleteCollection(context.Background(), metav1.DeleteOptions{}, metav1.ListOptions{ @@ -127,16 +130,27 @@ var _ = ginkgo.Describe("Placement", func() { }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) } - assertBindingClusterSet := func(clusterSetName string) { - ginkgo.By("Create clusterset/clustersetbinding") + assertCreatingClusterSet := func(clusterSetName string, matchLabel map[string]string) { + ginkgo.By("Create clusterset") clusterset := &clusterapiv1beta1.ManagedClusterSet{ ObjectMeta: metav1.ObjectMeta{ Name: clusterSetName, }, } + if matchLabel != nil { + clusterset.Spec.ClusterSelector = clusterapiv1beta1.ManagedClusterSelector{ + SelectorType: clusterapiv1beta1.LabelSelector, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: matchLabel, + }, + } + } _, err = clusterClient.ClusterV1beta1().ManagedClusterSets().Create(context.Background(), clusterset, metav1.CreateOptions{}) gomega.Expect(err).ToNot(gomega.HaveOccurred()) + } + assertCreatingClusterSetBinding := func(clusterSetName string) { + ginkgo.By("Create clustersetbinding") csb := &clusterapiv1beta1.ManagedClusterSetBinding{ ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, @@ -150,6 +164,12 @@ var _ = ginkgo.Describe("Placement", func() { gomega.Expect(err).ToNot(gomega.HaveOccurred()) } + assertBindingClusterSet := func(clusterSetName string, matchLabel map[string]string) { + ginkgo.By("Create clusterset/clustersetbinding") + assertCreatingClusterSet(clusterSetName, matchLabel) + assertCreatingClusterSetBinding(clusterSetName) + } + assertCreatingClusters := func(clusterSetName string, num int) { ginkgo.By(fmt.Sprintf("Create %d clusters", num)) for i := 0; i < num; i++ { @@ -188,7 +208,7 @@ var _ = ginkgo.Describe("Placement", func() { } ginkgo.It("Should schedule successfully", func() { - assertBindingClusterSet(clusterSet1Name) + assertBindingClusterSet(clusterSet1Name, nil) assertCreatingClusters(clusterSet1Name, 5) assertCreatingPlacement(placementName, noc(10), 5) @@ -203,8 +223,10 @@ var _ = ginkgo.Describe("Placement", func() { assertNumberOfDecisions(placementName, 5) assertPlacementStatus(placementName, 5, false) - // create 2 more clusters - assertCreatingClusters(clusterSet1Name, 2) + // create global clusterset + assertBindingClusterSet(clusterSetGlobal, map[string]string{}) + // create 2 more clusters belong to global clusterset + assertCreatingClusters("", 2) assertNumberOfDecisions(placementName, 6) assertPlacementStatus(placementName, 6, true) @@ -226,7 +248,7 @@ var _ = ginkgo.Describe("Placement", func() { }) ginkgo.It("Should delete placementdecision successfully", func() { - assertBindingClusterSet(clusterSet1Name) + assertBindingClusterSet(clusterSet1Name, nil) assertCreatingClusters(clusterSet1Name, 1) assertCreatingPlacement(placementName, nil, 1) diff --git a/test/integration/placement_test.go b/test/integration/placement_test.go index 1a03e97ef..8fb39a6f3 100644 --- a/test/integration/placement_test.go +++ b/test/integration/placement_test.go @@ -667,6 +667,30 @@ var _ = ginkgo.Describe("Placement", func() { assertPlacementConditionSatisfied(placementName, nod, true) }) + ginkgo.It("Should be satisfied once new clusters belong to labelselector type clusterset are added", func() { + ginkgo.By("Bind clusterset to the placement namespace") + assertCreatingLabelSelectorClusterSet(clusterSet1Name, map[string]string{ + "vendor": "openShift", + }) + assertCreatingClusterSetBinding(clusterSet1Name) + assertCreatingClusterWithLabel(clusterName+"1", map[string]string{ + "vendor": "openShift", + }) + assertCreatingPlacementWithDecision(placementName, noc(2), 1, clusterapiv1beta1.PrioritizerPolicy{}, []clusterapiv1beta1.Toleration{}) + + // add more clusters + assertCreatingClusterWithLabel(clusterName+"2", map[string]string{ + "vendor": "openShift", + }) + + nod := 2 + assertNumberOfDecisions(placementName, nod) + assertPlacementConditionSatisfied(placementName, nod, true) + + assertDeletingCluster(clusterName + "1") + assertDeletingCluster(clusterName + "2") + }) + ginkgo.It("Should schedule successfully once new clusterset is bound", func() { assertBindingClusterSet(clusterSet1Name) assertCreatingClusters(clusterSet1Name, 5) @@ -681,6 +705,34 @@ var _ = ginkgo.Describe("Placement", func() { assertPlacementConditionSatisfied(placementName, nod, false) }) + ginkgo.It("Should schedule successfully once new labelselector type clusterset is bound", func() { + ginkgo.By("Bind clusterset to the placement namespace") + assertCreatingLabelSelectorClusterSet(clusterSet1Name, map[string]string{ + "vendor": "openShift", + }) + assertCreatingClusterSetBinding(clusterSet1Name) + assertCreatingClusterWithLabel(clusterName+"1", map[string]string{ + "vendor": "openShift", + }) + assertCreatingPlacementWithDecision(placementName, noc(2), 1, clusterapiv1beta1.PrioritizerPolicy{}, []clusterapiv1beta1.Toleration{}) + + ginkgo.By("Bind one more labelselector clusterset to the placement namespace") + assertCreatingLabelSelectorClusterSet(clusterSet2Name, map[string]string{ + "vendor": "IKS", + }) + assertCreatingClusterSetBinding(clusterSet2Name) + assertCreatingClusterWithLabel(clusterName+"2", map[string]string{ + "vendor": "IKS", + }) + + nod := 2 + assertNumberOfDecisions(placementName, nod) + assertPlacementConditionSatisfied(placementName, nod, true) + + assertDeletingCluster(clusterName + "1") + assertDeletingCluster(clusterName + "2") + }) + ginkgo.It("Should create multiple placementdecisions once scheduled", func() { assertBindingClusterSet(clusterSet1Name) assertCreatingClusters(clusterSet1Name, 101) @@ -981,7 +1033,7 @@ var _ = ginkgo.Describe("Placement", func() { assertClusterNamesOfDecisions(placementName, []string{clusterNames[0], clusterNames[2]}) }) - ginkgo.It("Should re-schedule successfully once a new cluster added/deleted", func() { + ginkgo.It("Should re-schedule successfully once a new cluster with resources added/deleted", func() { // cluster settings clusterNames := []string{ clusterName + "-1", @@ -1066,10 +1118,10 @@ var _ = ginkgo.Describe("Placement", func() { "vendor": "openShift", }) assertCreatingClusterSetBinding(clusterSet1Name) - assertCreatingClusterWithLabel("cluster1", map[string]string{ + assertCreatingClusterWithLabel(clusterName+"1", map[string]string{ "vendor": "openShift", }) - assertCreatingClusterWithLabel("cluster2", map[string]string{ + assertCreatingClusterWithLabel(clusterName+"2", map[string]string{ "vendor": "IKS", }) assertCreatingPlacementWithDecision(placementName, noc(10), 1, clusterapiv1beta1.PrioritizerPolicy{}, []clusterapiv1beta1.Toleration{}) @@ -1088,6 +1140,12 @@ var _ = ginkgo.Describe("Placement", func() { }) assertNumberOfDecisions(placementName, 1) assertPlacementConditionSatisfied(placementName, 1, false) + + ginkgo.By("Delete the cluster") + assertDeletingCluster(clusterName + "1") + assertNumberOfDecisions(placementName, 0) + + assertDeletingCluster(clusterName + "2") }) ginkgo.It("Should re-schedule successfully once a clustersetbinding deleted/added", func() {