From 37d6a12ab46d370d0bc3c9c1875bce53b7d18ee7 Mon Sep 17 00:00:00 2001 From: Radim Hrazdil <32546791+rhrazdil@users.noreply.github.com> Date: Sun, 21 Nov 2021 11:21:11 +0100 Subject: [PATCH] Set NNCP Degraded as soon as first enactment fails (#877) Don't wait for all enactments to finish, set the Degraded condition as soon as first Failing enactment appears. Signed-off-by: Radim Hrazdil --- pkg/policyconditions/conditions.go | 19 +++++----- pkg/policyconditions/conditions_test.go | 4 +- test/e2e/handler/nnce_conditions_test.go | 48 +++++++++++++++++++++--- 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/pkg/policyconditions/conditions.go b/pkg/policyconditions/conditions.go index 54dbb963b..befe6d95d 100644 --- a/pkg/policyconditions/conditions.go +++ b/pkg/policyconditions/conditions.go @@ -3,6 +3,7 @@ package policyconditions import ( "context" "fmt" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -139,19 +140,17 @@ func update(apiWriter client.Client, apiReader client.Reader, policyReader clien if numberOfNmstateMatchingNodes == 0 { message := "Policy does not match any node" SetPolicyNotMatching(&policy.Status.Conditions, message) + } else if enactmentsCountByCondition.Failed() > 0 || enactmentsCountByCondition.Aborted() > 0 { + message := fmt.Sprintf("%d/%d nodes failed to configure", enactmentsCountByCondition.Failed(), numberOfNmstateMatchingNodes) + if enactmentsCountByCondition.Aborted() > 0 { + message += fmt.Sprintf(", %d nodes aborted configuration", enactmentsCountByCondition.Aborted()) + } + SetPolicyFailedToConfigure(&policy.Status.Conditions, message) } else if numberOfFinishedEnactments < numberOfNmstateMatchingNodes { SetPolicyProgressing(&policy.Status.Conditions, fmt.Sprintf("Policy is progressing %d/%d nodes finished", numberOfFinishedEnactments, numberOfNmstateMatchingNodes)) } else { - if enactmentsCountByCondition.Failed() > 0 || enactmentsCountByCondition.Aborted() > 0 { - message := fmt.Sprintf("%d/%d nodes failed to configure", enactmentsCountByCondition.Failed(), numberOfNmstateMatchingNodes) - if enactmentsCountByCondition.Aborted() > 0 { - message += fmt.Sprintf(", %d nodes aborted configuration", enactmentsCountByCondition.Aborted()) - } - SetPolicyFailedToConfigure(&policy.Status.Conditions, message) - } else { - message := fmt.Sprintf("%d/%d nodes successfully configured", enactmentsCountByCondition.Available(), enactmentsCountByCondition.Available()) - SetPolicySuccess(&policy.Status.Conditions, message) - } + message := fmt.Sprintf("%d/%d nodes successfully configured", enactmentsCountByCondition.Available(), enactmentsCountByCondition.Available()) + SetPolicySuccess(&policy.Status.Conditions, message) } err = apiWriter.Status().Update(context.TODO(), policy) diff --git a/pkg/policyconditions/conditions_test.go b/pkg/policyconditions/conditions_test.go index 0fa861523..7df797108 100644 --- a/pkg/policyconditions/conditions_test.go +++ b/pkg/policyconditions/conditions_test.go @@ -210,7 +210,7 @@ var _ = Describe("Policy Conditions", func() { Pods: newNmstatePods(3), Policy: p(SetPolicyProgressing, "Policy is progressing 2/3 nodes finished"), }), - Entry("when enactments are failed/progressing/success then policy is progressing", ConditionsCase{ + Entry("when enactments are failed/progressing/success then policy is degraded", ConditionsCase{ Enactments: []nmstatev1beta1.NodeNetworkConfigurationEnactment{ e("node1", "policy1", enactmentconditions.SetSuccess), e("node2", "policy1", enactmentconditions.SetProgressing), @@ -219,7 +219,7 @@ var _ = Describe("Policy Conditions", func() { }, Nodes: newNodes(4), Pods: newNmstatePods(4), - Policy: p(SetPolicyProgressing, "Policy is progressing 3/4 nodes finished"), + Policy: p(SetPolicyFailedToConfigure, "1/4 nodes failed to configure"), }), Entry("when all the enactments are at failing or success policy is degraded", ConditionsCase{ Enactments: []nmstatev1beta1.NodeNetworkConfigurationEnactment{ diff --git a/test/e2e/handler/nnce_conditions_test.go b/test/e2e/handler/nnce_conditions_test.go index 4ed2fba44..42190cbc5 100644 --- a/test/e2e/handler/nnce_conditions_test.go +++ b/test/e2e/handler/nnce_conditions_test.go @@ -118,14 +118,28 @@ var _ = Describe("[rfe_id:3503][crit:medium][vendor:cnv-qe@redhat.com][level:com Expect(abortedConditions).To(Equal(len(nodes)-failingConditions), "other nodes should have aborted enactment") } + By("Wait for enactments to reach failing or aborted state") + var wg sync.WaitGroup + wg.Add(len(nodes)) + for i, _ := range nodes { + node := nodes[i] + go func() { + defer wg.Done() + defer GinkgoRecover() + Byf("Check %s failing state is kept", node) + enactmentConditionsStatusEventually(node).Should( + SatisfyAny( + matchConditionsFrom(enactmentconditions.SetFailedToConfigure), + matchConditionsFrom(enactmentconditions.SetConfigurationAborted), + ), "should consistently keep failing or aborted conditions at enactments") + }() + } + wg.Wait() + By("Check policy is at degraded state") waitForDegradedTestPolicy() - By("Check there is one failing enactment and the rest are aborted") - checkEnactmentCounts(TestPolicy) - - By("Check that the enactment stays in failing or aborted state") - var wg sync.WaitGroup + By("Check that the enactments stay in failing or aborted state") wg.Add(len(nodes)) for i, _ := range nodes { node := nodes[i] @@ -142,8 +156,30 @@ var _ = Describe("[rfe_id:3503][crit:medium][vendor:cnv-qe@redhat.com][level:com } wg.Wait() - By("Check there is still one failing enactment and the rest are aborted") + By("Check there is up to maxUnavailable failing enactments and the rest are aborted") checkEnactmentCounts(TestPolicy) }) + + It("should mark policy as Degraded as soon as first enactment fails", func() { + failingEnactmentsCount := func(policy string) int { + failingConditions := 0 + for _, node := range nodes { + conditionList := enactmentConditionsStatus(node, TestPolicy) + found, _ := matchConditionsFrom(enactmentconditions.SetFailedToConfigure).Match(conditionList) + if found { + failingConditions++ + } + } + return failingConditions + } + + By("Waiting for first enactment to fail") + Eventually(func() int { + return failingEnactmentsCount(TestPolicy) + }).Should(BeNumerically(">=", 1)) + + By("Checking the policy is marked as Degraded") + Expect(policyConditionsStatus(TestPolicy)).Should(containPolicyDegraded(), "policy should be marked as Degraded") + }) }) })