Enhance overcloud openbao deployment - #2477
Conversation
Added variable ``raft_leader_index`` to the ``secret-store-deploy-overcloud.yml`` playbook. This variable controls which controller becomes the leader when deploying OpenBao. The default is 0 (Frist controller). This variable needs to track the index of current leader controller when fixing/restarting the cluster to prevent split brain.
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a playbook to identify the current OpenBao Raft leader. Wires that playbook into recovery deployment. Updates deployment to use a configurable leader index and adds a first-unseal timeout. Documents the cluster restoration procedure. ChangesOpenBao recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant LeaderPlaybook as get-current-raft-leader.yml
participant SecretStoreAPI
participant RecoveryPlaybook as fix-openbao-overcloud.yml
participant OvercloudDeploy as secret-store-deploy-overcloud.yml
Operator->>LeaderPlaybook: Run leader discovery
LeaderPlaybook->>SecretStoreAPI: Query HA and leader status
SecretStoreAPI-->>LeaderPlaybook: Return current leader
LeaderPlaybook-->>RecoveryPlaybook: Set raft_leader_index
RecoveryPlaybook->>OvercloudDeploy: Import deployment
OvercloudDeploy->>SecretStoreAPI: Use selected Raft leader IP
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
This needs to be backported at least to stackhpc/2025.1 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
etc/kayobe/ansible/secret-store/get-current-raft-leader.yml (1)
17-23: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRemove the unused
hvacinstallation.This playbook uses no
hvac-dependent module. Remove Lines 17–23 to avoid an unnecessary package and network dependency before leader discovery.🤖 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 `@etc/kayobe/ansible/secret-store/get-current-raft-leader.yml` around lines 17 - 23, Remove the Ensure Python hvac module is installed task from the playbook, including its pip installation, virtualenv, and become configuration; leave the raft leader discovery tasks unchanged.
🤖 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 `@etc/kayobe/ansible/secret-store/get-current-raft-leader.yml`:
- Around line 30-42: Update both ansible.builtin.uri tasks, “Query the HA
status” and “Query the leader,” to set use_proxy: false and clear https_proxy in
their task environments, matching the established secret-store request
configuration while preserving their existing URLs, headers, and registrations.
- Around line 47-48: Update the HA condition in get-current-raft-leader to
reference the documented case-sensitive Nodes field from ha_status_query.json
instead of nodes, preserving the existing length-greater-than-one check so
raft_leader is created when multiple nodes are returned.
- Around line 51-53: Update the Raft leader index assignment using a core
Ansible expression, or declare ansible.utils in the Ansible collection
requirements consumed by the supported installation commands. Ensure the lookup
in the raft_leader_index set_fact remains functional when ansible.utils is not
otherwise installed.
- Around line 44-53: Update the “Determine Raft leader” condition to read the HA
node count from ha_status_query.json.Nodes, then validate that
groups['raft_leader'] contains exactly one host before calculating
raft_leader_index. Reject zero or multiple detected leaders rather than applying
first and selecting an ambiguous controller.
In `@etc/kayobe/ansible/secret-store/secret-store-deploy-overcloud.yml`:
- Around line 26-35: Set stackhpc.hashicorp’s openbao_init_addr to the
controller selected by raft_leader_index, alongside openbao_raft_leaders, so
initialization checks and hashivault_init target the same leader used for
retry-join. Preserve the existing selected-controller address construction and
avoid leaving initialization bound to the default openbao_api_addr.
---
Nitpick comments:
In `@etc/kayobe/ansible/secret-store/get-current-raft-leader.yml`:
- Around line 17-23: Remove the Ensure Python hvac module is installed task from
the playbook, including its pip installation, virtualenv, and become
configuration; leave the raft leader discovery tasks 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c54e153f-49d7-4412-bf70-31e4d9eda846
📒 Files selected for processing (3)
doc/source/configuration/openbao.rstetc/kayobe/ansible/secret-store/get-current-raft-leader.ymletc/kayobe/ansible/secret-store/secret-store-deploy-overcloud.yml
|
Why are we doing it here, instead of fixing it in the hashicorp collection? |
|
@mnasiadka Currently The OpenBao role in hashicorp collection supports setting the leader node other than the first controller. It's the SKC playbook that fixed the leader to be the first controller. |
|
@mnasiadka
We can still automate this in SKC by having a wrapper playbook for |
5386a35 to
c89988c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@etc/kayobe/ansible/secret-store/get-current-raft-leader.yml`:
- Around line 47-56: Insert an ansible.builtin.assert immediately before the
“Set Raft leader index fact” task, requiring (groups['raft_leader'] |
default([])) | length == 1. Keep the existing raft_leader_index calculation
unchanged, and make the assertion fail when no leader or multiple leaders are
detected.
- Around line 30-44: Update both the “Query the HA status” and “Query the
leader” ansible.builtin.uri tasks to prevent redirects from forwarding the
X-Vault-Token header, preferably by setting follow_redirects to none while
preserving the existing requests and error handling.
- Around line 36-52: Update the “Determine Raft leader” task to require
successful completion of both URI tasks before accessing ha_status_query.json or
leader_query.json, using their registered success indicators. Also add an
explicit no-leader fallback for when no host joins raft_leader so the play does
not fail when leader discovery returns no match.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cf175bfb-814c-44d1-a0ab-baa5dbeeedc7
📒 Files selected for processing (4)
doc/source/configuration/openbao.rstetc/kayobe/ansible/secret-store/fix-openbao-overcloud.ymletc/kayobe/ansible/secret-store/get-current-raft-leader.ymletc/kayobe/ansible/secret-store/secret-store-deploy-overcloud.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- etc/kayobe/ansible/secret-store/secret-store-deploy-overcloud.yml
- doc/source/configuration/openbao.rst
45ee6c6 to
de8078a
Compare
de8078a to
8198b55
Compare
8198b55 to
6cd456c
Compare
2 participants
Currently fixing split brain OpenBao cluster is not a smooth operation.
Added following to improve the cluster restoration process
Also added 10 seconds timeout on unsealing first OpenBao node to prevent the deployment playbook from failing caused by slightly longer unsealing time when fixing the cluster..
Summary by CodeRabbit
New Features
Documentation