feat: add observe mode and a startup grace to the orphan sweep

Three changes aimed at making this safe to adopt on a fleet nobody has audited.

OBSERVE MODE. This controller deletes machines, and an operator adopting it
could otherwise only learn what it would do to their fleet by letting it do it.
A kill switch is reactive; observe mode is not. It runs every check, emits every
signal, and changes nothing -- not the fleet, not the servers' labels, nothing
outside the cluster. INSTANCE_GARBAGE_COLLECTION_MODE replaces the boolean,
since three states do not fit in one, and an unrecognised value refuses to start
rather than falling back to the mode that deletes.

A STARTUP GRACE. Grace is counted in consecutive sweeps, which a restart or a
leader handover resets -- and the instability that strands servers is exactly
what causes those. A fresh process could otherwise reach the threshold having
watched the cluster for only a few minutes. It must now also have been sweeping
for a full window before it may reclaim anything, so operator instability delays
reclamation rather than either skipping it forever or authorising it on a short
history.

This is what makes in-process counting safe rather than merely simple, and it
follows core: nodeclaim/consistency keeps first-seen times in an in-process
cache and writes nothing durable. Core's durable timeouts hang off status
conditions on objects it owns, never off the cloud resource. An earlier revision
of this branch stored the clock on the Hetzner server itself; it was withdrawn
because it made a safety-critical invariant depend on a remote write that can
fail silently, on a clock written by a process that may be gone, and on state an
operator can edit with one CLI command.

NODE EVENTS. Reclamations are recorded as GarbageCollected, and observe-mode
candidates as WouldGarbageCollect, typed Normal -- reclaiming an orphan is this
controller working, not a fault, and a cluster alerting on Warning events
against Nodes should not page every time the sweep does its job. Most orphans
never registered a Node, so this supplements the log and the metric rather than
replacing them, and the doc comment says so.

Three chart-level holes closed alongside them. The ClusterRole now grants
events.k8s.io, without which the manager's recorder has every event rejected 403
while the README tells operators to validate observe mode by reading them. The
mode is emitted unconditionally rather than through a "with" block that skips
falsy values, so "mode: false" -- the natural typo when migrating from the
boolean this replaces -- reaches the parser and stops the operator instead of
silently defaulting to enabled. And a "fail" guard rejects an upgrade still
carrying the removed "disabled" key, which Helm would otherwise merge while the
sweep ran on a fleet the operator believed was paused.

The clock is injected, matching core, so the startup grace is testable without
reaching into a fake provider.

Author: pkieszcz

README.md

@@ -25,7 +25,7 @@ version tag in production.
 - **Talos Linux and Ubuntu images**, resolved per architecture.
 - **Placement groups** for spreading nodes across physical hosts.
 - **Cost controls** — opt out of the billed public IPv4 (and/or IPv6) per node class for private-network clusters.
-- **Multi-cluster safe** — every managed server is tagged with the cluster name, so several clusters can share one Hetzner project without touching each other's nodes.
+- **Multi-cluster safe** — every managed server is tagged with the cluster name and the cluster's `kube-system` UID, so several clusters can share one Hetzner project without touching each other's nodes. Servers created before the UID label existed are matched on name alone, so give each cluster a distinct `clusterName` until the fleet has rolled.
 
 ## How it works
 
@@ -66,22 +66,53 @@ Two mechanisms cover this:
   cluster and this NodeClaim and matches the requested type, location and image.
 - **Garbage collection.** A sweep every two minutes reclaims servers Karpenter
   has no NodeClaim for, along with the Node objects they left behind. A server
-  must be seen reapable on three consecutive sweeps, and one whose node is
+  must be seen unowned on several consecutive sweeps, and one whose node is
   registered and still `Ready` is never touched — a machine carrying workloads is
-  core's to drain, not this sweep's to destroy. Sparing a server restarts its
-  count, so a machine the `Ready` guard protected does not sit on a spent grace
-  window waiting for its first NotReady blip.
+  core's to drain, not this sweep's to destroy.
+
+  Every path that declines to act resets the count, so the window always measures
+  an uninterrupted run of sweeps that found nothing in the way; a machine the
+  `Ready` guard protected never sits on a spent window waiting for its first
+  NotReady blip. The count is per-process, so a restart or leader handover starts
+  it again — and the operator must additionally have been sweeping for a full
+  window before it may reclaim anything, so instability delays reclamation rather
+  than authorising it on a short history.
 
 **`clusterName` must be unique per cluster within a Hetzner project.** Servers
 are labelled with it, and the sweep uses that label to decide what it owns. Two
 clusters sharing a name in one project would each see the other's servers as
 unclaimed. The operator therefore also stamps the UID of the cluster's
 `kube-system` namespace on every server it creates and refuses to touch a server
-carrying a different one, logging the collision once. That protects servers
-created from this version onward; servers predating it carry no UID and are
-still matched on name alone, so distinct names remain the thing to get right.
-
-Set `instanceGarbageCollection.disabled: true` to pause the sweep during
+carrying a different one, logging the collision once and counting it as
+`karpenter_hetzner_orphaned_server_gc_total{result="skipped_foreign_cluster"}` on
+every sweep.
+
+Two things this does not cover. It protects servers created from this version
+onward; servers predating it carry no UID and are still matched on name alone, so
+until a fleet has fully rolled, distinct names remain the thing to get right.
+And the UID identifies the *control plane*, not the servers: rebuilding a cluster
+from scratch mints a new `kube-system` UID, after which the previous
+incarnation's servers are refused forever — never reclaimed, still billing. The
+`skipped_foreign_cluster` counter is the signal for both. Recovering from a
+rebuild means relabelling those servers with the new UID
+(`hcloud server add-label <server> karpenter.sh/cluster-uid=<uid>`, where `<uid>`
+is `kubectl get ns kube-system -o jsonpath='{.metadata.uid}'`) or deleting them
+by hand.
+
+> **Upgrading an existing cluster.** This version adds a controller that
+> **deletes Hetzner servers**. On first start it reclaims every server in the
+> project that carries this cluster's labels and has no NodeClaim — which is the
+> point, but on a fleet nobody has audited it is worth seeing first.
+>
+> Set `instanceGarbageCollection.mode: observe` to run every check and report
+> what *would* be reclaimed without deleting anything. Watch
+> `karpenter_hetzner_orphaned_server_gc_total{result="would_reap"}` and the
+> `WouldGarbageCollect` events on the affected Nodes, satisfy yourself the list
+> is right, then switch to `enabled`. Reclamations are recorded as
+> `GarbageCollected` events on the Node, so `kubectl describe node` explains a
+> server that disappeared.
+
+Set `instanceGarbageCollection.mode: disabled` to pause the sweep during
 maintenance that removes NodeClaims wholesale (reinstalling the CRDs, restoring
 etcd, clearing finalizers by hand), so it does not act on a cluster that only
 looks empty. Provisioning and disruption keep working while it is off.
@@ -224,7 +255,7 @@ comments explaining every field.
 |---------|----------|-------------|
 | `HCLOUD_TOKEN` | yes | Hetzner Cloud API token |
 | `CLUSTER_NAME` | yes | Cluster identifier; scopes managed servers. Must be unique per Hetzner project — two clusters sharing a value will reclaim each other's servers |
-| `DISABLE_INSTANCE_GARBAGE_COLLECTION` | no (unset) | Set to `true` to pause the orphaned-server sweep (chart value: `instanceGarbageCollection.disabled`) |
+| `INSTANCE_GARBAGE_COLLECTION_MODE` | no (`enabled`) | `enabled`, `observe` or `disabled`; an unrecognised value stops the operator starting (chart value: `instanceGarbageCollection.mode`) |
 | `METRICS_PORT` | no (8080) | Prometheus metrics port |
 | `HEALTH_PROBE_PORT` | no (8081) | Health/readiness probe port |
 

charts/karpenter-provider-hetzner/Chart.yaml

@@ -32,4 +32,4 @@ annotations:
   artifacthub.io/containsSecurityUpdates: "false"
   artifacthub.io/changes: |
     - kind: added
-      description: "Reclaim Hetzner servers Karpenter no longer has a NodeClaim for, which would otherwise run and bill unowned; pause the sweep with instanceGarbageCollection.disabled during maintenance that removes NodeClaims wholesale"
+      description: "Reclaim Hetzner servers Karpenter no longer has a NodeClaim for, which would otherwise run and bill unowned; set instanceGarbageCollection.mode to observe to preview it, or disabled to pause it during maintenance that removes NodeClaims wholesale"

charts/karpenter-provider-hetzner/README.md

@@ -37,7 +37,7 @@ Existing `v1alpha1` objects are not migrated automatically; recreate them under
 | Key | Default | Description |
 |-----|---------|-------------|
 | `clusterName` | `""` (required) | Scopes which servers the controller manages; must be unique per Hetzner project |
-| `instanceGarbageCollection.disabled` | `false` | Pause the sweep that reclaims servers with no NodeClaim (see values.yaml) |
+| `instanceGarbageCollection.mode` | `enabled` | `enabled`, `observe` (report what would be reclaimed, delete nothing) or `disabled` (see values.yaml) |
 | `replicas` | `1` | Controller replicas |
 | `image.repository` | `ghcr.io/paperclipinc/karpenter-provider-hetzner` | Image |
 | `image.tag` | `""` | Empty tracks the chart appVersion; pin a tag in production |
@@ -87,6 +87,12 @@ When `serviceMonitor.enabled=true` the chart creates:
 - a `Service` named `karpenter-provider-hetzner-metrics` exposing port `http-metrics`
 - a `ServiceMonitor` that selects that Service and scrapes `/metrics` at the configured interval
 
-Requires the [Prometheus Operator](https://github.com/prometheus-operator/prometheus-operator) CRDs to be present. The controller exposes provider metrics under the `karpenter_hetzner_` prefix (server creates/deletes, durations, drift reasons, instance-type cache hits/misses, orphaned-server garbage-collection outcomes, adopted servers, and raw hcloud API call counts).
+Requires the [Prometheus Operator](https://github.com/prometheus-operator/prometheus-operator) CRDs to be present. The controller exposes provider metrics under the `karpenter_hetzner_` prefix (server creates/deletes, durations, drift reasons, instance-type cache hits/misses, orphaned-server garbage-collection outcomes, adopted servers, unpriceable nodes, and raw hcloud API call counts).
 
-Worth an alert: `karpenter_hetzner_orphaned_server_gc_total{result="error"}` means a server cannot be reclaimed and is still billing, and `karpenter_hetzner_server_adopt_total{result="declined"}` means a NodeClaim keeps colliding with a server adoption refuses to take.
+Worth an alert:
+
+- `karpenter_hetzner_orphaned_server_gc_total{result="error"}` — a server cannot be reclaimed and is still billing.
+- `karpenter_hetzner_server_adopt_total{result="declined"}` — a NodeClaim keeps colliding with a server adoption refuses to take.
+- `karpenter_hetzner_orphaned_server_gc_total{result="skipped_foreign_cluster"}` — servers carry this cluster's `clusterName` but another cluster's UID. Either two clusters share a name in one Hetzner project, or this cluster's control plane was rebuilt and these servers predate it. Either way they will never be reclaimed.
+
+Note that the metrics endpoint is scraped on every replica, but the sweeps behind these metrics run only on the leader. Aggregate with `max()` rather than `avg()`/`min()`, or a standby's zero will read as a healthy cluster.

charts/karpenter-provider-hetzner/templates/deployment.yaml

@@ -68,10 +68,17 @@ spec:
                   key: {{ .Values.auth.secretRef.key }}
             - name: CLUSTER_NAME
               value: {{ .Values.clusterName | quote }}
-            {{- if .Values.instanceGarbageCollection.disabled }}
-            - name: DISABLE_INSTANCE_GARBAGE_COLLECTION
-              value: "true"
+            {{- if hasKey .Values.instanceGarbageCollection "disabled" }}
+              {{- fail "instanceGarbageCollection.disabled has been replaced by instanceGarbageCollection.mode (enabled|observe|disabled). Your existing value is being ignored, which would leave the orphaned-server sweep RUNNING. Set mode explicitly and remove the disabled key." }}
             {{- end }}
+            # Emitted unconditionally rather than through `with`, which skips
+            # falsy values: `mode: false` -- the natural typo when migrating from
+            # the boolean this replaced -- would omit the variable, the operator
+            # would default to enabled, and a value meant to stop the sweep would
+            # start it. Passed through so an invalid value reaches parseGCMode,
+            # which refuses to start.
+            - name: INSTANCE_GARBAGE_COLLECTION_MODE
+              value: {{ .Values.instanceGarbageCollection.mode | quote }}
             - name: METRICS_PORT
               value: {{ .Values.metrics.port | quote }}
             - name: HEALTH_PROBE_PORT

charts/karpenter-provider-hetzner/templates/rbac.yaml

@@ -15,14 +15,23 @@ rules:
   - apiGroups: [""]
     resources: ["nodes", "nodes/status"]
     verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
-  # Core resources Karpenter's scheduling simulation observes.
+  # Core resources Karpenter's scheduling simulation observes. "namespaces" is
+  # also load-bearing for startup: the operator reads the kube-system namespace's
+  # UID to identify this cluster and refuses to start without it, so narrowing
+  # this rule takes the whole controller down, not just scheduling.
   - apiGroups: [""]
     resources: ["pods", "persistentvolumes", "persistentvolumeclaims", "replicationcontrollers", "namespaces"]
     verbs: ["get", "list", "watch"]
   - apiGroups: [""]
     resources: ["pods/eviction"]
     verbs: ["create"]
-  - apiGroups: [""]
+  # Karpenter core and leader election use the deprecated core-group recorder;
+  # this provider's garbage collector uses the manager's recorder, which writes
+  # to events.k8s.io/v1. RBAC authorizes on (apiGroup, resource), so the core
+  # group alone would 403 every GarbageCollected event -- silently, since only a
+  # klog line reports it, while the README tells operators to validate observe
+  # mode by reading those very events.
+  - apiGroups: ["", "events.k8s.io"]
     resources: ["events"]
     verbs: ["create", "patch"]
   - apiGroups: [""]

charts/karpenter-provider-hetzner/values.yaml

@@ -53,13 +53,22 @@ args: []
 clusterName: ""
 
 # The operator reclaims servers Karpenter no longer has a NodeClaim for, which
-# would otherwise run and bill with nothing pointing at them. Set disabled: true
-# to pause that sweep during maintenance that removes NodeClaims wholesale —
-# reinstalling the CRDs, restoring etcd, clearing finalizers by hand — so it does
-# not act on a cluster that only looks empty. Provisioning and disruption keep
-# working while it is off. Remember to turn it back on.
+# would otherwise run and bill with nothing pointing at them.
 instanceGarbageCollection:
-  disabled: false
+  # enabled  - reclaim orphaned servers.
+  # observe  - run every check and report what WOULD be reclaimed, deleting
+  #            nothing. Start here on an existing fleet: watch
+  #            karpenter_hetzner_orphaned_server_gc_total{result="would_reap"},
+  #            satisfy yourself it names the right machines, then switch to
+  #            enabled. A kill switch only helps after something has gone wrong.
+  # disabled - do not sweep at all. For maintenance that removes NodeClaims
+  #            wholesale (reinstalling the CRDs, restoring etcd, clearing
+  #            finalizers by hand), so the sweep does not act on a cluster that
+  #            only looks empty. Provisioning and disruption keep working.
+  #
+  # An unrecognised value stops the operator starting rather than falling back
+  # to enabled: a typo here would reap the fleet it was meant to protect.
+  mode: enabled
 
 metrics:
   port: 8080

cmd/controller/main.go

@@ -1,6 +1,9 @@
 package main
 
 import (
+	"context"
+	"time"
+
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	// Register karpenter core types into the default k8s scheme.
@@ -25,6 +28,10 @@ import (
 	"github.com/paperclipinc/karpenter-provider-hetzner/pkg/providers/instancetype"
 )
 
+// clusterUIDTimeout bounds the one API-server read the operator makes before the
+// manager -- and therefore the health probes -- are running.
+const clusterUIDTimeout = 30 * time.Second
+
 func main() {
 	ctx, op := operator.NewOperator()
 
@@ -41,19 +48,26 @@ func main() {
 		return
 	}
 
-	// Create the three providers.
 	// Identify this cluster independently of its operator-chosen name. CLUSTER_NAME
 	// is not guaranteed unique, and two clusters sharing one in a single Hetzner
 	// project would otherwise each treat the other's servers as its own -- which
 	// now means deleting them. The kube-system UID is unique per cluster and
 	// stable for its lifetime. Read through the API reader because the manager's
 	// cache is not running yet.
-	clusterUID, err := hetznerop.ClusterUID(ctx, op.GetAPIReader())
+	//
+	// Bound it: this runs before the manager starts, so the health probes are not
+	// listening yet and an apiserver that accepts the connection but never answers
+	// would hang the process where nothing can observe it. The rest config sets no
+	// per-request timeout of its own.
+	uidCtx, cancelUID := context.WithTimeout(ctx, clusterUIDTimeout)
+	clusterUID, err := hetznerop.ClusterUID(uidCtx, op.GetAPIReader())
+	cancelUID()
 	if err != nil {
 		log.FromContext(ctx).Error(err, "failed to read the cluster UID")
 		return
 	}
 
+	// Create the three providers.
 	instanceProvider := instance.NewProviderWithPlacementGroups(&hcloudClient.Server, &hcloudClient.PlacementGroup, cfg.ClusterName, clusterUID, &hcloudClient.Action)
 	typeProvider := instancetype.NewProvider(&hcloudClient.ServerType)
 	imageProvider := imagefamily.NewProvider(&hcloudClient.Image)
@@ -75,23 +89,36 @@ func main() {
 	// Our NodeClass status controller (network + image validation, Ready).
 	nodeClassController := nodeclass.NewController(op.GetClient(), &hcloudClient.Network, &hcloudClient.Firewall, &hcloudClient.SSHKey, imageProvider)
 
+	providerControllers := []controller.Controller{nodeClassController}
 	// Reap servers whose NodeClaim is gone. Karpenter core only garbage collects
 	// the opposite direction (NodeClaims with no instance), so without this an
 	// orphaned server runs and bills indefinitely.
-	providerControllers := []controller.Controller{nodeClassController}
-	if cfg.DisableInstanceGarbageCollection {
+	//
+	// Every mode is logged, not just the unusual ones: this controller deletes
+	// machines, so which mode took effect must be answerable from the operator's
+	// own startup logs rather than inferred from a values file.
+	//
+	// Every mode is named explicitly and `default` refuses to start. Routing the
+	// unknown case to the deleting branch would re-open, one layer down, exactly
+	// the hole parseGCMode exists to close: GCMode's zero value is "", not
+	// "enabled", so any Config built without LoadConfig -- or any mode added to
+	// the parser and forgotten here -- would silently select "delete servers".
+	switch cfg.InstanceGarbageCollectionMode {
+	case hetznerop.GCDisabled:
 		log.FromContext(ctx).Info("instance garbage collection is disabled; " +
 			"servers whose NodeClaim is gone will not be reclaimed")
-	} else {
-		// Log the enabled case too. DISABLE_INSTANCE_GARBAGE_COLLECTION leaves the
-		// sweep running on any value it does not recognise, so a line for one state
-		// only would let a typo'd pause ("disabled", "True!") look identical to a
-		// pause that took effect -- on the one flag whose job is protecting a fleet
-		// during maintenance that removes NodeClaims wholesale.
-		log.FromContext(ctx).Info("instance garbage collection is enabled; " +
-			"servers whose NodeClaim is gone will be reclaimed")
+	case hetznerop.GCObserve, hetznerop.GCEnabled:
+		mode := instancegc.Mode(cfg.InstanceGarbageCollectionMode)
+		log.FromContext(ctx).Info("instance garbage collection is active",
+			"mode", string(mode),
+			"reclaims", mode == instancegc.ModeEnabled)
 		providerControllers = append(providerControllers,
-			instancegc.NewController(op.GetClient(), instanceProvider, cfg.ClusterName, clusterUID))
+			instancegc.NewController(op.GetClient(), instanceProvider,
+				cfg.ClusterName, clusterUID, mode, op.Clock))
+	default:
+		log.FromContext(ctx).Error(nil, "unhandled instance garbage collection mode; refusing to start",
+			"mode", string(cfg.InstanceGarbageCollectionMode))
+		return
 	}
 
 	// Wire and start all controllers.

pkg/apis/v1/labels.go

@@ -34,3 +34,28 @@ const (
 	// label, and is treated as ours so existing fleets stay managed.
 	ServerLabelClusterUID = "karpenter.sh/cluster-uid"
 )
+
+// OwnedByCluster reports whether a server's labels mark it as belonging to the
+// installation identified by clusterName and clusterUID.
+//
+// This is the single definition of ownership. It is consulted from two places
+// that both act destructively on the answer -- the orphan sweep deletes, and
+// adoption hands a live machine to Karpenter, which eventually terminates it --
+// so the rule they apply has to be one rule. The legacy exemption below is the
+// part that must not drift: it is a migration affordance that will be tightened
+// once fleets have rolled, and tightening it in one caller but not the other
+// would either strand every pre-UID orphan or resume cross-cluster deletion.
+//
+// A UID that is present and different belongs to another cluster. A missing UID
+// predates the label and is treated as ours, because refusing those would strand
+// every server created before it existed.
+func OwnedByCluster(labels map[string]string, clusterName, clusterUID string) bool {
+	if labels[ServerLabelManagedBy] != ServerValueManagedBy {
+		return false
+	}
+	if labels[ServerLabelCluster] != clusterName {
+		return false
+	}
+	uid := labels[ServerLabelClusterUID]
+	return uid == "" || uid == clusterUID
+}

pkg/cloudprovider/cloudprovider_test.go

@@ -101,6 +101,17 @@ func (f *fakeServerClient) AllWithOpts(_ context.Context, _ hcloud.ServerListOpt
 	return out, nil
 }
 
+func (f *fakeServerClient) Update(_ context.Context, server *hcloud.Server, opts hcloud.ServerUpdateOpts) (*hcloud.Server, *hcloud.Response, error) {
+	s, ok := f.servers[server.ID]
+	if !ok {
+		return nil, nil, hcloud.Error{Code: hcloud.ErrorCodeNotFound, Message: "not found"}
+	}
+	if opts.Labels != nil {
+		s.Labels = opts.Labels
+	}
+	return s, nil, nil
+}
+
 type fakeServerTypeClient struct{ types []*hcloud.ServerType }
 
 func (f *fakeServerTypeClient) All(_ context.Context) ([]*hcloud.ServerType, error) {