feat(main): add image parameters for air-gapped cases#337
feat(main): add image parameters for air-gapped cases#337Andrey Kolkov (androndo) wants to merge 1 commit into
Conversation
github-actions
Bot
added
api-change
controllers
documentation
|
Warning Review limit reached
More reviews will be available in 34 minutes and 40 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughAdds ChangesEtcd Image Override and Pull Secrets
Sequence Diagram(s)sequenceDiagram
participant User as User/Helm
participant Operator as etcd-operator (main.go)
participant ClusterCtrl as EtcdClusterReconciler
participant MemberCtrl as EtcdMemberReconciler
participant resolveEtcdImage as resolveEtcdImage helper
participant Pod as etcd member Pod
User->>Operator: --etcd-image-repository (ETCD_IMAGE_REPOSITORY env)
Operator->>MemberCtrl: EtcdImageRepository field set
User->>ClusterCtrl: EtcdCluster with spec.image / spec.imagePullSecrets
ClusterCtrl->>ClusterCtrl: snapshotSpecIntoObserved (latches Image + ImagePullSecrets)
ClusterCtrl->>ClusterCtrl: specEqualsObserved detects image drift
ClusterCtrl->>MemberCtrl: EtcdMember.Spec.Image + ImagePullSecrets propagated
MemberCtrl->>resolveEtcdImage: resolveEtcdImage(member, EtcdImageRepository)
resolveEtcdImage-->>MemberCtrl: resolved image string + pullPolicy
MemberCtrl->>Pod: container.Image, ImagePullPolicy, spec.ImagePullSecrets
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for custom etcd container images and image pull secrets, primarily targeting air-gapped environments. It adds image and imagePullSecrets fields to EtcdCluster, ObservedCluster, and EtcdMember specs, allowing both operator-wide default overrides (via the --etcd-image-repository flag or Helm chart values) and per-cluster overrides. It also updates the migration logic to preserve custom images and pull secrets from legacy configurations, adds comprehensive unit and end-to-end tests, and updates the documentation. There are no review comments to provide feedback on.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![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 `@test/e2e/image_override_test.go`:
- Around line 76-80: The test assertion checking the pull policy on line 78 is
incorrect because it expects an empty string, but Kubernetes API server defaults
imagePullPolicy to IfNotPresent when the field is omitted and the image tag is
fixed (non-:latest). Since etcdMemberImage reads the actual persisted Pod from
the cluster, it will return the Kubernetes-defaulted value IfNotPresent, not an
empty string. Update the assertion condition from policy != "" to expect policy
== "IfNotPresent" instead, and update the error message to reflect that
IfNotPresent is the expected Kubernetes-defaulted behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 956b0115-4973-464f-8ee9-2122df51fdea
📒 Files selected for processing (21)
Makefileapi/v1alpha2/etcdcluster_types.goapi/v1alpha2/etcdmember_types.goapi/v1alpha2/zz_generated.deepcopy.gocharts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yamlcharts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yamlcharts/etcd-operator/templates/deployment.yamlcharts/etcd-operator/values.yamlcontrollers/etcdcluster_controller.gocontrollers/etcdmember_controller.gocontrollers/etcdmember_controller_test.gocontrollers/helpers.gocontrollers/helpers_test.godocs/installation.mddocs/migration.mdhack/e2e.shinternal/migrate/adopt.gointernal/migrate/translate.gointernal/migrate/translate_test.gomain.gotest/e2e/image_override_test.go
Andrey Kolkov (androndo)
force-pushed
the
feat/overrides-for-airgappeed-envs
branch
from
b0be94b to
dfa8611
Compare
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo)
force-pushed
the
feat/overrides-for-airgappeed-envs
branch
from
dfa8611 to
c62e793
Compare
Timofei Larkin (lllamnyp)
left a comment
Timofei Larkin (lllamnyp)
left a comment
There was a problem hiding this comment.
Request changes — narrow this to the operator-wide repository flag (+ pull secrets)
Thanks for tackling the air-gap case. The operator-wide repository default is the right primitive and I want to merge that part. But the per-cluster spec.image block introduces a second source of truth for the etcd version that can silently disagree with spec.version, and that's not something I'm willing to take on. Concretely:
Keep:
--etcd-image-repository/ETCD_IMAGE_REPOSITORYon the binary, the chart'setcdImage.repositoryvalue, and the env wiring in the Deployment.spec.imagePullSecrets(cluster → member mirror,status.observedlatching, and the migrate carry-through). A private mirror needs credentials, and pull secrets have none of the version-ambiguity problem below — they only affect whether the pull authenticates, never what version runs.
The repository flag cleanly serves the mirror-once-per-fleet case: it only changes where the image is pulled from, never what version runs.
Please drop (for now):
EtcdCluster.spec.image(EtcdImageSpec—repository,tag,pullPolicy) and its mirror onEtcdMember.spec.image- the
status.observed.imagelatching and theresolveEtcdImagetag/pull-policy handling - the migrate
etcdImageOverridereconstruction
The blocking problem is spec.image.tag. The operator treats spec.version as the source of truth for the running etcd version — it's injected as ETCD_VERSION and drives the restore version-compat pre-flight, the latched target, and drift detection. spec.image.tag feeds none of that; it only changes the pulled reference. So a spec where image.tag resolves to a different minor than spec.version is internally contradictory: the container runs one version while the operator reasons about another. Nothing validates that they agree, and the most damaging consequence lands exactly on the air-gap path this PR targets — the restore initContainer rebuilds the data dir to spec.version's format, then a mismatched binary boots on it. Adding a second, unvalidated way to set the version is a footgun I'd rather not introduce, even guarded by docs.
Future work (not blocking — please open issues, don't fold into this PR):
-
Observed member version in
status.spec.versioncommunicates intent but is currently relied on as the source of truth. The member should determine the etcd version it's actually running at runtime and write it to itsstatus; the operator's version-dependent logic should key off that observed value, not the spec field. This is the principled fix for the conflict above and would make a version override safe to reconsider later. -
Multi-version restore agent. The restore agent ships a single compiled
etcdutl(3.6.x in this build), so restore is silently pinned to the operator's own etcd minor regardless ofspec.version. If we want the operator to support running multiple etcd versions, the restore agent needs to run a matchingetcdutlper target version too — otherwise restore is unsupported for any cluster off the operator's minor.
2 participants
Summary by CodeRabbit
Release Notes
New Features
spec.image:repository,tag,pullPolicy) plusspec.imagePullSecretsto pull member (and restore initContainer) images from private registries.Bug Fixes
Documentation
Tests