tests/snr: add 5 destructive remediation tests (worker + master)

[gamado]

Summary

Port SNR worker and master remediation tests from Python (ocp-edge-auto) to Go/Ginkgo. These tests stop kubelet on nodes, NHC detects unhealthy state, SNR creates remediation CRs, nodes reboot and recover.

Tests added

• OCP-52416: Worker kubelet stop with NHC detection
• OCP-50772: ResourceDeletion strategy -- workload pod eviction
• OCP-61594: OutOfServiceTaint strategy -- workload pod eviction
• OCP-55059: Master kubelet stop with NHC detection
• OCP-56069: Simultaneous master + worker kubelet stop

Key implementation details

• waitForRemediationComplete handles both fast flow (remediation completes during oc debug timeout) and normal flow (test observes SNR CR lifecycle)
• stopKubeletForRemediation wraps helpers.StopKubelet with error suppression for oc debug timeout and debug pod teardown races
• deleteRemediationCR uses retry-safe get-delete-confirm pattern (aligned with FAR PR Add 6 standalone FAR destructive tests and improve test resiliency #49)
• selectMasterNode / countReadyMasterNodes try both master and control-plane role labels for OCP 4.14+ compatibility
• NHC CRs include minHealthy: 51% required by NHC admission webhook
• Workload pods use registry.k8s.io/pause:3.9 (multi-arch: x86 + ARM64)
• GVK declarations refactored from functions to package-level vars (aligned with FAR PR Add 6 standalone FAR destructive tests and improve test resiliency #49)

Files changed

• New: tests/snr-operator/tests/worker_remediation.go (3 tests)
• New: tests/snr-operator/tests/master_remediation.go (2 tests)
• Modified: tests/snr-operator/tests/helpers.go (+493 lines of remediation helpers)
• Modified: tests/snr-operator/internal/snrparams/const.go (+65 lines of constants)
• Modified: tests/snr-operator/README.md (destructive test documentation)
• Modified: config_lifecycle.go, processing_conditions.go (snrGVK() -> snrGVK var)

Test plan

• go build ./tests/snr-operator/... -- passes
• go vet ./tests/snr-operator/... -- passes
• gofmt -l tests/snr-operator/ -- no changes needed
• All 5 tests pass on AWS x86 OCP 4.22 (31m 47s)
• All 5 tests pass on nvd-srv-16 ARM64 OCP 4.22 (31m 29s)
• Negative validation: all 5 tests correctly fail with bogus SNRT
• /review-checklist -- 0 violations across 48 rules

Jira: RHWA-1077

Summary by CodeRabbit

• New Features

  • Added destructive remediation coverage for worker and master nodes.
  • Added scenarios validating automatic, resource-deletion, and out-of-service-taint remediation strategies.
  • Added verification for node reboot, recovery, workload eviction, and simultaneous master/worker remediation.

• Documentation

  • Expanded test prerequisites, commands, labels, and pass criteria for destructive test execution.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Changes

SNR destructive remediation coverage

Layer / File(s)	Summary
Destructive test documentation tests/snr-operator/README.md	Documents destructive prerequisites, label-filtered commands, and worker/master remediation scenarios.
Remediation constants and GVK wiring tests/snr-operator/internal/snrparams/const.go, tests/snr-operator/tests/helpers.go, tests/snr-operator/tests/config_lifecycle.go, tests/snr-operator/tests/processing_conditions.go	Adds remediation constants and changes SNR GVK helpers to package-level values used by existing tests.
Remediation helper workflow tests/snr-operator/tests/helpers.go	Adds log search, kubelet stopping, NHC/SNRT construction, node selection, remediation polling, workload eviction, and cleanup helpers.
Worker remediation suite tests/snr-operator/tests/worker_remediation.go	Tests automatic, ResourceDeletion, and OutOfServiceTaint remediation after worker kubelet disruption.
Master remediation suite tests/snr-operator/tests/master_remediation.go	Tests master-only and simultaneous master/worker remediation, reboot detection, recovery, and resource cleanup.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• medik8s/system-tests#17: Overlaps in SNR helper GVK wiring and condition-test areas.
• medik8s/system-tests#39: Overlaps in shared SNRC helper and GVK changes.

Suggested labels: lgtm

Suggested reviewers: ugreener, razo7, slintes, mshitrit

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: five destructive remediation tests covering worker and master nodes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gamado]

/test 4.22-konflux-e2e-snr-aws

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/snr-operator/tests/helpers.go (1)

206-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

findMessageInControllerLogs duplicates findMessageInDSPodLogs.

This function is a near-verbatim copy of findMessageInDSPodLogs (lines 170-205), differing only in the label selector and two error strings. Consider extracting a shared helper to avoid drift.

♻️ Suggested shared helper

// findMessageInPodLogs lists pods matching labelSelector and searches
// their recent logs for message. podKind is used only in error text.
func findMessageInPodLogs(labelSelector, podKind, message string, logWindow time.Duration) error {
	pods, listErr := pod.List(APIClient, medik8sparams.OperatorNs, metav1.ListOptions{
		LabelSelector: labelSelector,
	})
	if listErr != nil {
		return fmt.Errorf("failed to list SNR %s pods: %w", podKind, listErr)
	}
	if len(pods) == 0 {
		return fmt.Errorf("no SNR %s pods found", podKind)
	}

	var lastLogErr error
	for _, p := range pods {
		logStr, logErr := p.GetLog(logWindow, "")
		if logErr != nil {
			lastLogErr = fmt.Errorf("pod %s: %w", p.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 SNR %s pod logs (last %s)", message, podKind, logWindow)
}

Then have both callers delegate, e.g. findMessageInControllerLogs → findMessageInPodLogs(snrparams.OperatorControllerPodLabelSelector, "controller", message, logWindow).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/snr-operator/tests/helpers.go` around lines 206 - 245,
`findMessageInControllerLogs` is a near-duplicate of `findMessageInDSPodLogs`,
so it should delegate to a shared helper instead of maintaining separate
copy-pasted logic. Extract the common pod log search flow into a helper such as
`findMessageInPodLogs`, parameterized by label selector and pod kind/error text,
and have `findMessageInControllerLogs` and `findMessageInDSPodLogs` call it with
their respective selectors and names. Keep the existing behavior for listing
pods, checking `GetLog`, and returning the same error shapes, but centralize the
implementation to prevent drift.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/snr-operator/tests/master_remediation.go`:
- Around line 263-286: The simultaneous remediation test in
master_remediation.go is missing the node creation timestamp guard used by the
single-node test. Before triggering remediation, record the creation timestamps
for both targetMasterName and targetWorkerName using the same pattern as the
earlier test, then after waitForRemediationComplete and the WaitForNodeReady
checks, verify each node’s creation timestamp is unchanged to confirm reboot
rather than recreation. Use the existing remediation flow and helpers in the
test case where currentNHCNames is cleaned up, and keep the new assertions
alongside the boot ID-based recovery checks.

---

Nitpick comments:
In `@tests/snr-operator/tests/helpers.go`:
- Around line 206-245: `findMessageInControllerLogs` is a near-duplicate of
`findMessageInDSPodLogs`, so it should delegate to a shared helper instead of
maintaining separate copy-pasted logic. Extract the common pod log search flow
into a helper such as `findMessageInPodLogs`, parameterized by label selector
and pod kind/error text, and have `findMessageInControllerLogs` and
`findMessageInDSPodLogs` call it with their respective selectors and names. Keep
the existing behavior for listing pods, checking `GetLog`, and returning the
same error shapes, but centralize the implementation to prevent drift.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 05ce87cc-2b2b-418b-8fe8-d74d474602e9

📥 Commits

Reviewing files that changed from the base of the PR and between 1534e0d and 9009bad.

📒 Files selected for processing (8)

• tests/mdr-operator/README.md
• tests/snr-operator/README.md
• tests/snr-operator/internal/snrparams/const.go
• tests/snr-operator/tests/config_lifecycle.go
• tests/snr-operator/tests/helpers.go
• tests/snr-operator/tests/master_remediation.go
• tests/snr-operator/tests/processing_conditions.go
• tests/snr-operator/tests/worker_remediation.go

[gamado]

/test 4.22-konflux-e2e-snr-aws

[ugreener]

/lgtm

[razo7]

Consider adding a BeforeEach that verifies the controller deployment is Ready before each destructive test:

BeforeEach(func() {
    By("Verifying FAR controller is Ready before test")
    farDeployment, err := deployment.Pull(
        APIClient, farparams.OperatorDeploymentName, medik8sparams.OperatorNs)
    Expect(err).ToNot(HaveOccurred())
    Expect(farDeployment.IsReady(medik8sparams.DefaultTimeout)).To(BeTrue(),
        "FAR controller is not Ready - webhook will be unreachable")
})

With ContinueOnFailure, if a prior test leaves the cluster degraded, the SNR controller may not be ready when the next test starts. Adding an equivalent BeforeEach for snrparams.OperatorDeploymentName would catch this early instead of timing out 15 minutes into the test.

Same applies to master_remediation.go:23.

[gamado]

Fixed in 4c5f289

[gamado]

/test 4.22-konflux-e2e-snr-aws

[gamado]

/retest

[gamado]

/retest

[gamado]

/test 4.22-konflux-e2e-snr-aws

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gamado, mshitrit

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [mshitrit]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment