Fix deadlock with OrderedReady StatefulSets

README.md

@@ -188,6 +188,18 @@ You can limit Wave to only watch certain namespaces:
 --namespaces=your-namespace,other-namespace
 ```
 
+#### Gated Pod Deletion
+
+When the [webhooks](#webhooks) are enabled, Wave deletes the Pods of a
+StatefulSet that are stuck with its placeholder scheduler once the required
+ConfigMaps/Secrets appear. To keep those Pods instead, set:
+
+```
+--disable-gated-pod-deletion=true
+```
+
+The StatefulSet then stays in `Pending` until the Pods are deleted manually.
+
 ## Quick Start
 
 If you haven't yet got Wave running on your cluster, see
@@ -317,6 +329,17 @@ Pods will stay in state `Pending` instead of `ContainerCreating`.
 When required Secrets/ConfigMaps have been created Wave will restore the
 scheduler and add the config hash without requiring any restarts.
 
+A Pod's `spec.schedulerName` is immutable, so a Pod that was already created
+while scheduling was disabled stays `Pending` forever. Deployments and
+DaemonSets replace such Pods themselves, but a StatefulSet using
+`podManagementPolicy: OrderedReady` waits for them to become ready and never
+rolls out the restored pod template. Wave therefore deletes them once scheduling
+is re-enabled, so that the StatefulSet controller recreates them from the
+current revision. Only Pods that are owned by the StatefulSet, still use Wave's
+placeholder scheduler and have not been scheduled to a node are deleted, and
+Wave emits a `GatedPodDeleted` event for each of them. This can be turned off
+with [`--disable-gated-pod-deletion`](#gated-pod-deletion).
+
 ## Communication
 
 - Found a bug? Please open an issue.

charts/wave/templates/clusterrole.yaml

@@ -25,6 +25,14 @@ rules:
       - create
       - update
       - patch
+  - apiGroups:
+      - ""
+    resources:
+      - pods
+    verbs:
+      - list
+      - watch
+      - delete
   - apiGroups:
       - apps
     resources:

charts/wave/templates/deployment.yaml

@@ -44,6 +44,9 @@ spec:
           {{- if .Values.webhooks.enabled }}
             - --enable-webhooks=true
           {{- end }}
+          {{- if .Values.disableGatedPodDeletion }}
+            - --disable-gated-pod-deletion=true
+          {{- end }}
           volumeMounts:
           {{- if .Values.webhooks.enabled }}
             - mountPath: /tmp/k8s-webhook-server/serving-certs

charts/wave/values.yaml

@@ -50,6 +50,10 @@ serviceAccount:
 webhooks:
   enabled: false
 
+# Keep pods of a StatefulSet which are stuck with the placeholder scheduler that
+# the webhooks use to hold back pods with missing ConfigMaps/Secrets
+disableGatedPodDeletion: false
+
 # Period for reconciliation
 # syncPeriod: 5m
 

cmd/manager/main.go

@@ -52,6 +52,7 @@ var (
 	showVersion             = flag.Bool("version", false, "Show version and exit")
 	enableWebhooks          = flag.Bool("enable-webhooks", false, "Enable webhooks")
 	namespaces              = flag.String("namespaces", "", "Comma-separated list of namespaces to watch. Defaults to all namespaces.")
+	disableGatedPodDeletion = flag.Bool("disable-gated-pod-deletion", false, "Do not delete pods of a StatefulSet which are stuck with the placeholder scheduler after scheduling has been re-enabled")
 	setupLog                = ctrl.Log.WithName("setup")
 )
 
@@ -112,8 +113,9 @@ func main() {
 	// Setup all Controllers
 	setupLog.Info("Setting up controller")
 	controllerConfig := controller.Config{
-		UpdateRate:  *updateRate,
-		UpdateBurst: *updateBurst,
+		UpdateRate:              *updateRate,
+		UpdateBurst:             *updateBurst,
+		DisableGatedPodDeletion: *disableGatedPodDeletion,
 	}
 	if err := controller.AddToManager(mgr, controllerConfig); err != nil {
 		setupLog.Error(err, "unable to register controllers to the manager")

config/default/rbac/rbac_role.yaml

@@ -155,3 +155,11 @@ rules:
   - update
   - patch
   - delete
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - list
+  - watch
+  - delete

config/rbac/role.yaml

@@ -19,6 +19,12 @@ rules:
   - create
   - patch
   - update
+- resources:
+  - pods
+  verbs:
+  - delete
+  - list
+  - watch
 - apiGroups:
   - apps
   resources:

hack/run-test-in-minikube.sh

@@ -138,5 +138,61 @@ while ! kubectl get cm test-completed; do
   fi
 done
 
+# Without the webhooks scheduling is never disabled, so there is nothing to recover from
+if [ "$1" = "production" ]; then
+	echo Creating a StatefulSet whose Secret does not exist yet...
+	kubectl create -f - <<'EOF'
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: test-sts
+  annotations:
+    wave.pusher.com/update-on-config-change: "true"
+spec:
+  serviceName: test-sts
+  podManagementPolicy: OrderedReady
+  replicas: 1
+  selector:
+    matchLabels:
+      app: test-sts
+  template:
+    metadata:
+      labels:
+        app: test-sts
+    spec:
+      containers:
+      - name: test
+        image: nixery.dev/shell/kubectl
+        command: ["/bin/sh", "-ec", "sleep infinity"]
+        volumeMounts:
+        - name: secret
+          mountPath: /etc/secret
+      volumes:
+      - name: secret
+        secret:
+          secretName: test-sts
+EOF
+
+	# The webhook holds the pod back with an invalid scheduler. Its
+	# spec.schedulerName is immutable, so wave has to delete the pod once the
+	# Secret exists, otherwise the StatefulSet never becomes ready.
+	kubectl wait --for=create pod/test-sts-0 --timeout=60s
+	kubectl create secret generic test-sts --from-literal=test=init
+
+	ctr=0
+	while [ "$(kubectl get statefulset test-sts -o jsonpath='{.status.readyReplicas}')" != "1" ]; do
+	  echo Waiting for the StatefulSet to become ready
+	  sleep 10
+	  ctr=$((ctr+1))
+	  if [ "$ctr" -gt 30 ]; then
+		echo "StatefulSet did not recover after its Secret was created"
+		kubectl get pods -o wide
+		kubectl get pod test-sts-0 -o jsonpath='{.spec.schedulerName}'
+		kubectl describe statefulset test-sts
+		exit 1
+	  fi
+	done
+fi
+
 echo Test passed
 exit 0

pkg/controller/add_statefulset.go

@@ -24,6 +24,6 @@ import (
 func init() {
 	// AddToManagerFuncs is a list of functions to create controllers and add them to a manager.
 	AddToManagerFuncs = append(AddToManagerFuncs, func(mgr manager.Manager, cfg Config) error {
-		return statefulset.Add(mgr, cfg.UpdateRate, cfg.UpdateBurst)
+		return statefulset.Add(mgr, cfg.UpdateRate, cfg.UpdateBurst, cfg.DisableGatedPodDeletion)
 	})
 }

pkg/controller/controller.go

@@ -24,6 +24,10 @@ import (
 type Config struct {
 	UpdateRate  float64 // updates per second
 	UpdateBurst int     // maximum burst size
+
+	// DisableGatedPodDeletion keeps Pods which are stuck with Wave's placeholder
+	// scheduler instead of deleting them
+	DisableGatedPodDeletion bool
 }
 
 // AddToManagerFuncs is a list of functions to add all Controllers to the Manager

pkg/controller/statefulset/statefulset_controller.go

@@ -29,20 +29,23 @@ import (
 // +kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch;update;patch
 // +kubebuilder:rbac:groups=,resources=configmaps,verbs=get;list;watch;update;patch
 // +kubebuilder:rbac:groups=,resources=secrets,verbs=get;list;watch;update;patch
+// +kubebuilder:rbac:groups=,resources=pods,verbs=list;watch;delete
 // +kubebuilder:rbac:groups=,resources=events,verbs=create;update;patch
 
 // Add creates a new StatefulSet Controller and adds it to the Manager with default RBAC. The Manager will set fields on the Controller
 // and Start it when the Manager is Started.
-func Add(mgr manager.Manager, updateRate float64, updateBurst int) error {
-	r := newReconciler(mgr, updateRate, updateBurst)
+func Add(mgr manager.Manager, updateRate float64, updateBurst int, disableGatedPodDeletion bool) error {
+	r := newReconciler(mgr, updateRate, updateBurst, disableGatedPodDeletion)
 	return add(mgr, r, r.handler)
 }
 
 // newReconciler returns a new reconcile.Reconciler
-func newReconciler(mgr manager.Manager, updateRate float64, updateBurst int) *ReconcileStatefulSet {
+func newReconciler(mgr manager.Manager, updateRate float64, updateBurst int, disableGatedPodDeletion bool) *ReconcileStatefulSet {
+	handler := core.NewHandler[*appsv1.StatefulSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave"), updateRate, updateBurst)
+	handler.DisableGatedPodDeletion = disableGatedPodDeletion
 	return &ReconcileStatefulSet{
 		scheme:  mgr.GetScheme(),
-		handler: core.NewHandler[*appsv1.StatefulSet](mgr.GetClient(), mgr.GetEventRecorderFor("wave"), updateRate, updateBurst),
+		handler: handler,
 	}
 }
 

pkg/controller/statefulset/statefulset_controller_suite_test.go

@@ -137,7 +137,7 @@ var _ = BeforeSuite(func() {
 	m = utils.Matcher{Client: c}
 
 	var recFn reconcile.Reconciler
-	r := newReconciler(mgr, math.Inf(1), 1)
+	r := newReconciler(mgr, math.Inf(1), 1, false)
 	recFn, requestsStart, requests = core.SetupControllerTestReconcile(r)
 	Expect(add(mgr, recFn, r.handler)).NotTo(HaveOccurred())