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 <[email protected]>
nmstate · Nov 21, 2021 · 37d6a12 · 37d6a12
1 parent 8e35b11
commit 37d6a12
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 18 deletions.
diff --git 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
@@ -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
@@ -118,14 +118,28 @@ var _ = Describe("[rfe_id:3503][crit:medium][vendor:[email protected]][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:[email protected]][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")
+		})
 	})
 })