From 12c0825c31c1ad8addd87597e848edb78b0b732f Mon Sep 17 00:00:00 2001 From: Gal Amado Date: Thu, 13 Aug 2026 14:55:42 +0300 Subject: [PATCH 1/8] mdr-operator: add MDR condition tests (OCP-65763, OCP-66137, OCP-66351, OCP-66317) Port 4 MDR condition tests from ocp-edge-auto test_mdr_cli.py: - OCP-65763: NHC timed-out annotation sets RemediationStoppedByNHC - OCP-66137: Non-existent node sets RemediationCannotStartNodeNotFound - OCP-66351: Control-plane node sets RemediationCannotStartNoControllerOwner - OCP-66317: PermanentNodeDeletionExpected condition (platform-aware) Adds shared infrastructure: buildMDR/buildMDRWithAnnotations CR builders, verifyMDRConditionsByType condition checker, listControlPlaneNodes helper, findMessageInControllerLogs for log verification, and condition constants. RHWA-1249 Co-Authored-By: Claude Opus 4.6 --- tests/mdr-operator/README.md | 55 ++- .../mdr-operator/internal/mdrparams/const.go | 52 +++ tests/mdr-operator/tests/mdr_conditions.go | 340 ++++++++++++++++++ tests/mdr-operator/tests/mdr_helpers.go | 238 ++++++++++++ 4 files changed, 684 insertions(+), 1 deletion(-) create mode 100644 tests/mdr-operator/tests/mdr_conditions.go diff --git a/tests/mdr-operator/README.md b/tests/mdr-operator/README.md index 8ea6fc6278..b6c78fb31a 100644 --- a/tests/mdr-operator/README.md +++ b/tests/mdr-operator/README.md @@ -75,6 +75,59 @@ container or pod level). Only checks the `manager` container. - **Standalone**: `ginkgo --label-filter="mdr" --focus="runs as non-root" ./tests/mdr-operator/...` - **Pass criteria**: Pod runAsNonRoot=true; expected manager container exists; manager container runAsUser != 0; allowPrivilegeEscalation=false; readOnlyRootFilesystem=true; capabilities.drop=[ALL]; seccomp profile RuntimeDefault +## Condition Tests + +### 5. Verify MDR Conditions With NHC Timed-Out Annotation ([OCP-65763](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-65763)) + +Creates an MDR CR with the `remediation.medik8s.io/nhc-timed-out` annotation. +Verifies that the Processing and Succeeded conditions have reason +`RemediationStoppedByNHC`, and that the controller log contains the +"NHC time out annotation found, stopping remediation" message. + +- **Operators**: MDR v0.7.0+ +- **Cluster**: Any topology (MNO or SNO) +- **Environment**: Connected or disconnected +- **Standalone**: `ginkgo --label-filter="mdr" --focus="nhc-timed-out annotation" ./tests/mdr-operator/...` +- **Pass criteria**: Processing reason=RemediationStoppedByNHC; Succeeded reason=RemediationStoppedByNHC; controller log contains remediation-stopped message; MDR controller pod running after test + +### 6. Verify MDR Conditions With Non-Existent Node ([OCP-66137](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-66137)) + +Creates an MDR CR named `non-existent-node`. Verifies that the Processing and +Succeeded conditions have status=False and reason +`RemediationCannotStartNodeNotFound`. Guards against accidental real fencing +by asserting the node name does not exist in the cluster before creating the CR. + +- **Operators**: MDR v0.7.0+ +- **Cluster**: Any topology (MNO or SNO) +- **Environment**: Connected or disconnected +- **Standalone**: `ginkgo --label-filter="mdr" --focus="non-existent node" ./tests/mdr-operator/...` +- **Pass criteria**: Node name confirmed absent from cluster; Processing status=False reason=RemediationCannotStartNodeNotFound; Succeeded status=False reason=RemediationCannotStartNodeNotFound; MDR controller pod running after test + +### 7. Verify MDR Conditions With Control-Plane Node ([OCP-66351](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-66351)) + +Creates an MDR CR named after a random control-plane node. Verifies that the +Processing and Succeeded conditions have status=False and reason +`RemediationCannotStartNoControllerOwner`. + +- **Operators**: MDR v0.7.0+ +- **Cluster**: Any topology (MNO or SNO) +- **Environment**: Connected or disconnected +- **Standalone**: `ginkgo --label-filter="mdr" --focus="control-plane node name" ./tests/mdr-operator/...` +- **Pass criteria**: Processing status=False reason=RemediationCannotStartNoControllerOwner; Succeeded status=False reason=RemediationCannotStartNoControllerOwner; MDR controller pod running after test + +### 8. Verify PermanentNodeDeletionExpected Condition With Control-Plane Node ([OCP-66317](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-66317)) + +Creates an MDR CR for a control-plane node and verifies the +`PermanentNodeDeletionExpected` condition. Uses platform detection to assert +the correct status, reason, and message: baremetal clusters expect +status=False/KeepsNodeName, cloud clusters expect status=True/NewNodeName. + +- **Operators**: MDR v0.7.0+ +- **Cluster**: Any topology (MNO or SNO) +- **Environment**: Connected or disconnected +- **Standalone**: `ginkgo --label-filter="mdr" --focus="PermanentNodeDeletionExpected" ./tests/mdr-operator/...` +- **Pass criteria**: PermanentNodeDeletionExpected status/reason/message match platform (baremetal: False/KeepsNodeName/"...NOT expected..."; cloud: True/NewNodeName/"...expected..."); MDR controller pod running after test + ## Destructive Tests -- NHC-Triggered Remediation Tests that stop kubelet on a worker node, let NHC detect the unhealthy node @@ -88,7 +141,7 @@ provider provisions a new VM. The node is re-created (new creation timestamp). - At least 2 Ready worker nodes (target + spare for cluster schedulability) - `KUBECONFIG` set with cluster-admin access -### 5. MDR Remediation with Condition Transitions ([OCP-66138](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-66138)) +### 9. MDR Remediation with Condition Transitions ([OCP-66138](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-66138)) Stops kubelet on a worker node. NHC detects the unhealthy node and creates an MDR CR via the MDR template. Verifies the MDR CR status conditions diff --git a/tests/mdr-operator/internal/mdrparams/const.go b/tests/mdr-operator/internal/mdrparams/const.go index ecba6bbebf..ea55ff8abf 100644 --- a/tests/mdr-operator/internal/mdrparams/const.go +++ b/tests/mdr-operator/internal/mdrparams/const.go @@ -67,4 +67,56 @@ const ( // ConditionReasonRemediationStarted is the reason set on Processing and Succeeded // conditions when remediation begins. ConditionReasonRemediationStarted = "RemediationStarted" + + // --- Condition test constants (RHWA-1249) --- + + // ConditionReasonStoppedByNHC is the reason set when NHC timed-out annotation is present. + ConditionReasonStoppedByNHC = "RemediationStoppedByNHC" + + // ConditionReasonNodeNotFound is the reason set when the target node does not exist. + ConditionReasonNodeNotFound = "RemediationCannotStartNodeNotFound" + + // ConditionReasonNoControllerOwner is the reason set when the target node has no controller owner. + ConditionReasonNoControllerOwner = "RemediationCannotStartNoControllerOwner" + + // PermanentNodeDeletionExpectedConditionType is the condition indicating node name persistence. + PermanentNodeDeletionExpectedConditionType = "PermanentNodeDeletionExpected" + + // ConditionReasonKeepsNodeName is the reason for baremetal clusters where the node name is preserved. + ConditionReasonKeepsNodeName = "MachineDeletionOnBareMetalProviderKeepsNodeName" + + // ConditionReasonNewNodeName is the reason for cloud clusters where the node gets a new name. + ConditionReasonNewNodeName = "MachineDeletionOnCloudProviderCausesNewNodeName" + + // ConditionMessageKeepsNodeName is the expected message for baremetal PermanentNodeDeletionExpected. + ConditionMessageKeepsNodeName = "Machine will be deleted and the unhealthy node replaced. " + + "This is a BareMetal cluster provider: the new node is NOT expected to have a new name" + + // ConditionMessageNewNodeName is the expected message for cloud PermanentNodeDeletionExpected. + ConditionMessageNewNodeName = "Machine will be deleted and the unhealthy node replaced. " + + "This is a Cloud cluster provider: the new node is expected to have a new name" + + // ConditionStatusFalse is the status value "False" for condition checks. + ConditionStatusFalse = "False" + + // ConditionStatusTrue is the status value "True" for condition checks. + ConditionStatusTrue = "True" + + // NHCTimedOutAnnotationKey is the annotation that signals NHC timed out. + NHCTimedOutAnnotationKey = "remediation.medik8s.io/nhc-timed-out" + + // NHCTimedOutAnnotationValue is a valid RFC3339 timestamp for the annotation. + NHCTimedOutAnnotationValue = "2006-01-02T15:04:05Z07:00" + + // MDRRemStoppedLogMsg is the log message emitted when MDR stops due to NHC timeout. + MDRRemStoppedLogMsg = "NHC time out annotation found, stopping remediation" + + // MDRConditionTestName is the MDR CR name for the NHC timed-out condition test. + MDRConditionTestName = "mdr-test-timeout-annotation" + + // MDRNonExistentNodeTestName is the MDR CR name for the non-existent node test. + MDRNonExistentNodeTestName = "non-existent-node" + + // ControllerLogWindow is the time window for searching controller logs. + ControllerLogWindow = 5 * time.Minute ) diff --git a/tests/mdr-operator/tests/mdr_conditions.go b/tests/mdr-operator/tests/mdr_conditions.go new file mode 100644 index 0000000000..8108249301 --- /dev/null +++ b/tests/mdr-operator/tests/mdr_conditions.go @@ -0,0 +1,340 @@ +package tests + +import ( + "context" + "fmt" + "math/rand" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + configv1 "github.com/openshift/api/config/v1" + "github.com/rh-ecosystem-edge/eco-goinfra/pkg/deployment" + "github.com/rh-ecosystem-edge/eco-goinfra/pkg/pod" + "github.com/rh-ecosystem-edge/eco-goinfra/pkg/reportxml" + + "github.com/medik8s/system-tests/tests/internal/helpers" + "github.com/medik8s/system-tests/tests/internal/labels" + . "github.com/medik8s/system-tests/tests/internal/medik8sinittools" + "github.com/medik8s/system-tests/tests/internal/medik8sparams" + "github.com/medik8s/system-tests/tests/mdr-operator/internal/mdrparams" + + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe( + "MDR Condition tests", + Ordered, + ContinueOnFailure, + Label(labels.OperatorMDR, mdrparams.Label), func() { + BeforeAll(func() { + By("Verify MDR deployment is ready") + + mdrDeployment, err := deployment.Pull( + APIClient, mdrparams.OperatorDeploymentName, medik8sparams.OperatorNs) + Expect(err).ToNot(HaveOccurred(), "Failed to get MDR deployment") + Expect(mdrDeployment.IsReady(medik8sparams.DefaultTimeout)).To(BeTrue(), + "MDR deployment is not Ready") + + By("Pre-cleaning stale test resources from previous runs") + + cleanupMDRCR(mdrparams.MDRConditionTestName) + cleanupMDRCR(mdrparams.MDRNonExistentNodeTestName) + }) + + AfterAll(func() { + By("Cleaning up condition test MDR CRs") + + cleanupMDRCR(mdrparams.MDRConditionTestName) + cleanupMDRCR(mdrparams.MDRNonExistentNodeTestName) + + By("Verifying MDR controller pod is running") + + Eventually(verifyMDRControllerRunning, + medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), + "MDR controller pod should be running after condition tests") + }) + + Context("when MDR has nhc-timed-out annotation", func() { + It("Verify MDR conditions with nhc-timed-out annotation", + reportxml.ID("65763"), + Label(labels.TierAcceptance, labels.ComponentController, + labels.DisruptionNonDestructive, labels.PlatformAny, + labels.FrequencyWeekly), func() { + By("Creating MDR with nhc-timed-out annotation") + + mdrCR := buildMDRWithAnnotations(mdrparams.MDRConditionTestName, + map[string]string{ + mdrparams.NHCTimedOutAnnotationKey: mdrparams.NHCTimedOutAnnotationValue, + }) + + err := APIClient.Create(context.TODO(), mdrCR) + Expect(err).ToNot(HaveOccurred(), + "Failed to create MDR with nhc-timed-out annotation") + + deferDeleteMDRCR(mdrparams.MDRConditionTestName) + + By("Waiting for Processing and Succeeded conditions to reflect NHC timed-out state") + + Eventually(func() error { + liveMDR := &unstructured.Unstructured{} + liveMDR.SetGroupVersionKind(mdrGVK) + + getErr := APIClient.Get(context.TODO(), + client.ObjectKey{ + Name: mdrparams.MDRConditionTestName, + Namespace: medik8sparams.OperatorNs, + }, + liveMDR) + if getErr != nil { + return getErr + } + + return verifyMDRConditionsByType(liveMDR, + expectedCondition{ + conditionType: mdrparams.ProcessingConditionType, + reason: mdrparams.ConditionReasonStoppedByNHC, + }, + expectedCondition{ + conditionType: mdrparams.SucceededConditionType, + reason: mdrparams.ConditionReasonStoppedByNHC, + }, + ) + }, medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), + "MDR conditions should reflect NHC timed-out state") + + By("Verifying controller log contains remediation-stopped message") + + Eventually(func() error { + return findMessageInControllerLogs( + mdrparams.MDRRemStoppedLogMsg, mdrparams.ControllerLogWindow) + }, medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), + "MDR controller log should contain %q", mdrparams.MDRRemStoppedLogMsg) + }) + }) + + Context("when MDR targets a non-existent node", func() { + It("Verify MDR conditions with non-existent node name", + reportxml.ID("66137"), + Label(labels.TierAcceptance, labels.ComponentController, + labels.DisruptionNonDestructive, labels.PlatformAny, + labels.FrequencyWeekly), func() { + By("Verifying test node name does not exist in the cluster") + + nodeObj := &corev1.Node{} + nodeErr := APIClient.Get(context.TODO(), + client.ObjectKey{Name: mdrparams.MDRNonExistentNodeTestName}, + nodeObj) + Expect(k8serrors.IsNotFound(nodeErr)).To(BeTrue(), + "Node %q must not exist in the cluster to avoid triggering real fencing", + mdrparams.MDRNonExistentNodeTestName) + + By("Creating MDR with non-existent node name") + + mdrCR := buildMDR(mdrparams.MDRNonExistentNodeTestName) + + err := APIClient.Create(context.TODO(), mdrCR) + Expect(err).ToNot(HaveOccurred(), + "Failed to create MDR with non-existent node name") + + deferDeleteMDRCR(mdrparams.MDRNonExistentNodeTestName) + + By("Waiting for Processing and Succeeded conditions to reflect node-not-found state") + + Eventually(func() error { + liveMDR := &unstructured.Unstructured{} + liveMDR.SetGroupVersionKind(mdrGVK) + + getErr := APIClient.Get(context.TODO(), + client.ObjectKey{ + Name: mdrparams.MDRNonExistentNodeTestName, + Namespace: medik8sparams.OperatorNs, + }, + liveMDR) + if getErr != nil { + return getErr + } + + return verifyMDRConditionsByType(liveMDR, + expectedCondition{ + conditionType: mdrparams.ProcessingConditionType, + status: mdrparams.ConditionStatusFalse, + reason: mdrparams.ConditionReasonNodeNotFound, + }, + expectedCondition{ + conditionType: mdrparams.SucceededConditionType, + status: mdrparams.ConditionStatusFalse, + reason: mdrparams.ConditionReasonNodeNotFound, + }, + ) + }, medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), + "MDR conditions should reflect node-not-found state") + }) + }) + + Context("when MDR targets a control-plane node", func() { + var controlPlaneNodeName string + + BeforeAll(func() { + By("Selecting a random control-plane node") + + cpNodes, err := listControlPlaneNodes(context.TODO(), APIClient) + Expect(err).ToNot(HaveOccurred(), "Failed to list control-plane nodes") + Expect(cpNodes.Items).ToNot(BeEmpty(), "No control-plane nodes found") + + controlPlaneNodeName = cpNodes.Items[rand.Intn(len(cpNodes.Items))].Name + GinkgoWriter.Printf("Selected control-plane node: %s\n", controlPlaneNodeName) + }) + + AfterEach(func() { + if controlPlaneNodeName != "" { + cleanupMDRCR(controlPlaneNodeName) + } + }) + + It("Verify MDR conditions with control-plane node name", + reportxml.ID("66351"), + Label(labels.TierAcceptance, labels.ComponentController, + labels.DisruptionNonDestructive, labels.PlatformAny, + labels.FrequencyWeekly), func() { + By(fmt.Sprintf("Creating MDR for control-plane node %s", controlPlaneNodeName)) + + mdrCR := buildMDR(controlPlaneNodeName) + + err := APIClient.Create(context.TODO(), mdrCR) + Expect(err).ToNot(HaveOccurred(), + "Failed to create MDR for control-plane node %s", controlPlaneNodeName) + + deferDeleteMDRCR(controlPlaneNodeName) + + By("Waiting for Processing and Succeeded conditions to reflect no-controller-owner state") + + Eventually(func() error { + liveMDR := &unstructured.Unstructured{} + liveMDR.SetGroupVersionKind(mdrGVK) + + getErr := APIClient.Get(context.TODO(), + client.ObjectKey{ + Name: controlPlaneNodeName, + Namespace: medik8sparams.OperatorNs, + }, + liveMDR) + if getErr != nil { + return getErr + } + + return verifyMDRConditionsByType(liveMDR, + expectedCondition{ + conditionType: mdrparams.ProcessingConditionType, + status: mdrparams.ConditionStatusFalse, + reason: mdrparams.ConditionReasonNoControllerOwner, + }, + expectedCondition{ + conditionType: mdrparams.SucceededConditionType, + status: mdrparams.ConditionStatusFalse, + reason: mdrparams.ConditionReasonNoControllerOwner, + }, + ) + }, medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), + "MDR conditions should reflect no-controller-owner state") + }) + + It("Verify PermanentNodeDeletionExpected condition with control-plane node", + reportxml.ID("66317"), + Label(labels.TierAcceptance, labels.ComponentController, + labels.DisruptionNonDestructive, labels.PlatformAny, + labels.FrequencyWeekly), func() { + By(fmt.Sprintf("Creating MDR for control-plane node %s", controlPlaneNodeName)) + + mdrCR := buildMDR(controlPlaneNodeName) + + err := APIClient.Create(context.TODO(), mdrCR) + Expect(err).ToNot(HaveOccurred(), + "Failed to create MDR for control-plane node %s", controlPlaneNodeName) + + deferDeleteMDRCR(controlPlaneNodeName) + + By("Detecting cluster platform for expected condition values") + + platform, _, platformErr := helpers.DetectPlatform(context.TODO(), APIClient) + Expect(platformErr).ToNot(HaveOccurred(), "Failed to detect cluster platform") + + var expectedStatus, expectedReason, expectedMessage string + + switch platform { + case configv1.BareMetalPlatformType, configv1.NonePlatformType: + expectedStatus = mdrparams.ConditionStatusFalse + expectedReason = mdrparams.ConditionReasonKeepsNodeName + expectedMessage = mdrparams.ConditionMessageKeepsNodeName + default: + expectedStatus = mdrparams.ConditionStatusTrue + expectedReason = mdrparams.ConditionReasonNewNodeName + expectedMessage = mdrparams.ConditionMessageNewNodeName + } + + GinkgoWriter.Printf("Platform %s: expecting PermanentNodeDeletionExpected "+ + "status=%s reason=%s\n", platform, expectedStatus, expectedReason) + + By("Waiting for PermanentNodeDeletionExpected condition") + + Eventually(func() error { + liveMDR := &unstructured.Unstructured{} + liveMDR.SetGroupVersionKind(mdrGVK) + + getErr := APIClient.Get(context.TODO(), + client.ObjectKey{ + Name: controlPlaneNodeName, + Namespace: medik8sparams.OperatorNs, + }, + liveMDR) + if getErr != nil { + return getErr + } + + verifyErr := verifyMDRConditionsByType(liveMDR, + expectedCondition{ + conditionType: mdrparams.PermanentNodeDeletionExpectedConditionType, + status: expectedStatus, + reason: expectedReason, + }, + ) + if verifyErr != nil { + return verifyErr + } + + return verifyMDRConditionMessage(liveMDR, + mdrparams.PermanentNodeDeletionExpectedConditionType, + expectedMessage) + }, medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), + "PermanentNodeDeletionExpected condition should match platform %s expectations", + platform) + }) + }) + }) + +// verifyMDRControllerRunning checks the MDR controller pod is running +// with the expected replica count. +func verifyMDRControllerRunning() error { + listOptions := metav1.ListOptions{ + LabelSelector: mdrparams.OperatorControllerPodLabelSelector, + } + + allPods, listErr := pod.List(APIClient, medik8sparams.OperatorNs, listOptions) + if listErr != nil { + return fmt.Errorf("failed to list MDR pods: %w", listErr) + } + + mdrPods := helpers.FilterPodsByDeployment(allPods, mdrparams.OperatorDeploymentName) + runningCount := int32(len(helpers.FilterRunningPods(mdrPods))) + + if runningCount != mdrparams.ExpectedReplicas { + return fmt.Errorf("expected %d running MDR pod(s), found %d", + mdrparams.ExpectedReplicas, runningCount) + } + + return nil +} diff --git a/tests/mdr-operator/tests/mdr_helpers.go b/tests/mdr-operator/tests/mdr_helpers.go index efdf8a096c..fd667549fc 100644 --- a/tests/mdr-operator/tests/mdr_helpers.go +++ b/tests/mdr-operator/tests/mdr_helpers.go @@ -3,6 +3,7 @@ package tests import ( "context" "fmt" + "sort" "strings" "time" @@ -354,6 +355,243 @@ func getMDRCRCondition(nodeName, condType string) (map[string]interface{}, error return nil, fmt.Errorf("condition %s not found on MDR CR %s", condType, nodeName) } +// buildMDR builds an unstructured MachineDeletionRemediation CR. +func buildMDR(name string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": mdrparams.CRDGroup + "/" + mdrparams.CRDVersion, + "kind": "MachineDeletionRemediation", + "metadata": map[string]interface{}{ + "name": name, + "namespace": medik8sparams.OperatorNs, + }, + "spec": map[string]interface{}{}, + }, + } +} + +// buildMDRWithAnnotations builds an unstructured MachineDeletionRemediation CR +// with optional annotations. +func buildMDRWithAnnotations( + name string, annotations map[string]string, +) *unstructured.Unstructured { + metadata := map[string]interface{}{ + "name": name, + "namespace": medik8sparams.OperatorNs, + } + + if annotations != nil { + annotationMap := make(map[string]interface{}, len(annotations)) + for key, val := range annotations { + annotationMap[key] = val + } + + metadata["annotations"] = annotationMap + } + + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": mdrparams.CRDGroup + "/" + mdrparams.CRDVersion, + "kind": "MachineDeletionRemediation", + "metadata": metadata, + "spec": map[string]interface{}{}, + }, + } +} + +// expectedCondition defines the expected values for a status condition. +type expectedCondition struct { + conditionType string + status string // empty means don't check + reason string +} + +// verifyMDRConditionsByType checks MDR conditions by looking up each expected +// condition by its type field, not by positional index. +func verifyMDRConditionsByType( + mdrObj *unstructured.Unstructured, expected ...expectedCondition, +) error { + conditions, found, err := unstructured.NestedSlice( + mdrObj.Object, "status", "conditions") + if err != nil { + return fmt.Errorf("failed to get status.conditions: %w", err) + } + + if !found || len(conditions) == 0 { + return fmt.Errorf("no status.conditions found") + } + + for _, exp := range expected { + condMap, findErr := findMDRConditionByType(conditions, exp.conditionType) + if findErr != nil { + return findErr + } + + condReason, reasonFound, reasonErr := unstructured.NestedString(condMap, "reason") + if reasonErr != nil { + return fmt.Errorf("condition %q reason field error: %w", + exp.conditionType, reasonErr) + } + + if !reasonFound { + return fmt.Errorf("condition %q reason field not yet written by controller", + exp.conditionType) + } + + if condReason != exp.reason { + return fmt.Errorf("condition %q reason: expected %q, got %q", + exp.conditionType, exp.reason, condReason) + } + + if exp.status != "" { + condStatus, statusFound, statusErr := unstructured.NestedString(condMap, "status") + if statusErr != nil { + return fmt.Errorf("condition %q status field error: %w", + exp.conditionType, statusErr) + } + + if !statusFound { + return fmt.Errorf("condition %q status field not yet written by controller", + exp.conditionType) + } + + if condStatus != exp.status { + return fmt.Errorf("condition %q status: expected %q, got %q", + exp.conditionType, exp.status, condStatus) + } + } + } + + return nil +} + +// findMDRConditionByType finds a condition map by its type field. +func findMDRConditionByType( + conditions []interface{}, condType string, +) (map[string]interface{}, error) { + for _, cond := range conditions { + condMap, ok := cond.(map[string]interface{}) + if !ok { + continue + } + + typeName, _, _ := unstructured.NestedString(condMap, "type") + if typeName == condType { + return condMap, nil + } + } + + return nil, fmt.Errorf("condition with type %q not found", condType) +} + +// verifyMDRConditionMessage checks that the message field of a specific +// condition type matches the expected string. +func verifyMDRConditionMessage( + mdrObj *unstructured.Unstructured, condType, expectedMessage string, +) error { + conditions, found, err := unstructured.NestedSlice( + mdrObj.Object, "status", "conditions") + if err != nil { + return fmt.Errorf("failed to get status.conditions: %w", err) + } + + if !found || len(conditions) == 0 { + return fmt.Errorf("no status.conditions found") + } + + condMap, findErr := findMDRConditionByType(conditions, condType) + if findErr != nil { + return findErr + } + + condMessage, msgFound, msgErr := unstructured.NestedString(condMap, "message") + if msgErr != nil { + return fmt.Errorf("condition %q message field error: %w", condType, msgErr) + } + + if !msgFound { + return fmt.Errorf("condition %q message field not yet written by controller", condType) + } + + if condMessage != expectedMessage { + return fmt.Errorf("condition %q message: expected %q, got %q", + condType, expectedMessage, condMessage) + } + + return nil +} + +// deferDeleteMDRCR registers cleanup for an MDR CR via DeferCleanup. +func deferDeleteMDRCR(name string) { + DeferCleanup(func() { + cleanupMDRCR(name) + }) +} + +// listControlPlaneNodes returns all master/control-plane nodes, trying both +// the "master" and "control-plane" role labels for OCP 4.14+ compat. +func listControlPlaneNodes(ctx context.Context, k8sClient client.Client) (*corev1.NodeList, error) { + nodeList := &corev1.NodeList{} + + if err := k8sClient.List(ctx, nodeList, + client.MatchingLabels{"node-role.kubernetes.io/master": ""}); err != nil { + return nil, fmt.Errorf("failed to list master nodes: %w", err) + } + + if len(nodeList.Items) == 0 { + if err := k8sClient.List(ctx, nodeList, + client.MatchingLabels{"node-role.kubernetes.io/control-plane": ""}); err != nil { + return nil, fmt.Errorf("failed to list control-plane nodes: %w", err) + } + } + + sort.Slice(nodeList.Items, func(i, j int) bool { + return nodeList.Items[i].Name < nodeList.Items[j].Name + }) + + return nodeList, nil +} + +// findMessageInControllerLogs searches MDR controller manager pod logs +// for the given message within the specified time window. +func findMessageInControllerLogs(message string, logWindow time.Duration) error { + listOptions := metav1.ListOptions{ + LabelSelector: mdrparams.OperatorControllerPodLabelSelector, + } + + mdrPods, listErr := pod.List(APIClient, medik8sparams.OperatorNs, listOptions) + if listErr != nil { + return fmt.Errorf("failed to list MDR controller pods: %w", listErr) + } + + filteredPods := helpers.FilterPodsByDeployment(mdrPods, mdrparams.OperatorDeploymentName) + if len(filteredPods) == 0 { + return fmt.Errorf("no MDR controller pods found") + } + + var lastLogErr error + + for _, mdrPod := range filteredPods { + logStr, logErr := mdrPod.GetLog(logWindow, mdrparams.ManagerContainerName) + if logErr != nil { + lastLogErr = fmt.Errorf("pod %s: %w", mdrPod.Object.Name, logErr) + + continue + } + + if strings.Contains(logStr, message) { + return nil + } + } + + if lastLogErr != nil { + return fmt.Errorf("message %q not found; last log error: %w", message, lastLogErr) + } + + return fmt.Errorf("message %q not found in any MDR controller pod logs (last %s)", + message, logWindow) +} + // logMDRControllerState logs the MDR controller pod states for failure triage. func logMDRControllerState() { pods, err := pod.List(APIClient, medik8sparams.OperatorNs, From 74db4a9195a9bf19f95788bfd392414612d6193a Mon Sep 17 00:00:00 2001 From: Gal Amado Date: Thu, 13 Aug 2026 15:28:56 +0300 Subject: [PATCH 2/8] fix: platform-aware control-plane tests + update MDR condition messages - Test 4 (OCP-66351): On cloud with CPMS, control-plane Machines have a controller owner, so MDR sets RemediationStarted instead of NoControllerOwner. Now platform-aware. - Test 5 (OCP-66317): Update condition messages to match current MDR operator ("as part of remediation" replaces "unhealthy node replaced"). - All 4 condition tests pass on AWS (OCP 4.22 nightly). Co-Authored-By: Claude Opus 4.6 --- .../mdr-operator/internal/mdrparams/const.go | 4 +- tests/mdr-operator/tests/mdr_conditions.go | 59 ++++++++++++++----- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/tests/mdr-operator/internal/mdrparams/const.go b/tests/mdr-operator/internal/mdrparams/const.go index ea55ff8abf..00111bafab 100644 --- a/tests/mdr-operator/internal/mdrparams/const.go +++ b/tests/mdr-operator/internal/mdrparams/const.go @@ -89,11 +89,11 @@ const ( ConditionReasonNewNodeName = "MachineDeletionOnCloudProviderCausesNewNodeName" // ConditionMessageKeepsNodeName is the expected message for baremetal PermanentNodeDeletionExpected. - ConditionMessageKeepsNodeName = "Machine will be deleted and the unhealthy node replaced. " + + ConditionMessageKeepsNodeName = "Machine will be deleted as part of remediation. " + "This is a BareMetal cluster provider: the new node is NOT expected to have a new name" // ConditionMessageNewNodeName is the expected message for cloud PermanentNodeDeletionExpected. - ConditionMessageNewNodeName = "Machine will be deleted and the unhealthy node replaced. " + + ConditionMessageNewNodeName = "Machine will be deleted as part of remediation. " + "This is a Cloud cluster provider: the new node is expected to have a new name" // ConditionStatusFalse is the status value "False" for condition checks. diff --git a/tests/mdr-operator/tests/mdr_conditions.go b/tests/mdr-operator/tests/mdr_conditions.go index 8108249301..abb93db9ce 100644 --- a/tests/mdr-operator/tests/mdr_conditions.go +++ b/tests/mdr-operator/tests/mdr_conditions.go @@ -177,9 +177,19 @@ var _ = Describe( }) Context("when MDR targets a control-plane node", func() { - var controlPlaneNodeName string + var ( + controlPlaneNodeName string + platform configv1.PlatformType + ) BeforeAll(func() { + By("Detecting cluster platform") + + var platformErr error + + platform, _, platformErr = helpers.DetectPlatform(context.TODO(), APIClient) + Expect(platformErr).ToNot(HaveOccurred(), "Failed to detect cluster platform") + By("Selecting a random control-plane node") cpNodes, err := listControlPlaneNodes(context.TODO(), APIClient) @@ -187,7 +197,8 @@ var _ = Describe( Expect(cpNodes.Items).ToNot(BeEmpty(), "No control-plane nodes found") controlPlaneNodeName = cpNodes.Items[rand.Intn(len(cpNodes.Items))].Name - GinkgoWriter.Printf("Selected control-plane node: %s\n", controlPlaneNodeName) + GinkgoWriter.Printf("Selected control-plane node: %s (platform: %s)\n", + controlPlaneNodeName, platform) }) AfterEach(func() { @@ -211,7 +222,35 @@ var _ = Describe( deferDeleteMDRCR(controlPlaneNodeName) - By("Waiting for Processing and Succeeded conditions to reflect no-controller-owner state") + var expectedProcessing, expectedSucceeded expectedCondition + + switch platform { + case configv1.BareMetalPlatformType, configv1.NonePlatformType: + expectedProcessing = expectedCondition{ + conditionType: mdrparams.ProcessingConditionType, + status: mdrparams.ConditionStatusFalse, + reason: mdrparams.ConditionReasonNoControllerOwner, + } + expectedSucceeded = expectedCondition{ + conditionType: mdrparams.SucceededConditionType, + status: mdrparams.ConditionStatusFalse, + reason: mdrparams.ConditionReasonNoControllerOwner, + } + default: + expectedProcessing = expectedCondition{ + conditionType: mdrparams.ProcessingConditionType, + reason: mdrparams.ConditionReasonRemediationStarted, + } + expectedSucceeded = expectedCondition{ + conditionType: mdrparams.SucceededConditionType, + reason: mdrparams.ConditionReasonRemediationStarted, + } + } + + GinkgoWriter.Printf("Platform %s: expecting Processing reason=%s, Succeeded reason=%s\n", + platform, expectedProcessing.reason, expectedSucceeded.reason) + + By("Waiting for Processing and Succeeded conditions") Eventually(func() error { liveMDR := &unstructured.Unstructured{} @@ -228,19 +267,9 @@ var _ = Describe( } return verifyMDRConditionsByType(liveMDR, - expectedCondition{ - conditionType: mdrparams.ProcessingConditionType, - status: mdrparams.ConditionStatusFalse, - reason: mdrparams.ConditionReasonNoControllerOwner, - }, - expectedCondition{ - conditionType: mdrparams.SucceededConditionType, - status: mdrparams.ConditionStatusFalse, - reason: mdrparams.ConditionReasonNoControllerOwner, - }, - ) + expectedProcessing, expectedSucceeded) }, medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), - "MDR conditions should reflect no-controller-owner state") + "MDR conditions should match expected state for platform %s", platform) }) It("Verify PermanentNodeDeletionExpected condition with control-plane node", From 2d05611e2f4931015e55120bd1358b0840322c3d Mon Sep 17 00:00:00 2001 From: Gal Amado Date: Thu, 13 Aug 2026 16:54:28 +0300 Subject: [PATCH 3/8] fix: address review findings on condition tests - Fix NHCTimedOutAnnotationValue to real RFC3339 timestamp - Remove redundant DetectPlatform() in test 5, use BeforeAll var - Whitelist known cloud platforms (AWS/Azure/GCP/vSphere), Skip unknown - Add status=False checks to test 2 (NHC timed-out conditions) - Merge message field into expectedCondition struct, remove standalone verifyMDRConditionMessage function - Add DeferCleanup in control-plane BeforeAll for safety - Remove double cleanup (DeferCleanup + AfterEach) in tests 4+5 - Update README test 7 for platform-dependent behavior - Add comment documenting cloud CPMS remediation + cleanup timing All 4 condition tests pass on AWS OCP 4.22. Co-Authored-By: Claude Opus 4.6 --- tests/mdr-operator/README.md | 10 ++-- .../mdr-operator/internal/mdrparams/const.go | 5 +- tests/mdr-operator/tests/mdr_conditions.go | 40 ++++++------- tests/mdr-operator/tests/mdr_helpers.go | 56 +++++++------------ 4 files changed, 49 insertions(+), 62 deletions(-) diff --git a/tests/mdr-operator/README.md b/tests/mdr-operator/README.md index b6c78fb31a..321e282675 100644 --- a/tests/mdr-operator/README.md +++ b/tests/mdr-operator/README.md @@ -105,15 +105,17 @@ by asserting the node name does not exist in the cluster before creating the CR. ### 7. Verify MDR Conditions With Control-Plane Node ([OCP-66351](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-66351)) -Creates an MDR CR named after a random control-plane node. Verifies that the -Processing and Succeeded conditions have status=False and reason -`RemediationCannotStartNoControllerOwner`. +Creates an MDR CR named after a random control-plane node. Platform-aware: +on baremetal (no CPMS), control-plane Machines have no controller owner and +MDR sets RemediationCannotStartNoControllerOwner. On cloud (AWS/Azure/GCP/vSphere) +with CPMS, the Machine has a controller owner and MDR sets RemediationStarted. +Skips on unknown platforms. - **Operators**: MDR v0.7.0+ - **Cluster**: Any topology (MNO or SNO) - **Environment**: Connected or disconnected - **Standalone**: `ginkgo --label-filter="mdr" --focus="control-plane node name" ./tests/mdr-operator/...` -- **Pass criteria**: Processing status=False reason=RemediationCannotStartNoControllerOwner; Succeeded status=False reason=RemediationCannotStartNoControllerOwner; MDR controller pod running after test +- **Pass criteria**: Baremetal: Processing status=False reason=RemediationCannotStartNoControllerOwner, Succeeded status=False reason=RemediationCannotStartNoControllerOwner. Cloud: Processing reason=RemediationStarted, Succeeded reason=RemediationStarted. MDR controller pod running after test ### 8. Verify PermanentNodeDeletionExpected Condition With Control-Plane Node ([OCP-66317](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-66317)) diff --git a/tests/mdr-operator/internal/mdrparams/const.go b/tests/mdr-operator/internal/mdrparams/const.go index 00111bafab..3450643b75 100644 --- a/tests/mdr-operator/internal/mdrparams/const.go +++ b/tests/mdr-operator/internal/mdrparams/const.go @@ -105,8 +105,9 @@ const ( // NHCTimedOutAnnotationKey is the annotation that signals NHC timed out. NHCTimedOutAnnotationKey = "remediation.medik8s.io/nhc-timed-out" - // NHCTimedOutAnnotationValue is a valid RFC3339 timestamp for the annotation. - NHCTimedOutAnnotationValue = "2006-01-02T15:04:05Z07:00" + // NHCTimedOutAnnotationValue is an RFC3339 timestamp for the annotation. + // The MDR controller checks only for key presence, not value format. + NHCTimedOutAnnotationValue = "2024-01-01T00:00:00Z" // MDRRemStoppedLogMsg is the log message emitted when MDR stops due to NHC timeout. MDRRemStoppedLogMsg = "NHC time out annotation found, stopping remediation" diff --git a/tests/mdr-operator/tests/mdr_conditions.go b/tests/mdr-operator/tests/mdr_conditions.go index abb93db9ce..15a3574c9b 100644 --- a/tests/mdr-operator/tests/mdr_conditions.go +++ b/tests/mdr-operator/tests/mdr_conditions.go @@ -97,10 +97,12 @@ var _ = Describe( return verifyMDRConditionsByType(liveMDR, expectedCondition{ conditionType: mdrparams.ProcessingConditionType, + status: mdrparams.ConditionStatusFalse, reason: mdrparams.ConditionReasonStoppedByNHC, }, expectedCondition{ conditionType: mdrparams.SucceededConditionType, + status: mdrparams.ConditionStatusFalse, reason: mdrparams.ConditionReasonStoppedByNHC, }, ) @@ -199,6 +201,10 @@ var _ = Describe( controlPlaneNodeName = cpNodes.Items[rand.Intn(len(cpNodes.Items))].Name GinkgoWriter.Printf("Selected control-plane node: %s (platform: %s)\n", controlPlaneNodeName, platform) + + DeferCleanup(func() { + cleanupMDRCR(controlPlaneNodeName) + }) }) AfterEach(func() { @@ -220,10 +226,11 @@ var _ = Describe( Expect(err).ToNot(HaveOccurred(), "Failed to create MDR for control-plane node %s", controlPlaneNodeName) - deferDeleteMDRCR(controlPlaneNodeName) - var expectedProcessing, expectedSucceeded expectedCondition + // On cloud with CPMS, control-plane Machines have a controller owner, + // so MDR proceeds to RemediationStarted. The CR is cleaned up in + // AfterEach before Machine deletion progresses. switch platform { case configv1.BareMetalPlatformType, configv1.NonePlatformType: expectedProcessing = expectedCondition{ @@ -236,7 +243,8 @@ var _ = Describe( status: mdrparams.ConditionStatusFalse, reason: mdrparams.ConditionReasonNoControllerOwner, } - default: + case configv1.AWSPlatformType, configv1.AzurePlatformType, + configv1.GCPPlatformType, configv1.VSpherePlatformType: expectedProcessing = expectedCondition{ conditionType: mdrparams.ProcessingConditionType, reason: mdrparams.ConditionReasonRemediationStarted, @@ -245,6 +253,9 @@ var _ = Describe( conditionType: mdrparams.SucceededConditionType, reason: mdrparams.ConditionReasonRemediationStarted, } + default: + Skip(fmt.Sprintf("Skipping: unknown platform %s -- "+ + "cannot determine expected control-plane remediation behavior", platform)) } GinkgoWriter.Printf("Platform %s: expecting Processing reason=%s, Succeeded reason=%s\n", @@ -285,13 +296,6 @@ var _ = Describe( Expect(err).ToNot(HaveOccurred(), "Failed to create MDR for control-plane node %s", controlPlaneNodeName) - deferDeleteMDRCR(controlPlaneNodeName) - - By("Detecting cluster platform for expected condition values") - - platform, _, platformErr := helpers.DetectPlatform(context.TODO(), APIClient) - Expect(platformErr).ToNot(HaveOccurred(), "Failed to detect cluster platform") - var expectedStatus, expectedReason, expectedMessage string switch platform { @@ -299,10 +303,14 @@ var _ = Describe( expectedStatus = mdrparams.ConditionStatusFalse expectedReason = mdrparams.ConditionReasonKeepsNodeName expectedMessage = mdrparams.ConditionMessageKeepsNodeName - default: + case configv1.AWSPlatformType, configv1.AzurePlatformType, + configv1.GCPPlatformType, configv1.VSpherePlatformType: expectedStatus = mdrparams.ConditionStatusTrue expectedReason = mdrparams.ConditionReasonNewNodeName expectedMessage = mdrparams.ConditionMessageNewNodeName + default: + Skip(fmt.Sprintf("Skipping: unknown platform %s -- "+ + "cannot determine expected PermanentNodeDeletionExpected values", platform)) } GinkgoWriter.Printf("Platform %s: expecting PermanentNodeDeletionExpected "+ @@ -324,20 +332,14 @@ var _ = Describe( return getErr } - verifyErr := verifyMDRConditionsByType(liveMDR, + return verifyMDRConditionsByType(liveMDR, expectedCondition{ conditionType: mdrparams.PermanentNodeDeletionExpectedConditionType, status: expectedStatus, reason: expectedReason, + message: expectedMessage, }, ) - if verifyErr != nil { - return verifyErr - } - - return verifyMDRConditionMessage(liveMDR, - mdrparams.PermanentNodeDeletionExpectedConditionType, - expectedMessage) }, medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), "PermanentNodeDeletionExpected condition should match platform %s expectations", platform) diff --git a/tests/mdr-operator/tests/mdr_helpers.go b/tests/mdr-operator/tests/mdr_helpers.go index fd667549fc..e4065a86aa 100644 --- a/tests/mdr-operator/tests/mdr_helpers.go +++ b/tests/mdr-operator/tests/mdr_helpers.go @@ -404,6 +404,7 @@ type expectedCondition struct { conditionType string status string // empty means don't check reason string + message string // empty means don't check } // verifyMDRConditionsByType checks MDR conditions by looking up each expected @@ -460,6 +461,24 @@ func verifyMDRConditionsByType( exp.conditionType, exp.status, condStatus) } } + + if exp.message != "" { + condMessage, msgFound, msgErr := unstructured.NestedString(condMap, "message") + if msgErr != nil { + return fmt.Errorf("condition %q message field error: %w", + exp.conditionType, msgErr) + } + + if !msgFound { + return fmt.Errorf("condition %q message field not yet written by controller", + exp.conditionType) + } + + if condMessage != exp.message { + return fmt.Errorf("condition %q message: expected %q, got %q", + exp.conditionType, exp.message, condMessage) + } + } } return nil @@ -484,43 +503,6 @@ func findMDRConditionByType( return nil, fmt.Errorf("condition with type %q not found", condType) } -// verifyMDRConditionMessage checks that the message field of a specific -// condition type matches the expected string. -func verifyMDRConditionMessage( - mdrObj *unstructured.Unstructured, condType, expectedMessage string, -) error { - conditions, found, err := unstructured.NestedSlice( - mdrObj.Object, "status", "conditions") - if err != nil { - return fmt.Errorf("failed to get status.conditions: %w", err) - } - - if !found || len(conditions) == 0 { - return fmt.Errorf("no status.conditions found") - } - - condMap, findErr := findMDRConditionByType(conditions, condType) - if findErr != nil { - return findErr - } - - condMessage, msgFound, msgErr := unstructured.NestedString(condMap, "message") - if msgErr != nil { - return fmt.Errorf("condition %q message field error: %w", condType, msgErr) - } - - if !msgFound { - return fmt.Errorf("condition %q message field not yet written by controller", condType) - } - - if condMessage != expectedMessage { - return fmt.Errorf("condition %q message: expected %q, got %q", - condType, expectedMessage, condMessage) - } - - return nil -} - // deferDeleteMDRCR registers cleanup for an MDR CR via DeferCleanup. func deferDeleteMDRCR(name string) { DeferCleanup(func() { From c88ea579557a6e5b20fcc3932ffa720d2b501f9c Mon Sep 17 00:00:00 2001 From: Gal Amado Date: Thu, 13 Aug 2026 18:55:19 +0300 Subject: [PATCH 4/8] fix: skip control-plane tests on cloud to prevent Machine deletion On cloud platforms with CPMS, control-plane Machine objects have a controller owner. Creating an MDR CR for a control-plane node triggers real Machine deletion (~2s), which is destructive. Skip tests 4+5 (OCP-66351, OCP-66317) on cloud -- they run safely on baremetal where control-plane nodes have no controller owner. Co-Authored-By: Claude Opus 4.6 --- tests/mdr-operator/README.md | 22 ++--- tests/mdr-operator/tests/mdr_conditions.go | 98 +++++++--------------- 2 files changed, 43 insertions(+), 77 deletions(-) diff --git a/tests/mdr-operator/README.md b/tests/mdr-operator/README.md index 321e282675..f45f954a24 100644 --- a/tests/mdr-operator/README.md +++ b/tests/mdr-operator/README.md @@ -105,24 +105,24 @@ by asserting the node name does not exist in the cluster before creating the CR. ### 7. Verify MDR Conditions With Control-Plane Node ([OCP-66351](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-66351)) -Creates an MDR CR named after a random control-plane node. Platform-aware: -on baremetal (no CPMS), control-plane Machines have no controller owner and -MDR sets RemediationCannotStartNoControllerOwner. On cloud (AWS/Azure/GCP/vSphere) -with CPMS, the Machine has a controller owner and MDR sets RemediationStarted. -Skips on unknown platforms. +Creates an MDR CR named after a random control-plane node. On baremetal, +control-plane Machines have no controller owner and MDR sets +RemediationCannotStartNoControllerOwner. Skipped on cloud platforms where +CPMS gives control-plane Machines a controller owner, causing MDR to start +Machine deletion (destructive). - **Operators**: MDR v0.7.0+ -- **Cluster**: Any topology (MNO or SNO) +- **Cluster**: Any topology (MNO or SNO), baremetal only - **Environment**: Connected or disconnected -- **Standalone**: `ginkgo --label-filter="mdr" --focus="control-plane node name" ./tests/mdr-operator/...` -- **Pass criteria**: Baremetal: Processing status=False reason=RemediationCannotStartNoControllerOwner, Succeeded status=False reason=RemediationCannotStartNoControllerOwner. Cloud: Processing reason=RemediationStarted, Succeeded reason=RemediationStarted. MDR controller pod running after test +- **Standalone**: `ginkgo --label-filter="mdr && platform:baremetal" --focus="control-plane node name" ./tests/mdr-operator/...` +- **Pass criteria**: Processing status=False reason=RemediationCannotStartNoControllerOwner; Succeeded status=False reason=RemediationCannotStartNoControllerOwner; MDR controller pod running after test ### 8. Verify PermanentNodeDeletionExpected Condition With Control-Plane Node ([OCP-66317](https://polarion.engineering.redhat.com/polarion/#/project/OSE/workitem?id=OCP-66317)) Creates an MDR CR for a control-plane node and verifies the -`PermanentNodeDeletionExpected` condition. Uses platform detection to assert -the correct status, reason, and message: baremetal clusters expect -status=False/KeepsNodeName, cloud clusters expect status=True/NewNodeName. +`PermanentNodeDeletionExpected` condition on baremetal: status=False, +reason=KeepsNodeName, message confirms node name is preserved. +Skipped on cloud platforms (same reason as test 7). - **Operators**: MDR v0.7.0+ - **Cluster**: Any topology (MNO or SNO) diff --git a/tests/mdr-operator/tests/mdr_conditions.go b/tests/mdr-operator/tests/mdr_conditions.go index 15a3574c9b..904f18c821 100644 --- a/tests/mdr-operator/tests/mdr_conditions.go +++ b/tests/mdr-operator/tests/mdr_conditions.go @@ -192,6 +192,15 @@ var _ = Describe( platform, _, platformErr = helpers.DetectPlatform(context.TODO(), APIClient) Expect(platformErr).ToNot(HaveOccurred(), "Failed to detect cluster platform") + switch platform { + case configv1.BareMetalPlatformType, configv1.NonePlatformType: + GinkgoWriter.Printf("Platform %s: control-plane nodes have no controller owner\n", platform) + default: + Skip(fmt.Sprintf("Skipping control-plane tests on %s: "+ + "CPMS gives control-plane Machines a controller owner, "+ + "causing MDR to start Machine deletion (destructive)", platform)) + } + By("Selecting a random control-plane node") cpNodes, err := listControlPlaneNodes(context.TODO(), APIClient) @@ -199,8 +208,7 @@ var _ = Describe( Expect(cpNodes.Items).ToNot(BeEmpty(), "No control-plane nodes found") controlPlaneNodeName = cpNodes.Items[rand.Intn(len(cpNodes.Items))].Name - GinkgoWriter.Printf("Selected control-plane node: %s (platform: %s)\n", - controlPlaneNodeName, platform) + GinkgoWriter.Printf("Selected control-plane node: %s\n", controlPlaneNodeName) DeferCleanup(func() { cleanupMDRCR(controlPlaneNodeName) @@ -216,7 +224,7 @@ var _ = Describe( It("Verify MDR conditions with control-plane node name", reportxml.ID("66351"), Label(labels.TierAcceptance, labels.ComponentController, - labels.DisruptionNonDestructive, labels.PlatformAny, + labels.DisruptionNonDestructive, labels.PlatformBareMetal, labels.FrequencyWeekly), func() { By(fmt.Sprintf("Creating MDR for control-plane node %s", controlPlaneNodeName)) @@ -226,42 +234,7 @@ var _ = Describe( Expect(err).ToNot(HaveOccurred(), "Failed to create MDR for control-plane node %s", controlPlaneNodeName) - var expectedProcessing, expectedSucceeded expectedCondition - - // On cloud with CPMS, control-plane Machines have a controller owner, - // so MDR proceeds to RemediationStarted. The CR is cleaned up in - // AfterEach before Machine deletion progresses. - switch platform { - case configv1.BareMetalPlatformType, configv1.NonePlatformType: - expectedProcessing = expectedCondition{ - conditionType: mdrparams.ProcessingConditionType, - status: mdrparams.ConditionStatusFalse, - reason: mdrparams.ConditionReasonNoControllerOwner, - } - expectedSucceeded = expectedCondition{ - conditionType: mdrparams.SucceededConditionType, - status: mdrparams.ConditionStatusFalse, - reason: mdrparams.ConditionReasonNoControllerOwner, - } - case configv1.AWSPlatformType, configv1.AzurePlatformType, - configv1.GCPPlatformType, configv1.VSpherePlatformType: - expectedProcessing = expectedCondition{ - conditionType: mdrparams.ProcessingConditionType, - reason: mdrparams.ConditionReasonRemediationStarted, - } - expectedSucceeded = expectedCondition{ - conditionType: mdrparams.SucceededConditionType, - reason: mdrparams.ConditionReasonRemediationStarted, - } - default: - Skip(fmt.Sprintf("Skipping: unknown platform %s -- "+ - "cannot determine expected control-plane remediation behavior", platform)) - } - - GinkgoWriter.Printf("Platform %s: expecting Processing reason=%s, Succeeded reason=%s\n", - platform, expectedProcessing.reason, expectedSucceeded.reason) - - By("Waiting for Processing and Succeeded conditions") + By("Waiting for Processing and Succeeded conditions to reflect no-controller-owner state") Eventually(func() error { liveMDR := &unstructured.Unstructured{} @@ -278,15 +251,25 @@ var _ = Describe( } return verifyMDRConditionsByType(liveMDR, - expectedProcessing, expectedSucceeded) + expectedCondition{ + conditionType: mdrparams.ProcessingConditionType, + status: mdrparams.ConditionStatusFalse, + reason: mdrparams.ConditionReasonNoControllerOwner, + }, + expectedCondition{ + conditionType: mdrparams.SucceededConditionType, + status: mdrparams.ConditionStatusFalse, + reason: mdrparams.ConditionReasonNoControllerOwner, + }, + ) }, medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), - "MDR conditions should match expected state for platform %s", platform) + "MDR conditions should reflect no-controller-owner state") }) It("Verify PermanentNodeDeletionExpected condition with control-plane node", reportxml.ID("66317"), Label(labels.TierAcceptance, labels.ComponentController, - labels.DisruptionNonDestructive, labels.PlatformAny, + labels.DisruptionNonDestructive, labels.PlatformBareMetal, labels.FrequencyWeekly), func() { By(fmt.Sprintf("Creating MDR for control-plane node %s", controlPlaneNodeName)) @@ -296,25 +279,9 @@ var _ = Describe( Expect(err).ToNot(HaveOccurred(), "Failed to create MDR for control-plane node %s", controlPlaneNodeName) - var expectedStatus, expectedReason, expectedMessage string - - switch platform { - case configv1.BareMetalPlatformType, configv1.NonePlatformType: - expectedStatus = mdrparams.ConditionStatusFalse - expectedReason = mdrparams.ConditionReasonKeepsNodeName - expectedMessage = mdrparams.ConditionMessageKeepsNodeName - case configv1.AWSPlatformType, configv1.AzurePlatformType, - configv1.GCPPlatformType, configv1.VSpherePlatformType: - expectedStatus = mdrparams.ConditionStatusTrue - expectedReason = mdrparams.ConditionReasonNewNodeName - expectedMessage = mdrparams.ConditionMessageNewNodeName - default: - Skip(fmt.Sprintf("Skipping: unknown platform %s -- "+ - "cannot determine expected PermanentNodeDeletionExpected values", platform)) - } - - GinkgoWriter.Printf("Platform %s: expecting PermanentNodeDeletionExpected "+ - "status=%s reason=%s\n", platform, expectedStatus, expectedReason) + GinkgoWriter.Printf("Expecting PermanentNodeDeletionExpected "+ + "status=%s reason=%s\n", + mdrparams.ConditionStatusFalse, mdrparams.ConditionReasonKeepsNodeName) By("Waiting for PermanentNodeDeletionExpected condition") @@ -335,14 +302,13 @@ var _ = Describe( return verifyMDRConditionsByType(liveMDR, expectedCondition{ conditionType: mdrparams.PermanentNodeDeletionExpectedConditionType, - status: expectedStatus, - reason: expectedReason, - message: expectedMessage, + status: mdrparams.ConditionStatusFalse, + reason: mdrparams.ConditionReasonKeepsNodeName, + message: mdrparams.ConditionMessageKeepsNodeName, }, ) }, medik8sparams.DefaultTimeout, mdrparams.DefaultPollInterval).Should(Succeed(), - "PermanentNodeDeletionExpected condition should match platform %s expectations", - platform) + "PermanentNodeDeletionExpected condition should reflect baremetal expectations") }) }) }) From 98d1b621decba068307060c107e71922b16bee02 Mon Sep 17 00:00:00 2001 From: Gal Amado Date: Sun, 16 Aug 2026 14:21:08 +0300 Subject: [PATCH 5/8] fix: address CodeRabbit review comments on PR #89 - Fix godot lint: replace decorative separator comments with period-ending style - Align README test 8 docs with code: baremetal-only cluster, matching standalone filter and pass criteria - Extract checkConditionField helper to reduce verifyMDRConditionsByType cognitive complexity (34 -> ~16) Co-Authored-By: Claude Opus 4.6 --- tests/mdr-operator/README.md | 6 +- .../mdr-operator/internal/mdrparams/const.go | 4 +- tests/mdr-operator/tests/mdr_helpers.go | 68 ++++++++----------- 3 files changed, 32 insertions(+), 46 deletions(-) diff --git a/tests/mdr-operator/README.md b/tests/mdr-operator/README.md index f45f954a24..e00a6090c1 100644 --- a/tests/mdr-operator/README.md +++ b/tests/mdr-operator/README.md @@ -125,10 +125,10 @@ reason=KeepsNodeName, message confirms node name is preserved. Skipped on cloud platforms (same reason as test 7). - **Operators**: MDR v0.7.0+ -- **Cluster**: Any topology (MNO or SNO) +- **Cluster**: Any topology (MNO or SNO), baremetal only - **Environment**: Connected or disconnected -- **Standalone**: `ginkgo --label-filter="mdr" --focus="PermanentNodeDeletionExpected" ./tests/mdr-operator/...` -- **Pass criteria**: PermanentNodeDeletionExpected status/reason/message match platform (baremetal: False/KeepsNodeName/"...NOT expected..."; cloud: True/NewNodeName/"...expected..."); MDR controller pod running after test +- **Standalone**: `ginkgo --label-filter="mdr && platform:baremetal" --focus="PermanentNodeDeletionExpected" ./tests/mdr-operator/...` +- **Pass criteria**: PermanentNodeDeletionExpected status=False reason=KeepsNodeName message="...NOT expected to have a new name"; MDR controller pod running after test ## Destructive Tests -- NHC-Triggered Remediation diff --git a/tests/mdr-operator/internal/mdrparams/const.go b/tests/mdr-operator/internal/mdrparams/const.go index 3450643b75..9c5f2f094f 100644 --- a/tests/mdr-operator/internal/mdrparams/const.go +++ b/tests/mdr-operator/internal/mdrparams/const.go @@ -24,7 +24,7 @@ const ( // CSVNamePattern is the substring used to match the MDR operator ClusterServiceVersion by name. CSVNamePattern = "machine-deletion-remediation" - // --- Remediation test constants (RHWA-1248) --- + // Remediation test constants (RHWA-1248). // NHCCRDName is the CRD name for NodeHealthCheck, used to detect if NHC is installed. NHCCRDName = "nodehealthchecks.remediation.medik8s.io" @@ -68,7 +68,7 @@ const ( // conditions when remediation begins. ConditionReasonRemediationStarted = "RemediationStarted" - // --- Condition test constants (RHWA-1249) --- + // Condition test constants (RHWA-1249). // ConditionReasonStoppedByNHC is the reason set when NHC timed-out annotation is present. ConditionReasonStoppedByNHC = "RemediationStoppedByNHC" diff --git a/tests/mdr-operator/tests/mdr_helpers.go b/tests/mdr-operator/tests/mdr_helpers.go index e4065a86aa..f964dde748 100644 --- a/tests/mdr-operator/tests/mdr_helpers.go +++ b/tests/mdr-operator/tests/mdr_helpers.go @@ -428,57 +428,43 @@ func verifyMDRConditionsByType( return findErr } - condReason, reasonFound, reasonErr := unstructured.NestedString(condMap, "reason") - if reasonErr != nil { - return fmt.Errorf("condition %q reason field error: %w", - exp.conditionType, reasonErr) - } - - if !reasonFound { - return fmt.Errorf("condition %q reason field not yet written by controller", - exp.conditionType) - } - - if condReason != exp.reason { - return fmt.Errorf("condition %q reason: expected %q, got %q", - exp.conditionType, exp.reason, condReason) + if err := checkConditionField(condMap, exp.conditionType, "reason", exp.reason); err != nil { + return err } if exp.status != "" { - condStatus, statusFound, statusErr := unstructured.NestedString(condMap, "status") - if statusErr != nil { - return fmt.Errorf("condition %q status field error: %w", - exp.conditionType, statusErr) - } - - if !statusFound { - return fmt.Errorf("condition %q status field not yet written by controller", - exp.conditionType) - } - - if condStatus != exp.status { - return fmt.Errorf("condition %q status: expected %q, got %q", - exp.conditionType, exp.status, condStatus) + if err := checkConditionField(condMap, exp.conditionType, "status", exp.status); err != nil { + return err } } if exp.message != "" { - condMessage, msgFound, msgErr := unstructured.NestedString(condMap, "message") - if msgErr != nil { - return fmt.Errorf("condition %q message field error: %w", - exp.conditionType, msgErr) + if err := checkConditionField(condMap, exp.conditionType, "message", exp.message); err != nil { + return err } + } + } - if !msgFound { - return fmt.Errorf("condition %q message field not yet written by controller", - exp.conditionType) - } + return nil +} - if condMessage != exp.message { - return fmt.Errorf("condition %q message: expected %q, got %q", - exp.conditionType, exp.message, condMessage) - } - } +// checkConditionField validates a single field on a condition map. +func checkConditionField( + condMap map[string]interface{}, condType, field, expected string, +) error { + actual, found, err := unstructured.NestedString(condMap, field) + if err != nil { + return fmt.Errorf("condition %q %s field error: %w", condType, field, err) + } + + if !found { + return fmt.Errorf("condition %q %s field not yet written by controller", + condType, field) + } + + if actual != expected { + return fmt.Errorf("condition %q %s: expected %q, got %q", + condType, field, expected, actual) } return nil From 5471ea5d1856be7b69cb89bb9a41c9d9fcd629aa Mon Sep 17 00:00:00 2001 From: Gal Amado Date: Sun, 16 Aug 2026 23:46:40 +0300 Subject: [PATCH 6/8] mdr-operator: address razo7 review comments on PR #89 - Add pre-clean for control-plane node MDR CR in Context BeforeAll (name not known until runtime, so can't pre-clean at Describe level) - Replace all context.TODO() with context.Background() for consistency - Remove trailing period from section separator comments (not godot-relevant since they precede blank lines) Co-Authored-By: Claude Opus 4.6 --- .../mdr-operator/internal/mdrparams/const.go | 4 +-- tests/mdr-operator/tests/mdr_conditions.go | 26 +++++++++++-------- tests/mdr-operator/tests/mdr_helpers.go | 10 +++---- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/tests/mdr-operator/internal/mdrparams/const.go b/tests/mdr-operator/internal/mdrparams/const.go index 9c5f2f094f..dd574016e8 100644 --- a/tests/mdr-operator/internal/mdrparams/const.go +++ b/tests/mdr-operator/internal/mdrparams/const.go @@ -24,7 +24,7 @@ const ( // CSVNamePattern is the substring used to match the MDR operator ClusterServiceVersion by name. CSVNamePattern = "machine-deletion-remediation" - // Remediation test constants (RHWA-1248). + // Remediation test constants (RHWA-1248) // NHCCRDName is the CRD name for NodeHealthCheck, used to detect if NHC is installed. NHCCRDName = "nodehealthchecks.remediation.medik8s.io" @@ -68,7 +68,7 @@ const ( // conditions when remediation begins. ConditionReasonRemediationStarted = "RemediationStarted" - // Condition test constants (RHWA-1249). + // Condition test constants (RHWA-1249) // ConditionReasonStoppedByNHC is the reason set when NHC timed-out annotation is present. ConditionReasonStoppedByNHC = "RemediationStoppedByNHC" diff --git a/tests/mdr-operator/tests/mdr_conditions.go b/tests/mdr-operator/tests/mdr_conditions.go index 904f18c821..1cea8faab5 100644 --- a/tests/mdr-operator/tests/mdr_conditions.go +++ b/tests/mdr-operator/tests/mdr_conditions.go @@ -72,7 +72,7 @@ var _ = Describe( mdrparams.NHCTimedOutAnnotationKey: mdrparams.NHCTimedOutAnnotationValue, }) - err := APIClient.Create(context.TODO(), mdrCR) + err := APIClient.Create(context.Background(), mdrCR) Expect(err).ToNot(HaveOccurred(), "Failed to create MDR with nhc-timed-out annotation") @@ -84,7 +84,7 @@ var _ = Describe( liveMDR := &unstructured.Unstructured{} liveMDR.SetGroupVersionKind(mdrGVK) - getErr := APIClient.Get(context.TODO(), + getErr := APIClient.Get(context.Background(), client.ObjectKey{ Name: mdrparams.MDRConditionTestName, Namespace: medik8sparams.OperatorNs, @@ -128,7 +128,7 @@ var _ = Describe( By("Verifying test node name does not exist in the cluster") nodeObj := &corev1.Node{} - nodeErr := APIClient.Get(context.TODO(), + nodeErr := APIClient.Get(context.Background(), client.ObjectKey{Name: mdrparams.MDRNonExistentNodeTestName}, nodeObj) Expect(k8serrors.IsNotFound(nodeErr)).To(BeTrue(), @@ -139,7 +139,7 @@ var _ = Describe( mdrCR := buildMDR(mdrparams.MDRNonExistentNodeTestName) - err := APIClient.Create(context.TODO(), mdrCR) + err := APIClient.Create(context.Background(), mdrCR) Expect(err).ToNot(HaveOccurred(), "Failed to create MDR with non-existent node name") @@ -151,7 +151,7 @@ var _ = Describe( liveMDR := &unstructured.Unstructured{} liveMDR.SetGroupVersionKind(mdrGVK) - getErr := APIClient.Get(context.TODO(), + getErr := APIClient.Get(context.Background(), client.ObjectKey{ Name: mdrparams.MDRNonExistentNodeTestName, Namespace: medik8sparams.OperatorNs, @@ -189,7 +189,7 @@ var _ = Describe( var platformErr error - platform, _, platformErr = helpers.DetectPlatform(context.TODO(), APIClient) + platform, _, platformErr = helpers.DetectPlatform(context.Background(), APIClient) Expect(platformErr).ToNot(HaveOccurred(), "Failed to detect cluster platform") switch platform { @@ -203,13 +203,17 @@ var _ = Describe( By("Selecting a random control-plane node") - cpNodes, err := listControlPlaneNodes(context.TODO(), APIClient) + cpNodes, err := listControlPlaneNodes(context.Background(), APIClient) Expect(err).ToNot(HaveOccurred(), "Failed to list control-plane nodes") Expect(cpNodes.Items).ToNot(BeEmpty(), "No control-plane nodes found") controlPlaneNodeName = cpNodes.Items[rand.Intn(len(cpNodes.Items))].Name GinkgoWriter.Printf("Selected control-plane node: %s\n", controlPlaneNodeName) + By("Pre-cleaning stale control-plane MDR CR from previous runs") + + cleanupMDRCR(controlPlaneNodeName) + DeferCleanup(func() { cleanupMDRCR(controlPlaneNodeName) }) @@ -230,7 +234,7 @@ var _ = Describe( mdrCR := buildMDR(controlPlaneNodeName) - err := APIClient.Create(context.TODO(), mdrCR) + err := APIClient.Create(context.Background(), mdrCR) Expect(err).ToNot(HaveOccurred(), "Failed to create MDR for control-plane node %s", controlPlaneNodeName) @@ -240,7 +244,7 @@ var _ = Describe( liveMDR := &unstructured.Unstructured{} liveMDR.SetGroupVersionKind(mdrGVK) - getErr := APIClient.Get(context.TODO(), + getErr := APIClient.Get(context.Background(), client.ObjectKey{ Name: controlPlaneNodeName, Namespace: medik8sparams.OperatorNs, @@ -275,7 +279,7 @@ var _ = Describe( mdrCR := buildMDR(controlPlaneNodeName) - err := APIClient.Create(context.TODO(), mdrCR) + err := APIClient.Create(context.Background(), mdrCR) Expect(err).ToNot(HaveOccurred(), "Failed to create MDR for control-plane node %s", controlPlaneNodeName) @@ -289,7 +293,7 @@ var _ = Describe( liveMDR := &unstructured.Unstructured{} liveMDR.SetGroupVersionKind(mdrGVK) - getErr := APIClient.Get(context.TODO(), + getErr := APIClient.Get(context.Background(), client.ObjectKey{ Name: controlPlaneNodeName, Namespace: medik8sparams.OperatorNs, diff --git a/tests/mdr-operator/tests/mdr_helpers.go b/tests/mdr-operator/tests/mdr_helpers.go index f964dde748..d4faaf944c 100644 --- a/tests/mdr-operator/tests/mdr_helpers.go +++ b/tests/mdr-operator/tests/mdr_helpers.go @@ -113,7 +113,7 @@ func isNHCCRDInstalled() bool { crd := &apiextensionsv1.CustomResourceDefinition{} err := APIClient.Get( - context.TODO(), + context.Background(), types.NamespacedName{Name: mdrparams.NHCCRDName}, crd, ) @@ -200,19 +200,19 @@ func deleteRemediationCR( // cleanupNHCCR safely deletes a NodeHealthCheck CR by name. func cleanupNHCCR(name string) { deleteRemediationCR( - context.TODO(), APIClient, nhcGVK, name) + context.Background(), APIClient, nhcGVK, name) } // cleanupMDRT safely deletes a MachineDeletionRemediationTemplate CR by name. func cleanupMDRT(name string) { deleteRemediationCR( - context.TODO(), APIClient, mdrtGVK, name) + context.Background(), APIClient, mdrtGVK, name) } // cleanupMDRCR safely deletes a MachineDeletionRemediation CR by name. func cleanupMDRCR(name string) { deleteRemediationCR( - context.TODO(), APIClient, mdrGVK, name) + context.Background(), APIClient, mdrGVK, name) } // waitForMDRRemediationComplete polls until MDR remediation finishes. @@ -329,7 +329,7 @@ func getMDRCRCondition(nodeName, condType string) (map[string]interface{}, error mdrObj := &unstructured.Unstructured{} mdrObj.SetGroupVersionKind(mdrGVK) - if err := APIClient.Get(context.TODO(), types.NamespacedName{ + if err := APIClient.Get(context.Background(), types.NamespacedName{ Name: nodeName, Namespace: medik8sparams.OperatorNs, }, mdrObj); err != nil { From 03bac6c68d670b7ee35426ec962eedf963d95ac8 Mon Sep 17 00:00:00 2001 From: Gal Amado Date: Mon, 17 Aug 2026 11:49:15 +0300 Subject: [PATCH 7/8] mdr-operator: replace node role string literals with named constants Add MasterRoleLabel, ControlPlaneRoleLabel, and WorkerRoleLabel constants to mdrparams (mirroring medik8s/common/pkg/labels) and replace all hardcoded node-role.kubernetes.io/* strings in mdr_helpers.go. Co-Authored-By: Claude Opus 4.6 --- tests/mdr-operator/internal/mdrparams/const.go | 11 +++++++++++ tests/mdr-operator/tests/mdr_helpers.go | 8 ++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/mdr-operator/internal/mdrparams/const.go b/tests/mdr-operator/internal/mdrparams/const.go index dd574016e8..e7cad28705 100644 --- a/tests/mdr-operator/internal/mdrparams/const.go +++ b/tests/mdr-operator/internal/mdrparams/const.go @@ -120,4 +120,15 @@ const ( // ControllerLogWindow is the time window for searching controller logs. ControllerLogWindow = 5 * time.Minute + + // Node role labels (mirrors medik8s/common/pkg/labels) + + // MasterRoleLabel is the old role label for control-plane nodes. + MasterRoleLabel = "node-role.kubernetes.io/master" + + // ControlPlaneRoleLabel is the new role label for control-plane nodes. + ControlPlaneRoleLabel = "node-role.kubernetes.io/control-plane" + + // WorkerRoleLabel is the role label for worker nodes. + WorkerRoleLabel = "node-role.kubernetes.io/worker" ) diff --git a/tests/mdr-operator/tests/mdr_helpers.go b/tests/mdr-operator/tests/mdr_helpers.go index d4faaf944c..90615b42d7 100644 --- a/tests/mdr-operator/tests/mdr_helpers.go +++ b/tests/mdr-operator/tests/mdr_helpers.go @@ -77,7 +77,7 @@ func buildNHCForMDR(name, mdrtName string) *unstructured.Unstructured { "selector": map[string]interface{}{ "matchExpressions": []interface{}{ map[string]interface{}{ - "key": "node-role.kubernetes.io/worker", + "key": mdrparams.WorkerRoleLabel, "operator": "Exists", }, }, @@ -288,7 +288,7 @@ func waitForMDRRemediationComplete( // Worker count restored. Find the replacement node. nodeList := &corev1.NodeList{} if listErr := APIClient.List(ctx, nodeList, - client.MatchingLabels{"node-role.kubernetes.io/worker": ""}); listErr != nil { + client.MatchingLabels{mdrparams.WorkerRoleLabel: ""}); listErr != nil { return false, nil } @@ -502,13 +502,13 @@ func listControlPlaneNodes(ctx context.Context, k8sClient client.Client) (*corev nodeList := &corev1.NodeList{} if err := k8sClient.List(ctx, nodeList, - client.MatchingLabels{"node-role.kubernetes.io/master": ""}); err != nil { + client.MatchingLabels{mdrparams.MasterRoleLabel: ""}); err != nil { return nil, fmt.Errorf("failed to list master nodes: %w", err) } if len(nodeList.Items) == 0 { if err := k8sClient.List(ctx, nodeList, - client.MatchingLabels{"node-role.kubernetes.io/control-plane": ""}); err != nil { + client.MatchingLabels{mdrparams.ControlPlaneRoleLabel: ""}); err != nil { return nil, fmt.Errorf("failed to list control-plane nodes: %w", err) } } From 3e2f0690f6fd7d2f64980efb341cd5ae3b838fbc Mon Sep 17 00:00:00 2001 From: Gal Amado Date: Mon, 17 Aug 2026 15:39:23 +0300 Subject: [PATCH 8/8] mdr-operator: address ugreener review comments on PR #89 - Remove unused constants: ConditionReasonNewNodeName, ConditionMessageNewNodeName, ConditionStatusTrue - Check found+err in findMDRConditionByType (match checkConditionField) - Add By() annotation in DeferCleanup for Ginkgo report visibility Co-Authored-By: Claude Opus 4.6 --- tests/mdr-operator/internal/mdrparams/const.go | 10 ---------- tests/mdr-operator/tests/mdr_conditions.go | 1 + tests/mdr-operator/tests/mdr_helpers.go | 6 +++++- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/tests/mdr-operator/internal/mdrparams/const.go b/tests/mdr-operator/internal/mdrparams/const.go index e7cad28705..b8dba85127 100644 --- a/tests/mdr-operator/internal/mdrparams/const.go +++ b/tests/mdr-operator/internal/mdrparams/const.go @@ -85,23 +85,13 @@ const ( // ConditionReasonKeepsNodeName is the reason for baremetal clusters where the node name is preserved. ConditionReasonKeepsNodeName = "MachineDeletionOnBareMetalProviderKeepsNodeName" - // ConditionReasonNewNodeName is the reason for cloud clusters where the node gets a new name. - ConditionReasonNewNodeName = "MachineDeletionOnCloudProviderCausesNewNodeName" - // ConditionMessageKeepsNodeName is the expected message for baremetal PermanentNodeDeletionExpected. ConditionMessageKeepsNodeName = "Machine will be deleted as part of remediation. " + "This is a BareMetal cluster provider: the new node is NOT expected to have a new name" - // ConditionMessageNewNodeName is the expected message for cloud PermanentNodeDeletionExpected. - ConditionMessageNewNodeName = "Machine will be deleted as part of remediation. " + - "This is a Cloud cluster provider: the new node is expected to have a new name" - // ConditionStatusFalse is the status value "False" for condition checks. ConditionStatusFalse = "False" - // ConditionStatusTrue is the status value "True" for condition checks. - ConditionStatusTrue = "True" - // NHCTimedOutAnnotationKey is the annotation that signals NHC timed out. NHCTimedOutAnnotationKey = "remediation.medik8s.io/nhc-timed-out" diff --git a/tests/mdr-operator/tests/mdr_conditions.go b/tests/mdr-operator/tests/mdr_conditions.go index 1cea8faab5..88b7540af6 100644 --- a/tests/mdr-operator/tests/mdr_conditions.go +++ b/tests/mdr-operator/tests/mdr_conditions.go @@ -215,6 +215,7 @@ var _ = Describe( cleanupMDRCR(controlPlaneNodeName) DeferCleanup(func() { + By("Safety-net cleanup of control-plane MDR CR") cleanupMDRCR(controlPlaneNodeName) }) }) diff --git a/tests/mdr-operator/tests/mdr_helpers.go b/tests/mdr-operator/tests/mdr_helpers.go index 90615b42d7..8e431d1214 100644 --- a/tests/mdr-operator/tests/mdr_helpers.go +++ b/tests/mdr-operator/tests/mdr_helpers.go @@ -480,7 +480,11 @@ func findMDRConditionByType( continue } - typeName, _, _ := unstructured.NestedString(condMap, "type") + typeName, found, err := unstructured.NestedString(condMap, "type") + if err != nil || !found { + continue + } + if typeName == condType { return condMap, nil }