From 3f3ddda8ce989b3437bfe27d7886b9595cedf9fb Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Sun, 16 Aug 2026 09:23:16 +0400 Subject: [PATCH 1/9] feat(restore): run a version-matched etcdutl per target etcd version The restore agent rebuilt the data dir with a single etcdutl compiled into the operator image, silently pinning restore to the operator's own etcd minor: clusters on a different minor could not use spec.bootstrap.restore, and a mismatched binary could rebuild the data dir in the wrong on-disk format before etcd booted on it. Run the etcdutl that ships in the target etcd image (v) instead, so the rebuild and the etcd that boots on its result share a release by construction. The etcd image is distroless and offers no way to copy etcdutl out, so the restore seed now uses two init containers: an install-tools container (operator image) stages the operator binary onto a shared volume, and the restore container runs that binary from the etcd image, giving the agent both its own fetch/preflight/idempotency logic and the image's version-matched etcdutl. The agent execs `etcdutl snapshot restore` rather than the compiled-in snapshot library. Drops the version-compat pre-flight (and the ETCD_VERSION env it read): the binary now matches spec.version by construction, so restore supports any etcd version the operator supports. Removing the compiled-in etcdutl also drops the etcd server/bbolt/raft dependency tree from the operator binary. Closes #339 Signed-off-by: Andrey Kolkov Assisted-By: Claude Opus 4.8 (1M context) --- controllers/etcdmember_controller.go | 91 +++++++++----- controllers/restore_initcontainer_test.go | 38 ++++-- docs/concepts.md | 6 +- docs/installation.md | 6 +- docs/operations.md | 4 +- go.mod | 16 +-- go.sum | 44 ------- internal/agent/agent.go | 14 ++- internal/agent/restore.go | 133 +++++++++++--------- internal/agent/restore_test.go | 141 ++++++++++++++++------ main.go | 10 ++ 11 files changed, 301 insertions(+), 202 deletions(-) diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index c94157eb..cdd66798 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -778,18 +778,18 @@ func (r *EtcdMemberReconciler) buildPod(member *lll.EtcdMember, clusterFormed bo }) } - // Restore initContainer: when the seed carries a restore spec, populate + etcdImage := resolveEtcdImage(member, r.EtcdImageRepository) + + // Restore initContainers: when the seed carries a restore spec, populate // the data dir from the snapshot before etcd starts. The agent no-ops if // the data dir is already initialized, so it's safe across Pod restarts. var initContainers []corev1.Container if member.Spec.Restore != nil { - ic, extraVols := restoreInitContainer(member, pAddr, r.OperatorImage) - initContainers = append(initContainers, ic) + ics, extraVols := restoreInitContainers(member, pAddr, r.OperatorImage, etcdImage) + initContainers = append(initContainers, ics...) volumes = append(volumes, extraVols...) } - etcdImage := resolveEtcdImage(member, r.EtcdImageRepository) - return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: member.Name, @@ -870,13 +870,27 @@ func (r *EtcdMemberReconciler) buildPod(member *lll.EtcdMember, clusterFormed bo const restoreSrcMountPath = "/restore/src" -// restoreInitContainer builds the initContainer that restores the data dir -// from a snapshot before etcd starts. peerAddr is this member's peer URL; the -// agent feeds it (with the member name / initial-cluster / token) to etcdutl -// so the restored data matches the identity the etcd container will run with. -// For an S3 source the object key is exact (not a prefix); for a PVC source -// the volume is mounted read-only and PVC_SUBPATH points to the snapshot file. -func restoreInitContainer(member *lll.EtcdMember, peerAddr, operatorImage string) (corev1.Container, []corev1.Volume) { +// restoreToolsVolumeName is the shared volume onto which the install-tools +// initContainer copies the operator binary so the restore container (running +// the etcd image) can exec it. restoreToolsMountPath is where both mount it. +const ( + restoreToolsVolumeName = "restore-tools" + restoreToolsMountPath = "/tools" +) + +// restoreInitContainers builds the ordered initContainers that restore the data +// dir from a snapshot before etcd starts. The rebuild must run a version-matched +// etcdutl, so it runs from the target etcd image rather than the operator image; +// but that image is distroless and ships only etcd binaries, with no way to copy +// etcdutl out to the operator. So a first initContainer copies the operator +// binary onto a shared volume, and the restore container runs the etcd image +// with that binary as its entrypoint — giving the agent both its own logic and +// the image's etcdutl. peerAddr is this member's peer URL; the agent feeds it +// (with the member name / initial-cluster / token) to etcdutl so the restored +// data matches the identity the etcd container will run with. For an S3 source +// the object key is exact (not a prefix); for a PVC source the volume is mounted +// read-only and PVC_SUBPATH points to the snapshot file. +func restoreInitContainers(member *lll.EtcdMember, peerAddr, operatorImage, etcdImage string) ([]corev1.Container, []corev1.Volume) { src := member.Spec.Restore.Source env := []corev1.EnvVar{ {Name: "ETCD_DATA_DIR", Value: "/var/lib/etcd"}, @@ -884,12 +898,15 @@ func restoreInitContainer(member *lll.EtcdMember, peerAddr, operatorImage string {Name: "ETCD_INITIAL_CLUSTER", Value: member.Spec.InitialCluster}, {Name: "ETCD_INITIAL_CLUSTER_TOKEN", Value: member.Spec.ClusterToken}, {Name: "ETCD_PEER_URLS", Value: peerAddr}, - // The cluster's etcd version, for the agent's version-compat pre-flight - // (the restored data dir must match the etcd that boots on it). - {Name: "ETCD_VERSION", Value: member.Spec.Version}, } - mounts := []corev1.VolumeMount{{Name: "data", MountPath: "/var/lib/etcd"}} - var vols []corev1.Volume + mounts := []corev1.VolumeMount{ + {Name: "data", MountPath: "/var/lib/etcd"}, + {Name: restoreToolsVolumeName, MountPath: restoreToolsMountPath, ReadOnly: true}, + } + vols := []corev1.Volume{{ + Name: restoreToolsVolumeName, + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, + }} switch { case src.S3 != nil: @@ -924,18 +941,36 @@ func restoreInitContainer(member *lll.EtcdMember, peerAddr, operatorImage string mounts = append(mounts, corev1.VolumeMount{Name: "restore-src", MountPath: restoreSrcMountPath, ReadOnly: true}) } - return corev1.Container{ - Name: "restore", - Image: operatorImage, - Command: []string{"/manager", "restore-agent"}, - Env: env, - SecurityContext: &corev1.SecurityContext{ - AllowPrivilegeEscalation: ptrBool(false), - Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + restrictedSecurityContext := &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptrBool(false), + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + } + + installTools := corev1.Container{ + Name: "install-tools", + Image: operatorImage, + Command: []string{"/manager", "install-tools"}, + Env: []corev1.EnvVar{{Name: "TOOLS_DEST_DIR", Value: restoreToolsMountPath}}, + SecurityContext: restrictedSecurityContext, + VolumeMounts: []corev1.VolumeMount{ + {Name: restoreToolsVolumeName, MountPath: restoreToolsMountPath}, }, - VolumeMounts: mounts, - Resources: restoreAgentResources(), - }, vols + Resources: restoreAgentResources(), + } + + restore := corev1.Container{ + Name: "restore", + Image: etcdImage, + // Run the operator binary staged by install-tools, from the etcd image — + // so the agent can exec that image's version-matched etcdutl. + Command: []string{restoreToolsMountPath + "/manager", "restore-agent"}, + Env: env, + SecurityContext: restrictedSecurityContext, + VolumeMounts: mounts, + Resources: restoreAgentResources(), + } + + return []corev1.Container{installTools, restore}, vols } // dataLossRestartThreshold is how many times the etcd container must have diff --git a/controllers/restore_initcontainer_test.go b/controllers/restore_initcontainer_test.go index fa9caaeb..b1e9f0b8 100644 --- a/controllers/restore_initcontainer_test.go +++ b/controllers/restore_initcontainer_test.go @@ -51,6 +51,9 @@ func TestBuildPod_NoRestoreInitContainerWithoutSpec(t *testing.T) { if _, ok := findInitContainer(pod, "restore"); ok { t.Error("restore initContainer present though no restore spec was set") } + if _, ok := findInitContainer(pod, "install-tools"); ok { + t.Error("install-tools initContainer present though no restore spec was set") + } } func TestBuildPod_RestoreInitContainerS3(t *testing.T) { @@ -66,15 +69,37 @@ func TestBuildPod_RestoreInitContainerS3(t *testing.T) { r := &EtcdMemberReconciler{Scheme: testScheme(t), OperatorImage: "operator:latest"} pod := r.buildPod(seedMember(restore), false) + // install-tools stages the operator binary onto the shared volume so the + // restore container (etcd image) can exec it. + it, ok := findInitContainer(pod, "install-tools") + if !ok { + t.Fatal("install-tools initContainer missing") + } + if it.Image != "operator:latest" { + t.Errorf("install-tools image = %q, want operator:latest", it.Image) + } + if got, want := it.Command, []string{"/manager", "install-tools"}; len(got) != 2 || got[0] != want[0] || got[1] != want[1] { + t.Errorf("install-tools command = %v, want %v", got, want) + } + if m, ok := mountByName(it.VolumeMounts, "restore-tools"); !ok || m.MountPath != "/tools" || m.ReadOnly { + t.Errorf("install-tools restore-tools mount = %+v, want writable at /tools", m) + } + ic, ok := findInitContainer(pod, "restore") if !ok { t.Fatal("restore initContainer missing") } - if ic.Image != "operator:latest" { - t.Errorf("image = %q, want operator:latest", ic.Image) + // The restore container runs the target etcd image, so its bundled etcdutl + // matches spec.version — the whole point of restoring per-version. + if ic.Image != "quay.io/coreos/etcd:v3.6.4" { + t.Errorf("restore image = %q, want quay.io/coreos/etcd:v3.6.4 (version-matched)", ic.Image) } - if got, want := ic.Command, []string{"/manager", "restore-agent"}; len(got) != 2 || got[0] != want[0] || got[1] != want[1] { - t.Errorf("command = %v, want %v", got, want) + // It execs the operator binary staged on the shared volume, not the etcd image's entrypoint. + if got, want := ic.Command, []string{"/tools/manager", "restore-agent"}; len(got) != 2 || got[0] != want[0] || got[1] != want[1] { + t.Errorf("restore command = %v, want %v", got, want) + } + if m, ok := mountByName(ic.VolumeMounts, "restore-tools"); !ok || m.MountPath != "/tools" { + t.Errorf("restore restore-tools mount = %+v, want /tools", m) } // Restore identity must match what the etcd container will run with. @@ -91,11 +116,6 @@ func TestBuildPod_RestoreInitContainerS3(t *testing.T) { if vals["ETCD_DATA_DIR"] != "/var/lib/etcd" { t.Errorf("ETCD_DATA_DIR = %q, want /var/lib/etcd", vals["ETCD_DATA_DIR"]) } - // The cluster's etcd version must be passed for the agent's version-compat - // pre-flight (the restored data dir must match the etcd that boots on it). - if vals["ETCD_VERSION"] != "3.6.4" { - t.Errorf("ETCD_VERSION = %q, want 3.6.4", vals["ETCD_VERSION"]) - } if vals["SNAPSHOT_DEST_KIND"] != "s3" || vals["S3_KEY"] != "snapshots/b1.db" { t.Errorf("s3 source env = %+v", vals) } diff --git a/docs/concepts.md b/docs/concepts.md index 4abc8d81..09504832 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -469,7 +469,7 @@ All conditions carry `observedGeneration` so consumers can tell whether a condit ### Observed member version -`spec.version` is *intent* — it pins the image tag (`v`) and drives the restore version-compat gate. What etcd is **actually running** is a separate, observed fact. Once a member's Pod is Ready, the member controller reads the running version from that member's own etcd endpoint (the Maintenance `Status` RPC) and records it in `EtcdMember.status.version` (surfaced as the `Running` print column). The read is best-effort: a dial or RPC failure leaves the previous value in place and never affects `Ready` — readiness stays driven by Pod readiness and member-ID discovery alone. +`spec.version` is *intent* — it pins the image tag (`v`), which is also the etcd image whose `etcdutl` the restore agent runs. What etcd is **actually running** is a separate, observed fact. Once a member's Pod is Ready, the member controller reads the running version from that member's own etcd endpoint (the Maintenance `Status` RPC) and records it in `EtcdMember.status.version` (surfaced as the `Running` print column). The read is best-effort: a dial or RPC failure leaves the previous value in place and never affects `Ready` — readiness stays driven by Pod readiness and member-ID discovery alone. When the observed version diverges from the member's intended `spec.version`, the member surfaces `VersionDrifted=True/VersionMismatch`; when they agree it is `False/VersionMatched`; when intent is not yet known (`spec.version` empty) the condition is left unset. This condition is **informational only** — the operator does not act on it (it never keys reconciliation off the observed value). It exists so intent-vs-reality drift is *detectable rather than assumed*, which is the prerequisite for safely reconsidering a per-cluster image/version override. @@ -491,11 +491,11 @@ Snapshot integrity note: a `Maintenance.Snapshot` stream carries no appended has ### Restore (`spec.bootstrap.restore`) -Restore is a first-bootstrap-only path, not a controller that mutates a running cluster. When `spec.bootstrap.restore.source` is set, the cluster controller stamps the `RestoreSpec` onto the bootstrap **seed** `EtcdMember` (only the seed — scale-up members join the live cluster normally). The member controller's `buildPod` then prepends a `restore` init container (the operator image, `manager restore-agent`) that shares the etcd data volume. Before etcd starts, the agent fetches the snapshot (S3 download / PVC read) and runs `etcdutl` `snapshot.Restore` into the data dir, using the seed's exact identity — member name, `--initial-cluster`, cluster token, peer URL — so etcd accepts the rebuilt data dir. +Restore is a first-bootstrap-only path, not a controller that mutates a running cluster. When `spec.bootstrap.restore.source` is set, the cluster controller stamps the `RestoreSpec` onto the bootstrap **seed** `EtcdMember` (only the seed — scale-up members join the live cluster normally). The member controller's `buildPod` then prepends two init containers that share the etcd data volume: `install-tools` (operator image) copies the operator binary onto a shared volume, and `restore` runs that binary (`manager restore-agent`) — but from the **target etcd image**, so it reaches the version-matched `etcdutl` bundled there. Before etcd starts, the agent fetches the snapshot (S3 download / PVC read) and execs `etcdutl snapshot restore` into the data dir, using the seed's exact identity — member name, `--initial-cluster`, cluster token, peer URL — so etcd accepts the rebuilt data dir. The init container is idempotent: it no-ops if the data dir already contains a `member/` directory, so Pod restarts after first boot leave live data untouched and never re-download. Because `spec.bootstrap` is CEL-immutable post-create, the restore intent can't be added to or changed on a live cluster — restore happens once, at birth, or not at all. A restored cluster gets a fresh etcd cluster ID: it is a new cluster seeded with old data, not a continuation. -The rebuild uses the `etcdutl` vendored into the operator image, whose on-disk storage format is minor-version-specific. So restore requires `spec.version` to match that `etcdutl`'s minor (currently etcd **3.6.x**): the agent reads `spec.version` (passed as `ETCD_VERSION`) and **fails the restore early** with an actionable message if the major.minor differs, rather than rebuilding a data dir an older etcd would fail to boot. Restoring into a different minor means using an operator build whose `etcdutl` matches. (Non-restore clusters are unaffected — this gate only fires on the restore path.) +The rebuild's `etcdutl` is the one bundled in the target etcd image (`v`), not one compiled into the operator. Since a data dir's on-disk storage format is minor-version-specific, running the `etcdutl` that ships with the very etcd that will boot on the result keeps the two in lockstep by construction — so restore works for any etcd version the operator supports, not only the operator's own minor. The two init containers exist to bridge two distroless images that share no binaries: the etcd image has `etcdutl` but no way to copy it out, so `install-tools` brings the operator binary to the etcd image instead. This idempotency relies on the data dir being **persistent**. Restore is therefore rejected (by CEL) together with `spec.storage.medium: Memory`: a tmpfs data dir is wiped on every Pod restart, which would defeat the `member/`-exists guard and silently re-restore the original snapshot — reverting any writes since the restore, or breaking a multi-member cluster whose other members already moved past the restored cluster ID. Restore onto memory-backed storage is unsupported; use a PVC-backed cluster. diff --git a/docs/installation.md b/docs/installation.md index d8c6ab4c..6049b2d6 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -247,7 +247,7 @@ The `spec.tls` subtree is immutable post-create — flipping TLS on or off on an By default `spec.version` in an `EtcdCluster` becomes `quay.io/coreos/etcd:v`. For an air-gapped environment that mirrors the image to a private registry, repoint the **repository** operator-wide and supply per-cluster pull credentials: -- **Repository (operator-wide)** — set `etcdImage.repository` in the chart (env `ETCD_IMAGE_REPOSITORY` / flag `--etcd-image-repository`) to a registry/path, e.g. `registry.internal/mirror/etcd`. Every member Pod the operator creates pulls from it; the tag is always `v`. Mirror once per fleet — there is intentionally no per-cluster repository/tag override, because the operator keys every version-dependent behaviour (the restore version-compat pre-flight, the latched target, drift detection) off `spec.version`, and a per-cluster `tag` could silently disagree with it. +- **Repository (operator-wide)** — set `etcdImage.repository` in the chart (env `ETCD_IMAGE_REPOSITORY` / flag `--etcd-image-repository`) to a registry/path, e.g. `registry.internal/mirror/etcd`. Every member Pod the operator creates pulls from it; the tag is always `v`. Restore init containers pull the same image (that is where the version-matched `etcdutl` comes from), so mirroring it also covers restore. Mirror once per fleet — there is intentionally no per-cluster repository/tag override, because the operator keys every version-dependent behaviour (the etcd image tag, the latched target, drift detection) off `spec.version`, and a per-cluster `tag` could silently disagree with it. - **Pull credentials (per-cluster)** — `spec.imagePullSecrets` on an `EtcdCluster`: ```yaml @@ -259,9 +259,9 @@ By default `spec.version` in an `EtcdCluster` becomes `quay.io/coreos/etcd:v`) rather than a single bundled one (see the [restore runbook](operations.md#restoring-a-cluster-from-a-snapshot)). Operator's own toolchain (relevant when building from source): diff --git a/docs/operations.md b/docs/operations.md index 4c5a7975..b63f7a7c 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -280,7 +280,7 @@ kubectl logs -n job/my-etcd-2026-06-02-snapshot Restore is a **first-bootstrap-only** path: a *new* cluster initializes its seed member's data dir from a snapshot instead of starting empty. You cannot restore into an existing, already-bootstrapped cluster (`spec.bootstrap` is immutable post-create) — delete and recreate. -> **⚠️ Restore requires the cluster's etcd version to match the operator's `etcdutl`.** The restore agent rebuilds the data dir with the `etcdutl` vendored into the operator image (currently **etcd 3.6.x**), and that data dir carries 3.6's on-disk storage semantics — an etcd container of a different minor (e.g. 3.5.x) booting on it is unvalidated and can fail at the seed. The agent enforces this: it **fails the restore early with a clear message** if `spec.version`'s major.minor differs from its `etcdutl`. So a restored cluster must run a **3.6.x** `spec.version` (the example below uses `3.6.11`). This applies only to restore-on-bootstrap; non-restore clusters can run any supported version. To restore into a different minor, use an operator build whose `etcdutl` matches. +> **Restore rebuilds the data dir with a version-matched `etcdutl`.** The restore agent runs the `etcdutl` bundled in the target etcd image (`v`), the very version that then boots on the rebuilt data dir — so the on-disk storage format matches by construction. Restore works for any etcd version the operator supports; there is no requirement that `spec.version` match the operator's own build. > **⚠️ Restoring a snapshot from an auth-enabled cluster.** An etcd snapshot captures the data store *including its auth state* — users, roles, and the auth-enabled flag. A snapshot taken while auth was on restores into a cluster where **etcd boots with auth already ON**. You must therefore set `spec.auth` on the new `EtcdCluster` to match, or the operator can never manage it: > @@ -298,7 +298,7 @@ metadata: namespace: spec: replicas: 3 - version: 3.6.11 # must match the operator's etcdutl minor (3.6.x) — see warning above + version: 3.6.11 # restore rebuilds with this version's etcdutl storage: size: 1Gi bootstrap: diff --git a/go.mod b/go.mod index 3bbd56ce..22046840 100644 --- a/go.mod +++ b/go.mod @@ -14,8 +14,6 @@ require ( github.com/spf13/cobra v1.10.2 go.etcd.io/etcd/api/v3 v3.6.11 go.etcd.io/etcd/client/v3 v3.6.11 - go.etcd.io/etcd/etcdutl/v3 v3.6.11 - go.uber.org/zap v1.27.0 k8s.io/api v0.33.11 k8s.io/apimachinery v0.33.11 k8s.io/client-go v0.33.11 @@ -49,13 +47,11 @@ require ( github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-logr/logr v1.4.3 // indirect - github.com/go-logr/stdr v1.2.2 // indirect github.com/go-logr/zapr v1.3.0 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.23.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect - github.com/golang-jwt/jwt/v5 v5.2.2 // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/google/btree v1.1.3 // indirect github.com/google/gnostic-models v0.6.9 // indirect @@ -64,7 +60,6 @@ require ( github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect - github.com/jonboulle/clockwork v0.5.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/mailru/easyjson v0.7.7 // indirect @@ -80,20 +75,11 @@ require ( github.com/prometheus/procfs v0.15.1 // indirect github.com/spf13/pflag v1.0.10 // indirect github.com/x448/float16 v0.8.4 // indirect - github.com/xiang90/probing v0.0.0-20221125231312-a49e3df8f510 // indirect - go.etcd.io/bbolt v1.4.3 // indirect go.etcd.io/etcd/client/pkg/v3 v3.6.11 // indirect - go.etcd.io/etcd/pkg/v3 v3.6.11 // indirect - go.etcd.io/etcd/server/v3 v3.6.11 // indirect - go.etcd.io/raft/v3 v3.6.0 // indirect - go.opentelemetry.io/auto/sdk v1.2.1 // indirect - go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.59.0 // indirect go.opentelemetry.io/otel v1.40.0 // indirect - go.opentelemetry.io/otel/metric v1.40.0 // indirect go.opentelemetry.io/otel/sdk/metric v1.40.0 // indirect - go.opentelemetry.io/otel/trace v1.40.0 // indirect go.uber.org/multierr v1.11.0 // indirect - golang.org/x/crypto v0.49.0 // indirect + go.uber.org/zap v1.27.0 // indirect golang.org/x/net v0.52.0 // indirect golang.org/x/oauth2 v0.34.0 // indirect golang.org/x/sync v0.20.0 // indirect diff --git a/go.sum b/go.sum index 0e9fe0b7..87ba20da 100644 --- a/go.sum +++ b/go.sum @@ -42,12 +42,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= -github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= -github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/cockroachdb/datadriven v1.0.2 h1:H9MtNqVoVhvd9nCBwOyDjUEdZCREqbIdCJD93PBm/jA= -github.com/cockroachdb/datadriven v1.0.2/go.mod h1:a9RdTaap04u637JoCzcUoIcDmvwSUtcUFtT/C3kJlTU= github.com/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr4= github.com/coreos/go-semver v0.3.1/go.mod h1:irMmmIw/7yzSRPWryHsK7EYSg09caPQL03VsM8rvUec= github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs= @@ -69,7 +65,6 @@ github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nos github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E= github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ= -github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= @@ -89,8 +84,6 @@ github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZ github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= -github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8= -github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/btree v1.1.3 h1:CVpQJjYgC4VbzxeGVHfvZrv1ctoYCAI8vbl07Fcxlyg= @@ -109,17 +102,10 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 h1:JeSE6pjso5THxAzdVpqr6/geYxZytqFMBCOtn/ujyeo= github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674/go.mod h1:r4w70xmWCQKmi1ONH4KIaBptdivuRPyosB9RmPlGEwA= -github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 h1:+9834+KizmvFV7pXQGSXQTsaWhq2GjuNUt0aUU0YBYw= -github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.1 h1:qnpSQwGEnkcRpTqNOIR6bJbR0gAorgP9CSALpRcKoAA= -github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.1/go.mod h1:lXGCsh6c22WGtjr+qGHj1otzZpV/1kwTMAqkwZsnWRU= -github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.1.0 h1:pRhl55Yx1eC7BZ1N+BBWwnKaMyD8uC+34TLdndZMAKk= -github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.1.0/go.mod h1:XKMd7iuf/RGPSMJ/U4HP0zS2Z9Fh8Ps9a+6X26m/tmI= github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 h1:5ZPtiqj0JL5oKWmcsq4VMaAW5ukBEgSGXEN89zeH1Jo= github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3/go.mod h1:ndYquD05frm2vACXE1nsccT4oJzjhw2arTS2cpUD1PI= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/jonboulle/clockwork v0.5.0 h1:Hyh9A8u51kptdkR+cqRpT1EebBwTn1oK9YfGYbdFz6I= -github.com/jonboulle/clockwork v0.5.0/go.mod h1:3mZlmanh0g2NDKO5TWZVJAfofYk64M7XN3SzBPjZF60= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= @@ -169,10 +155,6 @@ github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoG github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= -github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= -github.com/soheilhy/cmux v0.1.5 h1:jjzc5WVemNEDTLwv9tlmemhC73tI08BNOIGwBOo10Js= -github.com/soheilhy/cmux v0.1.5/go.mod h1:T7TcVDs9LWfQgPlPsdngu6I6QIoyIFZDDC6sNE1GqG0= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= @@ -189,40 +171,20 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= -github.com/tmc/grpc-websocket-proxy v0.0.0-20220101234140-673ab2c3ae75 h1:6fotK7otjonDflCTK0BCfls4SPy3NcCVb5dqqmbRknE= -github.com/tmc/grpc-websocket-proxy v0.0.0-20220101234140-673ab2c3ae75/go.mod h1:KO6IkyS8Y3j8OdNO85qEYBsRPuteD+YciPomcXdrMnk= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= -github.com/xiang90/probing v0.0.0-20221125231312-a49e3df8f510 h1:S2dVYn90KE98chqDkyE9Z4N61UnQd+KOfgp5Iu53llk= -github.com/xiang90/probing v0.0.0-20221125231312-a49e3df8f510/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -go.etcd.io/bbolt v1.4.3 h1:dEadXpI6G79deX5prL3QRNP6JB8UxVkqo4UPnHaNXJo= -go.etcd.io/bbolt v1.4.3/go.mod h1:tKQlpPaYCVFctUIgFKFnAlvbmB3tpy1vkTnDWohtc0E= go.etcd.io/etcd/api/v3 v3.6.11 h1:XFGTgrJ8nak3kB4NgMG8t7NT+lEeuuvKQAqUHKVgkWQ= go.etcd.io/etcd/api/v3 v3.6.11/go.mod h1:HYfTh0jyh+uFgp6gMbxJteIDYY97yMuYz85Rnw6Gy9o= go.etcd.io/etcd/client/pkg/v3 v3.6.11 h1:e41mp315Yn3QMGPmEzCyLsMINgJXTY/dX8kM++1csxU= go.etcd.io/etcd/client/pkg/v3 v3.6.11/go.mod h1:DysuMe/inqRyC/1tjRR6hReH/VV9Lufs27YKSKBWWJg= go.etcd.io/etcd/client/v3 v3.6.11 h1:LAByD96VmmeuairkvdAcE0RZnrmGz/q3ceeWePo9bwc= go.etcd.io/etcd/client/v3 v3.6.11/go.mod h1:vOTDMCo+fGPEClJqcFEFSqZ+8e7WKV7AyqJjX//HR2w= -go.etcd.io/etcd/etcdutl/v3 v3.6.11 h1:MmpObzUWI3G0EVF3DRbAan7gZ8H28KgDCRr19/IOkCg= -go.etcd.io/etcd/etcdutl/v3 v3.6.11/go.mod h1:slFvkK8Sz+PhwRjCyDTFfS57VSsTdCDYYy0jq302KRo= -go.etcd.io/etcd/pkg/v3 v3.6.11 h1:tPKcVOJHqz1n60DBm3gR1dZ3vtEVOz10oKLn9ytqW1I= -go.etcd.io/etcd/pkg/v3 v3.6.11/go.mod h1:L/M2AmhhJ1+3WFRMiJv4Ra0z2hJGYVcsU6q+58NDFfc= -go.etcd.io/etcd/server/v3 v3.6.11 h1:ltQkUbTRM/YVwZGRGFXi+qcjyECMVoKcoCxISSpvtxg= -go.etcd.io/etcd/server/v3 v3.6.11/go.mod h1:WGWPgjHk4fWKoC1ftSMuPvUbdOBqeqvc/pDBPQgN1aw= -go.etcd.io/raft/v3 v3.6.0 h1:5NtvbDVYpnfZWcIHgGRk9DyzkBIXOi8j+DDp1IcnUWQ= -go.etcd.io/raft/v3 v3.6.0/go.mod h1:nLvLevg6+xrVtHUmVaTcTz603gQPHfh7kUAwV6YpfGo= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= -go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.59.0 h1:rgMkmiGfix9vFJDcDi1PK8WEQP4FLQwLDfhp5ZLpFeE= -go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.59.0/go.mod h1:ijPqXp5P6IRRByFVVg9DY8P5HkxkHE5ARIa+86aXPf4= go.opentelemetry.io/otel v1.40.0 h1:oA5YeOcpRTXq6NN7frwmwFR0Cn3RhTVZvXsP4duvCms= go.opentelemetry.io/otel v1.40.0/go.mod h1:IMb+uXZUKkMXdPddhwAHm6UfOwJyh4ct1ybIlV14J0g= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.34.0 h1:OeNbIYk/2C15ckl7glBlOBp5+WlYsOElzTNmiPW/x60= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.34.0/go.mod h1:7Bept48yIeqxP2OZ9/AqIpYS94h2or0aB4FypJTc8ZM= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.34.0 h1:tgJ0uaNS4c98WRNUEx5U3aDlrDOI5Rs+1Vifcw4DJ8U= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.34.0/go.mod h1:U7HYyW0zt/a9x5J1Kjs+r1f/d4ZHnYFclhYY2+YbeoE= go.opentelemetry.io/otel/metric v1.40.0 h1:rcZe317KPftE2rstWIBitCdVp89A2HqjkxR3c11+p9g= go.opentelemetry.io/otel/metric v1.40.0/go.mod h1:ib/crwQH7N3r5kfiBZQbwrTge743UDc7DTFVZrrXnqc= go.opentelemetry.io/otel/sdk v1.40.0 h1:KHW/jUzgo6wsPh9At46+h4upjtccTmuZCFAc9OJ71f8= @@ -231,8 +193,6 @@ go.opentelemetry.io/otel/sdk/metric v1.40.0 h1:mtmdVqgQkeRxHgRv4qhyJduP3fYJRMX4A go.opentelemetry.io/otel/sdk/metric v1.40.0/go.mod h1:4Z2bGMf0KSK3uRjlczMOeMhKU2rhUqdWNoKcYrtcBPg= go.opentelemetry.io/otel/trace v1.40.0 h1:WA4etStDttCSYuhwvEa8OP8I5EWu24lkOzp+ZYblVjw= go.opentelemetry.io/otel/trace v1.40.0/go.mod h1:zeAhriXecNGP/s2SEG3+Y8X9ujcJOTqQ5RgdEJcawiA= -go.opentelemetry.io/proto/otlp v1.5.0 h1:xJvq7gMzB31/d406fB8U5CBdyQGw4P399D1aQWU/3i4= -go.opentelemetry.io/proto/otlp v1.5.0/go.mod h1:keN8WnHxOy8PG0rQZjJJ5A2ebUoafqWp0eVQ4yIXvJ4= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= @@ -243,8 +203,6 @@ go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.49.0 h1:+Ng2ULVvLHnJ/ZFEq4KdcDd/cfjrrjjNSXNzxg0Y4U4= -golang.org/x/crypto v0.49.0/go.mod h1:ErX4dUh2UM+CFYiXZRTcMpEcN8b/1gxEuv3nODoYtCA= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -302,8 +260,6 @@ gopkg.in/evanphx/json-patch.v4 v4.12.0 h1:n6jtcsulIzXPJaxegRbvFNNrZDjbij7ny3gmSP gopkg.in/evanphx/json-patch.v4 v4.12.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWMG4EsWvDvM72M= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= -gopkg.in/natefinch/lumberjack.v2 v2.2.1 h1:bBRl1b0OH9s/DuPhuXpNl+VtCaJXFZ5/uEFST95x9zc= -gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYskCTPBJVb9jqSc= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/agent/agent.go b/internal/agent/agent.go index c7d54ac7..8a6ad357 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -77,9 +77,21 @@ const ( envInitialCluster = "ETCD_INITIAL_CLUSTER" envInitialToken = "ETCD_INITIAL_CLUSTER_TOKEN" envPeerURLs = "ETCD_PEER_URLS" // comma-separated - envEtcdVersion = "ETCD_VERSION" // cluster's spec.version, for the restore version-compat pre-flight + // envEtcdutlPath is the etcdutl the restore agent execs to rebuild the data + // dir. The restore container runs from the version-matched etcd image, so + // this points at that image's etcdutl — the binary and spec.version share a + // release by construction, which is what lets restore support any etcd minor. + envEtcdutlPath = "ETCDUTL_PATH" + + // install-tools-only: where the operator copies its own binary so the + // restore container (running the etcd image) can exec it. + envToolsDir = "TOOLS_DEST_DIR" ) +// defaultEtcdutlPath is etcdutl's location in the upstream etcd image +// (quay.io/coreos/etcd), used when envEtcdutlPath is unset. +const defaultEtcdutlPath = "/usr/local/bin/etcdutl" + // destination captures the resolved snapshot destination / restore source. type destination struct { kind string // "s3" | "pvc" diff --git a/internal/agent/restore.go b/internal/agent/restore.go index 93da58ab..31f81c05 100644 --- a/internal/agent/restore.go +++ b/internal/agent/restore.go @@ -13,17 +13,15 @@ package agent import ( "context" "fmt" + "io" "os" + "os/exec" "path/filepath" - "strings" "syscall" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/feature/s3/manager" "github.com/aws/aws-sdk-go-v2/service/s3" - etcdversion "go.etcd.io/etcd/api/v3/version" - "go.etcd.io/etcd/etcdutl/v3/snapshot" - "go.uber.org/zap" ) // RunRestore populates the etcd data dir from a snapshot before etcd starts. @@ -68,15 +66,6 @@ func RunRestore(ctx context.Context) error { return fmt.Errorf("restore source requires the exact snapshot file path within the volume (%s); got empty", envPVCSubPath) } - // Version-compat pre-flight: snapshot.Restore (this agent's etcdutl) writes a - // data dir with that etcdutl minor's storage semantics. A data dir rebuilt by - // a different minor than the etcd that will boot on it is unvalidated and can - // fail opaquely at the seed — the exact silent brick restore must avoid. Fail - // early with a clear, actionable message instead. - if err := checkRestoreVersionCompat(os.Getenv(envEtcdVersion), etcdutlMajorMinor()); err != nil { - return err - } - // A prior attempt may have crashed (OOM, node reboot) after staging a // snapshot download but before its deferred cleanup ran. We are past the // member/ no-op gate, so the data dir is uninitialized and any staged @@ -125,30 +114,14 @@ func RunRestore(ctx context.Context) error { } } - // etcdutl restore into a staging dir (it refuses to overwrite an - // existing OutputDataDir), then move member/ into the real data dir so - // etcd's --data-dir stays /var/lib/etcd. + // etcdutl refuses a non-empty output dir, so restore into a fresh staging + // subdir, then move member/ into the real data dir so etcd's --data-dir stays + // /var/lib/etcd. staging := filepath.Join(dataDir, ".restore") _ = os.RemoveAll(staging) // clean any partial prior attempt - var peerURLs []string - if p := os.Getenv(envPeerURLs); p != "" { - peerURLs = strings.Split(p, ",") - } - - mgr := snapshot.NewV3(zap.NewExample()) - if err := mgr.Restore(snapshot.RestoreConfig{ - SnapshotPath: snapPath, - Name: os.Getenv(envMemberName), - OutputDataDir: staging, - PeerURLs: peerURLs, - InitialCluster: os.Getenv(envInitialCluster), - InitialClusterToken: os.Getenv(envInitialToken), - // A clientv3 Maintenance.Snapshot stream has no appended integrity - // hash (unlike `etcdutl snapshot save`), so skip the check. - SkipHashCheck: true, - }); err != nil { - return fmt.Errorf("etcdutl restore: %w", err) + if err := runEtcdutlRestore(ctx, snapPath, staging); err != nil { + return err } if err := os.Rename(filepath.Join(staging, "member"), memberDir); err != nil { @@ -160,35 +133,79 @@ func RunRestore(ctx context.Context) error { return nil } -// etcdutlMajorMinor returns the "X.Y" of the etcd release the restore agent is -// built against. The etcdutl, api, and server modules ship in lockstep under -// one git tag, so the api module's compiled-in version.Version is a reliable -// proxy for the etcdutl that snapshot.Restore uses — and unlike build info it -// is a compile-time constant present in every build mode (including tests). -func etcdutlMajorMinor() string { - return majorMinor(etcdversion.Version) +// runEtcdutlRestore rebuilds the data dir under outputDir from snapPath by +// exec-ing the etcdutl binary shipped in the target etcd image — the version +// this cluster runs — rather than a single compiled-in one, so restore works +// across etcd minors. --skip-hash-check is required: a clientv3 +// Maintenance.Snapshot stream (how the snapshot agent captures snapshots) has +// no appended integrity hash, unlike `etcdutl snapshot save`. +func runEtcdutlRestore(ctx context.Context, snapPath, outputDir string) error { + etcdutl := os.Getenv(envEtcdutlPath) + if etcdutl == "" { + etcdutl = defaultEtcdutlPath + } + + args := []string{ + "snapshot", "restore", snapPath, + "--data-dir", outputDir, + "--name", os.Getenv(envMemberName), + "--initial-cluster", os.Getenv(envInitialCluster), + "--initial-cluster-token", os.Getenv(envInitialToken), + "--skip-hash-check", + } + if p := os.Getenv(envPeerURLs); p != "" { + args = append(args, "--initial-advertise-peer-urls", p) + } + + cmd := exec.CommandContext(ctx, etcdutl, args...) + cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("etcdutl snapshot restore: %w", err) + } + return nil } -// majorMinor extracts "X.Y" from a "X.Y.Z"(-ish) version, or "" if it lacks two -// leading components. -func majorMinor(v string) string { - parts := strings.SplitN(v, ".", 3) - if len(parts) < 2 || parts[0] == "" || parts[1] == "" { - return "" - } - return parts[0] + "." + parts[1] +// RunInstallTools copies the running operator binary into envToolsDir so the +// restore initContainer — which runs from the target etcd image, not the +// operator image — can exec it while still reaching that image's version-matched +// etcdutl. This bridges two distroless images that share no binaries: the etcd +// image has etcdutl but no way to copy it out, so we bring the operator to it. +func RunInstallTools() error { + dest := os.Getenv(envToolsDir) + if dest == "" { + return fmt.Errorf("%s is not set; nowhere to install the operator binary", envToolsDir) + } + self, err := os.Executable() + if err != nil { + return fmt.Errorf("locate running binary: %w", err) + } + if err := os.MkdirAll(dest, 0o755); err != nil { + return fmt.Errorf("create tools dir %s: %w", dest, err) + } + out := filepath.Join(dest, "manager") + if err := copyExecutable(self, out); err != nil { + return err + } + fmt.Printf("install-tools: copied %s to %s\n", self, out) + return nil } -// checkRestoreVersionCompat fails when the cluster's etcd version and the -// restore agent's etcdutl differ in major.minor. Empty/unparseable inputs skip -// the check (best-effort) rather than block a restore. -func checkRestoreVersionCompat(clusterVersion, etcdutlVersion string) error { - cm, um := majorMinor(clusterVersion), majorMinor(etcdutlVersion) - if cm == "" || um == "" { - return nil +func copyExecutable(src, dst string) error { + in, err := os.Open(src) + if err != nil { + return fmt.Errorf("open %s: %w", src, err) + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o755) + if err != nil { + return fmt.Errorf("create %s: %w", dst, err) + } + if _, err := io.Copy(out, in); err != nil { + out.Close() + return fmt.Errorf("copy %s to %s: %w", src, dst, err) } - if cm != um { - return fmt.Errorf("restore is only supported when the cluster's etcd version matches the restore agent's etcdutl (%s.x): spec.version=%s. A data dir rebuilt by a different etcd minor may not boot. Run an etcd %s.x cluster, or use an operator build whose etcdutl matches your etcd version", um, clusterVersion, um) + if err := out.Close(); err != nil { + return fmt.Errorf("finalize %s: %w", dst, err) } return nil } diff --git a/internal/agent/restore_test.go b/internal/agent/restore_test.go index 05031407..81f3d830 100644 --- a/internal/agent/restore_test.go +++ b/internal/agent/restore_test.go @@ -125,53 +125,116 @@ func TestEnsureRestoreSpace(t *testing.T) { } } -func TestCheckRestoreVersionCompat(t *testing.T) { - cases := []struct { - name, cluster, etcdutl string - wantErr bool - }{ - {"exact match", "3.6.11", "3.6", false}, - {"same minor, different patch", "3.6.0", "3.6", false}, - {"minor mismatch (3.5 cluster, 3.6 etcdutl)", "3.5.17", "3.6", true}, - {"major mismatch", "4.0.0", "3.6", true}, - {"empty cluster version skips", "", "3.6", false}, - {"empty etcdutl version skips", "3.5.17", "", false}, - {"unparseable cluster version skips", "garbage", "3.6", false}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - err := checkRestoreVersionCompat(tc.cluster, tc.etcdutl) - if tc.wantErr && err == nil { - t.Errorf("checkRestoreVersionCompat(%q, %q) = nil, want error", tc.cluster, tc.etcdutl) - } - if !tc.wantErr && err != nil { - t.Errorf("checkRestoreVersionCompat(%q, %q) = %v, want nil", tc.cluster, tc.etcdutl, err) - } - }) +// writeFakeEtcdutl writes an executable stub standing in for the etcd image's +// etcdutl: it records its argv to argsFile and, mimicking a real restore, +// creates member/ under the dir passed to --data-dir. Lets the exec path be +// exercised without the etcd binary. +func writeFakeEtcdutl(t *testing.T, argsFile string) string { + t.Helper() + script := "#!/bin/sh\n" + + ": > \"" + argsFile + "\"\n" + + "out=\"\"\nprev=\"\"\n" + + "for a in \"$@\"; do\n" + + " printf '%s\\n' \"$a\" >> \"" + argsFile + "\"\n" + + " if [ \"$prev\" = \"--data-dir\" ]; then out=\"$a\"; fi\n" + + " prev=\"$a\"\n" + + "done\n" + + "mkdir -p \"$out/member\"\n" + + "printf restored > \"$out/member/db\"\n" + path := filepath.Join(t.TempDir(), "etcdutl") + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatalf("write fake etcdutl: %v", err) } + return path } -// RunRestore must reject a version-incompatible restore early (before fetching -// the snapshot), since the agent's etcdutl (3.6.x) would rebuild a data dir an -// older etcd can't boot. The agent's etcdutl is 3.6.x, so a 3.5.x cluster fails. -func TestRunRestore_VersionMismatchFailsEarly(t *testing.T) { +// RunRestore must exec the (version-matched) etcdutl binary to rebuild the data +// dir, pass the member identity plus --skip-hash-check, and move the result into +// place. The etcdutl is faked so the exec path runs without the etcd image. +func TestRunRestore_ExecsEtcdutlAndMovesIntoPlace(t *testing.T) { dataDir := t.TempDir() // empty: no member/ dir, so the no-op gate is passed + mount := t.TempDir() + if err := os.WriteFile(filepath.Join(mount, "snap.db"), []byte("snapshot bytes"), 0o644); err != nil { + t.Fatalf("seed snapshot: %v", err) + } + argsFile := filepath.Join(t.TempDir(), "args") t.Setenv(envDataDir, dataDir) - t.Setenv(envEtcdVersion, "3.5.17") // mismatch vs the 3.6.x etcdutl the agent is built with - // No destination env: if the version gate didn't fire first, loadDestination - // would be the next failure — assert we fail on the version, not that. - t.Setenv(envDestKind, "s3") - t.Setenv(envS3Endpoint, "https://s3.example.com") - t.Setenv(envS3Bucket, "etcd") - t.Setenv(envS3Key, "snap.db") + t.Setenv(envMemberName, "c1-0") + t.Setenv(envInitialCluster, "c1-0=http://c1-0:2380") + t.Setenv(envInitialToken, "tok-xyz") + t.Setenv(envPeerURLs, "http://c1-0:2380") + t.Setenv(envEtcdutlPath, writeFakeEtcdutl(t, argsFile)) + t.Setenv(envDestKind, "pvc") + t.Setenv(envPVCMountPath, mount) + t.Setenv(envPVCSubPath, "snap.db") - err := RunRestore(context.Background()) - if err == nil { - t.Fatal("RunRestore with a mismatched etcd version = nil, want error") + if err := RunRestore(context.Background()); err != nil { + t.Fatalf("RunRestore = %v, want nil", err) + } + + // The restored member/ must be moved into the real data dir, and the staging + // dir cleaned up. + if _, err := os.Stat(filepath.Join(dataDir, "member", "db")); err != nil { + t.Errorf("restored member/ not in place: %v", err) + } + if _, err := os.Stat(filepath.Join(dataDir, ".restore")); !os.IsNotExist(err) { + t.Errorf(".restore staging dir not cleaned up (stat err=%v)", err) + } + + args, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("read recorded args: %v", err) + } + for _, want := range []string{"snapshot", "restore", "--skip-hash-check", "--name", "c1-0", "--initial-cluster-token", "tok-xyz"} { + if !strings.Contains(string(args), want) { + t.Errorf("etcdutl not invoked with %q; got:\n%s", want, args) + } } - if !strings.Contains(err.Error(), "restore is only supported when") { - t.Errorf("error was not the version-compat rejection: %v", err) +} + +// RunInstallTools copies the running binary to TOOLS_DEST_DIR/manager so the +// restore container (etcd image) can exec it. +func TestRunInstallTools(t *testing.T) { + dest := t.TempDir() + t.Setenv(envToolsDir, dest) + + if err := RunInstallTools(); err != nil { + t.Fatalf("RunInstallTools = %v, want nil", err) + } + + out := filepath.Join(dest, "manager") + fi, err := os.Stat(out) + if err != nil { + t.Fatalf("stat copied binary: %v", err) + } + if fi.Mode().Perm()&0o111 == 0 { + t.Errorf("copied binary is not executable: mode %v", fi.Mode()) + } + + self, err := os.Executable() + if err != nil { + t.Fatalf("os.Executable: %v", err) + } + want, err := os.ReadFile(self) + if err != nil { + t.Fatalf("read source binary: %v", err) + } + got, err := os.ReadFile(out) + if err != nil { + t.Fatalf("read copied binary: %v", err) + } + if len(got) != len(want) { + t.Errorf("copied binary size = %d, want %d (source)", len(got), len(want)) + } +} + +// RunInstallTools must fail loudly when its destination is unset rather than +// silently no-op, which would leave the restore container with no binary to exec. +func TestRunInstallTools_NoDestFails(t *testing.T) { + t.Setenv(envToolsDir, "") + if err := RunInstallTools(); err == nil { + t.Error("RunInstallTools with no TOOLS_DEST_DIR = nil, want error") } } diff --git a/main.go b/main.go index 3e22a4cf..015faea1 100644 --- a/main.go +++ b/main.go @@ -107,6 +107,16 @@ func main() { os.Exit(1) } return + case "install-tools": + // Runs from the operator image as the restore seed's first + // initContainer, copying this binary onto a shared volume so the + // restore-agent container — which runs the target etcd image for its + // version-matched etcdutl — can exec it. + if err := agent.RunInstallTools(); err != nil { + fmt.Fprintln(os.Stderr, "install-tools failed:", err) + os.Exit(1) + } + return } } From e4d0a61d6ffd1b7ca81d1f1218a810942a369cb4 Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Sun, 16 Aug 2026 11:57:20 +0400 Subject: [PATCH 2/9] test(restore): cover etcdutl failure aborts without initializing data dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A non-zero etcdutl exit must surface the error and leave member/ absent, never moving a nonexistent restored dir into place — the core "don't silently brick a data dir" contract, previously exercised only on the success path. Signed-off-by: Andrey Kolkov Assisted-By: Claude Opus 4.8 (1M context) --- internal/agent/restore_test.go | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/internal/agent/restore_test.go b/internal/agent/restore_test.go index 81f3d830..555e863d 100644 --- a/internal/agent/restore_test.go +++ b/internal/agent/restore_test.go @@ -193,6 +193,44 @@ func TestRunRestore_ExecsEtcdutlAndMovesIntoPlace(t *testing.T) { } } +// A non-zero etcdutl exit must abort the restore: RunRestore returns the error +// and must NOT move a nonexistent member/ into place, leaving the data dir +// uninitialized. This is the core "never silently brick a data dir" contract. +func TestRunRestore_EtcdutlFailureAborts(t *testing.T) { + dataDir := t.TempDir() // empty: no member/ dir, so the no-op gate is passed + mount := t.TempDir() + if err := os.WriteFile(filepath.Join(mount, "snap.db"), []byte("snapshot bytes"), 0o644); err != nil { + t.Fatalf("seed snapshot: %v", err) + } + // A fake etcdutl that fails and creates nothing. + fake := filepath.Join(t.TempDir(), "etcdutl") + if err := os.WriteFile(fake, []byte("#!/bin/sh\necho boom >&2\nexit 1\n"), 0o755); err != nil { + t.Fatalf("write fake etcdutl: %v", err) + } + + t.Setenv(envDataDir, dataDir) + t.Setenv(envMemberName, "c1-0") + t.Setenv(envInitialCluster, "c1-0=http://c1-0:2380") + t.Setenv(envInitialToken, "tok") + t.Setenv(envEtcdutlPath, fake) + t.Setenv(envDestKind, "pvc") + t.Setenv(envPVCMountPath, mount) + t.Setenv(envPVCSubPath, "snap.db") + + err := RunRestore(context.Background()) + if err == nil { + t.Fatal("RunRestore with a failing etcdutl = nil, want error") + } + if !strings.Contains(err.Error(), "etcdutl snapshot restore") { + t.Errorf("error did not wrap the etcdutl failure: %v", err) + } + // A failed rebuild must leave the data dir uninitialized — never move a + // nonexistent member/ into place. + if _, statErr := os.Stat(filepath.Join(dataDir, "member")); !os.IsNotExist(statErr) { + t.Errorf("member/ exists after a failed restore (stat err=%v); a failed rebuild must not initialize the data dir", statErr) + } +} + // RunInstallTools copies the running binary to TOOLS_DEST_DIR/manager so the // restore container (etcd image) can exec it. func TestRunInstallTools(t *testing.T) { From 2207315ec5b9ba58f31427e6474a9bd8672cce2a Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Sun, 16 Aug 2026 12:06:02 +0400 Subject: [PATCH 3/9] docs(api): describe restore as two initContainers; assert their order The Restore field comment (and its generated CRD description) said "a restore initContainer" (singular); the seed now runs install-tools plus restore. Regenerated the CRD to match, and pinned the correctness-critical ordering (install-tools before restore) in the controller test. Signed-off-by: Andrey Kolkov Assisted-By: Claude Opus 4.8 (1M context) --- api/v1alpha2/etcdmember_types.go | 2 +- .../etcd-operator.cozystack.io_etcdmembers.yaml | 2 +- controllers/restore_initcontainer_test.go | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/api/v1alpha2/etcdmember_types.go b/api/v1alpha2/etcdmember_types.go index 942b8281..46599fd4 100644 --- a/api/v1alpha2/etcdmember_types.go +++ b/api/v1alpha2/etcdmember_types.go @@ -174,7 +174,7 @@ type EtcdMemberSpec struct { // Restore is set only on the bootstrap seed when the parent cluster's // spec.bootstrap.restore is configured. It causes the member controller - // to run a restore initContainer that populates the data dir from the + // to run restore initContainers that populate the data dir from the // snapshot before etcd starts. Inert once the data dir is initialized. // +optional Restore *RestoreSpec `json:"restore,omitempty"` diff --git a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml index e928e619..fbad5192 100644 --- a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml +++ b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml @@ -1177,7 +1177,7 @@ spec: description: |- Restore is set only on the bootstrap seed when the parent cluster's spec.bootstrap.restore is configured. It causes the member controller - to run a restore initContainer that populates the data dir from the + to run restore initContainers that populate the data dir from the snapshot before etcd starts. Inert once the data dir is initialized. properties: source: diff --git a/controllers/restore_initcontainer_test.go b/controllers/restore_initcontainer_test.go index b1e9f0b8..a3006d55 100644 --- a/controllers/restore_initcontainer_test.go +++ b/controllers/restore_initcontainer_test.go @@ -45,6 +45,14 @@ func findInitContainer(pod *corev1.Pod, name string) (corev1.Container, bool) { return corev1.Container{}, false } +func initContainerNames(pod *corev1.Pod) []string { + names := make([]string, len(pod.Spec.InitContainers)) + for i, ic := range pod.Spec.InitContainers { + names[i] = ic.Name + } + return names +} + func TestBuildPod_NoRestoreInitContainerWithoutSpec(t *testing.T) { r := &EtcdMemberReconciler{Scheme: testScheme(t), OperatorImage: "operator:latest"} pod := r.buildPod(seedMember(nil), false) @@ -85,6 +93,12 @@ func TestBuildPod_RestoreInitContainerS3(t *testing.T) { t.Errorf("install-tools restore-tools mount = %+v, want writable at /tools", m) } + // install-tools must precede restore: it stages the binary the restore + // container execs, so the order is correctness-critical. + if got := initContainerNames(pod); len(got) != 2 || got[0] != "install-tools" || got[1] != "restore" { + t.Errorf("initContainer order = %v, want [install-tools restore]", got) + } + ic, ok := findInitContainer(pod, "restore") if !ok { t.Fatal("restore initContainer missing") From 34110763c6e2a95938d5bde2d1c576f2bebbaae2 Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Sun, 16 Aug 2026 12:06:41 +0400 Subject: [PATCH 4/9] docs(chart): the restore container runs the etcd image, not the operator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The image-invariant helper comment said the restore agent runs the operator image; the restore container now runs the target etcd image. Only the snapshot Job and the restore seed's install-tools initContainer share the operator image — which is what the invariant (image == OPERATOR_IMAGE) is about. Signed-off-by: Andrey Kolkov Assisted-By: Claude Opus 4.8 (1M context) --- charts/etcd-operator/templates/_helpers.tpl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/charts/etcd-operator/templates/_helpers.tpl b/charts/etcd-operator/templates/_helpers.tpl index 75c467b5..5e6cfe7c 100644 --- a/charts/etcd-operator/templates/_helpers.tpl +++ b/charts/etcd-operator/templates/_helpers.tpl @@ -52,7 +52,8 @@ would hand those permissions to every workload using the default SA. */ -}} {{/* Full operator image reference. Used for BOTH the manager container image and its OPERATOR_IMAGE env var — they MUST be identical, or the operator refuses to -start (the snapshot/restore agent runs this same image). +start (the snapshot Job and the restore seed's install-tools initContainer run +this same image). */}} {{- define "etcd-operator.image" -}} {{- printf "%s:%s" .Values.image.repository (.Values.image.tag | default .Chart.AppVersion) -}} From 7b81e7deb73c141779086d280bef3f13cec9a0be Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Sun, 16 Aug 2026 12:14:00 +0400 Subject: [PATCH 5/9] style: trim verbose comments on the version-matched restore path Let the code carry the obvious; keep only the non-obvious why (distroless cross-image exec, --skip-hash-check). Signed-off-by: Andrey Kolkov Assisted-By: Claude Opus 4.8 (1M context) --- controllers/etcdmember_controller.go | 30 +++++++++++----------------- internal/agent/agent.go | 14 +++---------- internal/agent/restore.go | 18 +++++++---------- main.go | 6 ++---- 4 files changed, 24 insertions(+), 44 deletions(-) diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index cdd66798..8d48a501 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -870,26 +870,22 @@ func (r *EtcdMemberReconciler) buildPod(member *lll.EtcdMember, clusterFormed bo const restoreSrcMountPath = "/restore/src" -// restoreToolsVolumeName is the shared volume onto which the install-tools -// initContainer copies the operator binary so the restore container (running -// the etcd image) can exec it. restoreToolsMountPath is where both mount it. +// restore-tools carries the operator binary from install-tools to the restore +// container, which runs the etcd image (for its version-matched etcdutl). const ( restoreToolsVolumeName = "restore-tools" restoreToolsMountPath = "/tools" ) // restoreInitContainers builds the ordered initContainers that restore the data -// dir from a snapshot before etcd starts. The rebuild must run a version-matched -// etcdutl, so it runs from the target etcd image rather than the operator image; -// but that image is distroless and ships only etcd binaries, with no way to copy -// etcdutl out to the operator. So a first initContainer copies the operator -// binary onto a shared volume, and the restore container runs the etcd image -// with that binary as its entrypoint — giving the agent both its own logic and -// the image's etcdutl. peerAddr is this member's peer URL; the agent feeds it -// (with the member name / initial-cluster / token) to etcdutl so the restored -// data matches the identity the etcd container will run with. For an S3 source -// the object key is exact (not a prefix); for a PVC source the volume is mounted -// read-only and PVC_SUBPATH points to the snapshot file. +// dir before etcd starts. The rebuild runs a version-matched etcdutl by running +// the agent from the target etcd image; that image can't copy etcdutl out, so +// install-tools first stages the operator binary onto a shared volume for the +// restore container to exec. peerAddr is this member's peer URL, fed (with +// member name / initial-cluster / token) to etcdutl so the restored data matches +// the identity the etcd container runs with. For an S3 source the object key is +// exact (not a prefix); for a PVC source the volume is mounted read-only and +// PVC_SUBPATH points to the snapshot file. func restoreInitContainers(member *lll.EtcdMember, peerAddr, operatorImage, etcdImage string) ([]corev1.Container, []corev1.Volume) { src := member.Spec.Restore.Source env := []corev1.EnvVar{ @@ -959,10 +955,8 @@ func restoreInitContainers(member *lll.EtcdMember, peerAddr, operatorImage, etcd } restore := corev1.Container{ - Name: "restore", - Image: etcdImage, - // Run the operator binary staged by install-tools, from the etcd image — - // so the agent can exec that image's version-matched etcdutl. + Name: "restore", + Image: etcdImage, Command: []string{restoreToolsMountPath + "/manager", "restore-agent"}, Env: env, SecurityContext: restrictedSecurityContext, diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 8a6ad357..641b0f17 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -77,19 +77,11 @@ const ( envInitialCluster = "ETCD_INITIAL_CLUSTER" envInitialToken = "ETCD_INITIAL_CLUSTER_TOKEN" envPeerURLs = "ETCD_PEER_URLS" // comma-separated - // envEtcdutlPath is the etcdutl the restore agent execs to rebuild the data - // dir. The restore container runs from the version-matched etcd image, so - // this points at that image's etcdutl — the binary and spec.version share a - // release by construction, which is what lets restore support any etcd minor. - envEtcdutlPath = "ETCDUTL_PATH" - - // install-tools-only: where the operator copies its own binary so the - // restore container (running the etcd image) can exec it. - envToolsDir = "TOOLS_DEST_DIR" + envEtcdutlPath = "ETCDUTL_PATH" // etcdutl to exec; default is the etcd image's + envToolsDir = "TOOLS_DEST_DIR" // where install-tools copies the operator binary ) -// defaultEtcdutlPath is etcdutl's location in the upstream etcd image -// (quay.io/coreos/etcd), used when envEtcdutlPath is unset. +// defaultEtcdutlPath is etcdutl's location in the upstream etcd image. const defaultEtcdutlPath = "/usr/local/bin/etcdutl" // destination captures the resolved snapshot destination / restore source. diff --git a/internal/agent/restore.go b/internal/agent/restore.go index 31f81c05..602f3cf1 100644 --- a/internal/agent/restore.go +++ b/internal/agent/restore.go @@ -133,12 +133,10 @@ func RunRestore(ctx context.Context) error { return nil } -// runEtcdutlRestore rebuilds the data dir under outputDir from snapPath by -// exec-ing the etcdutl binary shipped in the target etcd image — the version -// this cluster runs — rather than a single compiled-in one, so restore works -// across etcd minors. --skip-hash-check is required: a clientv3 -// Maintenance.Snapshot stream (how the snapshot agent captures snapshots) has -// no appended integrity hash, unlike `etcdutl snapshot save`. +// runEtcdutlRestore rebuilds the data dir under outputDir from snapPath, exec-ing +// the etcd image's own etcdutl (envEtcdutlPath) rather than a compiled-in one so +// it matches the target etcd version. --skip-hash-check is required: a clientv3 +// Maintenance.Snapshot stream carries no integrity hash. func runEtcdutlRestore(ctx context.Context, snapPath, outputDir string) error { etcdutl := os.Getenv(envEtcdutlPath) if etcdutl == "" { @@ -165,11 +163,9 @@ func runEtcdutlRestore(ctx context.Context, snapPath, outputDir string) error { return nil } -// RunInstallTools copies the running operator binary into envToolsDir so the -// restore initContainer — which runs from the target etcd image, not the -// operator image — can exec it while still reaching that image's version-matched -// etcdutl. This bridges two distroless images that share no binaries: the etcd -// image has etcdutl but no way to copy it out, so we bring the operator to it. +// RunInstallTools copies the running operator binary into envToolsDir, so the +// restore initContainer can exec it from the etcd image — which ships etcdutl +// but can't copy it out — while keeping the agent's own logic. func RunInstallTools() error { dest := os.Getenv(envToolsDir) if dest == "" { diff --git a/main.go b/main.go index 015faea1..6d961ce3 100644 --- a/main.go +++ b/main.go @@ -108,10 +108,8 @@ func main() { } return case "install-tools": - // Runs from the operator image as the restore seed's first - // initContainer, copying this binary onto a shared volume so the - // restore-agent container — which runs the target etcd image for its - // version-matched etcdutl — can exec it. + // Stages this binary for the restore-agent container to exec from the + // etcd image; no cluster I/O, so no timeout needed. if err := agent.RunInstallTools(); err != nil { fmt.Fprintln(os.Stderr, "install-tools failed:", err) os.Exit(1) From 9016b63ff142a036f03072c8281c83418947ffb6 Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Mon, 17 Aug 2026 15:20:26 +0400 Subject: [PATCH 6/9] =?UTF-8?q?fix(restore):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20early=20etcdutl=20check,=20PATH=20fallback,=20docs,?= =?UTF-8?q?=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - M2: resolve etcdutl BEFORE fetching the snapshot, so a target image with no etcdutl (etcd < 3.5 ships none) fails immediately with an actionable message instead of after a full S3 download and an indefinite re-download CrashLoop — restoring this path's fails-early guarantee. - M3: fall back to exec.LookPath("etcdutl") when it isn't at the default path, so ETCDUTL_PATH is no longer the only escape hatch for images that lay it out elsewhere (e.g. Bitnami). - M1: docs no longer overclaim — the guarantee is etcdutl↔etcd by construction; a snapshot from a newer etcd restored into an older spec.version is still unsupported (snapshot origin version isn't recorded or checked). - L4: test the version→image mapping across two versions, assert the restore-tools Volume exists (not just the mounts), and cover the peer-URL flag (present and the omitted-when-unset path) plus the missing-etcdutl fail-early path. - Give each restore initContainer its own SecurityContext (no shared pointer). Assisted-By: Claude Opus 4.8 (1M context) Signed-off-by: Andrey Kolkov --- controllers/etcdmember_controller.go | 14 +++-- controllers/restore_initcontainer_test.go | 26 +++++++++ docs/installation.md | 2 +- docs/operations.md | 4 +- internal/agent/restore.go | 42 +++++++++++--- internal/agent/restore_test.go | 67 ++++++++++++++++++++++- 6 files changed, 138 insertions(+), 17 deletions(-) diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index 8d48a501..d116cf86 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -937,9 +937,13 @@ func restoreInitContainers(member *lll.EtcdMember, peerAddr, operatorImage, etcd mounts = append(mounts, corev1.VolumeMount{Name: "restore-src", MountPath: restoreSrcMountPath, ReadOnly: true}) } - restrictedSecurityContext := &corev1.SecurityContext{ - AllowPrivilegeEscalation: ptrBool(false), - Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + // A fresh SecurityContext per container — not one pointer shared by both — + // so a later edit to one can't silently mutate the other. + restrictedSecurityContext := func() *corev1.SecurityContext { + return &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptrBool(false), + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + } } installTools := corev1.Container{ @@ -947,7 +951,7 @@ func restoreInitContainers(member *lll.EtcdMember, peerAddr, operatorImage, etcd Image: operatorImage, Command: []string{"/manager", "install-tools"}, Env: []corev1.EnvVar{{Name: "TOOLS_DEST_DIR", Value: restoreToolsMountPath}}, - SecurityContext: restrictedSecurityContext, + SecurityContext: restrictedSecurityContext(), VolumeMounts: []corev1.VolumeMount{ {Name: restoreToolsVolumeName, MountPath: restoreToolsMountPath}, }, @@ -959,7 +963,7 @@ func restoreInitContainers(member *lll.EtcdMember, peerAddr, operatorImage, etcd Image: etcdImage, Command: []string{restoreToolsMountPath + "/manager", "restore-agent"}, Env: env, - SecurityContext: restrictedSecurityContext, + SecurityContext: restrictedSecurityContext(), VolumeMounts: mounts, Resources: restoreAgentResources(), } diff --git a/controllers/restore_initcontainer_test.go b/controllers/restore_initcontainer_test.go index a3006d55..b4acec54 100644 --- a/controllers/restore_initcontainer_test.go +++ b/controllers/restore_initcontainer_test.go @@ -53,6 +53,27 @@ func initContainerNames(pod *corev1.Pod) []string { return names } +// The restore container's image must track spec.version exactly — the property +// the whole feature rests on. Asserting it at a single version elsewhere does +// not prove it varies with the version. +func TestBuildPod_RestoreImageTracksVersion(t *testing.T) { + r := &EtcdMemberReconciler{Scheme: testScheme(t), OperatorImage: "operator:latest"} + for _, version := range []string{"3.5.21", "3.6.11"} { + m := seedMember(&lll.RestoreSpec{Source: lll.SnapshotLocation{ + PVC: &lll.PVCSnapshotLocation{ClaimName: "snap-pvc", SubPath: "b1.db"}, + }}) + m.Spec.Version = version + pod := r.buildPod(m, false) + ic, ok := findInitContainer(pod, "restore") + if !ok { + t.Fatalf("version %s: restore initContainer missing", version) + } + if want := "quay.io/coreos/etcd:v" + version; ic.Image != want { + t.Errorf("version %s: restore image = %q, want %q", version, ic.Image, want) + } + } +} + func TestBuildPod_NoRestoreInitContainerWithoutSpec(t *testing.T) { r := &EtcdMemberReconciler{Scheme: testScheme(t), OperatorImage: "operator:latest"} pod := r.buildPod(seedMember(nil), false) @@ -115,6 +136,11 @@ func TestBuildPod_RestoreInitContainerS3(t *testing.T) { if m, ok := mountByName(ic.VolumeMounts, "restore-tools"); !ok || m.MountPath != "/tools" { t.Errorf("restore restore-tools mount = %+v, want /tools", m) } + // Both containers mount restore-tools by name; the backing Volume must + // actually exist, or the Pod is rejected at create and bootstrap bricks. + if v, ok := volumeByName(pod.Spec.Volumes, "restore-tools"); !ok || v.EmptyDir == nil { + t.Errorf("restore-tools volume = %+v, want an emptyDir", v) + } // Restore identity must match what the etcd container will run with. vals, secretKeys := envMap(ic.Env) diff --git a/docs/installation.md b/docs/installation.md index 6049b2d6..3ab5bd1a 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -261,7 +261,7 @@ By default `spec.version` in an `EtcdCluster` becomes `quay.io/coreos/etcd:v`) rather than a single bundled one (see the [restore runbook](operations.md#restoring-a-cluster-from-a-snapshot)). +The `spec.version` examples throughout these docs use **3.6.x** for consistency, but any supported etcd version works — including on the restore path, which rebuilds the data dir with the `etcdutl` from the target etcd image (`v`) rather than a single bundled one. That keeps `etcdutl` in lockstep with the etcd that boots on the result; it does **not** validate the snapshot's own origin version, so restoring a snapshot from a newer etcd into an older `spec.version` remains unsupported (see the [restore runbook](operations.md#restoring-a-cluster-from-a-snapshot)). Operator's own toolchain (relevant when building from source): diff --git a/docs/operations.md b/docs/operations.md index b63f7a7c..c05143f7 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -280,7 +280,9 @@ kubectl logs -n job/my-etcd-2026-06-02-snapshot Restore is a **first-bootstrap-only** path: a *new* cluster initializes its seed member's data dir from a snapshot instead of starting empty. You cannot restore into an existing, already-bootstrapped cluster (`spec.bootstrap` is immutable post-create) — delete and recreate. -> **Restore rebuilds the data dir with a version-matched `etcdutl`.** The restore agent runs the `etcdutl` bundled in the target etcd image (`v`), the very version that then boots on the rebuilt data dir — so the on-disk storage format matches by construction. Restore works for any etcd version the operator supports; there is no requirement that `spec.version` match the operator's own build. +> **Restore rebuilds the data dir with a version-matched `etcdutl`.** The restore agent runs the `etcdutl` bundled in the target etcd image (`v`), the very version that then boots on the rebuilt data dir — so the `etcdutl`↔etcd on-disk format matches by construction, for any etcd version the operator supports, with no requirement that `spec.version` match the operator's own build. +> +> This guarantees `etcdutl`↔etcd, **not** snapshot↔etcd: the snapshot's origin version is not recorded or checked anywhere. Restoring a snapshot taken from a **newer** etcd into an **older** `spec.version` (e.g. a 3.6 snapshot into a `3.5.x` cluster) runs an older `etcdutl` over a db written by a newer minor — unvalidated and unsupported. Restore into `spec.version` at or above the snapshot's source version. > **⚠️ Restoring a snapshot from an auth-enabled cluster.** An etcd snapshot captures the data store *including its auth state* — users, roles, and the auth-enabled flag. A snapshot taken while auth was on restores into a cluster where **etcd boots with auth already ON**. You must therefore set `spec.auth` on the new `EtcdCluster` to match, or the operator can never manage it: > diff --git a/internal/agent/restore.go b/internal/agent/restore.go index 602f3cf1..acfd0306 100644 --- a/internal/agent/restore.go +++ b/internal/agent/restore.go @@ -66,6 +66,13 @@ func RunRestore(ctx context.Context) error { return fmt.Errorf("restore source requires the exact snapshot file path within the volume (%s); got empty", envPVCSubPath) } + // Resolve etcdutl before fetching the snapshot: a target etcd image that + // ships no etcdutl (etcd < 3.5) must fail here, not after a full download. + etcdutl, err := resolveEtcdutl() + if err != nil { + return err + } + // A prior attempt may have crashed (OOM, node reboot) after staging a // snapshot download but before its deferred cleanup ran. We are past the // member/ no-op gate, so the data dir is uninitialized and any staged @@ -120,7 +127,7 @@ func RunRestore(ctx context.Context) error { staging := filepath.Join(dataDir, ".restore") _ = os.RemoveAll(staging) // clean any partial prior attempt - if err := runEtcdutlRestore(ctx, snapPath, staging); err != nil { + if err := runEtcdutlRestore(ctx, etcdutl, snapPath, staging); err != nil { return err } @@ -133,16 +140,33 @@ func RunRestore(ctx context.Context) error { return nil } -// runEtcdutlRestore rebuilds the data dir under outputDir from snapPath, exec-ing -// the etcd image's own etcdutl (envEtcdutlPath) rather than a compiled-in one so -// it matches the target etcd version. --skip-hash-check is required: a clientv3 -// Maintenance.Snapshot stream carries no integrity hash. -func runEtcdutlRestore(ctx context.Context, snapPath, outputDir string) error { - etcdutl := os.Getenv(envEtcdutlPath) - if etcdutl == "" { - etcdutl = defaultEtcdutlPath +// resolveEtcdutl locates the etcdutl binary to exec: ETCDUTL_PATH if set, the +// etcd image's usual /usr/local/bin/etcdutl next, and finally whatever is on +// PATH (covers images that lay it out elsewhere, e.g. Bitnami). Resolved and +// existence-checked before the snapshot is fetched so a target image with no +// etcdutl (etcd < 3.5 ships none) fails immediately instead of after a full +// download and an indefinite re-download CrashLoop. +func resolveEtcdutl() (string, error) { + if p := os.Getenv(envEtcdutlPath); p != "" { + if _, err := os.Stat(p); err != nil { + return "", fmt.Errorf("etcdutl (%s=%s): %w", envEtcdutlPath, p, err) + } + return p, nil + } + if _, err := os.Stat(defaultEtcdutlPath); err == nil { + return defaultEtcdutlPath, nil + } + if p, err := exec.LookPath("etcdutl"); err == nil { + return p, nil } + return "", fmt.Errorf("etcdutl not found at %s or on PATH; the target etcd image must ship etcdutl (etcd < 3.5 does not) — check spec.version", defaultEtcdutlPath) +} +// runEtcdutlRestore rebuilds the data dir under outputDir from snapPath, exec-ing +// the etcd image's own etcdutl (resolved by resolveEtcdutl) rather than a +// compiled-in one so it matches the target etcd version. --skip-hash-check is +// required: a clientv3 Maintenance.Snapshot stream carries no integrity hash. +func runEtcdutlRestore(ctx context.Context, etcdutl, snapPath, outputDir string) error { args := []string{ "snapshot", "restore", snapPath, "--data-dir", outputDir, diff --git a/internal/agent/restore_test.go b/internal/agent/restore_test.go index 555e863d..82f4ccc8 100644 --- a/internal/agent/restore_test.go +++ b/internal/agent/restore_test.go @@ -186,13 +186,77 @@ func TestRunRestore_ExecsEtcdutlAndMovesIntoPlace(t *testing.T) { if err != nil { t.Fatalf("read recorded args: %v", err) } - for _, want := range []string{"snapshot", "restore", "--skip-hash-check", "--name", "c1-0", "--initial-cluster-token", "tok-xyz"} { + for _, want := range []string{ + "snapshot", "restore", "--skip-hash-check", + "--name", "c1-0", + "--initial-cluster", "c1-0=http://c1-0:2380", + "--initial-cluster-token", "tok-xyz", + "--initial-advertise-peer-urls", "http://c1-0:2380", + } { if !strings.Contains(string(args), want) { t.Errorf("etcdutl not invoked with %q; got:\n%s", want, args) } } } +// ETCD_PEER_URLS is unset for some sources; --initial-advertise-peer-urls must +// then be omitted (etcdutl's VerifyBootstrap rejects its localhost:2380 default +// against a real --initial-cluster, so a spurious flag would fail closed). This +// pins the conditional shape that changed from the old []string field. +func TestRunRestore_OmitsPeerURLsFlagWhenUnset(t *testing.T) { + dataDir := t.TempDir() + mount := t.TempDir() + if err := os.WriteFile(filepath.Join(mount, "snap.db"), []byte("snapshot bytes"), 0o644); err != nil { + t.Fatalf("seed snapshot: %v", err) + } + argsFile := filepath.Join(t.TempDir(), "args") + + t.Setenv(envDataDir, dataDir) + t.Setenv(envMemberName, "c1-0") + t.Setenv(envInitialCluster, "c1-0=http://c1-0:2380") + t.Setenv(envInitialToken, "tok-xyz") + // envPeerURLs deliberately left unset. + t.Setenv(envEtcdutlPath, writeFakeEtcdutl(t, argsFile)) + t.Setenv(envDestKind, "pvc") + t.Setenv(envPVCMountPath, mount) + t.Setenv(envPVCSubPath, "snap.db") + + if err := RunRestore(context.Background()); err != nil { + t.Fatalf("RunRestore = %v, want nil", err) + } + args, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("read recorded args: %v", err) + } + if strings.Contains(string(args), "--initial-advertise-peer-urls") { + t.Errorf("--initial-advertise-peer-urls passed though ETCD_PEER_URLS was unset; got:\n%s", args) + } +} + +// A target image with no etcdutl (etcd < 3.5) must fail BEFORE the snapshot is +// fetched — the "fails early" guarantee — with an actionable message, not after +// a full download. +func TestRunRestore_MissingEtcdutlFailsBeforeFetch(t *testing.T) { + dataDir := t.TempDir() + mount := t.TempDir() // empty: the snapshot file is absent, so a fetch would fail first if it ran + + t.Setenv(envDataDir, dataDir) + t.Setenv(envMemberName, "c1-0") + // Point at a path that does not exist — stands in for an image with no etcdutl. + t.Setenv(envEtcdutlPath, filepath.Join(t.TempDir(), "no-etcdutl-here")) + t.Setenv(envDestKind, "pvc") + t.Setenv(envPVCMountPath, mount) + t.Setenv(envPVCSubPath, "snap.db") + + err := RunRestore(context.Background()) + if err == nil { + t.Fatal("RunRestore with no etcdutl = nil, want error") + } + if !strings.Contains(err.Error(), "etcdutl") { + t.Errorf("error did not mention etcdutl: %v", err) + } +} + // A non-zero etcdutl exit must abort the restore: RunRestore returns the error // and must NOT move a nonexistent member/ into place, leaving the data dir // uninitialized. This is the core "never silently brick a data dir" contract. @@ -393,6 +457,7 @@ func TestRunRestore_PVCDirectorySourceFails(t *testing.T) { } t.Setenv(envDataDir, dataDir) + t.Setenv(envEtcdutlPath, writeFakeEtcdutl(t, filepath.Join(t.TempDir(), "args"))) // resolve passes; the directory check is what must fire t.Setenv(envDestKind, "pvc") t.Setenv(envPVCMountPath, mount) t.Setenv(envPVCSubPath, "subdir") // exists, but is a directory From 7fbc90e46f36c7194fd047580436a4f6348a59f1 Mon Sep 17 00:00:00 2001 From: Timofei Larkin Date: Mon, 17 Aug 2026 15:50:33 +0300 Subject: [PATCH 7/9] docs(restore): carry the snapshot-origin caveat into the concepts doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runbook and the installation guide now say the version lockstep is etcdutl<->etcd and does not extend to the snapshot's own origin version. The concepts doc still claimed, unqualified, that restore works for any etcd version the operator supports — the one place left stating the broad form. Also make copyExecutable's discarded Closes explicit: the read side and the copy-failure path drop their errors deliberately, and only the success-path Close can hide a lost write. Assisted-By: Claude Opus 5 Signed-off-by: Timofei Larkin --- docs/concepts.md | 2 +- internal/agent/restore.go | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/concepts.md b/docs/concepts.md index 09504832..5459dfe9 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -495,7 +495,7 @@ Restore is a first-bootstrap-only path, not a controller that mutates a running The init container is idempotent: it no-ops if the data dir already contains a `member/` directory, so Pod restarts after first boot leave live data untouched and never re-download. Because `spec.bootstrap` is CEL-immutable post-create, the restore intent can't be added to or changed on a live cluster — restore happens once, at birth, or not at all. A restored cluster gets a fresh etcd cluster ID: it is a new cluster seeded with old data, not a continuation. -The rebuild's `etcdutl` is the one bundled in the target etcd image (`v`), not one compiled into the operator. Since a data dir's on-disk storage format is minor-version-specific, running the `etcdutl` that ships with the very etcd that will boot on the result keeps the two in lockstep by construction — so restore works for any etcd version the operator supports, not only the operator's own minor. The two init containers exist to bridge two distroless images that share no binaries: the etcd image has `etcdutl` but no way to copy it out, so `install-tools` brings the operator binary to the etcd image instead. +The rebuild's `etcdutl` is the one bundled in the target etcd image (`v`), not one compiled into the operator. Since a data dir's on-disk storage format is minor-version-specific, running the `etcdutl` that ships with the very etcd that will boot on the result keeps the two in lockstep by construction — so restore works for any etcd version the operator supports, not only the operator's own minor. That lockstep is `etcdutl`↔etcd only: the snapshot's own origin version is neither recorded nor checked, so restoring a snapshot taken from a newer etcd into an older `spec.version` remains unsupported (see the [restore runbook](operations.md#restoring-a-cluster-from-a-snapshot)). The two init containers exist to bridge two distroless images that share no binaries: the etcd image has `etcdutl` but no way to copy it out, so `install-tools` brings the operator binary to the etcd image instead. This idempotency relies on the data dir being **persistent**. Restore is therefore rejected (by CEL) together with `spec.storage.medium: Memory`: a tmpfs data dir is wiped on every Pod restart, which would defeat the `member/`-exists guard and silently re-restore the original snapshot — reverting any writes since the restore, or breaking a multi-member cluster whose other members already moved past the restored cluster ID. Restore onto memory-backed storage is unsupported; use a PVC-backed cluster. diff --git a/internal/agent/restore.go b/internal/agent/restore.go index acfd0306..bae1e264 100644 --- a/internal/agent/restore.go +++ b/internal/agent/restore.go @@ -215,13 +215,16 @@ func copyExecutable(src, dst string) error { if err != nil { return fmt.Errorf("open %s: %w", src, err) } - defer in.Close() + // Discarded deliberately: src is read-only, and on the copy failure below the + // io.Copy error is the one worth reporting. Only the success-path Close can + // surface a lost write, and that one is checked. + defer func() { _ = in.Close() }() out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o755) if err != nil { return fmt.Errorf("create %s: %w", dst, err) } if _, err := io.Copy(out, in); err != nil { - out.Close() + _ = out.Close() return fmt.Errorf("copy %s to %s: %w", src, dst, err) } if err := out.Close(); err != nil { From 7d28265ec6800cd6aab8780e2c7d76ffa20a26ac Mon Sep 17 00:00:00 2001 From: Timofei Larkin Date: Mon, 17 Aug 2026 16:59:48 +0300 Subject: [PATCH 8/9] fix(restore): pre-flight etcdutl for runnability, not mere existence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit os.Stat succeeds for a directory and for a non-executable file, so both passed the pre-flight, the snapshot was fetched, and the failure landed at exec — the late failure the pre-flight exists to prevent. Check for a regular file with an execute bit instead, which is what exec.LookPath already does for its own branch. A non-executable file at the default path now falls through to PATH rather than being returned. Assisted-By: Claude Opus 5 Signed-off-by: Timofei Larkin --- internal/agent/restore.go | 22 +++++++++++++++++++-- internal/agent/restore_test.go | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/internal/agent/restore.go b/internal/agent/restore.go index bae1e264..5bcb37b3 100644 --- a/internal/agent/restore.go +++ b/internal/agent/restore.go @@ -148,12 +148,12 @@ func RunRestore(ctx context.Context) error { // download and an indefinite re-download CrashLoop. func resolveEtcdutl() (string, error) { if p := os.Getenv(envEtcdutlPath); p != "" { - if _, err := os.Stat(p); err != nil { + if err := executableFile(p); err != nil { return "", fmt.Errorf("etcdutl (%s=%s): %w", envEtcdutlPath, p, err) } return p, nil } - if _, err := os.Stat(defaultEtcdutlPath); err == nil { + if err := executableFile(defaultEtcdutlPath); err == nil { return defaultEtcdutlPath, nil } if p, err := exec.LookPath("etcdutl"); err == nil { @@ -162,6 +162,24 @@ func resolveEtcdutl() (string, error) { return "", fmt.Errorf("etcdutl not found at %s or on PATH; the target etcd image must ship etcdutl (etcd < 3.5 does not) — check spec.version", defaultEtcdutlPath) } +// executableFile checks that path is something exec can actually run. A bare +// os.Stat also accepts directories and non-executable files, which would pass +// the pre-flight and then fail at exec — after the snapshot download the +// pre-flight exists to happen before. (exec.LookPath already checks this.) +func executableFile(path string) error { + fi, err := os.Stat(path) + if err != nil { + return err + } + if !fi.Mode().IsRegular() { + return fmt.Errorf("not a regular file (%s)", fi.Mode().Type()) + } + if fi.Mode().Perm()&0o111 == 0 { + return fmt.Errorf("not executable (mode %s)", fi.Mode().Perm()) + } + return nil +} + // runEtcdutlRestore rebuilds the data dir under outputDir from snapPath, exec-ing // the etcd image's own etcdutl (resolved by resolveEtcdutl) rather than a // compiled-in one so it matches the target etcd version. --skip-hash-check is diff --git a/internal/agent/restore_test.go b/internal/agent/restore_test.go index 82f4ccc8..bd76efc5 100644 --- a/internal/agent/restore_test.go +++ b/internal/agent/restore_test.go @@ -257,6 +257,42 @@ func TestRunRestore_MissingEtcdutlFailsBeforeFetch(t *testing.T) { } } +// os.Stat succeeds for a directory and for a non-executable file, so the +// pre-flight must check what exec actually needs. Otherwise these pass the gate, +// the snapshot is fetched, and the failure lands at exec — the late failure the +// pre-flight exists to prevent. +func TestRunRestore_UnrunnableEtcdutlFailsBeforeFetch(t *testing.T) { + nonExec := filepath.Join(t.TempDir(), "etcdutl") + if err := os.WriteFile(nonExec, []byte("#!/bin/sh\nexit 0\n"), 0o644); err != nil { + t.Fatalf("write non-executable etcdutl: %v", err) + } + cases := map[string]string{ + "directory": t.TempDir(), + "non-executable file": nonExec, + } + for name, path := range cases { + t.Run(name, func(t *testing.T) { + dataDir := t.TempDir() + mount := t.TempDir() // empty: a fetch would fail first if it ran + + t.Setenv(envDataDir, dataDir) + t.Setenv(envMemberName, "c1-0") + t.Setenv(envEtcdutlPath, path) + t.Setenv(envDestKind, "pvc") + t.Setenv(envPVCMountPath, mount) + t.Setenv(envPVCSubPath, "snap.db") + + err := RunRestore(context.Background()) + if err == nil { + t.Fatal("RunRestore with an unrunnable etcdutl = nil, want error") + } + if !strings.Contains(err.Error(), "etcdutl") { + t.Errorf("error did not mention etcdutl: %v", err) + } + }) + } +} + // A non-zero etcdutl exit must abort the restore: RunRestore returns the error // and must NOT move a nonexistent member/ into place, leaving the data dir // uninitialized. This is the core "never silently brick a data dir" contract. From 5fc57aa8af58b5f5cf300298549e8163e92aa1da Mon Sep 17 00:00:00 2001 From: Timofei Larkin Date: Mon, 17 Aug 2026 17:00:30 +0300 Subject: [PATCH 9/9] docs(restore): point the runbook at both init containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The troubleshooting snippet still named a single "restore" container. The seed now runs install-tools first, and a staging failure leaves restore never started — so the one log an operator is told to read is empty in exactly the case they are debugging. Assisted-By: Claude Opus 5 Signed-off-by: Timofei Larkin --- docs/operations.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/operations.md b/docs/operations.md index c05143f7..0e1d64b2 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -316,9 +316,12 @@ spec: name: s3-creds EOF -# The seed Pod runs an init container named "restore" before etcd starts. +# The seed Pod runs two init containers before etcd starts, in this order: +# "install-tools" stages the agent binary, then "restore" rebuilds the data dir. kubectl get etcdcluster.etcd-operator.cozystack.io my-etcd -n -w kubectl logs -n -c restore +# If "restore" never starts, staging failed — look there instead: +kubectl logs -n -c install-tools ``` Notes: