create placementdecision when misconfigured

Signed-off-by: haoqing0110 <qhao@redhat.com>
open-cluster-management-io · Dec 26, 2022 · b33c54e · b33c54e
1 parent 6c92d85
commit b33c54e
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 20 deletions.
diff --git a/pkg/controllers/framework/interface.go b/pkg/controllers/framework/interface.go
@@ -93,7 +93,7 @@ func NewStatus(plugin string, code Code, reasons ...string) *Status {
 		reasons: reasons,
 		plugin:  plugin,
 	}
-	if code == Error {
+	if s.IsError() {
 		s.err = errors.New(s.Message())
 	}
 	return s

diff --git a/pkg/controllers/scheduling/scheduling_controller.go b/pkg/controllers/scheduling/scheduling_controller.go
@@ -238,14 +238,6 @@ func (c *schedulingController) syncPlacement(ctx context.Context, syncCtx factor
 		status,
 	)
 
-	// update status and return error if schedule() returns error
-	if status.IsError() {
-		if err := c.updateStatus(ctx, placement, int32(len(scheduleResult.Decisions())), misconfiguredCondition, satisfiedCondition); err != nil {
-			return err
-		}
-		return status.AsError()
-	}
-
 	// requeue placement if requeueAfter is defined in scheduleResult
 	if syncCtx != nil && scheduleResult.RequeueAfter() != nil {
 		key, _ := cache.MetaNamespaceKeyFunc(placement)
@@ -254,13 +246,16 @@ func (c *schedulingController) syncPlacement(ctx context.Context, syncCtx factor
 		syncCtx.Queue().AddAfter(key, *t)
 	}
 
-	err = c.bind(ctx, placement, scheduleResult.Decisions(), scheduleResult.PrioritizerScores(), status)
-	if err != nil {
+	if err := c.bind(ctx, placement, scheduleResult.Decisions(), scheduleResult.PrioritizerScores(), status); err != nil {
 		return err
 	}
 
 	// update placement status if necessary to signal no bindings
-	return c.updateStatus(ctx, placement, int32(len(scheduleResult.Decisions())), misconfiguredCondition, satisfiedCondition)
+	if err := c.updateStatus(ctx, placement, int32(len(scheduleResult.Decisions())), misconfiguredCondition, satisfiedCondition); err != nil {
+		return err
+	}
+
+	return status.AsError()
 }
 
 // getManagedClusterSetBindings returns all bindings found in the placement namespace.

diff --git a/test/e2e/placement_test.go b/test/e2e/placement_test.go
@@ -85,6 +85,7 @@ var _ = ginkgo.Describe("Placement", func() {
 
 	assertNumberOfDecisions := func(placementName string, desiredNOD int) {
 		ginkgo.By("Check the number of decisions in placementdecisions")
+		// at least one decision for each placement
 		desiredNOPD := desiredNOD/maxNumOfClusterDecisions + 1
 		gomega.Eventually(func() bool {
 			pdl, err := clusterClient.ClusterV1beta1().PlacementDecisions(namespace).List(context.Background(), metav1.ListOptions{

diff --git a/test/integration/assertion_test.go b/test/integration/assertion_test.go
@@ -20,7 +20,7 @@ import (
 	"open-cluster-management.io/placement/test/integration/util"
 )
 
-func assertPlacementDecisionCreated(placement *clusterapiv1beta1.Placement, desiredNOPD int) {
+func assertPlacementDecisionCreated(placement *clusterapiv1beta1.Placement) {
 	ginkgo.By("Check if placementdecision is created")
 	gomega.Eventually(func() bool {
 		pdl, err := clusterClient.ClusterV1beta1().PlacementDecisions(placement.Namespace).List(context.Background(), metav1.ListOptions{
@@ -29,7 +29,7 @@ func assertPlacementDecisionCreated(placement *clusterapiv1beta1.Placement, desi
 		if err != nil {
 			return false
 		}
-		if len(pdl.Items) != desiredNOPD {
+		if len(pdl.Items) == 0 {
 			return false
 		}
 		for _, pd := range pdl.Items {
@@ -79,6 +79,7 @@ func assertPlacementDeleted(placementName, namespace string) {
 
 func assertNumberOfDecisions(placementName, namespace string, desiredNOD int) {
 	ginkgo.By("Check the number of decisions in placementdecisions")
+	// at least one decision for each placement
 	desiredNOPD := desiredNOD/maxNumOfClusterDecisions + 1
 	gomega.Eventually(func() bool {
 		pdl, err := clusterClient.ClusterV1beta1().PlacementDecisions(namespace).List(context.Background(), metav1.ListOptions{
@@ -402,7 +403,7 @@ func assertCreatingPlacement(name, namespace string, noc *int32, prioritizerPoli
 
 func assertCreatingPlacementWithDecision(name, namespace string, noc *int32, nod int, prioritizerPolicy clusterapiv1beta1.PrioritizerPolicy, tolerations []clusterapiv1beta1.Toleration) {
 	placement := assertCreatingPlacement(name, namespace, noc, prioritizerPolicy, tolerations)
-	assertPlacementDecisionCreated(placement, nod/maxNumOfClusterDecisions+1)
+	assertPlacementDecisionCreated(placement)
 	assertNumberOfDecisions(name, namespace, nod)
 	if noc != nil {
 		assertPlacementConditionSatisfied(name, namespace, nod, nod == int(*noc))

diff --git a/test/integration/placement_test.go b/test/integration/placement_test.go
@@ -97,13 +97,13 @@ var _ = ginkgo.Describe("Placement", func() {
 			// check if the placementdecisions are re-created
 			placement, err := clusterClient.ClusterV1beta1().Placements(namespace).Get(context.Background(), placementName, metav1.GetOptions{})
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
-			assertPlacementDecisionCreated(placement, 1)
+			assertPlacementDecisionCreated(placement)
 			assertNumberOfDecisions(placementName, namespace, 5)
 		})
 
 		ginkgo.It("Should create empty placementdecision when no cluster selected", func() {
 			placement := assertCreatingPlacement(placementName, namespace, nil, clusterapiv1beta1.PrioritizerPolicy{}, []clusterapiv1beta1.Toleration{})
-			assertPlacementDecisionCreated(placement, 1)
+			assertPlacementDecisionCreated(placement)
 			assertNumberOfDecisions(placementName, namespace, 0)
 		})
 
@@ -272,15 +272,14 @@ var _ = ginkgo.Describe("Placement", func() {
 			assertBindingClusterSet(clusterSet1Name, namespace)
 			assertCreatingClusters(clusterSet1Name, 1)
 
-			placement := assertCreatingPlacement(placementName, namespace, noc(1), clusterapiv1beta1.PrioritizerPolicy{}, []clusterapiv1beta1.Toleration{
+			assertCreatingPlacement(placementName, namespace, noc(1), clusterapiv1beta1.PrioritizerPolicy{}, []clusterapiv1beta1.Toleration{
 				{
 					Key:      "key1",
 					Operator: clusterapiv1beta1.TolerationOpExists,
 					Value:    "value1",
 				},
 			})
-			ginkgo.By("No placementdecision when misconfigured")
-			assertPlacementDecisionCreated(placement, 0)
+			assertNumberOfDecisions(placementName, namespace, 0)
 			assertPlacementConditionMisconfigured(placementName, namespace, true)
 
 			assertUpdatingPlacement(placementName, namespace, noc(1), clusterapiv1beta1.PrioritizerPolicy{}, []clusterapiv1beta1.Toleration{})