fix(platform): make etcd v1alpha2 adoption (migration 50) robust in-cluster - #3261
fix(platform): make etcd v1alpha2 adoption (migration 50) robust in-cluster#3261Andrey Kolkov (androndo) wants to merge 2 commits into
Conversation
…luster Migration 50 (etcd.aenix.io -> etcd-operator.cozystack.io/v1alpha2 adoption) had two defects that blocked every in-cluster 1.5 -> 1.6 upgrade on a cluster with an existing etcd: 1. Cert-SAN wait treated transient kubectl failures as "SAN absent". ensure_wildcard_sans checked/awaited the wildcard SAN with `kubectl get ... 2>/dev/null | grep`, so any transient GET failure (API discovery refresh, apiserver blip, throttling) produced an empty string indistinguishable from a genuine absence -> false miss, and the 120s wait never recovered. Replace the two ad-hoc checks with a _san_present helper that retries on an empty read (a real Certificate/Secret never has empty dnsNames/alt-names) and accepts the native wildcard from EITHER the issued Secret's cert-manager.io/alt-names annotation OR the Certificate spec.dnsNames (the source of truth for what cert-manager will issue). 2. etcd-migrate had no kubeconfig in-cluster. etcd-migrate only reads a kubeconfig file (-k/--kubeconfig, default /root/.kube/config) and, unlike kubectl, does not fall back to the mounted in-cluster ServiceAccount. The hook Job set no KUBECONFIG and passed no --kubeconfig, so both the dry-run and --apply aborted with "error building kubeconfig: stat /root/.kube/config: no such file". Synthesize an in-cluster kubeconfig from the mounted ServiceAccount and pass --kubeconfig to both etcd-migrate invocations. Verified end-to-end on a 1.5.2 -> 1.6.0-rc.1 upgrade: the adoption now completes in-place (pods never restarted, data intact) and the cluster reaches readyMembers=3 / Available=True. Refs: #3243, #3255 Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the reliability of the etcd v1alpha2 migration process (migration 50) for in-cluster upgrades. By addressing transient API errors during certificate validation and ensuring proper authentication for the migration tool, the changes ensure that the migration process is more resilient to environmental blips and correctly operates within the Kubernetes service account context. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces robustness improvements to the migration script in packages/core/platform/images/migrations/migrations/50. It adds a _san_present helper function with retries to handle transient API hiccups when checking for SAN presence, and synthesizes an in-cluster kubeconfig file for etcd-migrate to authenticate correctly. The review feedback identifies two key issues: a potential false positive in SAN matching due to substring matching on spec.dnsNames, and a potential failure on IPv6-only or dual-stack clusters when constructing the Kubernetes API server URL, both of which have actionable code suggestions to resolve them.
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.
| d=$(kubectl -n "$ns" get certificate.cert-manager.io "$cert" \ | ||
| -o jsonpath='{.spec.dnsNames}' 2>/dev/null || true) | ||
| if [ -n "$a" ] || [ -n "$d" ]; then | ||
| printf '%s' "$a" | tr ',' '\n' | grep -qxF "$native" && return 0 | ||
| printf '%s' "$d" | tr ',' '\n' | grep -qF "$native" && return 0 |
There was a problem hiding this comment.
Using {.spec.dnsNames} with tr ',' '\n' and grep -qF (substring match) can lead to false positives if the target wildcard SAN is a substring of another SAN (e.g., *.etcd.tenant-foo.svc matching *.etcd.tenant-foo.svc.cluster.local).
Instead, we can use {.spec.dnsNames[*]} to get a space-separated list of DNS names, and then use tr ' ' '\n' with grep -qxF for a robust, exact line match.
| d=$(kubectl -n "$ns" get certificate.cert-manager.io "$cert" \ | |
| -o jsonpath='{.spec.dnsNames}' 2>/dev/null || true) | |
| if [ -n "$a" ] || [ -n "$d" ]; then | |
| printf '%s' "$a" | tr ',' '\n' | grep -qxF "$native" && return 0 | |
| printf '%s' "$d" | tr ',' '\n' | grep -qF "$native" && return 0 | |
| d=$(kubectl -n "$ns" get certificate.cert-manager.io "$cert" \ | |
| -o jsonpath='{.spec.dnsNames[*]}' 2>/dev/null || true) | |
| if [ -n "$a" ] || [ -n "$d" ]; then | |
| printf '%s' "$a" | tr ',' '\n' | grep -qxF "$native" && return 0 | |
| printf '%s' "$d" | tr ' ' '\n' | grep -qxF "$native" && return 0 |
| - name: in-cluster | ||
| cluster: | ||
| certificate-authority: ${_sa_dir}/ca.crt | ||
| server: https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT:-443} |
There was a problem hiding this comment.
Using https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT:-443} can fail on IPv6-only or dual-stack clusters because raw IPv6 addresses in URLs must be enclosed in square brackets (e.g., https://[2001:db8::1]:443).
Since this migration hook runs inside the cluster where DNS is active, we can use the standard, built-in DNS name https://kubernetes.default.svc instead. This is fully robust and works seamlessly across both IPv4 and IPv6 environments.
| server: https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT:-443} | |
| server: https://kubernetes.default.svc |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughMigration 50 now retries wildcard SAN detection across cert-manager sources and uses a synthesized in-cluster kubeconfig for both ChangesMigration script 50 fixes
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/platform/images/migrations/migrations/50 (1)
175-182: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winWait on the Secret, not the Certificate spec
_san_presentreturns success fromCertificate.spec.dnsNames, and this loop runs right after that field is patched. That makes the first iteration succeed immediately, so migration can continue before cert-manager has re-issued the Secret. Check onlycert-manager.io/alt-nameshere.🤖 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 `@packages/core/platform/images/migrations/migrations/50` around lines 175 - 182, The polling loop around _san_present is checking Certificate.spec.dnsNames, which is updated immediately by the patch and lets the migration continue too early. Update the logic in this loop to wait only for cert-manager.io/alt-names on the Secret, using _san_present as the call site reference but removing the Certificate.spec.dnsNames dependency so success means the Secret has actually been re-issued.
🤖 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.
Outside diff comments:
In `@packages/core/platform/images/migrations/migrations/50`:
- Around line 175-182: The polling loop around _san_present is checking
Certificate.spec.dnsNames, which is updated immediately by the patch and lets
the migration continue too early. Update the logic in this loop to wait only for
cert-manager.io/alt-names on the Secret, using _san_present as the call site
reference but removing the Certificate.spec.dnsNames dependency so success means
the Secret has actually been re-issued.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53bd6a1c-3dfc-4613-950e-77d5cb9302b9
📒 Files selected for processing (1)
packages/core/platform/images/migrations/migrations/50
myasnikovdaniil
left a comment
There was a problem hiding this comment.
NOT LGTM — both fixes (#3243 cert-SAN wait, #3255 in-cluster kubeconfig) are correct and pass the existing suite, but they ship untested in a bats suite that exists specifically to pin this file's contract.
Business context: Migration 50 (adopt legacy etcd.aenix.io/v1alpha1 clusters onto etcd-operator.cozystack.io/v1alpha2) hard-fails every in-cluster 1.5→1.6 upgrade on a cluster with an existing etcd; this PR fixes the two documented blockers.
I verified the happy path: all 11 existing tests in hack/migration-50-etcd-adopt.bats still pass on this branch — no regression.
Blockers
B1: Both fixes ship with no test coverage in a suite built to pin this file's contract
hack/migration-50-etcd-adopt.bats drives the real script against a fake kubectl/etcd-migrate, and its header states the behaviours it pins "are the review blockers." This PR adds _san_present and the in-cluster kubeconfig synthesis (+ --kubeconfig on both etcd-migrate calls) and updates none of it.
Evidence:
migrations/50lines 389 and 435 pass--kubeconfig="${ETCD_MIGRATE_KUBECONFIG}"to the dry-run and--apply. The fakeetcd-migratealready logs its full argument vector, yet nothing in the test asserts the flag (grep -c kubeconfigon the.bats= 0). #3255 was a total upgrade blocker that shipped once precisely because no test assertedetcd-migrateis invoked with working auth — without an assertion, a future refactor can silently drop--kubeconfigand re-introduce it. This is ~2 lines: assert--kubeconfig=on theETCD-MIGRATEcmdlog entries.- The
_san_presenttransient-retry path (the core of the #3243 fix) is never exercised: the fake always returns a non-emptyalt-names, so thefor try in 1 2 3 4 5empty-read retry never iterates. A fake knob that returns empty N times then succeeds would pin it. - The existing "re-issues … certs" test passes only incidentally — the fake serves
get certificate … -o jsonpath='{.spec.dnsNames}'as empty, so the newdbranch is dead in-test and the assertion still rides the old Secret-annotation path. The new logic is genuinely uncovered.
At minimum add the --kubeconfig assertion; ideally the transient-retry case too.
Non-blocking follow-ups
-
The cert re-issue wait no longer waits for the Secret; the comment and echo now misstate what happens. Detail inline on line 176. In short:
_san_presentreturns as soon as the Certificatespec.dnsNamescarries the wildcard, but the patch a few lines above just added it tospec.dnsNames— so the wait breaks on the first iteration, before cert-manager re-issues the Secret. The comment on line 173 ("Wait for cert-manager to re-issue the Secret") and the echo on line 177 ("re-issued with native wildcard") then describe something that has not been confirmed. Happy path is fine (re-issue takes seconds, member replacement is far later), but the Secret-level confirmation this block was written to provide is gone. Either keep the wait checking the Secret only, or — if trusting the patched spec is intentional — drop the now-dead 120s loop and correct the comment/echo. -
Substring vs exact SAN match in
_san_present— inline on line 136. -
IPv6-only server URL in the synthesized kubeconfig — inline on line 371.
-
Nit: the
release-noteblock is empty for a change that unblocks every 1.5→1.6 upgrade — worth a line.
Items 2 and 3 overlap with the automated review already on this PR; I verified both independently and they hold.
| -o jsonpath='{.spec.dnsNames}' 2>/dev/null || true) | ||
| if [ -n "$a" ] || [ -n "$d" ]; then | ||
| printf '%s' "$a" | tr ',' '\n' | grep -qxF "$native" && return 0 | ||
| printf '%s' "$d" | tr ',' '\n' | grep -qF "$native" && return 0 |
There was a problem hiding this comment.
_san_present matches the Secret annotation with grep -qxF (exact line, L135) but the Certificate dnsNames with grep -qF (substring, here). The substring form is forced by reading {.spec.dnsNames}, which renders as a bracketed blob [a b c] that tr ',' '\n' cannot split. A wildcard that is a substring of a longer SAN (e.g. *.etcd.<ns>.svc inside *.etcd.<ns>.svc.cluster.local) would false-positive and skip the patch. This chart only issues the short .svc forms, so no live bug today — but it is fragile. Reading {.spec.dnsNames[*]} (space-separated) and matching with tr ' ' '\n' | grep -qxF makes both branches exact.
| if kubectl -n "$ns" get secret "$secret" \ | ||
| -o jsonpath='{.metadata.annotations.cert-manager\.io/alt-names}' 2>/dev/null \ | ||
| | tr ',' '\n' | grep -qxF "$native"; then | ||
| if _san_present "$ns" "$secret" "$cert" "$native"; then |
There was a problem hiding this comment.
This is the wait-for-re-issue loop, but _san_present returns as soon as the Certificate spec.dnsNames contains the wildcard — and the patch a few lines above just added it to spec.dnsNames. So this breaks on the first iteration, before cert-manager re-issues the Secret. The comment on L173 and the echo on L177 ("re-issued with native wildcard") then print during an unattended upgrade without the Secret actually being confirmed. On the happy path cert-manager re-issues within seconds and member replacement happens far later, so it works — but the Secret-level guarantee this block was written for is lost. Either scope the wait-loop check to the Secret annotation only, or, if trusting the patched spec is intentional, remove the now-dead 30×4s loop and fix the comment + echo.
| - name: in-cluster | ||
| cluster: | ||
| certificate-authority: ${_sa_dir}/ca.crt | ||
| server: https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT:-443} |
There was a problem hiding this comment.
On an IPv6-only cluster KUBERNETES_SERVICE_HOST is a bare IPv6 address, so this produces an invalid URL (https://fd00::1:443 — the host must be bracketed). Dual-stack uses the IPv4 primary, so the common path is unaffected, but server: https://kubernetes.default.svc is IP-family-agnostic, validated by the mounted SA CA, and matches how kubectl builds its own in-cluster server URL.
…t-gated wait, IPv6 kubeconfig) Review feedback on #3261 (gemini-code-assist, coderabbitai, myasnikovdaniil): - Exact SAN match. _san_present read the Certificate SANs as {.spec.dnsNames} (a bracketed JSON blob that `tr ','` cannot split) and matched with a substring `grep -qF`, so a wildcard that is a substring of a longer SAN (e.g. *.etcd.<ns>.svc inside *.etcd.<ns>.svc.cluster.local) could false-positive and skip the re-issue. Read {.spec.dnsNames[*]} (space separated) and match exactly with `grep -qxF`, matching the Secret-annotation branch. - Wait on the re-issued Secret, not the patched spec. The post-patch wait loop called _san_present, which returns as soon as the Certificate spec.dnsNames contains the wildcard -- but the patch just added it there, so the loop broke on the first iteration before cert-manager re-issued the Secret, and the "re-issued" log printed unconfirmed. Add _secret_has_san (Secret alt-names annotation only, same empty-read retry + exact match) and gate the wait on it. - IPv6-safe kubeconfig server. The synthesized kubeconfig used https://$KUBERNETES_SERVICE_HOST:$KUBERNETES_SERVICE_PORT, which is an invalid URL on an IPv6-only cluster (a bare IPv6 host must be bracketed). Use https://kubernetes.default.svc -- IP-family-agnostic, validated by the mounted SA CA, and the same server URL kubectl synthesizes in-cluster. Tests (the review blocker): extend the bats suite (and its fake kubectl) with two cases pinning these contracts -- a superstring dnsName must NOT skip the re-issue patch, and etcd-migrate must be invoked with a synthesized in-cluster kubeconfig whose server is kubernetes.default.svc (not the bare IPv6 host), authenticating via the SA token file + CA. Both the kubeconfig path and the SA directory are now env-overridable (ETCD_MIGRATE_KUBECONFIG, ETCD_ADOPT_SA_DIR) so the synthesis is exercisable off-cluster; production defaults are unchanged. Signed-off-by: Andrey Kolkov <androndo@gmail.com>
myasnikovdaniil
left a comment
There was a problem hiding this comment.
LGTM — the follow-up commit resolves all four points; both fixes are correct, now pinned by tests, and the full migration-50 suite is green (13/13).
Verified on b43abb2a:
- Exact SAN match —
_san_presentnow reads{.spec.dnsNames[*]}and matches withgrep -qxF, consistent with the Secret-annotation branch. Pinned by the new "superstring dnsName does not skip the re-issue patch" test (confirmed: reverting it togrep -qFmakes that test fail). - Secret-gated wait — the re-issue wait now uses
_secret_has_san, which checks only the issued Secret's alt-names (not the just-patched spec) while keeping the transient-empty-read retry; the echo is corrected. This restores the Secret-level confirmation the block exists for. - IPv6 kubeconfig —
server: https://kubernetes.default.svc, validated by the mounted SA CA and IP-family-agnostic. Pinned by the new kubeconfig test (asserts--kubeconfigon both etcd-migrate calls and that the bare IPv6 host never lands in the URL; confirmed dropping a--kubeconfigmakes it fail).
ETCD_ADOPT_SA_DIR / ETCD_MIGRATE_KUBECONFIG are test-only seams with unchanged production defaults.
Non-blocking nit (unchanged): the release-note block is still empty for a change that unblocks every 1.5→1.6 upgrade — worth a line for the changelog.
…sition fixes Brings the etcd-migrate migration-50 fixes (cert-SAN retry + Secret-gated wait, in-cluster kubeconfig) alongside the etcd-operator/chart transition fixes so the full 1.5->1.6 etcd v1alpha2 adoption path lands together.
…t-gated wait, IPv6 kubeconfig) Review feedback on #3261 (gemini-code-assist, coderabbitai, myasnikovdaniil): - Exact SAN match. _san_present read the Certificate SANs as {.spec.dnsNames} (a bracketed JSON blob that `tr ','` cannot split) and matched with a substring `grep -qF`, so a wildcard that is a substring of a longer SAN (e.g. *.etcd.<ns>.svc inside *.etcd.<ns>.svc.cluster.local) could false-positive and skip the re-issue. Read {.spec.dnsNames[*]} (space separated) and match exactly with `grep -qxF`, matching the Secret-annotation branch. - Wait on the re-issued Secret, not the patched spec. The post-patch wait loop called _san_present, which returns as soon as the Certificate spec.dnsNames contains the wildcard -- but the patch just added it there, so the loop broke on the first iteration before cert-manager re-issued the Secret, and the "re-issued" log printed unconfirmed. Add _secret_has_san (Secret alt-names annotation only, same empty-read retry + exact match) and gate the wait on it. - IPv6-safe kubeconfig server. The synthesized kubeconfig used https://$KUBERNETES_SERVICE_HOST:$KUBERNETES_SERVICE_PORT, which is an invalid URL on an IPv6-only cluster (a bare IPv6 host must be bracketed). Use https://kubernetes.default.svc -- IP-family-agnostic, validated by the mounted SA CA, and the same server URL kubectl synthesizes in-cluster. Tests (the review blocker): extend the bats suite (and its fake kubectl) with two cases pinning these contracts -- a superstring dnsName must NOT skip the re-issue patch, and etcd-migrate must be invoked with a synthesized in-cluster kubeconfig whose server is kubernetes.default.svc (not the bare IPv6 host), authenticating via the SA token file + CA. Both the kubeconfig path and the SA directory are now env-overridable (ETCD_MIGRATE_KUBECONFIG, ETCD_ADOPT_SA_DIR) so the synthesis is exercisable off-cluster; production defaults are unchanged. Signed-off-by: Andrey Kolkov <androndo@gmail.com>
…es (#3270) Consolidates the etcd `v1alpha2` transition fix for **in-cluster 1.5 → 1.6 upgrades** into a single PR, rebased on current `main`. Supersedes #3265 and #3261 — their commits are carried here (authorship preserved), so those PRs can be closed once this lands. Fresh installs use the new shapes directly and are unaffected, so fresh-install CI does not catch these; the 1.5 → 1.6 e2e upgrade path is the authoritative regression guard. ## What this PR does ### 1. Keep the legacy `etcd-headless` Service alive during adoption — `packages/extra/etcd` `etcd-migrate` adopts legacy clusters **in place**: the Pods keep their original `spec.subdomain: etcd-headless` and are dialed at `etcd-<i>.etcd-headless.<ns>.svc` until they roll onto the operator's native `<member>.etcd.<ns>.svc` domain. The v1alpha2 operator only creates the native `etcd` Service and the legacy `etcd-headless` Service is pruned, so those per-pod names stop resolving (`no such host`), `MemberList` fails, and `status.readyMembers` never populates — the `EtcdCluster` never goes `Ready` even though etcd is healthy and in quorum. We ship a chart-managed transitional headless `etcd-headless` Service (selector mirrors the operator's native `etcd` Service via `etcd-operator.cozystack.io/cluster`, `publishNotReadyAddresses: true`) — the DNS counterpart of the legacy `*.etcd-headless.<ns>.svc` SAN already kept for this window. Removable together with that SAN once members roll onto the native subdomain. ### 2. Survive the immutable controller-Deployment selector on upgrade — `packages/system/etcd-operator` (#3242) #2859 replaced the upstream etcd-operator chart with the cozystack-authored one, changing `Deployment.spec.selector.matchLabels`. `spec.selector` is immutable, so `helm upgrade` cannot patch the existing Deployment and the whole HelmRelease upgrade fails (`field is immutable`). A **pre-upgrade hook** (`templates/pre-upgrade-selector-fix.yaml`: ServiceAccount + Role + RoleBinding + Job) deletes the Deployment **only** when its live selector is the pre-1.6 one, so Helm recreates it cleanly. No-op when the selector already matches, never runs on fresh install. ### 3. Raise the operator's memory cold-start floor — `packages/system/etcd-operator` Steady-state working set is ~250Mi; the static `limits.memory: 128Mi` OOMKills a Pod that starts before the VPA admission webhook rewrites it. Raise the floor to `256Mi` (and VPA `minAllowed` to match) as defense in depth so the operator never depends on VPA timing to avoid crashing. ### 4. Make migration 50 (etcd adoption) robust in-cluster — `packages/core/platform/images/migrations/migrations/50` (was #3261) Exact server/peer cert-SAN match, Secret-gated wait on the adoption Secret, and in-cluster (IPv6-safe) kubeconfig handling, so the migration script drives the adoption reliably from inside the cluster. Covered by `hack/migration-50-etcd-adopt.bats`. ### 5. Hardening / review fixes (this PR's original scope) - **Hook `runAsUser: 65532`.** `pre-upgrade-selector-fix.yaml` set `runAsNonRoot: true` but no numeric `runAsUser`; `clastix/kubectl`'s image user is the non-numeric name `nonroot`, which the kubelet cannot verify against `runAsNonRoot` — so the hook Pod fails admission and silently blocks the very upgrade it exists to unblock. Adds `runAsUser: 65532`, matching the postgres-operator webhook-ready hook that runs the same image. - **Digest-pin the kubectl image.** The values comment claimed digest-pinning but shipped a floating `v1.32` tag. Reuses the digest postgres-operator vendors, adds the `renovate` annotation, and templates `repo:tag@digest`. - **Tests.** `etcd-operator/tests/selector-fix-hook_test.yaml` (hook wiring, weight ordering, namespaced least-privilege RBAC, numeric-non-root security context, digest-pinned image), `etcd-operator/tests/deployment_test.yaml` (256Mi cold-start floor), `extra/etcd/tests/etcd-cluster_test.yaml` (transitional `etcd-headless` Service). Each assertion was mutation-tested. ### 6. Derive the default S3 endpoint from the provisioned bucket — `packages/system/backupstrategy-controller` Derive the default S3 endpoint (and per-driver scheme / TLS / `secure_connection`) from the provisioned bucket Secret instead of requiring it to be hand-set, so etcd (and other) backup strategies get a working endpoint by default. Docs in `docs/operations/backup-classes.md`; covered by `tests/endpoint_form_test.yaml`. ### Verification - `helm unittest` green on current `main`: etcd-operator **18/18**, extra/etcd **18/18**, backupstrategy-controller **11/11**. - Behaviours 1 & 2 were reproduced and confirmed live on a 1.5.2 → 1.6.0-rc.1 adoption (3-node cluster) — recreating the `etcd-headless` Service took the adopted cluster to `readyMembers=3 / Available=True`. ```release-note fix(etcd): complete the v1alpha2 transition on in-cluster 1.5→1.6 upgrades — keep the legacy etcd-headless Service alive so adopted members stay resolvable, delete the pre-1.6 operator Deployment via a pre-upgrade hook to get past the immutable selector, raise the operator's memory floor so it does not OOM before the VPA scales it, and make the etcd adoption migration robust in-cluster. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved backup storage endpoint handling across supported backup drivers, including provisioned and external S3 storage. * Added compatibility support for legacy etcd pod discovery during migration. * Added an automated upgrade safeguard for etcd operator deployments. * **Bug Fixes** * Improved certificate SAN detection and etcd migration authentication. * Increased the etcd operator’s minimum startup memory to prevent early restarts. * **Documentation** * Clarified backup endpoint, TLS, and driver-specific behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
Completed in #3270 |
Pull request was closed
Migration 50 (etcd.aenix.io -> etcd-operator.cozystack.io/v1alpha2 adoption) had two defects that blocked every in-cluster 1.5 -> 1.6 upgrade on a cluster with an existing etcd:
Cert-SAN wait treated transient kubectl failures as "SAN absent". ensure_wildcard_sans checked/awaited the wildcard SAN with
kubectl get ... 2>/dev/null | grep, so any transient GET failure (API discovery refresh, apiserver blip, throttling) produced an empty string indistinguishable from a genuine absence -> false miss, and the 120s wait never recovered. Replace the two ad-hoc checks with a _san_present helper that retries on an empty read (a real Certificate/Secret never has empty dnsNames/alt-names) and accepts the native wildcard from EITHER the issued Secret's cert-manager.io/alt-names annotation OR the Certificate spec.dnsNames (the source of truth for what cert-manager will issue).etcd-migrate had no kubeconfig in-cluster. etcd-migrate only reads a kubeconfig file (-k/--kubeconfig, default /root/.kube/config) and, unlike kubectl, does not fall back to the mounted in-cluster ServiceAccount. The hook Job set no KUBECONFIG and passed no --kubeconfig, so both the dry-run and --apply aborted with "error building kubeconfig: stat /root/.kube/config: no such file". Synthesize an in-cluster kubeconfig from the mounted ServiceAccount and pass --kubeconfig to both etcd-migrate invocations.
Verified end-to-end on a 1.5.2 -> 1.6.0-rc.1 upgrade: the adoption now completes in-place (pods never restarted, data intact) and the cluster reaches readyMembers=3 / Available=True.
Refs: #3243, #3255
What this PR does
Screenshots
Release note
Summary by CodeRabbit
etcd-migratefrom the hook Pod’s ServiceAccount credentials, including during dry-run and apply.