WIP: CNTRLPLANE-3395: Lower maximum allowed etcd quota from 32 to 16GiB#2840
WIP: CNTRLPLANE-3395: Lower maximum allowed etcd quota from 32 to 16GiB#2840dusk125 wants to merge 1 commit intoopenshift:masterfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@dusk125: This pull request references CNTRLPLANE-3395 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Hello @dusk125! Some important instructions when contributing to openshift/api: |
📝 WalkthroughWalkthroughThis change reduces the maximum allowed value for the 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.1)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
openshift-ci
Bot
added
the
size/M
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@operator/v1/types_etcd.go`:
- Around line 45-49: The BackendQuotaGiB validation in operator/v1/types_etcd.go
was lowered to a maximum of 16, which breaks tests and existing CRs; update the
test suite operator/v1/tests/etcds.operator.openshift.io/EtcdBackendQuota.yaml
by removing or changing the case "Should be able to create with 32
BackendQuotaGiB" (and any other cases using values >16) and update expected
validation error strings to reference max 16 instead of 32 for tests that assert
failure; after adjusting tests, ensure you either document/implement a CRD
versioning/conversion or migration strategy for existing Etcd resources that may
have BackendQuotaGiB ≥17 so cluster upgrades remain safe (verify compliance with
OpenShift CRD upgrade rules).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e01d9449-15f8-4b64-908f-fb28441d8061
⛔ Files ignored due to path filters (6)
openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.featuregated-crd-manifests/etcds.operator.openshift.io/EtcdBackendQuota.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*
📒 Files selected for processing (4)
operator/v1/types_etcd.gopayload-manifests/crds/0000_12_etcd_01_etcds-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_12_etcd_01_etcds-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_12_etcd_01_etcds-TechPreviewNoUpgrade.crd.yaml
| // The value should be an integer not less than 8 and not more than 16. | ||
| // When not specified, the default value is 8. | ||
| // +kubebuilder:default:=8 | ||
| // +kubebuilder:validation:Minimum=8 | ||
| // +kubebuilder:validation:Maximum=32 | ||
| // +kubebuilder:validation:Maximum=16 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for existing Etcd resource configurations with backendQuotaGiB values
# Check for numeric values > 16 in YAML/JSON test fixtures or examples
rg -nP 'backendQuotaGiB[:\s]+([1][7-9]|[2-9][0-9]|[1-9][0-9]{2,})' --type yaml --type json -C 3
# Check for any references to the old maximum of 32
rg -nP '(maximum|max|limit)[:\s]*(of|is)?\s*32\s*(GiB|GB)?' --iglob '*etcd*' -C 3
# Look for migration documentation or upgrade notes
fd -e md -e txt --exec rg -l -i 'backendQuotaGiB|etcd.*quota' {} \;Repository: openshift/api
Length of output: 4239
🌐 Web query:
OpenShift API CRD schema changes backward compatibility and grandfathering existing resources
💡 Result:
OpenShift ensures backward compatibility for CRD schema changes primarily through Operator Lifecycle Manager (OLM) CRD upgrade safety checks and Kubernetes-native mechanisms. Existing resources (CRs) are grandfathered as the Kubernetes API server enforces schema compatibility for reads/writes based on stored versions, preventing breaking changes without conversion webhooks.[1][2][3][4][5][6] OLM's CRD upgrade safety preflight (for cluster extensions/operators) blocks non-backward-compatible CRD updates, such as changing scope, removing stored versions, making optional fields required, shrinking enum/min/max values, or adding required fields. Allowed changes include adding new enum values, making required fields optional, expanding min/max, or adding new versions without modifying existing ones.[1][2][3][4] OpenShift provides strict API compatibility guarantees (especially Tier 1), requiring round-tripping between versions without data loss (via annotations if needed).[7][8] For schema evolution: - Add new CRD versions alongside old ones (multiple served versions). - Use conversion webhooks (hub-spoke model) for breaking changes to enable lossless conversion.[6][5] - Change storage version only after webhook deployment; migrate existing CRs by updating them to rewrite in new storage version.[6][5] - Deprecate/serve=false old versions after migration. Recent OpenShift development (2026) introduces CompatibilityRequirement (apiextensions.openshift.io/v1alpha1, feature-gated under CRDCompatibilityRequirementOperator), allowing explicit compatibility assertions on target CRDs via schema validation on CRD updates and object validation on CR creates/updates. It supports excluded fields and required versions for nuanced control.[9][10][11][12][13][14][15] CRD schema checkers/tools like crd-schema-checker enforce compatibility during development.[16][17][18] Existing CRs remain valid/accessible post-schema changes if updates follow these rules; no automatic invalidation occurs.[1][6][4][5]
Citations:
- 1: https://docs.okd.io/4.17/extensions/ce/crd-upgrade-safety.html
- 2: https://docs.openshift.org/latest/extensions/ce/crd-upgrade-safety.html
- 3: https://docs.okd.io/4.20/extensions/ce/crd-upgrade-safety.html
- 4: https://operator-framework.github.io/operator-controller/concepts/crd-upgrade-safety/
- 5: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning
- 6: https://linuxcent.com/kubernetes-crd-versioning-conversion-webhook/
- 7: https://docs.redhat.com/en/documentation/openshift_container_platform/4.1/html/release_notes/ocp-4-1-versioning-policy
- 8: https://docs.redhat.com/en/documentation/openshift_container_platform/4.12/html/api_overview/understanding-api-support-tiers
- 9: 4959151
- 10: OCPCLOUD-3164: Add CompatibilityRequirement #2479
- 11: OCPCLOUD-3005: Initial implementation of CompatibilityRequirement reconciler and webhook cluster-capi-operator#458
- 12: Require APIExcludedFields Versions to be set #2727
- 13: 3cfb57e
- 14: 3c4723a
- 15: OCPCLOUD-3369: Implement RequiredVersions functionality for CompatibilitySchema cluster-capi-operator#471
- 16: Recurse when loading CRD schemas to check #1875
- 17: Update crd-schema-checker dependency #2062
- 18: https://github.com/openshift/crd-schema-checker
Breaking change requires test file updates and migration strategy for existing deployments with backendQuotaGiB ≥ 17.
The schema reduction from 32 to 16 GiB prevents users with existing deployments from updating Etcd resources with values between 17–32. The test file (operator/v1/tests/etcds.operator.openshift.io/EtcdBackendQuota.yaml) currently contains invalid test cases and outdated error message expectations:
- Line 66/75: Test "Should be able to create with 32 BackendQuotaGiB" will fail validation
- Line 93/150: Expected error messages reference max 32, now obsolete
Kubernetes grandfathers existing resources violating new constraints, but any update attempt fails validation. Update the test file to remove cases with values > 16 and adjust expected error messages accordingly. Additionally, verify that this breaking change aligns with OpenShift's CRD upgrade safety requirements or implement proper versioning/conversion strategy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@operator/v1/types_etcd.go` around lines 45 - 49, The BackendQuotaGiB
validation in operator/v1/types_etcd.go was lowered to a maximum of 16, which
breaks tests and existing CRs; update the test suite
operator/v1/tests/etcds.operator.openshift.io/EtcdBackendQuota.yaml by removing
or changing the case "Should be able to create with 32 BackendQuotaGiB" (and any
other cases using values >16) and update expected validation error strings to
reference max 16 instead of 32 for tests that assert failure; after adjusting
tests, ensure you either document/implement a CRD versioning/conversion or
migration strategy for existing Etcd resources that may have BackendQuotaGiB ≥17
so cluster upgrades remain safe (verify compliance with OpenShift CRD upgrade
rules).
|
@dusk125: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
No description provided.