From 7e4bc3683ae0e1ee480b72ffd553e1d1fe64bc18 Mon Sep 17 00:00:00 2001 From: Daisuke Miyamoto Date: Tue, 7 Jul 2026 21:44:47 +0000 Subject: [PATCH 1/2] fix(aws-pcs): keep needrestart guard in lock-step across CNG templates Follow-up to #1165 review: - normalize add-cng.yaml's guard-block comments to match the other three templates (ASCII dash, 'Managed by add-cng UserData') - add a lint-docs.sh check that the guard block is byte-identical across add-cng / add-cng-p5 / add-cng-p6-b200 / add-cng-p6-b300, so a later edit that lands in only some of the four hand-maintained copies fails the lint - chain the guard's mkdir/cat with && so the 'excluded from auto-restart' log line is only written when the drop-in was actually installed Verified: lint passes, detects a 1-char divergence when injected; the chained heredoc block runs clean under dash (and skips the log line when the write fails); validate-template passes on all four templates. --- architectures/aws-pcs/assets/add-cng-p5.yaml | 4 +-- .../aws-pcs/assets/add-cng-p6-b200.yaml | 4 +-- .../aws-pcs/assets/add-cng-p6-b300.yaml | 4 +-- architectures/aws-pcs/assets/add-cng.yaml | 8 +++--- architectures/aws-pcs/tests/lint-docs.sh | 25 +++++++++++++++++-- 5 files changed, 33 insertions(+), 12 deletions(-) diff --git a/architectures/aws-pcs/assets/add-cng-p5.yaml b/architectures/aws-pcs/assets/add-cng-p5.yaml index 7993b0530..0df4b22e0 100644 --- a/architectures/aws-pcs/assets/add-cng-p5.yaml +++ b/architectures/aws-pcs/assets/add-cng-p5.yaml @@ -405,8 +405,8 @@ Resources: # also covers the versioned units, e.g. slurmd-25.11.) # runcmd runs under /bin/sh (dash) - keep this POSIX-clean, no bashisms. - | - mkdir -p /etc/needrestart/conf.d - cat > /etc/needrestart/conf.d/90-pcs-slurm.conf <<'NRCONF' + mkdir -p /etc/needrestart/conf.d && + cat > /etc/needrestart/conf.d/90-pcs-slurm.conf <<'NRCONF' && # AWS PCS: never auto-restart slurmd - restarting it kills # the jobs running under it. Managed by add-cng UserData. $nrconf{override_rc} = { qr(^slurmd) => 0 }; diff --git a/architectures/aws-pcs/assets/add-cng-p6-b200.yaml b/architectures/aws-pcs/assets/add-cng-p6-b200.yaml index df360f04e..1cbafcb91 100644 --- a/architectures/aws-pcs/assets/add-cng-p6-b200.yaml +++ b/architectures/aws-pcs/assets/add-cng-p6-b200.yaml @@ -392,8 +392,8 @@ Resources: # also covers the versioned units, e.g. slurmd-25.11.) # runcmd runs under /bin/sh (dash) - keep this POSIX-clean, no bashisms. - | - mkdir -p /etc/needrestart/conf.d - cat > /etc/needrestart/conf.d/90-pcs-slurm.conf <<'NRCONF' + mkdir -p /etc/needrestart/conf.d && + cat > /etc/needrestart/conf.d/90-pcs-slurm.conf <<'NRCONF' && # AWS PCS: never auto-restart slurmd - restarting it kills # the jobs running under it. Managed by add-cng UserData. $nrconf{override_rc} = { qr(^slurmd) => 0 }; diff --git a/architectures/aws-pcs/assets/add-cng-p6-b300.yaml b/architectures/aws-pcs/assets/add-cng-p6-b300.yaml index 8f83f424d..6aa219b4b 100644 --- a/architectures/aws-pcs/assets/add-cng-p6-b300.yaml +++ b/architectures/aws-pcs/assets/add-cng-p6-b300.yaml @@ -395,8 +395,8 @@ Resources: # also covers the versioned units, e.g. slurmd-25.11.) # runcmd runs under /bin/sh (dash) - keep this POSIX-clean, no bashisms. - | - mkdir -p /etc/needrestart/conf.d - cat > /etc/needrestart/conf.d/90-pcs-slurm.conf <<'NRCONF' + mkdir -p /etc/needrestart/conf.d && + cat > /etc/needrestart/conf.d/90-pcs-slurm.conf <<'NRCONF' && # AWS PCS: never auto-restart slurmd - restarting it kills # the jobs running under it. Managed by add-cng UserData. $nrconf{override_rc} = { qr(^slurmd) => 0 }; diff --git a/architectures/aws-pcs/assets/add-cng.yaml b/architectures/aws-pcs/assets/add-cng.yaml index 895a75bab..4b17f9ce3 100644 --- a/architectures/aws-pcs/assets/add-cng.yaml +++ b/architectures/aws-pcs/assets/add-cng.yaml @@ -461,12 +461,12 @@ Resources: # slurmd auto-restart is suppressed. (PCS runs the controller managed-side; # slurmd is the only Slurm systemd service on login/compute nodes. qr(^slurmd) # also covers the versioned units, e.g. slurmd-25.11.) - # runcmd runs under /bin/sh (dash) — keep this POSIX-clean, no bashisms. + # runcmd runs under /bin/sh (dash) - keep this POSIX-clean, no bashisms. - | - mkdir -p /etc/needrestart/conf.d - cat > /etc/needrestart/conf.d/90-pcs-slurm.conf <<'NRCONF' + mkdir -p /etc/needrestart/conf.d && + cat > /etc/needrestart/conf.d/90-pcs-slurm.conf <<'NRCONF' && # AWS PCS: never auto-restart slurmd - restarting it kills - # the jobs running under it. Managed by add-cng.yaml UserData. + # the jobs running under it. Managed by add-cng UserData. $nrconf{override_rc} = { qr(^slurmd) => 0 }; NRCONF echo "needrestart: slurmd excluded from auto-restart" | tee /var/log/pcs-needrestart-guard.log diff --git a/architectures/aws-pcs/tests/lint-docs.sh b/architectures/aws-pcs/tests/lint-docs.sh index e1b7333cf..42a6fd7f0 100755 --- a/architectures/aws-pcs/tests/lint-docs.sh +++ b/architectures/aws-pcs/tests/lint-docs.sh @@ -64,7 +64,28 @@ for prm in $params; do grep -q "\`$prm\`" docs/PARAMETERS.md || report "deploy-all parameter '$prm' is not documented in docs/PARAMETERS.md" done -# 4. Same-file Markdown anchor links in README.md resolve to a real heading. +# 4. The needrestart/slurmd guard block must be byte-identical across the four +# CNG templates. It is hand-duplicated (no shared include), so an edit that +# lands in only some of the copies is exactly the drift this catches. +guard_extract() { # print the guard block: comment header through the log line + awk '/--- Protect running jobs from unattended-upgrades \/ needrestart ---/{p=1} + p{print} + p&&/pcs-needrestart-guard\.log/{exit}' "$1" | sed -E 's/^[[:space:]]+//' +} +ref=$(guard_extract assets/add-cng.yaml) +if [ -z "$ref" ]; then + report "needrestart guard block not found in assets/add-cng.yaml" +else + for t in add-cng-p5 add-cng-p6-b200 add-cng-p6-b300; do + other=$(guard_extract "assets/$t.yaml") + if [ "$other" != "$ref" ]; then + report "needrestart guard block in assets/$t.yaml differs from assets/add-cng.yaml (keep the four copies byte-identical):" + diff <(echo "$ref") <(echo "$other") | sed 's/^/ /' + fi + done +fi + +# 5. Same-file Markdown anchor links in README.md resolve to a real heading. # (Cross-file and external links are out of scope — kept simple on purpose.) while IFS= read -r anchor; do # build the set of heading slugs in README @@ -76,6 +97,6 @@ while IFS= read -r anchor; do done < <(grep -oE '\]\(#[a-z0-9-]+\)' README.md | sed -E 's/\]\(#//; s/\)//' | sort -u) if [ "$fail" -eq 0 ]; then - echo "docs lint: PASS (no stale parameter references, no empty=skip wording, all deploy-all params documented, README anchors resolve)" + echo "docs lint: PASS (no stale parameter references, no empty=skip wording, all deploy-all params documented, needrestart guard in lock-step, README anchors resolve)" fi exit $fail From 040d5fc3d98977bb908fd73f1d9af4f5ecfe3938 Mon Sep 17 00:00:00 2001 From: Daisuke Miyamoto Date: Wed, 8 Jul 2026 14:37:27 +0000 Subject: [PATCH 2/2] chore(aws-pcs): address lint nits from #1168 review - comment the whitespace-normalization blind spot in guard_extract (relative intra-block indentation drift is not detectable) - printf instead of echo when feeding the failure diff --- architectures/aws-pcs/tests/lint-docs.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/architectures/aws-pcs/tests/lint-docs.sh b/architectures/aws-pcs/tests/lint-docs.sh index 42a6fd7f0..bfa2c0bef 100755 --- a/architectures/aws-pcs/tests/lint-docs.sh +++ b/architectures/aws-pcs/tests/lint-docs.sh @@ -68,6 +68,11 @@ done # CNG templates. It is hand-duplicated (no shared include), so an edit that # lands in only some of the copies is exactly the drift this catches. guard_extract() { # print the guard block: comment header through the log line + # Leading whitespace is stripped because the four templates legitimately nest + # the block at different depths. Side effect: the check cannot see RELATIVE + # indentation drift inside the block (e.g. an indented NRCONF terminator, or + # <<'NRCONF' switched to the tab-stripping <<-'NRCONF' in one template) — + # those would change deployed behavior while still comparing as identical. awk '/--- Protect running jobs from unattended-upgrades \/ needrestart ---/{p=1} p{print} p&&/pcs-needrestart-guard\.log/{exit}' "$1" | sed -E 's/^[[:space:]]+//' @@ -80,7 +85,7 @@ else other=$(guard_extract "assets/$t.yaml") if [ "$other" != "$ref" ]; then report "needrestart guard block in assets/$t.yaml differs from assets/add-cng.yaml (keep the four copies byte-identical):" - diff <(echo "$ref") <(echo "$other") | sed 's/^/ /' + diff <(printf '%s\n' "$ref") <(printf '%s\n' "$other") | sed 's/^/ /' fi done fi