Skip to content

Tighten persistence.deploymentStrategy guard to require existingClaim, not persistence.enabled #6862

Description

@eamonnmoloney

Description

The persistence.deploymentStrategy guard admits a combination that can only ever hurt the user: Recreate together with chart-managed (ephemeral) persistence.

Every component that exposes the knob validates it like this:

{{- if and (eq $deploymentStrategy "Recreate") (not .Values.<component>.persistence.enabled) -}}
{{- fail "... Recreate requires <component>.persistence.enabled: true ..." -}}
{{- end -}}

That check was written when chart-managed persistence meant a single shared PersistentVolumeClaim, where Recreate was the mitigation for Multi-Attach deadlocks during helm upgrade (ADR 0092).

Since the chart-managed path moved to a per-pod generic ephemeral volume, that premise no longer holds. Each pod now gets its own PVC, so a surge pod never contends with the outgoing pod and RollingUpdate is always safe. The only remaining case where Recreate earns its downtime is existingClaim, where the chart does not control the access mode and the user-supplied volume may be ReadWriteOnce.

So the guard keys on the wrong field. It should key on existingClaim, not persistence.enabled.

Expected vs Actual Behavior

Given:

connectors:
  persistence:
    enabled: true
    deploymentStrategy: Recreate   # no existingClaim

Actual: renders successfully. The Deployment gets strategy.type: Recreate, so every helm upgrade tears the pod down before starting its replacement — downtime bought for nothing, since the volume is already per-pod.

Expected: the render fails with a message explaining that Recreate is only meaningful alongside existingClaim.

Motivation / Use Case

This is a silent, self-inflicted availability regression. Nothing warns the user; the manifest renders cleanly and the cost only shows up as unexplained downtime on every subsequent upgrade. The knob's own @param documentation already tells users the correct rule — the template just doesn't enforce it:

Set to "Recreate" only when using existingClaim with a ReadWriteOnce volume that cannot tolerate concurrent attach during a rollout (introduces brief downtime per upgrade).

Tightening the guard makes the template agree with its documentation.

Acceptance Criteria

  • Recreate + existingClaim renders successfully (unchanged).
  • Recreate without existingClaim fails the render with a message naming existingClaim as the precondition.
  • RollingUpdate (the default) is unaffected in every combination.
  • Applied consistently across all components exposing the knob, so no behavioural split is introduced.
  • Unit tests cover the newly-rejected combination for each component.
  • @param wording in values.yaml updated to state the existingClaim precondition as a requirement rather than a recommendation.
  • ADR 0092 amended to record that the guard condition changed once the chart-managed path became ephemeral.

Additional Context

Affected surface

Charts and components currently carrying the guard:

Chart Component Chart-managed path Tighten?
8.8 Web Modeler restapi ephemeral yes
8.9 Web Modeler restapi shared PVC no — see below
8.10 Web Modeler restapi ephemeral yes
8.10 Connectors ephemeral yes, once #6522 merges
8.10 Identity ephemeral yes, once #6522 merges

Optimize is out of scope: it hardcodes Recreate and does not expose the knob.

Precondition: do not tighten 8.9 yet

Chart 8.9 must be excluded until #6027 merges. Its Web Modeler restapi still mounts a shared chart-managed PVC:

charts/camunda-platform-8.9/templates/web-modeler/deployment-restapi.yaml
197:          persistentVolumeClaim:      # existingClaim
200:          persistentVolumeClaim:      # chart-managed — still shared

On that shared-PVC path Recreate + persistence.enabled is genuinely load-bearing — it is exactly the ADR 0092 Multi-Attach mitigation. Tightening 8.9 before the ephemeral change lands would remove a working mitigation and reintroduce the upgrade deadlock. Either sequence this after #6027, or scope the change to 8.8/8.10 and follow up.

Why this wasn't fixed in #6522

Raised during review of #6522, which converts Connectors and Identity /tmp to per-pod ephemeral volumes in 8.10. Deliberately left out of scope: that PR would have had to either tighten only its own two components — creating exactly the kind of cross-component behavioural split ADR 0092 set out to close — or edit Web Modeler across three charts, well outside its stated scope. Filing separately so the change lands uniformly.

References

Environment

  • Chart version: 8.8, 8.9, 8.10
  • Platform: any — this is a render-time validation issue, not runtime or platform specific

Metadata

Metadata

Assignees

No one assigned

    Labels

    cleanupRelating to code cleanup, resolving technical debt, or process cleanupkind/bugSomething isn't working as intendedkind/internalGeneral internal work and improvements (not customer facing)likelihood/lowObserved rarely / rather unlikely edge-caseseverity/midMarks a bug as having a noticeable impact but with a known workaroundsupportMarks an issue as related to a customer support request (don't edit the lable)triage:completed

    Type

    No type

    Fields

    Urgency

    someday

    Projects

    Status
    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions