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

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[:])
 }