-
Notifications
You must be signed in to change notification settings - Fork 478
Update CRDB operator examples and instructions for deprecated fields #21543
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify project configuration. |
| Specify labels in `cockroachdb.crdbCluster.podTemplate.spec.podLabels` and annotations in `cockroachdb.crdbCluster.podTemplate.spec.podAnnotations` in the values file used to [deploy the cluster]({% link {{ page.version.version }}/deploy-cockroachdb-with-cockroachdb-operator.md %}#initialize-the-cluster): | ||
|
|
||
| ~~~ yaml | ||
| cockroachdb: | ||
| crdbCluster: | ||
| podLabels: | ||
| app.kubernetes.io/version: v25.1.4 | ||
| podAnnotations | ||
| operator: https://raw.githubusercontent.com/cockroachdb/helm-charts/refs/heads/master/cockroachdb-parent/charts/cockroachdb/values.yaml | ||
| podTemplate: | ||
| spec: | ||
| podLabels: | ||
| app.kubernetes.io/version: v25.1.4 | ||
| podAnnotations | ||
| operator: https://raw.githubusercontent.com/cockroachdb/helm-charts/refs/heads/master/cockroachdb-parent/charts/cockroachdb/values.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pritesh-lahoti @NishanthNalluri I noticed that the values.yaml file doesn't have these fields included, even commented out. Is that something we ought to add for completeness? Or otherwise is there a reason we excluded pod labels/annotations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good callout, @jhlodin . The labels and annotations are now part of podTemplate.metadata.labels and podTemplate.metadata.annotations respectively.
We would need to update this example accordingly. Also, the sample values are slightly off, so we could update as below:
cockroachdb:
crdbCluster:
podTemplate:
metadata:
labels:
app.kubernetes.io/name: "cockroachdb"
annotations:
kubernetes.io/description: "This is a CockroachDB pod."
| - The `values.yaml` configuration maps to a subset of `cockroach start` flags when CockroachDB is initialized. | ||
|
|
||
| This page describes configuration options that allow advanced users to manually override the pod template and `cockroach start` flags as needed for deployment. | ||
| This page describes configuration options that allow advanced users to manually override pod template metadata and `cockroach start` flags as needed for deployment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NishanthNalluri @pritesh-lahoti Originally this page was written when the only purpose of podTemplate fields was to modify the spec from default, but now much of the default pod config is defined here. Is there a reason to keep this content? Is there a situation where a customer may need to modify other fields under podTemplate than the ones we've moved there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jhlodin I believe both the sections (pod template and start flags) should stay.
We could update the examples to not repeat the ones that we already have documented elsewhere, e.g. we could remove the affinity related example, but retain the one for init-containers and containers, as this is definitely a use-case for advanced customers.
On that note, we have resolved the changes to not require the containers field within pod template (https://cockroachlabs.atlassian.net/browse/CC-33346), so we could take this opportunity to refactor the same too.
6f65125 to
84ae52c
Compare
| Specify labels in `cockroachdb.crdbCluster.podTemplate.spec.podLabels` and annotations in `cockroachdb.crdbCluster.podTemplate.spec.podAnnotations` in the values file used to [deploy the cluster]({% link {{ page.version.version }}/deploy-cockroachdb-with-cockroachdb-operator.md %}#initialize-the-cluster): | ||
|
|
||
| ~~~ yaml | ||
| cockroachdb: | ||
| crdbCluster: | ||
| podLabels: | ||
| app.kubernetes.io/version: v25.1.4 | ||
| podAnnotations | ||
| operator: https://raw.githubusercontent.com/cockroachdb/helm-charts/refs/heads/master/cockroachdb-parent/charts/cockroachdb/values.yaml | ||
| podTemplate: | ||
| spec: | ||
| podLabels: | ||
| app.kubernetes.io/version: v25.1.4 | ||
| podAnnotations | ||
| operator: https://raw.githubusercontent.com/cockroachdb/helm-charts/refs/heads/master/cockroachdb-parent/charts/cockroachdb/values.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good callout, @jhlodin . The labels and annotations are now part of podTemplate.metadata.labels and podTemplate.metadata.annotations respectively.
We would need to update this example accordingly. Also, the sample values are slightly off, so we could update as below:
cockroachdb:
crdbCluster:
podTemplate:
metadata:
labels:
app.kubernetes.io/name: "cockroachdb"
annotations:
kubernetes.io/description: "This is a CockroachDB pod."
| - The `values.yaml` configuration maps to a subset of `cockroach start` flags when CockroachDB is initialized. | ||
|
|
||
| This page describes configuration options that allow advanced users to manually override the pod template and `cockroach start` flags as needed for deployment. | ||
| This page describes configuration options that allow advanced users to manually override pod template metadata and `cockroach start` flags as needed for deployment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jhlodin I believe both the sections (pod template and start flags) should stay.
We could update the examples to not repeat the ones that we already have documented elsewhere, e.g. we could remove the affinity related example, but retain the one for init-containers and containers, as this is definitely a use-case for advanced customers.
On that note, we have resolved the changes to not require the containers field within pod template (https://cockroachlabs.atlassian.net/browse/CC-33346), so we could take this opportunity to refactor the same too.
| podTemplate: | ||
| spec: | ||
| affinity: | ||
| podAntiAffinity: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This needs to be further indented to be within affinity.
https://cockroachlabs.atlassian.net/browse/DOC-15288
Update docs as needed to account for cockroachdb/helm-charts#578
TODO: Backport to other CRDB versions once these changes are reviewed.