feat(restore): run a version-matched etcdutl per target etcd version (#360)

Author: lllamnyp

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"`

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:

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) -}}

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,26 +870,39 @@ 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) {
+// 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 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{
 		{Name: "ETCD_DATA_DIR", Value: "/var/lib/etcd"},
 		{Name: "ETCD_MEMBER_NAME", Value: member.Name},
 		{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 +937,38 @@ 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{
+	// 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{
+		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,
+		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

controllers/restore_initcontainer_test.go

@@ -45,12 +45,44 @@ 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
+}
+
+// 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)
 	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 +98,48 @@ 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)
+	}
+
+	// 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")
 	}
-	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)
+	}
+	// 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 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)
+	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.
@@ -91,11 +156,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)
 	}

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<version>`) 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<version>`), 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<spec.version>`), 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.