OCPEDGE-2826: fix fencing_validator ocdebug fence dispatch race condition

[Neilhamza]

Summary

Fixes the fence() function in the fencing_validator script for the ocdebug transport mode. The fence command was silently failing due to a bash parsing issue, causing the disruptive fencing validation CI lane to time out on 4.22, 4.23, and 5.0.

Root cause: The compound command A && B || C & disown backgrounds the entire AND-OR list (not just the nohup fallback), causing the oc debug pod to exit immediately and kill the systemd-run process before it dispatches the fence unit.

Fix: Replace &&/|| with if/else to scope & disown to only the nohup fallback, and use --no-block on systemd-run.

Failing CI lanes

• periodic-ci-openshift-release-main-nightly-4.22-e2e-metal-ovn-two-node-fencing-validation
• periodic-ci-openshift-release-main-nightly-4.23-e2e-metal-ovn-two-node-fencing-validation
• periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-validation

Example failure: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/periodic-ci-openshift-release-main-nightly-4.22-e2e-metal-ovn-two-node-fencing-validation/2081600945665347584

Jira: https://redhat.atlassian.net/browse/OCPEDGE-2826

Test plan

• Reproduced the bug on a live TNF cluster: unpatched script times out waiting for master-0 NotReady (fence never happens)
• Verified the fix: patched script successfully fences both nodes sequentially
• Full disruptive validation passes end-to-end with the fix ("Disruptive validation PASSED", exit 0)
• CI lane passes with the fix

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved remote fencing command execution by using an explicit inline check for systemd-run.
  • When available, fence actions are started via systemd-run with non-blocking behavior and a dedicated unit name.
  • When not available, the fence action now reliably falls back to running pcs stonith fence in the background.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[coderabbitai]

Walkthrough

The fencing validator’s ocdebug transport now explicitly checks for systemd-run, launches fencing non-blocking with a named unit when available, and otherwise uses a background nohup command with disown.

Changes

Fencing dispatch

Layer / File(s)	Summary
Remote fence command dispatch templates/master/00-master/two-node-with-fencing/files/fencing-validator.yaml	The fence() function uses systemd-run --no-block when available and falls back to background nohup bash -lc execution with disown.

Estimated code review effort: 2 (Simple) | ~5 minutes

Suggested reviewers: eggfoobar, jerpeter1

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
No-Sensitive-Data-In-Logs	❌ Error	fencing-validator logs internal node names/IPs (e.g. conductor=$CONDUCTOR, $NODE_A/$NODE_B, $h), which can expose internal hostnames.	Redact host identifiers from log messages or replace them with sanitized labels/hashes before printing.

✅ Passed checks (14 passed)

Check name	Status	Explanation
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.
Stable And Deterministic Test Names	✅ Passed	Only a fencing-validator shell script changed; no Ginkgo test titles were added or modified, so there are no unstable test names.
Test Structure And Quality	✅ Passed	No Ginkgo test code was added or modified; the PR only changes a YAML script, so the test-structure checklist is not applicable.
Microshift Test Compatibility	✅ Passed	Only a YAML bash script changed; no new Ginkgo e2e tests or test names/apis were added, so the MicroShift test check is not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Only a fencing script line changed; no Ginkgo tests were added or modified, so no SNO-specific test compatibility issue applies.
Topology-Aware Scheduling Compatibility	✅ Passed	The patch only adds --collect to systemd-run in a fencing script; it introduces no pod scheduling fields or topology-dependent constraints.
Ote Binary Stdout Contract	✅ Passed	PASS: The only code change is adding --collect to systemd-run; no new process-level stdout writes or logging changes were introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo tests were added; the diff only tweaks a bash validator and adds no IPv4 or external-network assumptions.
No-Weak-Crypto	✅ Passed	The changed fencing script adds only shell dispatch logic; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret/token comparisons appear.
Container-Privileges	✅ Passed	Changed file is only an inline script; subtree search found no privileged, hostPID/Network/IPC, allowPrivilegeEscalation, or root/capability settings.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: fixing the ocdebug fence dispatch race condition in fencing_validator.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 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
`@templates/master/00-master/two-node-with-fencing/files/fencing-validator.yaml`:
- Around line 474-476: Update the systemd-run invocation in the fencing command
within the host_run block to retain the --collect option alongside --no-block
and the existing fence-$t unit name. Preserve the fallback nohup behavior
unchanged.

🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 703147ed-cb9f-459d-a5d0-2c71adbc6ecd

📥 Commits

Reviewing files that changed from the base of the PR and between 3b4a5c7 and 0cd0458.

📒 Files selected for processing (1)

• templates/master/00-master/two-node-with-fencing/files/fencing-validator.yaml

[Neilhamza]

/test ?

[Neilhamza]

@coderabbitai resolve

[coderabbitai]

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

[fracappa]

/lgtm

[openshift-merge-bot]

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn
/test e2e-aws-ovn-upgrade
/test e2e-gcp-op-ocl-part1
/test e2e-gcp-op-ocl-part2
/test e2e-gcp-op-part1
/test e2e-gcp-op-part2
/test e2e-gcp-op-single-node
/test e2e-hypershift

[isabella-janssen]

/lgtm

Deferring to TNF team's LGTM as this impacts them.

[isabella-janssen]

/retitle NO-ISSUE: fix fencing_validator ocdebug fence dispatch race condition

[openshift-ci-robot]

@Neilhamza: This pull request explicitly references no jira issue.

Details

In response to this:

Summary

Fixes the fence() function in the fencing_validator script for the ocdebug transport mode. The fence command was silently failing due to a bash parsing issue, causing the disruptive fencing validation CI lane to time out on 4.22, 4.23, and 5.0.

Root cause: The compound command A && B || C & disown backgrounds the entire AND-OR list (not just the nohup fallback), causing the oc debug pod to exit immediately and kill the systemd-run process before it dispatches the fence unit.

Fix: Replace &&/|| with if/else to scope & disown to only the nohup fallback, and use --no-block on systemd-run.

Failing CI lanes

• periodic-ci-openshift-release-main-nightly-4.22-e2e-metal-ovn-two-node-fencing-validation
• periodic-ci-openshift-release-main-nightly-4.23-e2e-metal-ovn-two-node-fencing-validation
• periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-two-node-fencing-validation

Example failure: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/periodic-ci-openshift-release-main-nightly-4.22-e2e-metal-ovn-two-node-fencing-validation/2081600945665347584

Jira: https://redhat.atlassian.net/browse/OCPEDGE-2826

Test plan

• Reproduced the bug on a live TNF cluster: unpatched script times out waiting for master-0 NotReady (fence never happens)
• Verified the fix: patched script successfully fences both nodes sequentially
• Full disruptive validation passes end-to-end with the fix ("Disruptive validation PASSED", exit 0)
• CI lane passes with the fix

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
• Improved remote fencing command execution by using an explicit inline check for systemd-run.
• When available, fence actions are started via systemd-run with non-blocking behavior and a dedicated unit name.
• When not available, the fence action now reliably falls back to running pcs stonith fence in the background.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fracappa, isabella-janssen, Neilhamza

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 [isabella-janssen]

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

[isabella-janssen]

/retest-required

[Neilhamza]

/retest

[Neilhamza]

/test ci/prow/e2e-aws-ovn

[Neilhamza]

/test e2e-aws-ovn

[Neilhamza]

/verified by ci

[openshift-ci-robot]

@Neilhamza: This PR has been marked as verified by ci.

Details

In response to this:

/verified by ci

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

@Neilhamza: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[Neilhamza]

/cherry-pick release-4.22

[openshift-cherrypick-robot]

@Neilhamza: new pull request created: #6360

Details

In response to this:

/cherry-pick release-4.22

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[Neilhamza]

/cherry-pick release-4.22

[openshift-cherrypick-robot]

@Neilhamza: new pull request could not be created: failed to create pull request against openshift/machine-config-operator#release-4.22 from head openshift-cherrypick-robot:cherry-pick-6341-to-release-4.22: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for openshift-cherrypick-robot:cherry-pick-6341-to-release-4.22."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request","status":"422"}

Details

In response to this:

/cherry-pick release-4.22

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.