chore: Cherry pick again (#254)

* fix(test): uninstall downgraded package before cleanup in downgrade-after-uninstall (#241)

The test left mypkg@1.2.3 freshly installed at chainsaw's automatic
CLEANUP, where the new delete-time uninstall finalizer (from #200) must
run uninstall pods on every node before releasing the CR. That exceeds
chainsaw's default cleanup window, causing "context deadline exceeded".

Add a final uninstall-v1 step that flips uninstall.apply=true on the
downgraded version and waits for nodeState to empty, so the CR has no
installed packages when cleanup deletes it.

Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>

* fix: deadlock in webhook controller when upgrading from old versions (#243)

* fix: close StageInterrupt trap + harden core e2e pool (#242)

* fix(test): harden core e2e pool against finalizer-cleanup races and improve diagnosability

Two changes across the six core-pool chainsaw tests:

1. Add `timeouts.cleanup: 120s` to every core-pool test. Chainsaw's
   default cleanup window (~30s) is too tight for the project's CR
   finalizer, which must uncordon nodes, remove labels/annotations,
   and GC package pods before releasing. Same failure mode that hit
   downgrade-after-uninstall (PR #241).

2. Add `catch:` blocks to `depends-on` and `simple-skyhook`, mirroring
   the pattern used by the other four core tests. When these tests
   flake, CI now captures the Node, Skyhook CR, and package Pods for
   diagnosis instead of failing without artifacts.

No assert logic changes — this is a foundation pass to remove a known
flake class and make future flakes diagnosable.

Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>

* fix(operator): close StageInterrupt trap when configUpdates signal decays

A package whose only interrupt is a `configInterrupts` entry (no top-level
`interrupt` block) could get stuck at `StageInterrupt/StateSkipped`
permanently when `Status.ConfigUpdates` cleared or never persisted (e.g.
due to a 409 on the spec patch). The state machine queried the dynamic
`HasInterrupt(config)` signal at four points, and once that signal
decayed to false, the package was untouchable: `ProgressSkipped`
wouldn't promote it, `NextStage` wouldn't advance it past Interrupt,
and `GetComplete`/`IsPackageComplete` wouldn't count it complete.

Decouple progression-past-Interrupt from the dynamic signal:

- `NextStage`: add `StageInterrupt → StagePostInterrupt` to the
  no-interrupt default map. The with-interrupt full-replacement map is
  preserved as-is — PR #200 deliberately omits `StageUninstall →
  StageApply` from it so with-interrupt uninstalls route via
  `StageUninstallInterrupt`; collapsing the maps would silently
  re-enable that transition.
- `ProgressSkipped`: drop the `HasInterrupt` gate. The only writer of
  `StateSkipped` at `StageInterrupt` is `ProcessInterrupt`'s
  budget-contention branch, which already decided the package needed
  an interrupt to schedule. Stage alone is sufficient.
- `GetComplete`, `IsPackageComplete`: treat `StagePostInterrupt` as
  unconditionally terminal. The only way to reach it is via the
  interrupt cycle; gating the terminal check on a signal that can
  decay is redundant with the entry gate and only becomes load-bearing
  when the trap fires.

Reproducer in `skyhook_types_test.go` exercises all three sites.

Surfaced by the `chainsaw/config-skyhook` "update while running" step,
which patches the spec mid-flight and concurrently triggers the
`HandleMigrations` and `processSkyhooksPerNode` 409 conflict paths —
those stretch convergence enough that ConfigUpdates can be lost
between the package reaching StageInterrupt and ProgressSkipped firing.

Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>

* refactor(operator): GetComplete delegates to IsPackageComplete

GetComplete and IsPackageComplete encoded the same terminal-state logic
twice. Have GetComplete iterate the spec packages and call
IsPackageComplete for each, and move the explanatory comment to
IsPackageComplete where the predicate lives.

Behavior preserved: the prior implementation iterated node state and
filtered to spec packages by name+version match; the new one iterates
spec packages and looks them up in node state by unique name (name|version).
Same set of packages either way.

Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>

---------

Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>

---------

Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>
Co-authored-by: ayuskauskas <ayuskauskas@nvidia.com>

Author: lockwobr

docs/README.md

@@ -30,6 +30,8 @@ This directory contains user and operator documentation for Skyhook. Here you'll
 
   - [Operator Status Definitions](operator-status-definitions.md): Definitions of Status, State, Stage, and Condition concepts used throughout the Skyhook operator.
 
+  - [Webhook Bootstrap Lease](designs/webhook-bootstrap-lease.md): Why the admission-webhook cert bootstrap runs on a dedicated leader-election lease, and the runbook for upgrading from v0.7.x (where the new lease does not yet exist).
+
 - **Process**
   - [Releases](releases.md):
       Release notes and upgrade information for Skyhook.

docs/designs/webhook-bootstrap-lease.md

@@ -0,0 +1,139 @@
+# Webhook bootstrap lease
+
+This document describes why the operator's webhook cert bootstrap runs under a
+**dedicated** leader-election lease, separate from the main reconcile lease, and
+what guarantees that design does (and does not) provide.
+
+## Background: the v0.7.x → v0.15.x deadlock
+
+Starting in v0.8.0 the operator self-bootstraps its admission-webhook TLS:
+on startup the leader generates a CA + serving cert, writes `Secret/webhook-cert`
+in the operator namespace, and patches the `caBundle` of the
+`MutatingWebhookConfiguration` and `ValidatingWebhookConfiguration`. Earlier
+versions (≤ v0.7.x) relied on a different bootstrap model and have no knowledge
+of `webhook-cert`.
+
+On an in-place rollout from v0.7.x to a self-bootstrapping version, the cluster
+can enter the following dead state:
+
+1. The new pod (v0.15.x) starts as a **follower**: the leader-election lease
+   is still held by an old v0.7.x pod.
+2. The new pod's webhook controller is gated on leader election, so it never
+   creates `webhook-cert`. Its readiness probe (which waits for the secret)
+   never goes green.
+3. The old leader has no concept of `webhook-cert` and never creates it. The
+   old leader also has nothing that would cause it to release the lease.
+4. Meanwhile, Argo/Helm applying the new manifests has reset the
+   `MutatingWebhookConfiguration` `caBundle` to empty, so the apiserver no
+   longer trusts the v0.7.x pods either. With `failurePolicy: Fail`, all
+   CREATE/UPDATE on Skyhook CRs fail with
+   `x509: certificate signed by unknown authority`.
+
+Net result: the new pod waits forever for a secret that requires leadership to
+create, the old pod holds leadership forever because the new pod never goes
+Ready, and Argo cannot reconcile any further Skyhook config changes.
+
+The only field operations that broke the cycle were manual:
+
+- Delete the old-version pods to force a lease handover, *or*
+- Delete the lease object directly (`Lease/3c22c1ae.nvidia.com`).
+
+## Design: a second lease, owned by a second manager
+
+The webhook bootstrap is now driven by a **dedicated `ctrl.Manager`** with its
+own leader-election lease name (`nodewright-webhook-bootstrap.nvidia.com`), separate from the
+main reconcile manager's lease (`3c22c1ae.nvidia.com`).
+
+```
+┌────────────────────────────────────────────────────────────────────────┐
+│ operator process (one per pod)                                         │
+│                                                                        │
+│  ┌────────────────────────────────────┐                                │
+│  │ reconcile manager                  │  lease: 3c22c1ae.nvidia.com    │
+│  │  - SkyhookReconciler (leader-gated)│                                │
+│  │  - SecretCertWatcher (every pod)   │                                │
+│  │  - webhook server (every pod)      │                                │
+│  │  - readyz / healthz                │                                │
+│  └────────────────────────────────────┘                                │
+│                                                                        │
+│  ┌────────────────────────────────────┐                                │
+│  │ webhook bootstrap manager          │  lease: nodewright-webhook-bootstrap.nvidia.com │
+│  │  - WebhookController (leader-gated)│                                │
+│  │     creates webhook-cert Secret    │                                │
+│  │     patches caBundle on            │                                │
+│  │     {Validating,Mutating}WebhookConfiguration │                     │
+│  └────────────────────────────────────┘                                │
+└────────────────────────────────────────────────────────────────────────┘
+```
+
+`SecretCertWatcher` (`NeedLeaderElection() == false`) continues to run on the
+reconcile manager on every pod. It watches the namespace's Secrets via the
+reconcile manager's cache and syncs `webhook-cert` to the local disk so this
+pod's webhook server can serve TLS. That part is unchanged.
+
+Both managers share the same `rest.Config` and `scheme` and start under the
+same signal handler via `errgroup.WithContext`. If either manager exits with
+an error the process exits non-zero.
+
+### Invariants this preserves
+
+- **Single writer to `webhook-cert` and to the webhook configuration caBundle.**
+  Exactly one pod holds `nodewright-webhook-bootstrap.nvidia.com` at a time. There is no
+  split-brain risk between operator versions that both understand this lease.
+- **Webhook server runs on every pod.** The webhook server, its readiness
+  probe, and `SecretCertWatcher` are all on the reconcile manager, not gated on
+  the bootstrap lease — so all pods can serve admission traffic as soon as
+  `webhook-cert` exists locally.
+- **`failurePolicy: Fail` on the admission webhooks remains in effect.** We
+  do not need to relax admission to make this work.
+
+### What this fixes
+
+Any *future* upgrade discontinuity between two versions that both understand
+the dedicated bootstrap lease: a new pod can win
+`nodewright-webhook-bootstrap.nvidia.com` and complete the cert bootstrap independently of
+who holds the main reconcile lease. Readiness flips green, Service endpoints
+rotate, the rollout completes.
+
+### What this does NOT fix
+
+This design **cannot** retroactively fix the v0.7.x → v0.15.x upgrade
+specifically, because v0.7.x has no knowledge of the new lease. Operators
+on v0.7.x still need the manual workaround:
+
+```bash
+# In the operator's namespace:
+kubectl -n skyhook get pods                                                 # identify old-version pods
+kubectl -n skyhook delete pod <old-pod-1> <old-pod-2>                       # free the lease
+# Or, equivalently:
+kubectl -n skyhook delete lease 3c22c1ae.nvidia.com
+
+# Verify recovery:
+kubectl -n skyhook get secret webhook-cert -w
+kubectl get mutatingwebhookconfiguration skyhook-operator-mutating-webhook \
+  -o jsonpath='{.webhooks[0].clientConfig.caBundle}' | wc -c                # must be > 0
+kubectl -n skyhook rollout status deploy/skyhook-operator-controller-manager
+```
+
+The runbook above should be added to release notes for any future major
+operator version that changes the webhook bootstrap model.
+
+## Implementation notes
+
+- Lease names live as constants in `operator/cmd/manager/main.go`:
+  `reconcileLeaseID`, `webhookBootstrapLeaseID`. **Do not rename them** without
+  a coordinated upgrade plan — renaming the bootstrap lease re-introduces
+  exactly the deadlock this design avoids.
+- The bootstrap manager is only constructed when `ENABLE_WEBHOOKS=true`.
+- The bootstrap manager disables its own metrics endpoint
+  (`metricsserver.Options{BindAddress: "0"}`) and does not bind a health-probe
+  port; the reconcile manager already owns those.
+- The bootstrap manager runs its own cache, which adds informers for
+  `Secret/webhook-cert` and the two webhook configurations only. The reconcile
+  manager's cache also watches Secrets via `SecretCertWatcher`. Two
+  Secret informers in the operator namespace is the cost of the split and is
+  considered acceptable.
+- `WebhookSecretReadyzCheck` reads through the bootstrap manager's client and
+  is registered on the reconcile manager's health-probe server. Cache-sync
+  races during startup naturally result in NotReady, which is the desired
+  behavior.

k8s-tests/chainsaw/skyhook/config-skyhook/chainsaw-test.yaml

@@ -24,6 +24,7 @@ metadata:
 spec:
   timeouts:
     assert: 240s ## 5 steps with full package lifecycles including mid-flight updates
+    cleanup: 120s ## finalizer needs time to uncordon, remove labels/annotations, and GC pods
   catch: ## if errors, print the most important info
     - get:
         apiVersion: v1

k8s-tests/chainsaw/skyhook/depends-on/chainsaw-test.yaml

@@ -24,6 +24,24 @@ metadata:
 spec:
   timeouts:
     assert: 90s
+    cleanup: 120s ## finalizer needs time to uncordon, remove labels/annotations, and GC pods
+  catch: ## if errors, print the most important info
+    - get:
+        apiVersion: v1
+        kind: Node
+        selector: skyhook.nvidia.com/test-node=skyhooke2e
+        format: yaml
+    - get:
+        apiVersion: skyhook.nvidia.com/v1alpha1
+        kind: Skyhook
+        name: depends-on
+        format: yaml
+    - get:
+        apiVersion: v1
+        kind: Pod
+        namespace: skyhook
+        selector: skyhook.nvidia.com/name=depends-on
+        format: yaml
   steps:
   - name: setup
     description: Reset node state, create skyhook with dependencies, and assert it completes

k8s-tests/chainsaw/skyhook/downgrade-after-uninstall/README.md

@@ -20,6 +20,10 @@ the complementary case where no uninstall is required.
 3. Update the spec to v1.2.3. The webhook accepts the downgrade because the
    package is absent from all tracked nodes. The new version installs and
    reaches `stage=config, state=complete`.
+4. Flip `uninstall.apply: true` on v1.2.3 and wait for the package to be
+   absent from node state again. This is purely operational — it leaves the
+   CR with no installed packages so chainsaw's automatic CLEANUP can delete
+   it without waiting on the delete-time uninstall finalizer.
 
 ## Key Features Tested
 
@@ -31,7 +35,8 @@ the complementary case where no uninstall is required.
 
 ## Files
 
-- `chainsaw-test.yaml` — Main test: install v2 → uninstall v2 → downgrade to v1
+- `chainsaw-test.yaml` — Main test: install v2 → uninstall v2 → downgrade to v1 → uninstall v1
 - `skyhook.yaml` — Initial v2.1.4 Skyhook, `uninstall.enabled: true`
-- `update-trigger-uninstall.yaml` — Patch setting `uninstall.apply: true`
+- `update-trigger-uninstall.yaml` — Patch setting `uninstall.apply: true` on v2.1.4
 - `update-downgrade.yaml` — Patch changing version to v1.2.3
+- `update-trigger-uninstall-v1.yaml` — Patch setting `uninstall.apply: true` on v1.2.3 to clear node state before cleanup

k8s-tests/chainsaw/skyhook/downgrade-after-uninstall/chainsaw-test.yaml

@@ -99,3 +99,21 @@ spec:
             name: downgrade-after-uninstall
           status:
             status: complete
+
+  - name: uninstall-v1
+    description: >
+      Uninstall the downgraded v1.2.3 package so the CR has no installed
+      packages at chainsaw cleanup time — keeps the finalizer's delete-time
+      uninstall flow off the cleanup critical path.
+    try:
+    - update:
+        file: update-trigger-uninstall-v1.yaml
+    - assert:
+        resource:
+          apiVersion: v1
+          kind: Node
+          metadata:
+            labels:
+              skyhook.nvidia.com/test-node: skyhooke2e
+            annotations:
+              skyhook.nvidia.com/nodeState_downgrade-after-uninstall: '{}'

k8s-tests/chainsaw/skyhook/downgrade-after-uninstall/update-trigger-uninstall-v1.yaml

@@ -0,0 +1,33 @@
+# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: skyhook.nvidia.com/v1alpha1
+kind: Skyhook
+metadata:
+  name: downgrade-after-uninstall
+spec:
+  nodeSelectors:
+    matchLabels:
+      skyhook.nvidia.com/test-node: skyhooke2e
+  interruptionBudget:
+    count: 1
+  packages:
+    mypkg:
+      version: "1.2.3"
+      image: ghcr.io/nvidia/skyhook/agentless
+      uninstall:
+        enabled: true
+        apply: true

k8s-tests/chainsaw/skyhook/simple-skyhook/chainsaw-test.yaml

@@ -25,6 +25,24 @@ spec:
   timeouts:
     assert: 240s
     exec: 90s
+    cleanup: 120s ## finalizer needs time to uncordon, remove labels/annotations, and GC pods
+  catch: ## if errors, print the most important info
+    - get:
+        apiVersion: v1
+        kind: Node
+        selector: skyhook.nvidia.com/test-node=skyhooke2e
+        format: yaml
+    - get:
+        apiVersion: skyhook.nvidia.com/v1alpha1
+        kind: Skyhook
+        name: simple-skyhook
+        format: yaml
+    - get:
+        apiVersion: v1
+        kind: Pod
+        namespace: skyhook
+        selector: skyhook.nvidia.com/name=simple-skyhook
+        format: yaml
   steps:
   - name: deploy
     description: Reset state, apply a skyhook with three packages, and verify all complete with correct metrics

k8s-tests/chainsaw/skyhook/simple-update-skyhook/chainsaw-test.yaml

@@ -24,6 +24,7 @@ metadata:
 spec:
   timeouts:
     assert: 240s
+    cleanup: 120s ## finalizer needs time to uncordon, remove labels/annotations, and GC pods
   catch: ## if errors, print the most important info
   - get:
       apiVersion: v1

k8s-tests/chainsaw/skyhook/strict-order/chainsaw-test.yaml

@@ -25,6 +25,7 @@ spec:
   timeouts:
     assert: 90s
     exec: 90s
+    cleanup: 120s ## finalizer needs time to uncordon, remove labels/annotations, and GC pods
   catch: ## if errors, print the most important info
   - get:
       apiVersion: v1