Skip to content

Commit 75bb70e

Browse files
committed
fix(e2e): arm the tenant failure handler before the pool apply
The exit handler was armed after the KubernetesNodes apply, so a failing pool apply exited under errexit with no handler installed: the parent went unclassified, and the report's "pool release does not exist" line described a run that could never reach it. Arm it right after the parent Kubernetes apply instead. The snapshot half still no-ops until the tenant kubeconfig is set, so the only change is that a failed pool apply now gets the parent classified and the absent pool reported. The ordering pin now also requires the handler to be armed before the pool apply. Assisted-by: LLM Signed-off-by: Marian Koreniuk <[email protected]>
1 parent 4d7d336 commit 75bb70e

2 files changed

Lines changed: 46 additions & 24 deletions

File tree

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

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5559,11 +5559,13 @@ cozy_report_helmrelease_remediation() {
55595559
cozy_diag_phase_has_time "HelmRelease remediation footprint of ${_parent}" || return 0
55605560
echo "=== ${_label}tenant HelmRelease remediation footprint (management cluster, ns ${_ns}) — classification only, this does not decide the run ==="
55615561
cozy_guard_helmrelease "$_ns" "${_parent}" required || true
5562-
# The pool release is created by a later apply than the parent, so a run that
5563-
# failed between the two has none to read. Its absence is then a fact about
5564-
# where the run stopped, not a failed read, and is reported as such. Only an
5565-
# exit 0 with empty output means absent: --ignore-not-found makes NotFound
5566-
# that, so any non-zero exit is a read that failed and is named as one.
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.
55675569
_pool=$(cozy_pool_helmrelease_name "${_parent}")
55685570
cozy_diag_phase_has_time "HelmRelease remediation footprint of ${_pool}" || return 0
55695571
if [ "$(date +%s)" -ge "${_deadline}" ]; then
@@ -5762,6 +5764,28 @@ ${ouroboros_addon}
57625764
version: "${k8s_version}"
57635765
EOF
57645766

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+
57655789
# Worker node pool is a separate KubernetesNodes resource since the Phase 2
57665790
# split. Named "<cluster>-md0" so it produces MachineDeployment
57675791
# kubernetes-${test_name}-md0 — the same object the pre-split
@@ -5820,22 +5844,6 @@ ${talos_block}
58205844
storageClass: "${storage_class}"
58215845
version: "${k8s_version}"
58225846
EOF
5823-
# Armed here, at the apply, rather than once the tenant answers. The parent
5824-
# HelmRelease exists from this point on -- the aggregated apiserver renders it
5825-
# from the CR above -- and everything between here and the tenant LB is a
5826-
# management-cluster wait on that release or on what it installs: the
5827-
# KamajiControlPlane, the control-plane Deployments, the MachineDeployment,
5828-
# the admin-kubeconfig Secret. That window is the one the guard was written
5829-
# for. An install action that times out while admin-kubeconfig is still being
5830-
# provisioned leaves the failed Snapshot this reads, and every one of those
5831-
# waits exits under errexit with no reading at all if the handler is armed
5832-
# after them.
5833-
#
5834-
# Arming this early costs the snapshot nothing: _tenant_snapshot_on_fail
5835-
# returns on an unset CURRENT_TENANT_KC, which is assigned where the
5836-
# kubeconfig becomes usable, so it no-ops until then.
5837-
CURRENT_TENANT_PARENT_HR="kubernetes-${test_name}"
5838-
trap '_tenant_failure_on_exit' EXIT
58395847
# Wait for the tenant-test namespace to be active
58405848
kubectl wait namespace tenant-test --timeout=20s --for=jsonpath='{.status.phase}'=Active
58415849

‎hack/run-kubernetes-remediation_test.bats‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,9 +387,10 @@ STUB
387387
}
388388

389389
@test "the failure-path report says when the pool release was never created" {
390-
# The pool is applied after the parent, so a run that failed between the two
391-
# has no pool release. That is a fact about where the run stopped, and the
392-
# report says so instead of reporting a failed read of a missing object.
390+
# The pool is applied after the parent, and the exit handler is armed between
391+
# the two, so a run whose pool apply failed reaches the report with no pool
392+
# release. That is a fact about where the run stopped, and the report says so
393+
# instead of reporting a failed read of a missing object.
393394
. hack/e2e-chainsaw/_lib/run-kubernetes.sh
394395
tmp=$(mktemp -d)
395396
cozy_make_kubectl_stub "$tmp"
@@ -1484,6 +1485,19 @@ $out" >&2
14841485
echo "expected the handler armed (line $arm) before the control-plane wait (line $cp_wait)" >&2
14851486
exit 1
14861487
fi
1488+
# And before the pool apply, which follows the parent's. A failing pool
1489+
# apply exits under errexit, so armed after it that run leaves the parent
1490+
# unclassified, and the report's absent-pool line describes a run that never
1491+
# reaches it.
1492+
pool_apply=$(grep -n '^kind: KubernetesNodes$' "$lib" | head -n 1 | cut -d: -f1)
1493+
if [ -z "$pool_apply" ]; then
1494+
echo "expected to find the KubernetesNodes apply in $lib" >&2
1495+
exit 1
1496+
fi
1497+
if [ "$arm" -ge "$pool_apply" ]; then
1498+
echo "expected the handler armed (line $arm) before the pool apply (line $pool_apply)" >&2
1499+
exit 1
1500+
fi
14871501
# The parent name is part of this one, because it is what tells the
14881502
# node-join call apart from the handler's. What follows it is not: a
14891503
# section label is presentation and may change without the wiring changing.

0 commit comments

Comments
 (0)