fix(ansible): enable gather_facts for HP masternode play#731
Open
ktechmidas wants to merge 1 commit intov1.0-devfrom
Open
fix(ansible): enable gather_facts for HP masternode play#731ktechmidas wants to merge 1 commit intov1.0-devfrom
ktechmidas wants to merge 1 commit intov1.0-devfrom
Conversation
…A.MD - Changed gather_facts from false to true in the HP masternodes play to fix geerlingguy.filebeat role which needs ansible_facts.os_family - Added INFRA.MD documenting dashmate operations, common fixes, and deployment procedures Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdded comprehensive testnet infrastructure operations documentation covering architecture, dashmate commands, common issues, ProTx lifecycle, and AWS management procedures. Modified Ansible deployment configuration to enable fact gathering for HP masternodes during deployment execution. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
INFRA.MD (1)
258-258: Remove hardcoded source line references from runbook text.The “line 338” reference will drift on future edits and become inaccurate. Reference the play name instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@INFRA.MD` at line 258, Remove the hardcoded "line 338" reference and instead refer to the specific Ansible play in deploy.yml that sets gather_facts (the play that configures/installs geerlingguy.filebeat); update the wording to say "the deploy.yml play that sets gather_facts for the Filebeat role" or similar so future edits won't break the pointer and readers can locate the setting by play name rather than a line number.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ansible/deploy.yml`:
- Line 338: Change the play to keep gather_facts: false, add strategy: free and
the dashmate_deploy tag on the play, and create a pre-task that runs the ansible
setup module to collect only required facts (e.g., ansible_facts.os_family)
instead of full facts; update the play header (gather_facts → false, add
strategy: free and tags: [dashmate_deploy]) and add a lightweight setup pre-task
using the setup module with the filter parameter to fetch only the minimal facts
downstream roles need.
In `@INFRA.MD`:
- Around line 283-293: The "Current Node Status" table in INFRA.MD exposes
concrete hostnames and an IP (e.g., rows referencing hp-masternode-3,
hp-masternode-4, hp-masternode-6, hp-masternode-16, hp-masternode-18,
hp-masternode-22, hp-masternode-29 and the IP 68.67.122.3); remove or redact
these identifiers and the public IP from the committed doc and either move the
detailed operational status to a private runbook or replace the table with
non-identifying examples (e.g., "masternode-A", "masternode-B", status examples)
and a note pointing to the secure runbook for real values so no live
infrastructure details remain in the public repo.
---
Nitpick comments:
In `@INFRA.MD`:
- Line 258: Remove the hardcoded "line 338" reference and instead refer to the
specific Ansible play in deploy.yml that sets gather_facts (the play that
configures/installs geerlingguy.filebeat); update the wording to say "the
deploy.yml play that sets gather_facts for the Filebeat role" or similar so
future edits won't break the pointer and readers can locate the setting by play
name rather than a line number.
| hosts: hp_masternodes | ||
| become: true | ||
| gather_facts: false | ||
| gather_facts: true |
There was a problem hiding this comment.
Keep gather_facts disabled; gather only required facts explicitly.
Turning this on breaks the fast-deploy baseline for this play. Keep gather_facts: false and add a minimal setup pre-task to fetch only the facts needed by downstream roles (for example OS family).
Proposed change
- name: Set up core and platform on HP masternodes
hosts: hp_masternodes
become: true
- gather_facts: true
+ gather_facts: false
# Using strategy: free for parallel execution to improve deployment speed
# This is intentional for performance optimization
strategy: free # noqa: run-once[play]
serial: 0
pre_tasks:
+ - name: Gather required OS fact for role conditionals/templates
+ ansible.builtin.setup:
+ filter:
+ - ansible_os_family
- name: Check inventory for HP masternodes
ansible.builtin.set_fact:
node: "{{ hp_masternodes[inventory_hostname] }}"As per coding guidelines "ansible/deploy.yml: Add dashmate_deploy tag, set gather_facts: false, and use strategy: free in ansible/deploy.yml to enable fast, parallel deployments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ansible/deploy.yml` at line 338, Change the play to keep gather_facts: false,
add strategy: free and the dashmate_deploy tag on the play, and create a
pre-task that runs the ansible setup module to collect only required facts
(e.g., ansible_facts.os_family) instead of full facts; update the play header
(gather_facts → false, add strategy: free and tags: [dashmate_deploy]) and add a
lightweight setup pre-task using the setup module with the filter parameter to
fetch only the minimal facts downstream roles need.
Comment on lines
+283
to
+293
| ## Current Node Status (as of 2026-02-26) | ||
|
|
||
| | Node | Status | Notes | | ||
| |------|--------|-------| | ||
| | hp-masternode-3 | READY, PoSe=0, Platform syncing | Freshly registered with new IP 68.67.122.3 | | ||
| | hp-masternode-4 | READY, PoSe=0, Platform syncing | Re-registered in previous session | | ||
| | hp-masternode-6 | READY, PoSe=0, Platform syncing | Re-registered in previous session | | ||
| | hp-masternode-16 | READY, PoSe=0, Platform up | rs-dapi metrics config updated | | ||
| | hp-masternode-18 | Syncing (99.97%) | Recently unbanned, waiting for core sync | | ||
| | hp-masternode-22 | Rebuilding chainstate | EvoDB corruption + disk full, logs truncated, rebuilding | | ||
| | hp-masternode-29 | Rebuilding chainstate | Stuck on conflicting block, evoDB wiped, rebuilding | |
There was a problem hiding this comment.
Avoid committing live node status and concrete infrastructure identifiers.
This section exposes operational state and host mapping (including a concrete public IP). Move this to a private runbook or redact to non-identifying examples to reduce reconnaissance risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@INFRA.MD` around lines 283 - 293, The "Current Node Status" table in INFRA.MD
exposes concrete hostnames and an IP (e.g., rows referencing hp-masternode-3,
hp-masternode-4, hp-masternode-6, hp-masternode-16, hp-masternode-18,
hp-masternode-22, hp-masternode-29 and the IP 68.67.122.3); remove or redact
these identifiers and the public IP from the committed doc and either move the
detailed operational status to a private runbook or replace the table with
non-identifying examples (e.g., "masternode-A", "masternode-B", status examples)
and a note pointing to the secure runbook for real values so no live
infrastructure details remain in the public repo.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
gather_factsfromfalsetotruein the HP masternodes play to fixgeerlingguy.filebeatrole which requiresansible_facts.os_familyINFRA.MDdocumenting dashmate operations, common fixes, and deployment proceduresTest plan
gather_facts: true🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores