Remove Kubernetes < 1.35 fallback, require ImageVolume (K8s 1.35+)#275
Open
WentingWu666666 wants to merge 6 commits intodocumentdb:mainfrom
Open
Remove Kubernetes < 1.35 fallback, require ImageVolume (K8s 1.35+)#275WentingWu666666 wants to merge 6 commits intodocumentdb:mainfrom
WentingWu666666 wants to merge 6 commits intodocumentdb:mainfrom
Conversation
Remove the legacy combined-image deployment mode for Kubernetes < 1.35. The operator now requires Kubernetes 1.35+ and uses ImageVolume exclusively to mount the DocumentDB extension image. Changes: - Remove combined mode constants (DEFAULT_COMBINED_DOCUMENTDB_IMAGE, UID/GID) - Remove useImageVolume parameter from GetCnpgClusterSpec and GetDocumentDBImageForInstance - Remove ImageVolumeSupported field and detectImageVolumeSupport from controller - Add validateK8sVersion check in SetupWithManager to fail early on K8s < 1.35 - Remove combined mode tests and add validateK8sVersion tests - Remove K8s 1.34 entries from E2E test matrix - Update documentation to require K8s 1.35+ Closes documentdb#269 Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
…r path Add tests to improve patch coverage from 73.7% to 100%: - SetupWithManager: test error return when K8s version validation fails - Reconcile: integration test exercising the full happy path through upgradeDocumentDBIfNeeded with a pre-existing CNPG cluster Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
WentingWu666666
requested review from
alaye-ms,
hossain-rayhan and
xgerman
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy “combined image” fallback path used for Kubernetes clusters < 1.35 and makes ImageVolume-based extension mounting the only supported deployment mode, enforcing Kubernetes 1.35+ at operator startup.
Changes:
- Enforces Kubernetes >= 1.35 at startup via
validateK8sVersion()and removesImageVolumeSupporteddetection/fallback behavior. - Simplifies CNPG cluster spec generation and image selection to always use ImageVolume-mounted extension images.
- Updates unit tests, docs, changelog, and CI E2E matrix to reflect the new Kubernetes 1.35+ requirement.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| operator/src/internal/utils/util_test.go | Updates tests for GetDocumentDBImageForInstance() after removing the mode parameter. |
| operator/src/internal/utils/util.go | Simplifies GetDocumentDBImageForInstance() to always return the extension image path. |
| operator/src/internal/utils/constants.go | Removes combined-mode constants and introduces MinK8sMinorVersion = 35. |
| operator/src/internal/controller/documentdb_controller_test.go | Replaces ImageVolume detection tests with Kubernetes version validation tests and adjusts controller tests accordingly. |
| operator/src/internal/controller/documentdb_controller.go | Removes ImageVolume support flag, always runs upgrade logic, and adds startup Kubernetes version validation. |
| operator/src/internal/cnpg/cnpg_cluster_test.go | Updates call sites for GetCnpgClusterSpec() after removing the useImageVolume parameter and deletes combined-mode tests. |
| operator/src/internal/cnpg/cnpg_cluster.go | Removes combined-mode spec branch; always configures ImageVolume extension mounting. |
| docs/operator-public-documentation/preview/index.md | Updates quickstart prerequisites to require Kubernetes 1.35+ and removes fallback guidance. |
| CHANGELOG.md | Updates unreleased notes to reflect ImageVolume-only mode and the breaking Kubernetes 1.35+ requirement. |
| .github/workflows/test-E2E.yml | Removes Kubernetes v1.34 entries from the E2E test matrix. |
WentingWu666666
force-pushed
the
users/wentingwu/remove-combined-mode
branch
from
54cfbaa to
797b148
Compare
- Parse K8s major version in validateK8sVersion so future major versions (>1) are treated as supported instead of rejected. Non-numeric major versions now return a clear error. - Replace hardcoded "GA in K8s 1.35" in the unsupported-version error message with the MinK8sMinorVersion constant for consistency. - Fail fast in SetupWithManager when Clientset is nil and the default SQLExecutor is used, catching misconfiguration at startup instead of a nil-pointer panic during reconciliation. - Fix existing SetupWithManager test to supply a Clientset, and add a dedicated test for the new Clientset-nil guard. - Add test cases for major version > 1 and non-numeric major version. Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
Hoist the Clientset nil check to the top of SetupWithManager so both the default SQLExecutor assignment and validateK8sVersion are protected by a single guard. This eliminates the duplicated nil check that was previously inside validateK8sVersion. - SetupWithManager now returns an error immediately when Clientset is nil, before any other initialization logic runs. - validateK8sVersion drops its own nil-Clientset early-return; its doc comment now states callers must ensure Clientset is non-nil. - Test "should not override pre-set SQLExecutor" supplies a Clientset via kubefake.NewSimpleClientset() to satisfy the new guard. - Removed the "should skip validation when Clientset is nil" test for validateK8sVersion that path no longer exists and is already covered by the SetupWithManager nil-Clientset test. Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
| - **ImageVolume Deployment**: The operator uses ImageVolume (GA in Kubernetes 1.35) to mount the DocumentDB extension as a separate image alongside a standard PostgreSQL base image. | ||
|
|
||
| ### Breaking Changes | ||
| - **Removed combined mode support**: The legacy combined-image deployment mode for Kubernetes < 1.35 has been removed. Kubernetes 1.35+ is now required. |
Collaborator
There was a problem hiding this comment.
Just a note, we should cut a major release in March for this breaking change.
From 0.1.3 to 1.0.0.
But if we don't want to go to 1.0.0 before GA, then a minor cut is Ok- need more discussion.
Collaborator
Author
There was a problem hiding this comment.
I remember 1.0.0 means GA. Since we are not yet GA, I believe any breaking change is acceptable in preview and could be included in the next minor release, right?
Remove documentDBImage, gatewayImage, and documentDBVersion from playground scripts, deployment examples, samples, and docs so the operator defaults handle image selection. Changes span: - documentdb-playground/ (aks-setup, aws-setup, telemetry, tls, multi-cloud-deployment, aks-fleet-deployment) - operator/src/config/samples/ - operator/src/scripts/deployment-examples/ - docs/developer-guides/ Also removes related script infrastructure: env vars, CLI flags, sed substitutions, and YAML template placeholders that fed the now-removed fields. The operator-upgrade-guide test-documentdb-versioned.yaml retains its pinned version intentionally (that is the purpose of the test). Verified on kind K8s 1.35: a DocumentDB CR with no image overrides starts successfully with operator defaults. Signed-off-by: Wenting Wu <wentingwu@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes the legacy single-image deployment fallback that was used on Kubernetes clusters older than 1.35 (where the ImageVolume feature was not yet GA). The operator now exclusively uses ImageVolume to mount the DocumentDB extension image separately from the PostgreSQL base image.
A startup validation is added so the operator fails fast with a clear error if the cluster is running Kubernetes < 1.35.
Changes
Operator Source
DEFAULT_COMBINED_DOCUMENTDB_IMAGE,COMBINED_IMAGE_POSTGRES_UID,COMBINED_IMAGE_POSTGRES_GID, andMinK8sMinorVersionForImageVolume. AddedMinK8sMinorVersion = 35.ImageVolumeSupportedfield anddetectImageVolumeSupport()method. AddedvalidateK8sVersion()called fromSetupWithManager()to enforce K8s >= 1.35 at startup.useImageVolumeparameter fromGetCnpgClusterSpec()and the single-image fallback branch.GetDocumentDBImageForInstance()to always return the ImageVolume-based image.Tests
detectImageVolumeSupporttests withvalidateK8sVersiontests (7 cases covering K8s 1.34, 1.35, 1.36+, nil Clientset, "+" suffix, ServerVersion error, non-numeric minor).useImageVolumeparameter from allGetCnpgClusterSpeccall sites; deleted the single-image test block.Documentation & CI
Breaking Changes
How It Works
On K8s < 1.35, the operator logged this at startup in
SetupWithManager:This causes the manager to call
os.Exit(1), and the pod entersCrashLoopBackOff. Users can see the error viakubectl logs.