fix(teuto-cnpg/updateMethod): set update method to switchover, set env-vars to make it compatible with newer barman #1572
Conversation
…v-vars to make it compatible with newer barman
WalkthroughThe cluster configuration was modified to add two AWS-related environment variables and a new field specifying the primary update method as "switchover". No other fields or logic were changed in the cluster specification. Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/teuto-cnpg/templates/cluster.yaml (1)
8-12: Hard-coded env vars reduce chart flexibilityEmbedding the checksum variables directly in the template prevents users from overriding or disabling them via
values.yaml.Recommend parameterising:
- podTemplate: - env: - - name: AWS_REQUEST_CHECKSUM_CALCULATION - value: when_required - - name: AWS_RESPONSE_CHECKSUM_VALIDATION - value: when_required + podTemplate: + env: +{{- with .Values.extraEnv }} +{{ toYaml . | indent 4 }} +{{- end }}and documenting
extraEnvinvalues.yaml.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/teuto-cnpg/templates/cluster.yaml(1 hunks)
🔇 Additional comments (2)
charts/teuto-cnpg/templates/cluster.yaml (2)
15-15:primaryUpdateMethod: switchoverrequires CNPG ≥ 1.20The
primaryUpdateMethodknob was introduced fairly recently. Ensure every cluster that will consume this chart is running a CloudNative-PG operator version that supports it; otherwise the apply will fail.If you still support older operator versions, consider guarding this field behind a
Valuesflag:{{- if .Values.enablePrimarySwitchover }} primaryUpdateMethod: switchover {{- end }}
8-12: Verifyspec.envsupport in CloudNative-PG Cluster CRDFile: charts/teuto-cnpg/templates/cluster.yaml (lines 8–12)
The Cluster CRD for CloudNative-PG does not expose a top-level
spec.env; environment variables belong underspec.podTemplate. Ifspec.envis unrecognized, applying this chart will fail with:unknown field "env" in io.cnpg.postgresql.v1.Cluster.specSuggested change:
spec: - env: - - name: AWS_REQUEST_CHECKSUM_CALCULATION - value: when_required - - name: AWS_RESPONSE_CHECKSUM_VALIDATION - value: when_required + podTemplate: + env: + - name: AWS_REQUEST_CHECKSUM_CALCULATION + value: when_required + - name: AWS_RESPONSE_CHECKSUM_VALIDATION + value: when_requiredSteps to verify:
- Inspect your CRD schema (e.g. check
crds/in the chart or runkubectl get crd clusters.cnpg.io -o yaml).- Run a server-side dry-run:
helm template charts/teuto-cnpg | kubectl apply --dry-run=server -f -to confirm the manifest is accepted by the API server.
🤖 I have diffed this beep boop"/$namespace/$kind/$name.yaml" for normal resources
|
|
For reference: cloudnative-pg/cloudnative-pg#7715 |
marvinWolff
changed the title
|
@marvinWolff can this be merged now? |
Summary by CodeRabbit