mco-1597: Validate MCN condition status on node degrade

Author: isabella-janssen

test/extended/machine_config/helpers.go

@@ -3,6 +3,7 @@ package machine_config
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math/rand"
 	"time"
@@ -39,6 +40,7 @@ const (
 )
 
 // TODO: add error message returns for `.NotTo(o.HaveOccurred())` cases.
+// TODO: fix caplitalization of helper funcs
 
 // skipUnlessTargetPlatform skips the test if it is running on the target platform
 func skipUnlessTargetPlatform(oc *exutil.CLI, platformType osconfigv1.PlatformType) {
@@ -401,7 +403,7 @@ func WaitForMCPToBeReady(oc *exutil.CLI, machineConfigClient *machineconfigclien
 	return nil
 }
 
-// GetCordonedNodes get cordoned nodes (if maxUnavailable > 1 ) otherwise return the 1st cordoned node
+// `GetCordonedNodes` get cordoned nodes (if maxUnavailable > 1 ) otherwise return the 1st cordoned node
 func GetCordonedNodes(oc *exutil.CLI, mcpName string) []corev1.Node {
 	// Wait for the MCP to start updating
 	o.Expect(waitForMCPConditionStatus(oc, mcpName, "Updating", "True")).NotTo(o.HaveOccurred(), "Waiting for 'Updating' status change failed.")
@@ -413,6 +415,7 @@ func GetCordonedNodes(oc *exutil.CLI, mcpName string) []corev1.Node {
 		o.Expect(nodeErr).NotTo(o.HaveOccurred(), "Error getting nodes from %v MCP.", mcpName)
 		o.Expect(nodes).ShouldNot(o.BeEmpty(), "No nodes found for %v MCP.", mcpName)
 
+		// TOOD: cleanup
 		for _, node := range nodes {
 			unschedulable := node.Spec.Unschedulable
 			if unschedulable {
@@ -451,27 +454,30 @@ func waitForMCPConditionStatus(oc *exutil.CLI, mcpName string, conditionType mcf
 
 		framework.Logf("Waiting for %v MCP's %v condition to be %v.", mcp.Name, conditionType, status)
 		return false
-	}, 5*time.Minute, 3*time.Second).Should(o.BeTrue())
+	}, 2*time.Minute, 3*time.Second).Should(o.BeTrue())
 	return nil
 }
 
 // `waitForMCNConditionStatus` waits until the desired MCN condition matches the desired status (ex. wait until "Updated" is "False")
 func waitForMCNConditionStatus(clientSet *machineconfigclient.Clientset, mcnName string, conditionType string, status metav1.ConditionStatus, timeout time.Duration, interval time.Duration) error {
 	o.Eventually(func() bool {
-		// Get MCN & desried condition status
+		framework.Logf("Waiting for MCN %v %v condition to be %v.", mcnName, conditionType, status)
+
+		// Get MCN & check if the MCN condition status matches the desired status
 		workerNodeMCN, workerErr := clientSet.MachineconfigurationV1alpha1().MachineConfigNodes().Get(context.TODO(), mcnName, metav1.GetOptions{})
 		o.Expect(workerErr).NotTo(o.HaveOccurred())
-		conditionStatus := getMCNConditionStatus(workerNodeMCN, conditionType)
-		if conditionStatus == status {
-			return true
-		}
-
-		framework.Logf("Waiting for MCN %v %v condition to be %v.", mcnName, conditionType, status)
-		return false
+		return checkMCNConditionStatus(workerNodeMCN, conditionType, status)
 	}, timeout, interval).Should(o.BeTrue())
 	return nil
 }
 
+// `checkMCNConditionStatus` checks that an MCN condition matches the desired status (ex. confirm "Updated" is "False")
+func checkMCNConditionStatus(mcn *v1alpha1.MachineConfigNode, conditionType string, status metav1.ConditionStatus) bool {
+	conditionStatus := getMCNConditionStatus(mcn, conditionType)
+	framework.Logf("MCN %v %v condition is %v.", mcn.Name, conditionType, conditionStatus)
+	return conditionStatus == status
+}
+
 // `getMCNConditionStatus` returns the status of the desired condition type for MCN, or an empty string if the condition does not exist
 func getMCNConditionStatus(mcn *v1alpha1.MachineConfigNode, conditionType string) metav1.ConditionStatus {
 	// Loop through conditions and return the status of the desired condition type
@@ -508,3 +514,70 @@ func confirmUpdatedMCNStatus(clientSet *machineconfigclient.Clientset, mcnName s
 	framework.Logf("Node %s update is complete and MCN is valid.", mcnName)
 	return true
 }
+
+// TODO: consolidate with similar functions
+func GetDegradedNode(oc *exutil.CLI, mcpName string) (corev1.Node, error) {
+	// Get nodes in desired pool
+	nodes, nodeErr := GetNodesByRole(oc, mcpName)
+	o.Expect(nodeErr).NotTo(o.HaveOccurred())
+	o.Expect(nodes).ShouldNot(o.BeEmpty())
+
+	// Get degraded node
+	for _, node := range nodes {
+		// TODO: create generalized get node state helper
+		state := node.Annotations["machineconfiguration.openshift.io/state"]
+		if state == "Degraded" {
+			return node, nil
+		}
+	}
+
+	return corev1.Node{}, errors.New("no degraded node found")
+}
+
+// `recoverFromDegraded` updates the current and desired machine configs so that the pool can recover from degraded state once the offending MC is deleted
+func recoverFromDegraded(oc *exutil.CLI, mcpName string) error {
+	framework.Logf("Recovering %s pool from degraded state", mcpName)
+
+	// Get nodes from degraded MCP & update the desired config of the degraded node to force a recovery update
+	nodes, nodeErr := GetNodesByRole(oc, mcpName)
+	o.Expect(nodeErr).NotTo(o.HaveOccurred())
+	o.Expect(nodes).ShouldNot(o.BeEmpty())
+	for _, node := range nodes {
+		framework.Logf("Restoring desired config for node: %s", node.Name)
+		state := node.Annotations["machineconfiguration.openshift.io/state"]
+		if state == "Done" {
+			framework.Logf("Node %s is updated and does not need to be recovered", node.Name)
+		} else {
+			err := restoreDesiredConfig(oc, node)
+			if err != nil {
+				return fmt.Errorf("error restoring desired config in node %s. Error: %s", node.Name, err)
+			}
+		}
+	}
+
+	// Wait for MCP to not be in degraded status
+	mcpErr := waitForMCPConditionStatus(oc, mcpName, "Degraded", "False")
+	o.Expect(mcpErr).NotTo(o.HaveOccurred(), fmt.Sprintf("could not recover %v MCP from the degraded status.", mcpName))
+	mcpErr = waitForMCPConditionStatus(oc, mcpName, "Updated", "True")
+	o.Expect(mcpErr).NotTo(o.HaveOccurred(), fmt.Sprintf("%v MCP could not reach an updated state.", mcpName))
+	return nil
+}
+
+// TODO: generalize with get node status to just pass in the general node annotation label
+func getCurrentMachineConfig(node corev1.Node) string {
+	return node.Annotations["machineconfiguration.openshift.io/currentConfig"]
+}
+
+// `restoreDesiredConfig` updates the value of a node's desiredConfig annotation to be equal to the value of its currentConfig (desiredConfig=currentConfig)
+func restoreDesiredConfig(oc *exutil.CLI, node corev1.Node) error {
+	// Get current config
+	currentConfig := getCurrentMachineConfig(node)
+	if currentConfig == "" {
+		return fmt.Errorf("currentConfig annotation is empty for node %s", node.Name)
+	}
+
+	// Update desired config to be equal to current config
+	framework.Logf("Node: %s is restoring desiredConfig value to match currentConfig value: %s", node.Name, currentConfig)
+	configErr := oc.Run("patch").Args(fmt.Sprintf("node/%v", node.Name), "--patch", fmt.Sprintf(`{"metadata":{"annotations":{"machineconfiguration.openshift.io/desiredConfig":"%v"}}}`, currentConfig), "--type=merge").Execute()
+	return configErr
+}

test/extended/machine_config/machine_config_node.go

@@ -31,6 +31,7 @@ var _ = g.Describe("[sig-mco][OCPFeatureGate:MachineConfigNode][Serial]", func()
 		MCOMachineConfigBaseDir     = exutil.FixturePath("testdata", "machine_config", "machineconfig")
 		infraMCPFixture             = filepath.Join(MCOMachineConfigPoolBaseDir, "infra-mcp.yaml")
 		testFileMCFixture           = filepath.Join(MCOMachineConfigBaseDir, "0-worker-mc.yaml")
+		invalidMCFixture            = filepath.Join(MCOMachineConfigBaseDir, "1-worker-invalid-mc.yaml")
 		oc                          = exutil.NewCLIWithoutNamespace("machine-config")
 	)
 
@@ -41,6 +42,10 @@ var _ = g.Describe("[sig-mco][OCPFeatureGate:MachineConfigNode][Serial]", func()
 	g.It("Should properly transition through MCN conditions on node update [apigroup:machineconfiguration.openshift.io]", func() {
 		ValidateMCNConditionTransitions(oc, testFileMCFixture)
 	})
+
+	g.It("Should properly report MCN conditions on node degrade [apigroup:machineconfiguration.openshift.io]", func() {
+		ValidateMCNConditionOnNodeDegrade(oc, invalidMCFixture)
+	})
 })
 
 // `ValidateMCNProperties` checks that MCN properties match the corresponding node properties
@@ -158,7 +163,7 @@ func ValidateMCNConditionTransitions(oc *exutil.CLI, fixture string) {
 
 	// Delete MC on failure or test completion
 	defer func() {
-		deleteMCErr := oc.Run("delete").Args("machineconfig", "99-worker-testfile").Execute()
+		deleteMCErr := oc.Run("delete").Args("machineconfig", "90-worker-testfile").Execute()
 		o.Expect(deleteMCErr).NotTo(o.HaveOccurred())
 	}()
 
@@ -168,6 +173,7 @@ func ValidateMCNConditionTransitions(oc *exutil.CLI, fixture string) {
 	workerNode := updatingNodes[0]
 
 	// Validate transition through conditions for MCN
+	// TODO: make consts for the statuses
 	framework.Logf("Checking Updated=False")
 	err := waitForMCNConditionStatus(clientSet, workerNode.Name, "Updated", "False", 1*time.Minute, 5*time.Second)
 	o.Expect(err).NotTo(o.HaveOccurred())
@@ -183,7 +189,6 @@ func ValidateMCNConditionTransitions(oc *exutil.CLI, fixture string) {
 	framework.Logf("Checking Drained=Unknown")
 	err = waitForMCNConditionStatus(clientSet, workerNode.Name, "Drained", "Unknown", 30*time.Second, 2*time.Second)
 	o.Expect(err).NotTo(o.HaveOccurred())
-	// Failing here!
 	framework.Logf("Checking Drained=True")
 	err = waitForMCNConditionStatus(clientSet, workerNode.Name, "Drained", "True", 7*time.Minute, 10*time.Second)
 	o.Expect(err).NotTo(o.HaveOccurred())
@@ -222,3 +227,42 @@ func ValidateMCNConditionTransitions(oc *exutil.CLI, fixture string) {
 	framework.Logf("Checking all conditions other than 'Updated' are False.")
 	o.Expect(confirmUpdatedMCNStatus(clientSet, workerNode.Name)).Should(o.BeTrue())
 }
+
+// `ValidateMCNConditionOnNodeDegrade` check that Conditions properly update on a node update
+func ValidateMCNConditionOnNodeDegrade(oc *exutil.CLI, fixture string) {
+	// Create client set for test
+	clientSet, clientErr := machineconfigclient.NewForConfig(oc.KubeFramework().ClientConfig())
+	o.Expect(clientErr).NotTo(o.HaveOccurred())
+
+	// Apply invalid MC targeting worker pool
+	mcErr := oc.Run("apply").Args("-f", fixture).Execute()
+	o.Expect(mcErr).NotTo(o.HaveOccurred())
+
+	// Cleanup MC and fix node degradation on failure or test completion
+	defer func() {
+		// Delete the applied MC
+		deleteMCErr := oc.Run("delete").Args("machineconfig", "91-worker-testfile-invalid").Execute()
+		o.Expect(deleteMCErr).NotTo(o.HaveOccurred())
+
+		// Recover the degraded MCP
+		recoverErr := recoverFromDegraded(oc, worker)
+		o.Expect(recoverErr).NotTo(o.HaveOccurred())
+	}()
+
+	// Wait for worker MCP to be in a degraded state with one degraded machine
+	// TODO consolidate into helper func that doesn't require getting MCP more than required
+	o.Expect(waitForMCPConditionStatus(oc, worker, "Degraded", "True")).NotTo(o.HaveOccurred(), "Error waiting for %v MCP to be in a degraded state.")
+	workerMcp, err := clientSet.MachineconfigurationV1().MachineConfigPools().Get(context.TODO(), worker, metav1.GetOptions{})
+	o.Expect(err).NotTo(o.HaveOccurred(), "Error getting %v MCP.", worker)
+	o.Expect(workerMcp.Status.DegradedMachineCount).To(o.BeNumerically("==", 1), "Degraded machine count is not 1.")
+
+	// Get degraded worker node
+	degradedNode, degradedNodeErr := GetDegradedNode(oc, worker)
+	o.Expect(degradedNodeErr).NotTo(o.HaveOccurred(), "Error getting degraded node for %v MCP.", worker)
+
+	// Validate MCN of degraded node
+	degradedNodeMCN, degradedErr := clientSet.MachineconfigurationV1alpha1().MachineConfigNodes().Get(context.TODO(), degradedNode.Name, metav1.GetOptions{})
+	o.Expect(degradedErr).NotTo(o.HaveOccurred())
+	o.Expect(checkMCNConditionStatus(degradedNodeMCN, "AppliedFilesAndOS", "Unknown"))
+	o.Expect(checkMCNConditionStatus(degradedNodeMCN, "UpdateExecuted", "Unknown"))
+}

test/extended/testdata/bindata.go

@@ -3,7 +3,7 @@ kind: MachineConfig
 metadata:
   labels:
     machineconfiguration.openshift.io/role: worker
-  name: 99-worker-testfile
+  name: 90-worker-testfile
 spec:
   config:
     ignition:

test/extended/testdata/machine_config/machineconfig/0-worker-mc.yaml

@@ -0,0 +1,19 @@
+apiVersion: machineconfiguration.openshift.io/v1
+kind: MachineConfig
+metadata:
+  labels:
+    machineconfiguration.openshift.io/role: worker
+  name: 91-worker-testfile-invalid
+spec:
+  config:
+    ignition:
+      version: 3.2.0
+    storage:
+      files:
+      - contents:
+          source: data:text/plain;charset=utf-8;base64,dGVzdA==
+        mode: 420
+        path: /etc/wronguser-test-file.test
+        user:
+          name: wronguser
+

test/extended/testdata/machine_config/machineconfig/1-worker-invalid-mc.yaml

@@ -102,6 +102,8 @@ spec:
     tests:
     - testName: '[sig-mco][OCPFeatureGate:MachineConfigNode][Serial] Should have MCN
         properties matching associated node properties [apigroup:machineconfiguration.openshift.io]'
+    - testName: '[sig-mco][OCPFeatureGate:MachineConfigNode][Serial] Should properly
+        report MCN conditions on node degrade [apigroup:machineconfiguration.openshift.io]'
     - testName: '[sig-mco][OCPFeatureGate:MachineConfigNode][Serial] Should properly
         transition through MCN conditions on node update [apigroup:machineconfiguration.openshift.io]'
   - featureGate: ManagedBootImages