fix(bootstrap): make bootstrap Job reconcile idempotent (no immutable-template churn)

The admin bootstrap Job (spec.auth.adminUser) could have its running pod
killed ~1s after start by the Job controller itself (SuccessfulDelete
immediately after "Started container bootstrap"), then BackoffLimitExceeded,
leaving bootstrapStatus at bootstrap_pending (issue #83).

A Kubernetes Job's pod template is immutable after creation, so any reconcile
that re-renders or patches spec.template makes the Job controller churn and can
delete the active pod. Separately, the Job was built with explicit pod-template
labels but no explicit selector, so the Job controller could adopt a
leftover/orphaned pod from a previous bootstrap Job of the same name (e.g.
after a manual `kubectl delete job` + operator recreate) and then reap it.

This makes the bootstrap Job lifecycle deterministic and non-churning:

- BuildBootstrapJob now sets an explicit, unique spec.selector +
  manualSelector=true keyed to the Job name (plus a matching per-Job pod
  label), so this Job's pods can never be adopted from a prior Job.
- BuildBootstrapJob stamps a content-hash annotation over the
  operator-controlled inputs (image, script, admin email, password secret ref).
- reconcileBootstrapJob is strictly create-if-absent. If the Job exists and the
  hash matches, it is left entirely untouched (spec.template is never patched).
  Only when the hash differs is the stale Job deleted (foreground propagation)
  and recreated on a later reconcile -- never an illegal in-place template
  update.

Adds an envtest regression test that reconciles the bootstrap Job N times and
asserts the Job is neither updated (same resourceVersion) nor recreated (same
UID), plus a test that a config change triggers delete+recreate.

Closes #83

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: stubbi

internal/controller/instance_controller.go

@@ -1279,22 +1279,52 @@ func (r *InstanceReconciler) reconcileBootstrapJob(ctx context.Context, instance
 		return nil
 	}
 
-	// Check if Job already exists (it should only run once)
+	// A Job's pod template is immutable after creation, so this reconcile is
+	// strictly create-if-absent and never patches an existing Job in place.
+	// Patching spec.template (even a no-op re-render) makes the Job controller
+	// churn and can delete the running bootstrap pod, tripping
+	// BackoffLimitExceeded (issue #83). If the operator-controlled inputs change
+	// we delete and recreate the Job instead, gated on a content-hash so a
+	// steady-state reconcile is a no-op.
 	existing := &batchv1.Job{}
 	err := r.Get(ctx, types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, existing)
 	if err == nil {
-		// Job already exists, nothing to do
+		// Job already exists. If its content hash matches the desired spec,
+		// leave it completely alone (do NOT touch spec.template).
+		if existing.Annotations[resources.BootstrapHashAnnotation] ==
+			desired.Annotations[resources.BootstrapHashAnnotation] {
+			return nil
+		}
+
+		// The bootstrap config changed. A Job's template cannot be updated, so
+		// delete the stale Job (foreground propagation so its pods are removed
+		// before we recreate) and recreate on a later reconcile. We do not
+		// recreate in the same pass to avoid racing the API server's
+		// asynchronous cascade delete.
+		logf.FromContext(ctx).Info("bootstrap Job content changed; replacing",
+			"job", desired.Name)
+		propagation := metav1.DeletePropagationForeground
+		if delErr := r.Delete(ctx, existing, &client.DeleteOptions{
+			PropagationPolicy: &propagation,
+		}); delErr != nil && !apierrors.IsNotFound(delErr) {
+			return fmt.Errorf("deleting stale bootstrap Job: %w", delErr)
+		}
 		return nil
 	}
 	if !apierrors.IsNotFound(err) {
 		return fmt.Errorf("checking bootstrap Job: %w", err)
 	}
 
-	// Job does not exist, create it
+	// Job does not exist, create it.
 	if err := controllerutil.SetControllerReference(instance, desired, r.Scheme); err != nil {
 		return fmt.Errorf("setting owner reference on bootstrap Job: %w", err)
 	}
 	if err := r.Create(ctx, desired); err != nil { // reconcile-guard:allow
+		if apierrors.IsAlreadyExists(err) {
+			// Lost a create race with the just-deleted Job's cascade; the next
+			// reconcile will converge.
+			return nil
+		}
 		return fmt.Errorf("creating bootstrap Job: %w", err)
 	}
 

internal/controller/instance_controller_test.go

@@ -26,6 +26,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
+	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -662,4 +663,117 @@ var _ = Describe("Instance Controller", func() {
 			Expect(meta.FindStatusCondition(updated.Status.Conditions, ConditionSchedulerGatingValid)).To(BeNil())
 		})
 	})
+
+	// Regression for issue #83: the bootstrap Job (spec.auth.adminUser) must be
+	// reconciled idempotently. A Job's pod template is immutable, so re-rendering
+	// or patching it on every reconcile makes the Job controller churn and kill
+	// the running bootstrap pod (SuccessfulDelete ~1s after start ->
+	// BackoffLimitExceeded). Steady-state reconciles must leave the Job entirely
+	// untouched; only a real config change may replace it (delete + recreate).
+	Context("When reconciling the admin bootstrap Job", func() {
+		ctx := context.Background()
+
+		newBootstrapInstance := func(name, email string) *paperclipv1alpha1.Instance {
+			return &paperclipv1alpha1.Instance{
+				ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"},
+				Spec: paperclipv1alpha1.InstanceSpec{
+					Image: paperclipv1alpha1.ImageSpec{Tag: "v1.0.0"},
+					Auth: paperclipv1alpha1.AuthSpec{
+						AdminUser: &paperclipv1alpha1.AdminUserSpec{
+							Email: email,
+							PasswordSecretRef: corev1.SecretKeySelector{
+								LocalObjectReference: corev1.LocalObjectReference{Name: "admin-secret"},
+								Key:                  "password",
+							},
+						},
+					},
+				},
+			}
+		}
+
+		It("does not update or recreate the Job across repeated reconciles", func() {
+			const bootName = "bootstrap-idem"
+			nn := types.NamespacedName{Name: bootName, Namespace: "default"}
+			jobNN := types.NamespacedName{Name: bootName + "-bootstrap", Namespace: "default"}
+
+			Expect(k8sClient.Create(ctx, newBootstrapInstance(bootName, "admin@test.com"))).To(Succeed())
+			DeferCleanup(func() {
+				resource := &paperclipv1alpha1.Instance{}
+				if err := k8sClient.Get(ctx, nn, resource); err == nil {
+					_ = k8sClient.Delete(ctx, resource)
+				}
+			})
+			r := &InstanceReconciler{Client: k8sClient, Scheme: k8sClient.Scheme()}
+
+			By("reconciling twice so the finalizer requeue passes and the Job is created")
+			reconcileN(ctx, r, nn, 2)
+
+			job := &batchv1.Job{}
+			Expect(k8sClient.Get(ctx, jobNN, job)).To(Succeed())
+			originalUID := job.UID
+			originalRV := job.ResourceVersion
+			Expect(job.Annotations).To(HaveKey(resources.BootstrapHashAnnotation))
+
+			By("asserting the Job has an explicit, unique selector so its pods cannot be adopted")
+			Expect(job.Spec.ManualSelector).NotTo(BeNil())
+			Expect(*job.Spec.ManualSelector).To(BeTrue())
+			Expect(job.Spec.Selector).NotTo(BeNil())
+			Expect(job.Spec.Selector.MatchLabels).To(HaveKeyWithValue(resources.BootstrapJobLabel, jobNN.Name))
+			Expect(job.Spec.Template.Labels).To(HaveKeyWithValue(resources.BootstrapJobLabel, jobNN.Name))
+
+			By("reconciling several more times")
+			reconcileN(ctx, r, nn, 5)
+
+			By("verifying the Job was neither updated (same resourceVersion) nor recreated (same UID)")
+			after := &batchv1.Job{}
+			Expect(k8sClient.Get(ctx, jobNN, after)).To(Succeed())
+			Expect(after.UID).To(Equal(originalUID), "bootstrap Job was recreated on a steady-state reconcile")
+			Expect(after.ResourceVersion).To(Equal(originalRV), "bootstrap Job spec was mutated on a steady-state reconcile")
+		})
+
+		It("replaces the Job (delete + recreate) only when the bootstrap config changes", func() {
+			const bootName = "bootstrap-replace"
+			nn := types.NamespacedName{Name: bootName, Namespace: "default"}
+			jobNN := types.NamespacedName{Name: bootName + "-bootstrap", Namespace: "default"}
+
+			Expect(k8sClient.Create(ctx, newBootstrapInstance(bootName, "admin@test.com"))).To(Succeed())
+			DeferCleanup(func() {
+				resource := &paperclipv1alpha1.Instance{}
+				if err := k8sClient.Get(ctx, nn, resource); err == nil {
+					_ = k8sClient.Delete(ctx, resource)
+				}
+			})
+			r := &InstanceReconciler{Client: k8sClient, Scheme: k8sClient.Scheme()}
+			reconcileN(ctx, r, nn, 2)
+
+			job := &batchv1.Job{}
+			Expect(k8sClient.Get(ctx, jobNN, job)).To(Succeed())
+			originalUID := job.UID
+
+			By("changing the admin email, which changes the bootstrap content hash")
+			updated := &paperclipv1alpha1.Instance{}
+			Expect(k8sClient.Get(ctx, nn, updated)).To(Succeed())
+			updated.Spec.Auth.AdminUser.Email = "different@test.com"
+			Expect(k8sClient.Update(ctx, updated)).To(Succeed())
+
+			By("the next reconcile deletes the stale Job (template is immutable, cannot patch)")
+			reconcileN(ctx, r, nn, 1)
+			// Foreground deletion may leave the object briefly with a deletion
+			// timestamp; remove any finalizers the envtest GC won't process.
+			stale := &batchv1.Job{}
+			if err := k8sClient.Get(ctx, jobNN, stale); err == nil && stale.DeletionTimestamp != nil {
+				stale.Finalizers = nil
+				_ = k8sClient.Update(ctx, stale)
+			}
+			Eventually(func() bool {
+				return errors.IsNotFound(k8sClient.Get(ctx, jobNN, &batchv1.Job{}))
+			}).Should(BeTrue(), "stale bootstrap Job should be deleted")
+
+			By("a subsequent reconcile recreates the Job with a new UID")
+			reconcileN(ctx, r, nn, 1)
+			recreated := &batchv1.Job{}
+			Expect(k8sClient.Get(ctx, jobNN, recreated)).To(Succeed())
+			Expect(recreated.UID).NotTo(Equal(originalUID), "Job should have been recreated, not patched in place")
+		})
+	})
 })

internal/resources/bootstrap.go

@@ -1,6 +1,8 @@
 package resources
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 	"strings"
 
@@ -11,6 +13,20 @@ import (
 	paperclipv1alpha1 "github.com/paperclipinc/paperclip-operator/api/v1alpha1"
 )
 
+// BootstrapHashAnnotation records a content hash of the operator-controlled
+// inputs to the bootstrap Job. Because a Job's pod template is immutable after
+// creation, the reconciler never patches an existing Job in place; instead it
+// compares this annotation and, only when it differs, deletes and recreates the
+// Job. A steady-state reconcile sees an unchanged hash and is a no-op.
+const BootstrapHashAnnotation = "paperclip.ai/bootstrap-spec-hash"
+
+// BootstrapJobLabel is a per-Job unique pod label. It is part of the Job's
+// explicit selector (manualSelector) so the Job controller can never adopt a
+// pod left over from a previous bootstrap Job of the same name. Adopting a
+// stale/orphaned pod is what made the Job controller reap its own pod ~1s after
+// start and then report BackoffLimitExceeded (issue #83).
+const BootstrapJobLabel = "paperclip.ai/bootstrap-job"
+
 // sanitizeJSONString escapes special characters for safe embedding in a JSON string literal
 // inside a shell script. Prevents JSON injection via user-controlled CRD fields.
 func sanitizeJSONString(s string) string {
@@ -156,18 +172,34 @@ echo "Admin bootstrap finished successfully."
 	backoffLimit := int32(3)
 	ttl := int32(3600) // Clean up completed job after 1 hour
 
-	return &batchv1.Job{
+	jobName := BootstrapJobName(instance)
+
+	// Pod template labels: the standard component labels plus a per-Job unique
+	// label. The unique label is also the Job's explicit selector
+	// (manualSelector=true) so this Job's pods can never be confused with — or
+	// adopted from — a previous bootstrap Job of the same name. Without an
+	// explicit, unique selector the Job controller could adopt a leftover/
+	// orphaned pod and then delete it, killing the running bootstrap pod ~1s
+	// after start and tripping BackoffLimitExceeded (issue #83).
+	podLabels := LabelsWithComponent(instance, "bootstrap")
+	podLabels[BootstrapJobLabel] = jobName
+
+	job := &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      BootstrapJobName(instance),
+			Name:      jobName,
 			Namespace: instance.Namespace,
 			Labels:    LabelsWithComponent(instance, "bootstrap"),
 		},
 		Spec: batchv1.JobSpec{
 			BackoffLimit:            &backoffLimit,
 			TTLSecondsAfterFinished: &ttl,
+			ManualSelector:          Ptr(true),
+			Selector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{BootstrapJobLabel: jobName},
+			},
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: LabelsWithComponent(instance, "bootstrap"),
+					Labels: podLabels,
 				},
 				Spec: corev1.PodSpec{
 					RestartPolicy:                corev1.RestartPolicyOnFailure,
@@ -210,4 +242,30 @@ echo "Admin bootstrap finished successfully."
 			},
 		},
 	}
+
+	// Stamp a content hash of the operator-controlled inputs. The reconciler
+	// uses this to decide whether an existing Job is up to date (no-op) or must
+	// be replaced (delete + recreate); it never patches the immutable template
+	// in place.
+	if job.Annotations == nil {
+		job.Annotations = map[string]string{}
+	}
+	job.Annotations[BootstrapHashAnnotation] = bootstrapSpecHash(image, script, admin)
+
+	return job
+}
+
+// bootstrapSpecHash returns a stable hash over the operator-controlled inputs to
+// the bootstrap Job. It intentionally excludes server-defaulted/mutable fields
+// so a steady-state reconcile produces an identical hash and is a no-op.
+func bootstrapSpecHash(image, script string, admin *paperclipv1alpha1.AdminUserSpec) string {
+	// The script already embeds the resolved admin name, service/base URLs and
+	// the JSON sign-up payload, so it captures most config drift. Include the
+	// image, admin email and the password secret reference explicitly so a
+	// change to credentials forces a fresh Job.
+	payload := fmt.Sprintf("image=%s\x00script=%s\x00email=%s\x00secret=%s/%s\x00",
+		image, script, admin.Email,
+		admin.PasswordSecretRef.Name, admin.PasswordSecretRef.Key)
+	sum := sha256.Sum256([]byte(payload))
+	return hex.EncodeToString(sum[:])
 }