kernel-1095: Harden remediate.sh with pipefail and drain timeout - #5961
Sebastien Tardif (SebTardif) wants to merge 3 commits into
Conversation
set -euo pipefail so a failed kubectl|grep cannot become totalNodes=0 and scale the pool to 1. Skip empty pools, bound kubectl drain at 5m, and read providerID via jsonpath so pipefail cannot abort after drain. Introduced in Azure#3350 (2022-11-19). Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
There was a problem hiding this comment.
🟡 Changes recommended
The script can still exit after kubectl cordon (for example on drain timeout) and leave nodes unschedulable without a cleanup trap, which is an operational risk introduced/strengthened by set -euo pipefail.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens the examples/kernel-1095-issue/remediate.sh remediation script to reduce unsafe scaling behavior and prevent kubectl drain from blocking indefinitely during kernel 5.4.0-1095-azure remediation.
Changes:
- Switches the script to
set -euo pipefailand improves quoting throughout. - Makes node counting and bad-kernel selection pipelines safer under
pipefail(avoidsgrep-driven false zeros / failures). - Adds a
kubectl drain --timeout=5mto prevent indefinite hangs and parsesproviderIDviajsonpath.
File summaries
| File | Description |
|---|---|
| examples/kernel-1095-issue/remediate.sh | Hardens the remediation flow (pipefail-safe counting, drain timeout, safer providerID parsing, improved quoting). |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Add a per-node EXIT trap after cordon so a drain timeout or reimage error does not leave the node unschedulable. Clear the trap after a successful uncordon. Fix the totalNodes comment to say wc is still used. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
There was a problem hiding this comment.
🟢 Approval recommended
The changes align with the stated hardening goals and reduce operational risk (pipeline failures and indefinite drains) without introducing high-risk complexity.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Autoscaler-off pools still scaled +1 even when nodeList was empty. Skip the pool before az aks scale so a re-run does not churn nodes. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
There was a problem hiding this comment.
🟢 Approval recommended
The changes directly address the stated failure modes (pipeline masking and unbounded drain) without introducing new verified correctness issues in the updated script logic.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Pengfei Ni (@feiskyer) can you please review? |
Harden
examples/kernel-1095-issue/remediate.shso a failed node count cannot scale the pool to 1, and sokubectl draincannot block reimage forever.Problem
The script is still the documented remediation for kernel
5.4.0-1095-azure(#3350, 2022-11-19). Two issues:pipefail.totalNodes=$(kubectl get nodes ... | grep $np | wc -l)succeeds with0ifkubectlorgrepfails, becausewc -lexits 0. The script then runsaz aks scale --node-count $((0+1)).kubectl draindefaults--timeoutto 0. A PDB or stuck pod blocksaz vmss reimageindefinitely.Repo Copilot instructions require
set -euo pipefailfor shell scripts.Change
set -euo pipefailkubectl get nodes --no-headers | wc -l(nogrepin that pipeline)totalNodesis 0awkso a no-match is empty, not a failed pipelinekubectl drain ... --timeout=5mproviderIDwithjsonpathso a missedgrepcannot abort after drain (node left cordoned)Validation
shellcheck examples/kernel-1095-issue/remediate.shis clean. This repo has no bash test suite for the script. I did not run it against a live cluster.Related
--timeout