Skip to content

Commit 013b166

Browse files
authored
fix(e2e): read the KubernetesNodes pool release in the HelmRelease guard (#4970)
`cozy_guard_all_helmreleases` in `hack/e2e-chainsaw/_lib/run-kubernetes.sh` reads the parent `kubernetes-<test>` release and the addons selected by the `kubernetes-<test>-` prefix. The worker pool is `kubernetes-nodes-<test>-md0`, an Application HelmRelease the aggregated apiserver stamps with `RetryOnFailure` like the parent, and it sits on the node-join path through the talos-reconcile Job it renders. Its name does not start with the addon prefix, so a failed action on it was never read, on the passing path or in the failure-path report. This reads it on both paths, with the `required` strategy check the parent gets, since the apiserver stamps both. The pool name is derived from the parent (`cozy_pool_helmrelease_name`), the way the addon prefix is, so the two cannot drift. The failure-path report checks that the release exists before reading it, with `get hr --ignore-not-found -o name`: a run whose pool apply failed has no pool release (exit 0, empty output), and the report says so instead of reporting a failed read. For that run to reach the report at all, `run_kubernetes_test` now arms the exit handler (`_tenant_failure_on_exit`, with `CURRENT_TENANT_PARENT_HR`) right after the parent `Kubernetes` apply instead of after the `KubernetesNodes` apply: before, a failing pool apply exited under errexit with no handler installed, and the parent went unclassified. The snapshot half still no-ops until the tenant kubeconfig is set. A non-zero exit (a timeout, a kill, a Forbidden) is reported as the failed read it is, through `_cozy_guard_read_failure`, never as absence. The pool reads are gated on the report's own deadline as well as the phase clock, like the addon listing. Unit tests in `hack/run-kubernetes-remediation_test.bats` cover a failing pool release on the passing path, a clean one (the composite returns zero), an empty strategy on the pool, the report with and without the pool release, a cut-off existence read, and a pool read past the report budget; the ordering pin now also requires the handler to be armed before the pool apply. The kubectl stub gained an existence read: an `ABSENT` marker prints nothing and exits 0 under `--ignore-not-found` (NotFound without it, as kubectl does), and the failure markers apply to it as `<name>.exists`. Verified with `hack/cozytest.sh` and Bats on `hack/run-kubernetes-remediation_test.bats` (51 tests), `hack/run-kubernetes-node-join_test.bats` and `hack/remediation-guard.bats`. Closes #4737 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Kubernetes release checks now include worker-pool releases alongside the parent release and add-ons, and flag missing or invalid install strategies and remediation failures. * Failure reports distinguish worker-pool releases that are absent from those whose status cannot be read. Existing releases are assessed against the phase gate and report time budget; when the budget is exhausted, skipped checks are identified. * Failure handling now begins immediately after the parent resource is applied, covering errors that occur while applying the worker pool. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 048c029 + 75bb70e commit 013b166

2 files changed

Lines changed: 354 additions & 36 deletions

File tree

‎hack/e2e-chainsaw/_lib/run-kubernetes.sh‎

Lines changed: 71 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5230,9 +5230,10 @@ _cozy_guard_kubectl() {
52305230
cozy_guard_helmrelease() {
52315231
local _ns="$1"
52325232
local _hr="$2"
5233-
# Non-empty on a release whose install strategy the cluster is known to set,
5234-
# which is the parent Application HelmRelease and nothing else here. See the
5235-
# branch that reads it for why the check cannot be applied to the addons.
5233+
# Non-empty on a release whose install strategy the cluster is known to set:
5234+
# the parent Application HelmRelease and the worker pool release, which is an
5235+
# Application HelmRelease too. See the branch that reads it for why the check
5236+
# cannot be applied to the addons.
52365237
local _strategy_required="${3:-}"
52375238
local _probe _strategy _history _ready _read_rc
52385239
_cozy_guard_read_bounds
@@ -5437,16 +5438,32 @@ cozy_guard_addon_helmreleases() {
54375438
# keeps the parent's own verdict, which an early return past the addons would
54385439
# drop.
54395440
#
5441+
# The worker pool release of parent kubernetes-<test>. The pool is a
5442+
# KubernetesNodes named <test>-md0 (see the apply in the run), and the
5443+
# aggregated apiserver names its HelmRelease kubernetes-nodes-<test>-md0. That
5444+
# name does not start with the addon prefix kubernetes-<test>-, so the addon
5445+
# listing never selects it, and it is read on its own. Derived from the parent
5446+
# rather than passed in, for the same reason as the addon prefix below.
5447+
cozy_pool_helmrelease_name() {
5448+
local _parent="$1"
5449+
printf 'kubernetes-nodes-%s-md0\n' "${_parent#kubernetes-}"
5450+
}
5451+
54405452
# The addon prefix is derived here rather than passed in, so the parent name and
54415453
# the prefix that selects its children cannot drift apart.
54425454
cozy_guard_all_helmreleases() {
54435455
local _ns="$1"
54445456
local _parent="$2"
54455457
local _failed=0
5446-
# The parent is asked to prove its install strategy read back non-empty; the
5447-
# addons are not, because most of them legitimately set none. See the branch
5448-
# in cozy_guard_helmrelease that reads this argument.
5458+
# The parent and the pool release are asked to prove their install strategy
5459+
# read back non-empty: both are Application HelmReleases, which the apiserver
5460+
# stamps. The addons are not, because most of them legitimately set none. See
5461+
# the branch in cozy_guard_helmrelease that reads this argument.
54495462
cozy_guard_helmrelease "$_ns" "${_parent}" required || _failed=1
5463+
# The pool release sits on the node-join path, through the talos-reconcile
5464+
# Job it renders, so a failed action on it is as much a finding as one on the
5465+
# parent. By the time this runs the pool has scaled, so the release exists.
5466+
cozy_guard_helmrelease "$_ns" "$(cozy_pool_helmrelease_name "${_parent}")" required || _failed=1
54505467
cozy_guard_addon_helmreleases "$_ns" "${_parent}-" || _failed=1
54515468
# These releases are now classified, and the same flag the failure-path report
54525469
# uses records it. The two are not alternatives: this call runs before the
@@ -5495,7 +5512,8 @@ _COZY_GUARD_REPORT_DONE=0
54955512
# failing path would replace the caller's own exit status, and the tenant
54965513
# crust-gather snapshot hangs off that exit.
54975514
#
5498-
# Bounded on its own clock, checked before the listing and before each release.
5515+
# Bounded on its own clock, checked before the pool release, before the listing
5516+
# and before each addon release.
54995517
# What comes after this on the exit path is the tenant snapshot, which needs the
55005518
# rest of the op, so a namespace whose apiserver answers slowly must cost a
55015519
# bounded number of seconds here rather than however many releases happen to be
@@ -5515,7 +5533,7 @@ cozy_report_helmrelease_remediation() {
55155533
local _ns="$1"
55165534
local _parent="$2"
55175535
local _label="${3:-}"
5518-
local _addon_hrs _hr _deadline _budget
5536+
local _addon_hrs _hr _pool _pool_found _read_rc _deadline _budget
55195537
[ "${_COZY_GUARD_REPORT_DONE}" -eq 0 ] || return 0
55205538
# An empty parent name means the run failed before the release was created, so
55215539
# there is no history anywhere to read yet.
@@ -5541,6 +5559,29 @@ cozy_report_helmrelease_remediation() {
55415559
cozy_diag_phase_has_time "HelmRelease remediation footprint of ${_parent}" || return 0
55425560
echo "=== ${_label}tenant HelmRelease remediation footprint (management cluster, ns ${_ns}) — classification only, this does not decide the run ==="
55435561
cozy_guard_helmrelease "$_ns" "${_parent}" required || true
5562+
# The pool release is created by a later apply than the parent, and
5563+
# run_kubernetes_test arms the exit handler between the two, so a run whose
5564+
# pool apply failed reaches this with none to read. Its absence is then a fact
5565+
# about where the run stopped, not a failed read, and is reported as such.
5566+
# Only an exit 0 with empty output means absent: --ignore-not-found makes
5567+
# NotFound that, so any non-zero exit is a read that failed and is named as
5568+
# one.
5569+
_pool=$(cozy_pool_helmrelease_name "${_parent}")
5570+
cozy_diag_phase_has_time "HelmRelease remediation footprint of ${_pool}" || return 0
5571+
if [ "$(date +%s)" -ge "${_deadline}" ]; then
5572+
echo "=== HelmRelease remediation footprint: stopped after ${_budget}s; ${_pool} and the addon releases of ${_parent} were not read, so nothing was observed about any of them ===" >&2
5573+
return 0
5574+
fi
5575+
_cozy_guard_read_bounds
5576+
_read_rc=0
5577+
_pool_found=$(_cozy_guard_kubectl get hr -n "$_ns" "${_pool}" --ignore-not-found -o name) || _read_rc=$?
5578+
if [ "${_read_rc}" -ne 0 ]; then
5579+
echo "Checking whether ${_pool} exists failed - $(_cozy_guard_read_failure "${_read_rc}"); nothing was observed about it either way." >&2
5580+
elif [ -z "${_pool_found}" ]; then
5581+
echo "» ${_pool} does not exist - the run ended before the worker pool was applied, or its HelmRelease was never created; nothing to read"
5582+
else
5583+
cozy_guard_helmrelease "$_ns" "${_pool}" required || true
5584+
fi
55445585
cozy_diag_phase_has_time "the addon HelmRelease listing of ${_parent}" || return 0
55455586
if [ "$(date +%s)" -ge "${_deadline}" ]; then
55465587
echo "=== HelmRelease remediation footprint: stopped after ${_budget}s; the addon releases of ${_parent} were not listed, so nothing was observed about any of them ===" >&2
@@ -5723,6 +5764,28 @@ ${ouroboros_addon}
57235764
version: "${k8s_version}"
57245765
EOF
57255766

5767+
# Armed here, at the parent apply, rather than once the tenant answers. The
5768+
# parent HelmRelease exists from this point on -- the aggregated apiserver
5769+
# creates it before it answers the apply above -- and everything between here
5770+
# and the tenant LB is a management-cluster wait on that release or on what it
5771+
# installs: the KamajiControlPlane, the control-plane Deployments, the
5772+
# MachineDeployment, the admin-kubeconfig Secret. That window is the one the
5773+
# guard was written for. An install action that times out while
5774+
# admin-kubeconfig is still being provisioned leaves the failed Snapshot this
5775+
# reads, and every one of those waits exits under errexit with no reading at
5776+
# all if the handler is armed after them.
5777+
#
5778+
# Ahead of the pool apply below as well, not after it: a pool apply that fails
5779+
# exits under errexit too, and armed after it that run would leave the parent
5780+
# unclassified. Armed here, it is the run the report's "pool release does not
5781+
# exist" line describes.
5782+
#
5783+
# Arming this early costs the snapshot nothing: _tenant_snapshot_on_fail
5784+
# returns on an unset CURRENT_TENANT_KC, which is assigned where the
5785+
# kubeconfig becomes usable, so it no-ops until then.
5786+
CURRENT_TENANT_PARENT_HR="kubernetes-${test_name}"
5787+
trap '_tenant_failure_on_exit' EXIT
5788+
57265789
# Worker node pool is a separate KubernetesNodes resource since the Phase 2
57275790
# split. Named "<cluster>-md0" so it produces MachineDeployment
57285791
# kubernetes-${test_name}-md0 — the same object the pre-split
@@ -5781,22 +5844,6 @@ ${talos_block}
57815844
storageClass: "${storage_class}"
57825845
version: "${k8s_version}"
57835846
EOF
5784-
# Armed here, at the apply, rather than once the tenant answers. The parent
5785-
# HelmRelease exists from this point on -- the aggregated apiserver renders it
5786-
# from the CR above -- and everything between here and the tenant LB is a
5787-
# management-cluster wait on that release or on what it installs: the
5788-
# KamajiControlPlane, the control-plane Deployments, the MachineDeployment,
5789-
# the admin-kubeconfig Secret. That window is the one the guard was written
5790-
# for. An install action that times out while admin-kubeconfig is still being
5791-
# provisioned leaves the failed Snapshot this reads, and every one of those
5792-
# waits exits under errexit with no reading at all if the handler is armed
5793-
# after them.
5794-
#
5795-
# Arming this early costs the snapshot nothing: _tenant_snapshot_on_fail
5796-
# returns on an unset CURRENT_TENANT_KC, which is assigned where the
5797-
# kubeconfig becomes usable, so it no-ops until then.
5798-
CURRENT_TENANT_PARENT_HR="kubernetes-${test_name}"
5799-
trap '_tenant_failure_on_exit' EXIT
58005847
# Wait for the tenant-test namespace to be active
58015848
kubectl wait namespace tenant-test --timeout=20s --for=jsonpath='{.status.phase}'=Active
58025849

0 commit comments

Comments
 (0)