Skip to content

Conversation

@tomjelinek
Copy link
Member

@tomjelinek tomjelinek commented Dec 1, 2025

Enhancement:
Apply special stonith-watchdog-timeout cluster property handling to fencing-watchdog-timeout as well

Reason:
fencing-watchdog-timeout is replacing stonith-watchdog-timeout in future pacemaker version

Result:
The role supports future pacemaker versions

Issue Tracker Tickets (Jira or BZ if any):
https://issues.redhat.com/browse/RHELHA-388

Summary by Sourcery

Ensure watchdog-related cluster properties remain consistent across current and future Pacemaker versions by handling both legacy and new property names.

New Features:

  • Configure the fencing-watchdog-timeout cluster property alongside stonith-watchdog-timeout to support newer Pacemaker feature sets.

Enhancements:

  • Preserve both stonith-watchdog-timeout and fencing-watchdog-timeout when creating and pushing CIB configurations via pcs and crmsh.
  • Align CIB- and ACL-related task descriptions for clearer test intent.

Tests:

  • Update cluster property tests to expect both fencing-watchdog-timeout and stonith-watchdog-timeout attributes where applicable.
  • Adjust CIB-focused tests to account for the new fencing-watchdog-timeout property and remove now-unneeded schema-version fetching.

Especially do not load cib schema version before a cluster is created by
running the role in a test, as that fails due to CIB nonexistence.
@tomjelinek tomjelinek requested a review from richm as a code owner December 1, 2025 09:08
@sourcery-ai
Copy link

sourcery-ai bot commented Dec 1, 2025

Reviewer's Guide

Adds support for the new Pacemaker fencing-watchdog-timeout cluster property by setting it alongside the existing stonith-watchdog-timeout, updates CIB filtering logic, and adjusts tests/fixtures to expect both properties while performing a minor test name cleanup and removing now-unneeded CIB schema version fetching in a couple of test suites.

Sequence diagram for setting both watchdog timeout properties via pcs

sequenceDiagram
    actor Admin
    participant AnsibleTask_sbd
    participant pcs_cli
    participant Pacemaker_old
    participant Pacemaker_new

    Admin->>AnsibleTask_sbd: Run sbd.yml role
    AnsibleTask_sbd->>AnsibleTask_sbd: Evaluate ha_cluster_sbd_enabled
    AnsibleTask_sbd->>pcs_cli: pcs --force property set\n  fencing-watchdog-timeout=value\n  stonith-watchdog-timeout=value

    alt Pacemaker_with_feature_set_below_3_20_5
        pcs_cli->>Pacemaker_old: Apply CIB change
        Pacemaker_old->>Pacemaker_old: Ignore fencing-watchdog-timeout
        Pacemaker_old->>Pacemaker_old: Use stonith-watchdog-timeout
    else Pacemaker_with_feature_set_3_20_5_or_higher
        pcs_cli->>Pacemaker_new: Apply CIB change
        Pacemaker_new->>Pacemaker_new: Prefer fencing-watchdog-timeout
        Pacemaker_new->>Pacemaker_new: Treat stonith-watchdog-timeout as deprecated
    end
Loading

Flow diagram for create-and-push-cib CIB filtering with both watchdog properties

flowchart TD
    A[Start create-and-push-cib task] --> B[Generate or load temporary CIB XML]
    B --> C[Apply XPath filter to remove dynamic properties\n dc-version, have-watchdog, last-lrm-refresh,\n stonith-watchdog-timeout, fencing-watchdog-timeout]
    C --> D[Write filtered CIB to tempfile CIB_file]
    D --> E[Push CIB_file to cluster using crmsh or pcs]
    E --> F[Cluster CIB now contains synced\n stonith-watchdog-timeout and fencing-watchdog-timeout]
    F --> G[End]
Loading

File-Level Changes

Change Details Files
Set both fencing-watchdog-timeout and stonith-watchdog-timeout cluster properties when configuring SBD so the role works with current and future Pacemaker versions.
  • Extend the pcs command used to set cluster properties to also set fencing-watchdog-timeout to the same value as stonith-watchdog-timeout based on ha_cluster_sbd_enabled.
  • Add inline documentation explaining the Pacemaker feature set change and why setting both properties is safe across versions.
tasks/shell_pcs/sbd.yml
Update test inventories/fixtures to expect both fencing-watchdog-timeout and stonith-watchdog-timeout properties where watchdog timeouts are asserted.
  • Replace single stonith-watchdog-timeout attr lists with attr lists containing both fencing-watchdog-timeout and stonith-watchdog-timeout with matching values in multiple cluster scenario tests.
  • Add fencing-watchdog-timeout with value 0 to property expectations in CIB-based tests that assert default or specific cluster properties.
  • Keep ordering and values consistent to avoid brittle test failures.
tests/tests_cluster_advanced_knet_full.yml
tests/tests_cluster_advanced_knet_implicit.yml
tests/tests_cluster_advanced_udp_full.yml
tests/tests_cluster_basic.yml
tests/tests_cluster_basic_cloud_packages.yml
tests/tests_cluster_basic_disabled.yml
tests/tests_export_doesnt_destroy_cluster.yml
tests/tests_export_firewall_selinux.yml
tests/tests_pcs_permissions.yml
tests/tests_cib_properties_empty.yml
tests/tests_cib_properties_one_set.yml
tests/tests_cib_rsc_op_defaults.yml
Ensure CIB export/import filtering keeps both watchdog timeout properties when creating and pushing CIBs.
  • Extend the XPath filter that preserves selected cluster properties during CIB processing to also include fencing-watchdog-timeout alongside stonith-watchdog-timeout in both crmsh and pcs task variants.
tasks/shell_crmsh/create-and-push-cib.yml
tasks/shell_pcs/create-and-push-cib.yml
Minor test maintenance unrelated to the new property but included in this PR.
  • Rename a test task label from "Check node attributes configuration" to "Check acl configuration" for clarity in ACL tests.
  • Remove with_cib_schema_version: true variable from fetch_versions.yml includes in two CIB-related tests where it is no longer needed.
tests/tests_cib_acls.yml
tests/tests_cib_constraints_create_and_load_info.yml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@tomjelinek tomjelinek changed the title Fencing watchdog timeout feat: add support for fencing-watchdog-timeout Dec 1, 2025
@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.92%. Comparing base (f58d15e) to head (dc828c1).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #335   +/-   ##
=======================================
  Coverage   90.92%   90.92%           
=======================================
  Files          19       19           
  Lines        1212     1212           
=======================================
  Hits         1102     1102           
  Misses        110      110           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@richm
Copy link
Contributor

richm commented Dec 1, 2025

[citest]

@richm richm merged commit f1a49dc into linux-system-roles:main Dec 1, 2025
41 of 44 checks passed
@tomjelinek tomjelinek deleted the fencing-watchdog-timeout branch December 2, 2025 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants