Salt Phase 1: bootstrap + Makefile #1
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo_admin/pal-e-platform!1
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "issue-pal-e-platform-salt-bootstrap"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Changes
Makefile: unified CLI with 8 targets (salt-bootstrap, salt-test, salt-apply, salt-uninstall, tofu-init, tofu-plan, tofu-apply, tofu-fmt)salt/bootstrap.sh: idempotent install of salt-onedir via paru, generates /etc/salt/master.d/roots.conf with dynamic repo path, starts (not enables) services, accepts minion key, verifies test.ping. Supports --remove for clean uninstall.salt/master.conf: listens on 127.0.0.1 (localhost only), warning log level. file_roots/pillar_roots are NOT here -- generated dynamically by bootstrap.sh so repo path is resolved at install time.salt/minion.conf: master 127.0.0.1, minion ID "archbox", warning log levelsalt/states/top.sls: empty state tree (Phase 2 populates with package/service/kernel/user states)salt/pillar/top.sls: empty pillar tree (Phase 2 populates with GPG-encrypted secrets)Design decisions:
No changes to terraform/ or any existing files. Purely additive.
Test Plan
make salt-bootstrapon the Arch boxsalt-call test.pingreturns Truemake salt-testruns dry-run highstate successfullymake tofu-planstill works (existing Terraform unbroken)make salt-uninstallcleanly removes SaltReview Checklist
Related Notes
issue-pal-e-platform-salt-bootstrap-- the issue this PR addressesplan-2026-02-26-salt-host-management-- Phase 1 of the salt host management planhost-inventory-archbox-- complete host audit that informs Phase 2 state definitionsReview round 1 — 2 issues found and fixed:
Blocking:
--localflag onsalt-callin Makefile —salt-call --localreads config from/etc/salt/minion, butfile_rootsandpillar_rootsare set in the master config (/etc/salt/master.d/roots.conf). This meansmake salt-testandmake salt-applywould not find the state tree. Fix: dropped--localso salt-call communicates with the local master which has the correct roots.Improvement:
systemctl start→restartin bootstrap.sh — On idempotent re-runs where config may have changed,startis a no-op if the service is already running. Changed torestartso config changes are always picked up.Review round 2 -- clean pass.
All 6 files reviewed. Zero issues found. The
--localfix from round 1 is correct. Bootstrap script is idempotent, handles both install and uninstall paths, and the Makefile provides all 8 required targets.Ready for user review and manual testing on the Arch box.
Design Decision Correction
The PR body mentions a design decision about using
--local flag on salt-call in Makefile: local file client since master+minion are colocated. However, this was superseded by commit006854c, which removed--localfrom the salt-call invocation.The Makefile now correctly uses
salt-callwithout--local, allowing it to properly readfile_rootsandpillar_rootsfrom the master configuration. This is the correct approach since the master config is the source of truth for these paths.The code is correct — this comment is just clarifying that the PR body's design decision should reflect the final implementation.
QA Review Fixes — All 6 Warnings Addressed
Pushed commit
794c352with fixes for all QA warnings:W1. PR body mentions
--localbut code doesn't use itNo code change needed. Confirmed grep shows zero
--localreferences in any code or comments. The PR body "Design decisions" section is inaccurate — the Makefilesalt-callcommands correctly omit--localbecause salt-call already defaults to local mode when contacting the master on localhost. The previous commit (006854c) already dropped--localfrom the code. The PR body should be read with this correction in mind.W2.
sudo -u parunesting could prompt for passwordFixed. Added
sudo -u "${BUILD_USER}" sudo -vwarmup before theparucall ininstall_salt(). If the cache refresh fails, a warning is emitted but execution continues (paru's--noconfirmhandles the rest).W3.
do_remove()doesn't clean up the minion keyFixed. Added
salt-key --delete="${MINION_ID}" --yesat the top ofdo_remove(), before stopping services. This prevents stale accepted keys from causing silent auth failures after uninstall+reinstall. Also reordered service stops: minion first, then master (minion needs master running to process the key delete).W4.
do_remove()doesn't clean up/etc/salt/masterand/etc/salt/minionFixed. Added
rm -f /etc/salt/masterandrm -f /etc/salt/minionalongside the existingroots.confcleanup. The/etc/salt/directory itself is still preserved (comment retained).W5.
sleep 2after starting salt-master is fragileFixed. Replaced with a port poll loop that checks
ss -tlnpfor port 4506 (salt-master ret port), polling every 1s for up to 15s. If it still isn't listening after 15s, a warning is emitted and execution continues rather than hard-failing.W6. No
.gitignoreentries for Salt cache/runtime artifactsFixed. Added to existing
.gitignore:W2: Add sudo credential cache warmup before paru call to prevent password prompt when sudo cache is expired inside root shell. W3: Delete minion key in do_remove() before stopping master to prevent stale accepted key causing silent auth failure on reinstall. W4: Remove /etc/salt/master and /etc/salt/minion in do_remove() since bootstrap.sh is responsible for placing them. W5: Replace fragile sleep 2 with port poll loop (up to 15s) that waits for salt-master to listen on port 4506. W6: Add Salt runtime artifacts (__pycache__, *.pyc) to .gitignore. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>W1 clarification: The PR description mentions
--local flag on salt-call in Makefileunder Design decisions. This was removed during review — the Makefile correctly usessalt-call state.apply(master-minion mode, not masterless--local). The PR body is stale on this point; the code is correct.