Add spec.pod: pod-level customization + PodDisruptionBudget on TerdutServer #2

New Issue

2026-10-02T13:26:50Z

niklas commented

2026-10-02 13:26:50 +00:00

Context

TerdutServer currently has no way to customize the pod it creates beyond image/replicas/env derived from named fields. reconcileDeployment (internal/controller/terdutserver_deployment.go) builds the PodTemplateSpec entirely inline, setting only Labels, one init container, and a main container — no Resources, SecurityContext, Affinity, Tolerations, TopologySpreadConstraints, Annotations, ServiceAccountName, ImagePullSecrets, or extra volumes/env.

Original ask: pod annotations, topology spread constraints, and pod (anti-)affinity, for running on real clusters (zone spread, avoiding noisy neighbors, node-pool pinning). Since this operator is headed for a public release, scope was widened past that one deployment's needs by researching Zalando postgres-operator, CloudNativePG, and Crunchy PGO, then narrowed back down with an interview. Full design discussion & research findings are recorded in the originating conversation.

Convention decided: direct corev1 type reuse throughout (corev1.Toleration, corev1.Affinity, corev1.TopologySpreadConstraint, corev1.ResourceRequirements, corev1.SecurityContext, corev1.EnvVar, etc.) — matches how Zalando/CNPG expose these same knobs, and matches this repo's own SweeperSpec doc-comment policy ("wrap only when a round-trip through a different type buys something"). New fields live under a nested spec.pod struct, matching the CRD's existing per-concern grouping (NetworkingSpec, DatabaseSpec, SweeperSpec, ...) rather than flattening onto spec directly.

Explicitly deferred this round (interviewed and declined): priorityClassName, extra pod labels (beyond annotations), HPA.

Scope

New PodSpec type on TerdutServerSpec.Pod:

annotations map[string]string
nodeSelector map[string]string
tolerations []corev1.Toleration
affinity *corev1.Affinity — covers node affinity + pod affinity + pod anti-affinity in one field; pure user-supplied passthrough, no operator-generated default (this operator doesn't support replicas > 1 as a topology)
topologySpreadConstraints []corev1.TopologySpreadConstraint
resources corev1.ResourceRequirements (main container; currently unset entirely — pre-existing gap this closes)
securityContext *corev1.PodSecurityContext (pod-level)
containerSecurityContext *corev1.SecurityContext (main container only, not wait-for-postgres)
serviceAccountName string
extraEnv []corev1.EnvVar, extraEnvFrom []corev1.EnvFromSource (appended after buildEnv()'s fixed vars)
extraVolumes []corev1.Volume, extraVolumeMounts []corev1.VolumeMount (main container only)
imagePullSecrets []corev1.LocalObjectReference
disruptionBudget *PodDisruptionBudgetSpec (new wrapper: minAvailable/maxUnavailable *intstr.IntOrString, mutually exclusive via CEL XValidation, mirroring DatabaseSpec's existing dsn/postgresClusterRef pattern)

Todo

API types (api/v1alpha1/terdutserver_types.go): add PodSpec and PodDisruptionBudgetSpec types, new Pod PodSpec field on TerdutServerSpec (after AllowedTeams); add corev1/intstr imports.
Deployment wiring (internal/controller/terdutserver_deployment.go): wire srv.Spec.Pod.* into the PodTemplateSpec/main-container literal in reconcileDeployment (annotations, nodeSelector, tolerations, affinity, topologySpreadConstraints, securityContext, serviceAccountName, imagePullSecrets, volumes, envFrom, volumeMounts, resources, containerSecurityContext); append ExtraEnv at the end of buildEnv()'s return.
PodDisruptionBudget reconcile (new internal/controller/terdutserver_pdb.go): reconcilePodDisruptionBudget — create/update a policyv1.PodDisruptionBudget (selector = labelsFor(srv), owned via SetControllerReference) when spec.pod.disruptionBudget is set; delete (ignoring NotFound) when cleared, mirroring the delete-idiom already in terdutalertsource_controller.go's rotateKind.
Controller wiring (internal/controller/terdutserver_controller.go): call reconcilePodDisruptionBudget after reconcileService in Reconcile; add Owns(&policyv1.PodDisruptionBudget{}) in SetupWithManager; add RBAC marker +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;create;update;patch;delete.
Regenerate: make manifests generate (CRD schema, RBAC role, deepcopy) — review the diff, specifically confirm the minAvailable/maxUnavailable CEL rule renders correctly on this doubly-nested optional pointer-to-struct field, and that intstr.IntOrString auto-detects to x-kubernetes-int-or-string: true with no extra marker (both unproven paths in this repo today).
Chart (charts/terdut-operator): regenerate via kubebuilder edit --plugins helm.kubebuilder.io/v2-alpha --output-dir charts --force, then re-add by hand a pod: passthrough block in templates/terdutserver/terdutserver.yaml (same idiom as sweeper/notify/oidc) and a documented pod: {} stanza in values.yaml (with resources called out as a named example).
DESIGN.md: add a pod: block to §4.1's canonical example (representative subset: resources, tolerations, disruptionBudget) + prose on the "pure passthrough, no auto-generated affinity" decision; extend §7 (ownership) to list PodDisruptionBudget as the one conditionally-created/deleted child object; extend §9 (RBAC) with the new policy/poddisruptionbudgets permissions; optionally note in §13 that priorityClassName/extra pod labels/HPA were considered and deferred.
Sample (config/samples/terdut_v1alpha1_terdutserver.yaml): optionally add a commented-out # pod: ... example, same style as the existing database alternate-path comment.
Tests (internal/controller/terdutserver_controller_test.go): Describe("spec.pod", ...) asserting resources/tolerations/extraEnv/extraVolumes+Mounts/serviceAccountName land in the right spot on the Deployment (and not on wait-for-postgres); Describe("spec.pod.disruptionBudget", ...) asserting create/owner-ref and delete-on-clear; a case proving the API server rejects both minAvailable+maxUnavailable set together, and neither set.
Verify: make fmt lint test helm-lint (CI gate); manual kind spot-check (apply a TerdutServer with spec.pod.resources/tolerations/affinity/disruptionBudget set, confirm the Deployment and kubectl get pdb reflect it, then clear disruptionBudget and confirm the PDB is deleted) — matches this repo's existing manual-e2e-pass precedent, not a CI job.

Full design writeup (including the Zalando/CloudNativePG/PGO research this was based on) is in the plan file from the planning session: ~/.claude/plans/terdut-operator-s-terdutserver-needs-a-snazzy-teapot.md.

## Context `TerdutServer` currently has no way to customize the pod it creates beyond image/replicas/env derived from named fields. `reconcileDeployment` (`internal/controller/terdutserver_deployment.go`) builds the `PodTemplateSpec` entirely inline, setting only `Labels`, one init container, and a main container — no `Resources`, `SecurityContext`, `Affinity`, `Tolerations`, `TopologySpreadConstraints`, `Annotations`, `ServiceAccountName`, `ImagePullSecrets`, or extra volumes/env. Original ask: pod annotations, topology spread constraints, and pod (anti-)affinity, for running on real clusters (zone spread, avoiding noisy neighbors, node-pool pinning). Since this operator is headed for a public release, scope was widened past that one deployment's needs by researching Zalando postgres-operator, CloudNativePG, and Crunchy PGO, then narrowed back down with an interview. Full design discussion & research findings are recorded in the originating conversation. **Convention decided:** direct corev1 type reuse throughout (`corev1.Toleration`, `corev1.Affinity`, `corev1.TopologySpreadConstraint`, `corev1.ResourceRequirements`, `corev1.SecurityContext`, `corev1.EnvVar`, etc.) — matches how Zalando/CNPG expose these same knobs, and matches this repo's own `SweeperSpec` doc-comment policy ("wrap only when a round-trip through a different type buys something"). New fields live under a nested `spec.pod` struct, matching the CRD's existing per-concern grouping (`NetworkingSpec`, `DatabaseSpec`, `SweeperSpec`, ...) rather than flattening onto `spec` directly. **Explicitly deferred this round** (interviewed and declined): `priorityClassName`, extra pod labels (beyond annotations), HPA. ## Scope New `PodSpec` type on `TerdutServerSpec.Pod`: - `annotations map[string]string` - `nodeSelector map[string]string` - `tolerations []corev1.Toleration` - `affinity *corev1.Affinity` — covers node affinity + pod affinity + pod anti-affinity in one field; pure user-supplied passthrough, no operator-generated default (this operator doesn't support replicas > 1 as a topology) - `topologySpreadConstraints []corev1.TopologySpreadConstraint` - `resources corev1.ResourceRequirements` (main container; currently unset entirely — pre-existing gap this closes) - `securityContext *corev1.PodSecurityContext` (pod-level) - `containerSecurityContext *corev1.SecurityContext` (main container only, not `wait-for-postgres`) - `serviceAccountName string` - `extraEnv []corev1.EnvVar`, `extraEnvFrom []corev1.EnvFromSource` (appended after `buildEnv()`'s fixed vars) - `extraVolumes []corev1.Volume`, `extraVolumeMounts []corev1.VolumeMount` (main container only) - `imagePullSecrets []corev1.LocalObjectReference` - `disruptionBudget *PodDisruptionBudgetSpec` (new wrapper: `minAvailable`/`maxUnavailable *intstr.IntOrString`, mutually exclusive via CEL `XValidation`, mirroring `DatabaseSpec`'s existing `dsn`/`postgresClusterRef` pattern) ## Todo - [ ] **API types** (`api/v1alpha1/terdutserver_types.go`): add `PodSpec` and `PodDisruptionBudgetSpec` types, new `Pod PodSpec` field on `TerdutServerSpec` (after `AllowedTeams`); add `corev1`/`intstr` imports. - [ ] **Deployment wiring** (`internal/controller/terdutserver_deployment.go`): wire `srv.Spec.Pod.*` into the `PodTemplateSpec`/main-container literal in `reconcileDeployment` (annotations, nodeSelector, tolerations, affinity, topologySpreadConstraints, securityContext, serviceAccountName, imagePullSecrets, volumes, envFrom, volumeMounts, resources, containerSecurityContext); append `ExtraEnv` at the end of `buildEnv()`'s return. - [ ] **PodDisruptionBudget reconcile** (new `internal/controller/terdutserver_pdb.go`): `reconcilePodDisruptionBudget` — create/update a `policyv1.PodDisruptionBudget` (selector = `labelsFor(srv)`, owned via `SetControllerReference`) when `spec.pod.disruptionBudget` is set; delete (ignoring `NotFound`) when cleared, mirroring the delete-idiom already in `terdutalertsource_controller.go`'s `rotateKind`. - [ ] **Controller wiring** (`internal/controller/terdutserver_controller.go`): call `reconcilePodDisruptionBudget` after `reconcileService` in `Reconcile`; add `Owns(&policyv1.PodDisruptionBudget{})` in `SetupWithManager`; add RBAC marker `+kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;create;update;patch;delete`. - [ ] **Regenerate**: `make manifests generate` (CRD schema, RBAC role, deepcopy) — review the diff, specifically confirm the `minAvailable`/`maxUnavailable` CEL rule renders correctly on this doubly-nested optional pointer-to-struct field, and that `intstr.IntOrString` auto-detects to `x-kubernetes-int-or-string: true` with no extra marker (both unproven paths in this repo today). - [ ] **Chart** (`charts/terdut-operator`): regenerate via `kubebuilder edit --plugins helm.kubebuilder.io/v2-alpha --output-dir charts --force`, then re-add by hand a `pod:` passthrough block in `templates/terdutserver/terdutserver.yaml` (same idiom as `sweeper`/`notify`/`oidc`) and a documented `pod: {}` stanza in `values.yaml` (with `resources` called out as a named example). - [ ] **DESIGN.md**: add a `pod:` block to §4.1's canonical example (representative subset: `resources`, `tolerations`, `disruptionBudget`) + prose on the "pure passthrough, no auto-generated affinity" decision; extend §7 (ownership) to list PodDisruptionBudget as the one conditionally-created/deleted child object; extend §9 (RBAC) with the new `policy`/`poddisruptionbudgets` permissions; optionally note in §13 that `priorityClassName`/extra pod labels/HPA were considered and deferred. - [ ] **Sample** (`config/samples/terdut_v1alpha1_terdutserver.yaml`): optionally add a commented-out `# pod: ...` example, same style as the existing `database` alternate-path comment. - [ ] **Tests** (`internal/controller/terdutserver_controller_test.go`): `Describe("spec.pod", ...)` asserting resources/tolerations/extraEnv/extraVolumes+Mounts/serviceAccountName land in the right spot on the Deployment (and not on `wait-for-postgres`); `Describe("spec.pod.disruptionBudget", ...)` asserting create/owner-ref and delete-on-clear; a case proving the API server rejects both `minAvailable`+`maxUnavailable` set together, and neither set. - [ ] **Verify**: `make fmt lint test helm-lint` (CI gate); manual `kind` spot-check (apply a `TerdutServer` with `spec.pod.resources`/`tolerations`/`affinity`/`disruptionBudget` set, confirm the Deployment and `kubectl get pdb` reflect it, then clear `disruptionBudget` and confirm the PDB is deleted) — matches this repo's existing manual-e2e-pass precedent, not a CI job. Full design writeup (including the Zalando/CloudNativePG/PGO research this was based on) is in the plan file from the planning session: `~/.claude/plans/terdut-operator-s-terdutserver-needs-a-snazzy-teapot.md`.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: niklas/terdut-operator#2