OCPBUGS-109740: Fix tnf_cluster_in_service during per-node maintenance

[lucaconsalvi]

Summary

• Fix tnf_cluster_in_service incorrectly staying 1 when all nodes are placed in maintenance individually via pcs node maintenance --all
• Expand isClusterInMaintenance to also detect when every node has its per-node maintenance flag set, since this does not set the cluster-level maintenance-mode property
• This ensures TNFClusterInMaintenance fires correctly instead of two noisy per-node TNFNodeInMaintenance alerts

Test plan

• Unit tests for isClusterInMaintenance covering: cluster-level property, all nodes individually in maintenance, single node in maintenance, no nodes, empty property
• XML integration test with new all_nodes_maintenance.xml fixture verifying end-to-end cluster condition output
• All existing TNF tests pass
• On-cluster verification: pcs node maintenance --all → check tnf_cluster_in_service == 0

Fixes: https://redhat.atlassian.net/browse/OCPEDGE-2901

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved maintenance status detection for clusters where all nodes are individually in maintenance mode.
  • Correctly preserves non-maintenance status for empty node lists.
  • Ensures clusters with every node in maintenance are reported as unhealthy and out of service.
• Tests
  • Expanded coverage for single-node, multi-node, empty-list, and combined maintenance scenarios.

[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

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d1792a7-1501-42de-b835-4ab97647117c

📥 Commits

Reviewing files that changed from the base of the PR and between 4703f21 and c443186.

📒 Files selected for processing (3)

• pkg/tnf/pkg/pacemaker/statuscollector.go
• pkg/tnf/pkg/pacemaker/statuscollector_test.go
• pkg/tnf/pkg/pacemaker/testdata/all_nodes_maintenance.xml

---

Walkthrough

Pacemaker maintenance detection now recognizes cluster-wide maintenance and the case where all nodes are individually in maintenance. Tests cover these combinations and add an XML scenario for an unhealthy, out-of-service cluster.

Changes

Maintenance Detection

Layer / File(s)	Summary
Maintenance status evaluation pkg/tnf/pkg/pacemaker/statuscollector.go	isClusterInMaintenance returns true when cluster-wide maintenance is enabled or every listed node is in maintenance. Empty node lists return false.
Maintenance scenarios and fixture pkg/tnf/pkg/pacemaker/statuscollector_test.go, pkg/tnf/pkg/pacemaker/testdata/all_nodes_maintenance.xml	Tests cover node-level maintenance combinations and validate the all-nodes scenario as unhealthy and not in service.

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

Mergeability Score: 🔵 Low · up to c4431

The change correctly targets per-node maintenance detection, but cluster maintenance status could still be misclassified for incomplete or non-member node data. The PR is mergeable with explicit owner awareness and follow-up to align the node-set handling.

Suggested reviewers: jaypoulz

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
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	The changed tests use Go t.Run with static table names; no Ginkgo titles or dynamic values were introduced. Node names remain only in test bodies or unchanged tests.
Test Structure And Quality	✅ Passed	The changed tests use Go testing/testify, not Ginkgo. Each table subtest covers one maintenance case, uses fixtures only, creates no resources, and has no cluster waits requiring timeouts.
Microshift Test Compatibility	✅ Passed	The changed tests use Go testing.Test and t.Run with local Pacemaker XML fixtures; no new Ginkgo e2e test, MicroShift-unavailable API, namespace, or unsupported assumption is introduced.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR adds standard Go testing.T unit/table tests in statuscollector_test.go; the diff adds no Ginkgo e2e constructs or multi-node OpenShift test.
Topology-Aware Scheduling Compatibility	✅ Passed	The parent-to-HEAD diff changes only Pacemaker maintenance evaluation, tests, and XML data; it adds no deployment, controller, replica, affinity, topology spread, selector, taint, or PDB scheduling...
Ote Binary Stdout Contract	✅ Passed	The patch only changes maintenance evaluation, tests, and XML; added lines contain no stdout writes or process-level setup, and no OTE entry point changed.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR adds standard Go tests and an XML fixture, not Ginkgo e2e tests. Added lines contain no IPv4 assumptions or external connectivity; existing IPv4 fixtures are unchanged.
No-Weak-Crypto	✅ Passed	The diff only changes maintenance detection and XML tests. Added-line scanning found no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons; existing SHA-256 use is unchanged.
Container-Privileges	✅ Passed	The parent-to-HEAD diff changes only Go tests and an XML fixture; it adds no container or Kubernetes manifest privilege settings.
No-Sensitive-Data-In-Logs	✅ Passed	The diff changes maintenance evaluation and tests only. It adds no logging calls or log fields, and the changed path emits only existing generic status messages.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the bug and the main change: fixing tnf_cluster_in_service during per-node maintenance.

✨ 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.}

[lucaconsalvi]

On-cluster verification

Tested on a cluster-bot TNF cluster (5.1.0-0.nightly), 2-node bare metal topology.

Baseline

Cluster: InService=True (InService), Healthy=True (ClusterHealthy)
Node 0:  InService=True (InService), Healthy=True (NodeHealthy)
Node 1:  InService=True (InService), Healthy=True (NodeHealthy)

After pcs node maintenance --all

Verified that maintenance-mode cluster property stays false (pacemaker only sets per-node flags):

Pacemaker Nodes:
 Maintenance: master-0 master-1

Cluster Properties:
  maintenance-mode=false (default)

After the next status collector run:

Cluster: InService=False (InMaintenance), Healthy=False (ClusterUnhealthy)
Node 0:  InService=False (InMaintenance)
Node 1:  InService=False (InMaintenance)

PASS — tnf_cluster_in_service correctly reports 0, enabling TNFClusterInMaintenance to fire and suppressing per-node TNFNodeInMaintenance alerts via the tnf_cluster_in_service == 1 guard.

After pcs node unmaintenance --all

Cluster: InService=True (InService), Healthy=True (ClusterHealthy)
Node 0:  InService=True (InService), Healthy=True (NodeHealthy)
Node 1:  InService=True (InService), Healthy=True (NodeHealthy)

PASS — cluster recovers to healthy baseline.

[openshift-ci-robot]

@lucaconsalvi: This pull request references OCPEDGE-2901 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

• Fix tnf_cluster_in_service incorrectly staying 1 when all nodes are placed in maintenance individually via pcs node maintenance --all
• Expand isClusterInMaintenance to also detect when every node has its per-node maintenance flag set, since this does not set the cluster-level maintenance-mode property
• This ensures TNFClusterInMaintenance fires correctly instead of two noisy per-node TNFNodeInMaintenance alerts

Test plan

• Unit tests for isClusterInMaintenance covering: cluster-level property, all nodes individually in maintenance, single node in maintenance, no nodes, empty property
• XML integration test with new all_nodes_maintenance.xml fixture verifying end-to-end cluster condition output
• All existing TNF tests pass
• On-cluster verification: pcs node maintenance --all → check tnf_cluster_in_service == 0

Fixes: https://redhat.atlassian.net/browse/OCPEDGE-2901

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
• Improved maintenance status detection for clusters where all nodes are individually in maintenance mode.
• Correctly preserves non-maintenance status for empty node lists.
• Ensures clusters with every node in maintenance are reported as unhealthy and out of service.
• Tests
• Expanded coverage for single-node, multi-node, empty-list, and combined maintenance scenarios.

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.

[fonta-rh]

Reviewed the logic change in isClusterInMaintenance, test coverage, cross-file interactions with the TNFNodeInMaintenance alert guard, and edge cases.

• Empty-nodes guard prevents vacuous-truth on the allMaintenance check
• != "true" correctly handles absent maintenance attribute (no nil deref risk)
• TNFNodeInMaintenance's tnf_cluster_in_service == 1 guard now correctly suppresses during pcs node maintenance --all, which was the bug
• Healthcheck consumer at healthcheck.go handles the new behavior correctly
• Test fixture all_nodes_maintenance.xml covers the new path

Clean fix at the right level — expanding isClusterInMaintenance generalizes the mechanism rather than adding a special case.

/lgtm

[openshift-merge-bot]

Scheduling required tests:
/test e2e-agnostic-ovn
/test e2e-agnostic-ovn-upgrade
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-aws-ovn-single-node
/test e2e-gcp-operator
/test e2e-gcp-operator-disruptive
/test e2e-metal-ipi-ovn-ipv6
/test e2e-operator

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fonta-rh

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:

• pkg/tnf/OWNERS [fonta-rh]

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

[dhensel-rh]

/lgtm

[lucaconsalvi]

/verified by @lucaconsalvi

[openshift-ci-robot]

@lucaconsalvi: This PR has been marked as verified by @lucaconsalvi.

Details

In response to this:

/verified by @lucaconsalvi

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.

[lucaconsalvi]

/retest

[openshift-ci-robot]

@lucaconsalvi: This pull request references Jira Issue OCPBUGS-109740, which is invalid:

• expected the bug to target either version "5.1.0." or "openshift-5.1.0.", but it targets "5.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Summary

• Fix tnf_cluster_in_service incorrectly staying 1 when all nodes are placed in maintenance individually via pcs node maintenance --all
• Expand isClusterInMaintenance to also detect when every node has its per-node maintenance flag set, since this does not set the cluster-level maintenance-mode property
• This ensures TNFClusterInMaintenance fires correctly instead of two noisy per-node TNFNodeInMaintenance alerts

Test plan

• Unit tests for isClusterInMaintenance covering: cluster-level property, all nodes individually in maintenance, single node in maintenance, no nodes, empty property
• XML integration test with new all_nodes_maintenance.xml fixture verifying end-to-end cluster condition output
• All existing TNF tests pass
• On-cluster verification: pcs node maintenance --all → check tnf_cluster_in_service == 0

Fixes: https://redhat.atlassian.net/browse/OCPEDGE-2901

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
• Improved maintenance status detection for clusters where all nodes are individually in maintenance mode.
• Correctly preserves non-maintenance status for empty node lists.
• Ensures clusters with every node in maintenance are reported as unhealthy and out of service.
• Tests
• Expanded coverage for single-node, multi-node, empty-list, and combined maintenance scenarios.

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.

[dhensel-rh]

/hold

[openshift-ci-robot]

@dhensel-rh: The /verified command must be used with one of the following actions: by, later, remove, or bypass. See https://docs.ci.openshift.org/docs/architecture/jira/#premerge-verification for more information.

Details

In response to this:

Update: Verification SUCCESSFUL

After fixing deployment issues with the custom image, I can confirm PR #1678 works correctly and resolves OCPBUGS-109740.

What went wrong initially

My first verification attempt failed because the custom-built image was missing required dependencies (nsenter, sudo, proper user setup). The pacemaker-status-collector CronJob jobs failed, so I never got a successful status update with the PR code.

Successful verification (Run 2)

Built proper image using ubi9:latest with full dependencies and successfully deployed to the pacemaker-status-collector CronJob.

Results with all nodes in maintenance (`pcs node maintenance --all`):

• ✅ tnf_cluster_in_service = 0 (correct!)
• ✅ PacemakerCluster.status.conditions[type=InService].status = "False"
• ✅ PacemakerCluster.status.conditions[type=InService].reason = "InMaintenance"

Results after removing maintenance (`pcs node unmaintenance --all`):

• ✅ tnf_cluster_in_service = 1 (back to normal)
• ✅ All metrics return to expected state

Conclusion

PR #1678 VERIFIED ✅ - The fix works exactly as designed and matches @lucaconsalvi's verification results. Apologies for the initial noise - the issue was entirely with my deployment approach, not the PR code.

This will correctly:

• Fire TNFClusterInMaintenance when all nodes are in per-node maintenance
• Suppress the noisy per-node TNFNodeInMaintenance alerts

/verified

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.

[dhensel-rh]

/unhold

[lucaconsalvi]

/retest

[lucaconsalvi]

/jira refresh

[openshift-ci-robot]

@lucaconsalvi: This pull request references Jira Issue OCPBUGS-109740, which is invalid:

• expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is ON_QA instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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.

[lucaconsalvi]

/jira refresh

[openshift-ci-robot]

@lucaconsalvi: This pull request references Jira Issue OCPBUGS-109740, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.1.0) matches configured target version for branch (5.1.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (dhensel@redhat.com), skipping review request.

Details

In response to this:

/jira refresh

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.

[lucaconsalvi]

/cherry-pick release-5.0

[openshift-cherrypick-robot]

@lucaconsalvi: once the present PR merges, I will cherry-pick it on top of release-5.0 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-5.0

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.

[lucaconsalvi]

/retest

[openshift-merge-bot]

/retest-required

Remaining retests: 0 against base HEAD 6b88b76 and 2 for PR HEAD c443186 in total

[openshift-ci]

@lucaconsalvi: 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.

[openshift-ci-robot]

@lucaconsalvi: Jira Issue Verification Checks: Jira Issue OCPBUGS-109740
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-109740 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

Summary

• Fix tnf_cluster_in_service incorrectly staying 1 when all nodes are placed in maintenance individually via pcs node maintenance --all
• Expand isClusterInMaintenance to also detect when every node has its per-node maintenance flag set, since this does not set the cluster-level maintenance-mode property
• This ensures TNFClusterInMaintenance fires correctly instead of two noisy per-node TNFNodeInMaintenance alerts

Test plan

• Unit tests for isClusterInMaintenance covering: cluster-level property, all nodes individually in maintenance, single node in maintenance, no nodes, empty property
• XML integration test with new all_nodes_maintenance.xml fixture verifying end-to-end cluster condition output
• All existing TNF tests pass
• On-cluster verification: pcs node maintenance --all → check tnf_cluster_in_service == 0

Fixes: https://redhat.atlassian.net/browse/OCPEDGE-2901

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
• Improved maintenance status detection for clusters where all nodes are individually in maintenance mode.
• Correctly preserves non-maintenance status for empty node lists.
• Ensures clusters with every node in maintenance are reported as unhealthy and out of service.
• Tests
• Expanded coverage for single-node, multi-node, empty-list, and combined maintenance scenarios.

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-cherrypick-robot]

@lucaconsalvi: new pull request created: #1681

Details

In response to this:

/cherry-pick release-5.0

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.

[openshift-merge-robot]

Fix included in release 5.1.0-0.nightly-2026-08-18-031603