From b33c54e5a762c745824b6e9fb109537aaf310a00 Mon Sep 17 00:00:00 2001 From: haoqing0110 Date: Thu, 22 Dec 2022 09:55:53 +0000 Subject: [PATCH] create placementdecision when misconfigured Signed-off-by: haoqing0110 --- pkg/controllers/framework/interface.go | 2 +- .../scheduling/scheduling_controller.go | 17 ++++++----------- test/e2e/placement_test.go | 1 + test/integration/assertion_test.go | 7 ++++--- test/integration/placement_test.go | 9 ++++----- 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/pkg/controllers/framework/interface.go b/pkg/controllers/framework/interface.go index dd45ad28..48b25339 100644 --- 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 index 2f31791a..37dd7818 100644 --- 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 index f3022a51..f9818ed1 100644 --- 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 index 66a5c81b..2e055ae7 100644 --- 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 index d9df7a11..d7a83d64 100644 --- 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{})