Same two holes as the migration's read classification, in the unpin
hook. Capturing stderr into $pol (2>&1) contaminated the success path:
kubectl warns on successful reads, so an unannotated object read as
"pinned" and drew a spurious annotate call -- one per named object.
And the *"not found"* pattern classified a transient conversion-webhook
failure as absence, letting the Job report success while the
keep-annotated objects leaked past the uninstall.
--ignore-not-found makes the whole classification unnecessary here:
absence folds into the same empty-output no-op branch as "present but
unannotated", which is exactly the semantics unpin() wants, and any
non-zero exit is a real failure that set -e turns into a failed Job.
The case/esac disappears; stderr stays on the Job log where it is
visible instead of parsed. This also avoids tempfile juggling under the
container's readOnlyRootFilesystem.
The bats suite gains a previously missing failure-path case and
regression tests for both holes, red against the previous commit: a
failed KMT listing must fail the Job (FAKE_LIST_FAIL), a read failure
whose message contains "not found" must fail the Job, and a
success-path stderr warning must not fake a pinned state
(FAKE_WARN_STDERR). Absence is now modeled the way real kubectl
behaves under --ignore-not-found: success with empty output.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
The pre-delete unpin hook's existence check swallowed every kubectl get
failure (`... >/dev/null 2>&1 || return 0`), four lines below the
comment declaring "Failures are NOT swallowed". kubectl exits 1 for
NotFound and for a transient apiserver error alike, so a timeout during
uninstall read as "object absent": the unpin was skipped, the Job
succeeded, and Helm then skipped the still-keep-annotated
MachineDeployment -- leaking the adopted pool's worker VMs into the
tenant namespace, running and unmanaged, after the pool was deleted.
The KMT-listing path was already hardened for exactly this class; the
existence check was the remaining instance.
Classify by stderr: NotFound (native pool, or already gone) stays a
no-op; any other failure fails the Job, bounded by backoffLimit and
activeDeadlineSeconds, with logs retained. The existence check and the
annotation read fold into a single request.
The embedded script's behavior cannot be asserted from the manifest, so
a new bats suite renders the chart, extracts the Job's script, and
executes it against a fake kubectl: NotFound no-ops and succeeds, a
transient named-get error fails the Job (red against the previous
script, which printed "unpin done" and exited 0), keep-annotated
objects are unpinned with the 6-hex hash anchor respected, and an
unpinned existing object is left untouched.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
Timofei Larkin