Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#187 Check Capabilities to render PodDisruptionBudget manifest #188

Merged
merged 5 commits into from
Dec 1, 2022

Conversation

vovtz
Copy link
Contributor

@vovtz vovtz commented Oct 24, 2022

Fixes #187

@joejulian
Copy link
Contributor

The PDB is critical to prevent outages, split brain, and data loss. I think we should just prevent the chart from being used on k8s < 1.21 (which was merged before I saw this issue/PR). What do you think?

@vovtz
Copy link
Contributor Author

vovtz commented Oct 27, 2022

For our current use case there is no HA requirement, we plan to use Redpanda only to create ephemeral one-node ‘clusters’ that are used in component testing. In future we might decide to use Redpanda in production too, but for now we use Kafka.

(BTW, we obviously shouldn’t run K8s versions that are EOL anyway - we urgently requested to upgrade.)

@@ -35,3 +36,4 @@ spec:
app.kubernetes.io/name: {{ template "redpanda.name" . }}
app.kubernetes.io/instance: {{ .Release.Name | quote }}
maxUnavailable: {{ .Values.statefulset.budget.maxUnavailable | int64 }}
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For our current use case there is no HA requirement, we plan to use Redpanda only to create ephemeral one-node ‘clusters’ that are used in component testing.

In that case, we should probably add a value under "below here be dragons" with a big warning about not using in production, then use that new value in an else, here.

something like:

yesyesyesno:
  pod_disruption_budget:
    enabled: true
Suggested change
{{- end }}
{{- else }}
{{- if .Values.yesyesyesno.pod_disruption_budget.enabled }}
fail "You must use Kubernetes 1.21+ for a production cluster. If you're not using this in production and you know why doing this is a bad idea, you can set yesyesyesno.pod_disruption_budget.enabled=false to continue."
{{- end }
{{- end }}

(The parent key is not what I suggest, it's just a humorous placeholder)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I committed largely what you propose above, including an attempt for a more serious key name 😄

Copy link
Contributor

@joejulian joejulian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. This seems like a good compromise.

@joejulian
Copy link
Contributor

rebased

@joejulian
Copy link
Contributor

Will rebase and merge after #202

@joejulian joejulian merged commit d50fa93 into redpanda-data:main Dec 1, 2022
@vovtz vovtz deleted the patch-1 branch December 5, 2022 17:41
RafalKorepta pushed a commit to redpanda-data/redpanda-operator that referenced this pull request Dec 3, 2024
redpanda-data/helm-charts#187 Check `Capabilities` to render `PodDisruptionBudget` manifest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PodDisruptionBudget resource should be optional - it now prevents deployment on K8s 1.20 and below
2 participants