fix(elasticsearch): skip bootstrap-only checks when joining existing cluster#151
Conversation
…cluster Adds `not elasticsearch_cluster_set_up | bool` to the four bootstrap-password tasks in elasticsearch-security.yml plus the master-count check in main.yml, so a joining node skips the checks that only make sense for fresh-bootstrap. Reopens issue #148 after the previous attempt (#149, reverted in #150) regressed fresh-install scenarios. The previous attempt put the gate on the whole "Runtime cluster setup" block. That block contains an inner `set_fact` that flips elasticsearch_cluster_set_up to true after the initial passwords file is written. Ansible re-evaluates block-level whens per task against the live variable, so every inner task after that flip silently skipped, including the fetch that sets elasticstack_password. Downstream tasks then saw it undefined and either fell back to a hardcoded path that doesn't exist on custom-path scenarios, or failed outright. Gating per task avoids that — each task's when is independent and doesn't get re-evaluated against later state. The four gated tasks all already had `not elasticsearch_passwords_file.stat.exists | bool` in their when, so they're a clean place for the additional guard. The master-count check in main.yml was uncontroversial in #149 — it sits before the security tasks run and the inner flip never reaches it — so it returns unchanged. No molecule scenario covers the join-existing-cluster path: it would need a multi-node setup where the joining node sees the passwords file as absent (stat delegated to the CA host), which the existing single-node scenarios can't reproduce. Existing scenarios still validate the fresh-bootstrap path and would catch a regression like the one #149 introduced.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1 participant
Reopens #148 after the first attempt (#149) was reverted in #150.
The previous attempt put the gate on the whole "Runtime cluster setup" block in
elasticsearch-security.yml. That block contains an innerset_factthat flipselasticsearch_cluster_set_uptotruepartway through, after the initial passwords file is written. Ansible re-evaluates a block-levelwhen:against each task's live variable state, so every task after that internal flip silently skipped — includingFetch elastic password after initial setup, which is what setselasticstack_password. Downstream consumers (cluster settings, watermarks, the molecule sharedset_ci_watermarkshelper) then sawelasticstack_passwordundefined and either fell back to a hardcoded/usr/share/elasticsearch/initial_passwordspath that doesn't exist on custom-path scenarios, or failed outright. CI on main caught it onelasticsearch_custom,elasticsearch_cert_content, andelasticsearch_diagnostics.This version gates per task instead. Each of the four bootstrap-only tasks gets
- not elasticsearch_cluster_set_up | booladded to its existingwhen:list (alongsidenot elasticsearch_passwords_file.stat.exists | boolwhich they all already had). Per-taskwhen:is evaluated independently, so the innerset_factdoesn't poison subsequent tasks. The four gated tasks areCheck for API with bootstrap password, theRecover elastic user after interrupted security setupsub-block, theFail if bootstrap password check failed and recovery was not attemptedtask, andCheck for cluster status with bootstrap password. A comment on the block explains why the gating is at task level rather than block level.The master-count check in
main.ymlreturns unchanged from #149 — it sits before the security tasks run and the inner flip never reaches it, so it was uncontroversial and just got reverted along with #149 for a clean rollback.No molecule scenario exercises the join-existing-cluster path: it would need a multi-node setup where the joining node sees the passwords file as absent (stat delegated to the CA host), which single-node scenarios can't reproduce. Adding one is more disruptive than the fix warrants. The existing scenarios still validate the fresh-bootstrap regression class — they're what flagged #149's issue when I ran the full elasticsearch-role workflow against main, and they'll catch the same shape of regression on this PR too.