Add FAR destructive test constants and cluster helpers

[ugreener]

Summary

Add timeout constants, fence agent identifiers, AWS credential field names, and cluster-level helper functions to support FAR destructive E2E tests. This is the foundation layer that subsequent PRs (#23 and #22) build upon.

Changes

• Extended tests/far-operator/internal/farparams/const.go with: timeout constants (NodeReadyTimeout, NodeNotReadyTimeout, NodeRebootTimeout, OcDebugTimeout, FARConditionTimeout), fence agent names (fence_aws, fence_ipmilan), node identifier parameters (--plug, --ipport), AWS credential Secret/field names, and the controller lease prefix
• Added OperatorControllerPodLabels two-label map to tests/far-operator/internal/farparams/farvars.go for controller pod selection matching the control-plane: controller-manager + app.kubernetes.io/name labeling convention used across medik8s operators
• New tests/far-operator/internal/farutils/harness.go with cluster helpers: platform detection via Infrastructure CR, AWS instance ID extraction from providerID, AWS credentials from CCO Secret, worker node selection with leader exclusion and deterministic ordering, leader controller node discovery via lease inspection, AWS node parameter map building, ready worker count, and controller pod readiness check

Dependencies

Part 1 of 3 for RHWA-962. No dependencies. Node disruption helpers in #23, test scaffold in #22.

Merge order: this PR first, then #23, then #22.

Jira: RHWA-962

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ugreener

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 [ugreener]

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new farparams/const.go file defining 21 exported constants (polling intervals, CRD names, PSA labels, fence agent identifiers, AWS credential keys, node/FAR timeouts, and a lease prefix), extends farparams/farvars.go with an OperatorControllerPodLabels selector map, and introduces farutils/harness.go with nine exported functions for platform detection, AWS credential retrieval, worker-node selection/counting, controller Lease-based leader discovery, AWS fence parameter construction, and FAR controller Pod listing.

Changes

FAR Test Infrastructure

Layer / File(s)	Summary
farparams constants and label selectors tests/far-operator/internal/farparams/const.go, tests/far-operator/internal/farparams/farvars.go	Defines 21 exported constants (polling interval, CRD names, PSA labels, priority class, controller label/lease keys, fence agent binary/parameter names, AWS credential fields, node/FAR timeout durations) and the OperatorControllerPodLabels selector map consumed by GetFARControllerPods.
Platform detection and AWS helpers tests/far-operator/internal/farutils/harness.go	Adds DetectPlatform (fetches Infrastructure/cluster, returns platform type and AWS region), FenceAgentForPlatform (maps platform to fence agent binary and node-identifier parameter), ExtractAWSInstanceID (parses aws:// providerID), and GetAWSCredentials (reads CCO-provisioned Secret fields with missing-key errors).
Worker node selection, counting, and readiness predicate tests/far-operator/internal/farutils/harness.go	Adds SelectWorkerNode (sorts and filters worker nodes by name exclusion, scheduling state, and NodeReady condition), CountReadyWorkerNodes (counts Ready schedulable workers), and the internal isNodeReady predicate scanning NodeReady conditions.
Controller discovery, AWS node parameters, and Pod listing tests/far-operator/internal/farutils/harness.go	Adds GetActiveFARControllerNode (Lease-based leader discovery returning the leader Pod's spec.nodeName), BuildAWSNodeParameters (builds the fence_aws --plug map from Ready/schedulable workers and extracted instance IDs), and GetFARControllerPods (lists controller Pods filtered by label selectors and all-containers-Ready).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• lyfofvipin
• maximunited
• clobrano

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 94.44% which is sufficient. The required threshold is 80.00%.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add FAR destructive test constants and cluster helpers' clearly and concisely summarizes the main changes: introducing new constants and helper functions for FAR destructive tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

tests/far-operator/internal/farutils/harness.go (1)

244-265: ⚡ Quick win

Hardcoded label value duplicates GetFARControllerPods.

Line 248 hardcodes "fence-agents-remediation-operator", which also appears in GetFARControllerPods (Line 213). Consider extracting this as a constant in farparams (e.g., ControllerPodLabelValue) to maintain consistency and simplify future updates.

🤖 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/far-operator/internal/farutils/harness.go` around lines 244 - 265, The
hardcoded string "fence-agents-remediation-operator" in the
ListFARControllerPodsByLabel function duplicates the same value used in
GetFARControllerPods. Extract this value as a constant in the farparams package
(e.g., ControllerPodLabelValue) and replace the hardcoded string on line 248
with a reference to this new constant. Additionally, update the
GetFARControllerPods function to use the same constant to maintain consistency
across the codebase.

tests/far-operator/internal/farutils/node.go (1)

142-159: 💤 Low value

Hardcoded poll interval instead of farparams.DefaultPollInterval.

Line 147 uses a hardcoded 10*time.Second interval, while WaitForNodeNotReady and WaitForNodeReady use farparams.DefaultPollInterval (5 seconds). If the longer interval is intentional for reboot detection, consider extracting it as a named constant (e.g., RebootPollInterval) for clarity and maintainability.

🤖 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/far-operator/internal/farutils/node.go` around lines 142 - 159, The
WaitForNodeReboot function uses a hardcoded 10*time.Second poll interval in the
wait.PollUntilContextTimeout call, which differs from the standard interval used
in similar functions like WaitForNodeNotReady and WaitForNodeReady that use
farparams.DefaultPollInterval. Either replace the hardcoded interval with
farparams.DefaultPollInterval for consistency, or if the longer interval is
intentional for reboot detection, extract it as a named constant (e.g.,
RebootPollInterval) in the farparams package for clarity and maintainability.

🤖 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/far-operator/internal/farutils/harness.go`:
- Around line 132-162: The GetActiveFARControllerNode function has three
robustness issues that need to be addressed. First, replace the strings.Contains
check when looking for the controller lease to use strings.HasPrefix or exact
matching instead, as Contains is too loose and could match unintended leases
with similar names. Second, add validation after splitting the holderIdentity by
underscore to ensure the split result has at least one element and that it's
non-empty before using parts[0] as the pod name, otherwise return an appropriate
error. Third, before returning pod.Spec.NodeName, validate that it is not an
empty string and return an error if the pod has not been scheduled yet
(indicating an empty NodeName).

In `@tests/far-operator/internal/farutils/node.go`:
- Around line 42-50: The StopKubelet function uses fragile string matching with
strings.Contains() to detect connection refused errors instead of the proper
IsConnectionRefused() function available in
vendor/k8s.io/apimachinery/pkg/util/net/util.go. Replace the string matching
check in StopKubelet with a call to IsConnectionRefused() to properly detect
connection refused errors. Additionally, review the StartKubelet function (lines
52-57) to ensure consistent error handling between both functions - consider
whether it should also handle the connection refused case using the same
IsConnectionRefused() approach to maintain consistency.
- Around line 89-138: The WaitForNodeNotReady function has inconsistent error
handling compared to WaitForNodeReady. When the client.Get call fails in
WaitForNodeNotReady, it returns the error immediately (return false, err),
terminating polling, while WaitForNodeReady suppresses the error and continues
polling (return false, nil). Change the error handling in WaitForNodeNotReady to
suppress Get errors by returning false, nil instead of false, err, making it
consistent with WaitForNodeReady and the broader codebase pattern of suppressing
transient errors during polling operations.

---

Nitpick comments:
In `@tests/far-operator/internal/farutils/harness.go`:
- Around line 244-265: The hardcoded string "fence-agents-remediation-operator"
in the ListFARControllerPodsByLabel function duplicates the same value used in
GetFARControllerPods. Extract this value as a constant in the farparams package
(e.g., ControllerPodLabelValue) and replace the hardcoded string on line 248
with a reference to this new constant. Additionally, update the
GetFARControllerPods function to use the same constant to maintain consistency
across the codebase.

In `@tests/far-operator/internal/farutils/node.go`:
- Around line 142-159: The WaitForNodeReboot function uses a hardcoded
10*time.Second poll interval in the wait.PollUntilContextTimeout call, which
differs from the standard interval used in similar functions like
WaitForNodeNotReady and WaitForNodeReady that use farparams.DefaultPollInterval.
Either replace the hardcoded interval with farparams.DefaultPollInterval for
consistency, or if the longer interval is intentional for reboot detection,
extract it as a named constant (e.g., RebootPollInterval) in the farparams
package for clarity and maintainability.

🪄 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: 6f0e31a7-be0f-4d41-af83-35cbd197b3f3

📥 Commits

Reviewing files that changed from the base of the PR and between 79c5b13 and d82d3b1.

📒 Files selected for processing (3)

• tests/far-operator/internal/farparams/const.go
• tests/far-operator/internal/farutils/harness.go
• tests/far-operator/internal/farutils/node.go

[ugreener]

(This AI review was posted in error and has been retracted. Please disregard.)

[ugreener]

(This AI review was posted in error and has been retracted. Please disregard.)

[ugreener]

/test 4.22-konflux-e2e-far-aws

[razo7]

Looks good, but I think we have an issue with lease name

[razo7]

Why did you choose "fence-agents-remediation" for the lease name?

The const is set to "fence-agents-remediation", but FAR's actual leader election lease name is "cb305759.medik8s.io" (see in cmd/main.go:120):

LeaderElectionID: "cb305759.medik8s.io",

So strings.HasPrefix("cb305759.medik8s.io", "fence-agents-remediation") → false. This function will always return "FAR leader lease not found or has no holder".

IIUC this is invisible in PR #20 CI because the function isn't called yet, but it'll surface when PR #22 uses it.

[razo7]

In the FAR repo, these helpers are split into:

• test/e2e/utils/cluster.go - platform detection, credentials, node info
• test/e2e/utils/command.go - controller pod management

Why harness.go? Would cluster.go (or splitting into cluster.go + controller.go) communicate the contents more clearly?

Also: some of these helpers are not FAR-specific. DetectPlatform, SelectWorkerNode, CountReadyWorkerNodes, isNodeReady would work unchanged for NHC, SNR, or any medik8s operator. Should they live in the shared tests/internal/ package instead of under farutils/?