From 5b8f20b89fb4dde1a316d929579d4bc804015116 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Mon, 27 Jul 2026 12:47:42 +1000 Subject: [PATCH] test: assert ModifyVolume resize reaches the guest block device (#585) --- tests/e2e/harness/guest_data.go | 20 +++ tests/e2e/single/guestchurn_test.go | 33 ++-- tests/e2e/single/volume_test.go | 270 +++++++++++++++++++++++----- 3 files changed, 263 insertions(+), 60 deletions(-) diff --git a/tests/e2e/harness/guest_data.go b/tests/e2e/harness/guest_data.go index cc42af5b0..88dec1bc9 100644 --- a/tests/e2e/harness/guest_data.go +++ b/tests/e2e/harness/guest_data.go @@ -124,6 +124,26 @@ func WaitForNewGuestDisk(t *testing.T, tgt SSHTarget, before map[string]struct{} return found } +// LsblkDeviceBytes returns the raw byte size of /dev/ as the guest kernel +// sees it. Byte-exact rather than GiB-rounded (unlike LsblkRootGiB) so a resize +// that lands short by less than 1 GiB still fails a caller's comparison. +func LsblkDeviceBytes(t *testing.T, tgt SSHTarget, dev string) int64 { + t.Helper() + out, err := GuestExec(tgt, fmt.Sprintf("lsblk -b -d -n -o SIZE /dev/%s", dev)) + if err != nil { + t.Fatalf("LsblkDeviceBytes(%s): %v\n%s", dev, err, out) + } + // Read the last non-empty line rather than the whole output: an SSH banner + // or sudo notice ahead of the size would otherwise break the parse. + lines := strings.Split(strings.TrimSpace(out), "\n") + raw := strings.TrimSpace(lines[len(lines)-1]) + size, err := strconv.ParseInt(raw, 10, 64) + if err != nil { + t.Fatalf("LsblkDeviceBytes(%s): parse %q: %v\nfull output:\n%s", dev, raw, err, out) + } + return size +} + // GuestFormatWriteSentinel formats /dev/ as ext4 with the given label, // mounts it, writes sizeMiB of random data to the sentinel file, fsyncs, and // returns the file's sha256. Unmounts before returning so the volume can be diff --git a/tests/e2e/single/guestchurn_test.go b/tests/e2e/single/guestchurn_test.go index ef1da0def..d64efb8c3 100644 --- a/tests/e2e/single/guestchurn_test.go +++ b/tests/e2e/single/guestchurn_test.go @@ -170,7 +170,7 @@ func runGuestChurnDurability(t *testing.T, fix *Fixture) { var wantSha string writeOK := t.Run("WriteSentinel", func(t *testing.T) { - tgt := resolveGuestChurnTarget(t, fix, instanceID, keyPath) + tgt := resolveGuestSSHTarget(t, fix, instanceID, keyPath) harness.Step(t, "create-volume size=1 az=%s", az) // e2e:allow-create — the sentinel volume is the subject under test. @@ -219,7 +219,7 @@ func runGuestChurnDurability(t *testing.T, fix *Fixture) { // invalidates every later perturbation's result. verifySentinel := func(t *testing.T, stage string) { t.Helper() - tgt := resolveGuestChurnTarget(t, fix, instanceID, keyPath) + tgt := resolveGuestSSHTarget(t, fix, instanceID, keyPath) gotSha := harness.GuestReadSentinelSha(t, tgt, "/dev/disk/by-label/"+guestChurnSentinelLabel, guestChurnSentinelLabel) require.Equalf(t, wantSha, gotSha, "sentinel sha256 mismatch after %s", stage) harness.Detail(t, "sentinel_verified_after", stage, "sha256", gotSha) @@ -251,7 +251,7 @@ func runGuestChurnRound(t *testing.T, fix *Fixture, instanceID, origType, keyPat harness.SkipIfNoOVN(t) requireSSHHealthy(t) - tgt := resolveGuestChurnTarget(t, fix, instanceID, keyPath) + tgt := resolveGuestSSHTarget(t, fix, instanceID, keyPath) def := harness.EnsureDefaultVPC(t, fix.Harness) require.NotEmpty(t, def.SubnetID, "default subnet ID required") require.NotEmpty(t, def.SGID, "default SG ID required") @@ -343,7 +343,7 @@ func runGuestChurnRound(t *testing.T, fix *Fixture, instanceID, origType, keyPat if eniAttached { t.Run("DaemonRestartReadoptsSlot", func(t *testing.T) { - tgt := resolveGuestChurnTarget(t, fix, instanceID, keyPath) + tgt := resolveGuestSSHTarget(t, fix, instanceID, keyPath) // Restart the daemon; the guest must survive. harness.Step(t, "restart spinifex-daemon (guest QEMU survives)") @@ -359,7 +359,7 @@ func runGuestChurnRound(t *testing.T, fix *Fixture, instanceID, origType, keyPat verifySentinel(t, "DaemonRestartReadoptsSlot") t.Run("DetachENI", func(t *testing.T) { - tgt := resolveGuestChurnTarget(t, fix, instanceID, keyPath) + tgt := resolveGuestSSHTarget(t, fix, instanceID, keyPath) // Detach: succeeds only if the reconciler re-adopted the slot. harness.Step(t, "detach-network-interface %s (proves slot re-adoption)", attachmentID) @@ -527,7 +527,7 @@ func runGuestChurnRound(t *testing.T, fix *Fixture, instanceID, origType, keyPat }) t.Run("Reboot", func(t *testing.T) { - inst := describeGuestChurnInstance(t, fix, instanceID) + inst := describeSingletonInstance(t, fix, instanceID) host, port := harness.InstancePublicSSHHost(t, inst) waitForSSHReady(t, host, port, keyPath) @@ -591,11 +591,11 @@ func runGuestChurnRound(t *testing.T, fix *Fixture, instanceID, origType, keyPat verifySentinel(t, "Reboot") } -// describeGuestChurnInstance returns the current *ec2.Instance for -// instanceID. Every perturbation this scenario drives can change the -// instance's public hostfwd endpoint (type change, reboot, stop/start), so -// stages re-describe rather than trust an *ec2.Instance captured earlier. -func describeGuestChurnInstance(t *testing.T, fix *Fixture, instanceID string) *ec2.Instance { +// describeSingletonInstance returns the current *ec2.Instance for +// instanceID. Any perturbation of the singleton can change its public hostfwd +// endpoint (type change, reboot, stop/start), so callers re-describe rather +// than trust an *ec2.Instance captured earlier. +func describeSingletonInstance(t *testing.T, fix *Fixture, instanceID string) *ec2.Instance { t.Helper() out, err := fix.AWS.EC2.DescribeInstances(&ec2.DescribeInstancesInput{ InstanceIds: []*string{aws.String(instanceID)}, @@ -606,14 +606,13 @@ func describeGuestChurnInstance(t *testing.T, fix *Fixture, instanceID string) * return out.Reservations[0].Instances[0] } -// resolveGuestChurnTarget re-describes instanceID and returns a fresh SSH +// resolveGuestSSHTarget re-describes instanceID and returns a fresh SSH // target, waiting for the handshake to succeed. The public hostfwd port can -// rebind across every perturbation this scenario drives, so every stage -// re-discovers it rather than trusting a target captured before the -// perturbation. -func resolveGuestChurnTarget(t *testing.T, fix *Fixture, instanceID, keyPath string) harness.SSHTarget { +// rebind across any perturbation of the guest, so callers re-discover it +// rather than trusting a target captured before the perturbation. +func resolveGuestSSHTarget(t *testing.T, fix *Fixture, instanceID, keyPath string) harness.SSHTarget { t.Helper() - inst := describeGuestChurnInstance(t, fix, instanceID) + inst := describeSingletonInstance(t, fix, instanceID) host, port := harness.InstancePublicSSHHost(t, inst) waitForSSHReady(t, host, port, keyPath) return harness.SSHTarget{User: "ubuntu", Host: host, Port: port, KeyPath: keyPath} diff --git a/tests/e2e/single/volume_test.go b/tests/e2e/single/volume_test.go index 585e2c661..a217a3c01 100644 --- a/tests/e2e/single/volume_test.go +++ b/tests/e2e/single/volume_test.go @@ -5,6 +5,9 @@ package single import ( "errors" "fmt" + "regexp" + "strconv" + "strings" "testing" "time" @@ -15,72 +18,69 @@ import ( "github.com/stretchr/testify/require" ) -// runVolumeLifecycle exercises create → modify → attach → detach → -// delete on a fresh 10 GiB volume against the running Phase 5 instance. +// Resize parameters. The volume is created at volumeResizeFromGiB, carries a +// sentinel of volumeResizeSentinelMiB random bytes across the resize, and ends +// at volumeResizeToGiB. The 4 MiB sentinel matches the guest-churn one: large +// enough to be a meaningful checksum target, small enough that the predastore +// round-trip stays inside this phase's budget. +const ( + volumeResizeLabel = "e2eresize" + volumeResizeSentinelMiB = 4 + volumeResizeFromGiB = int64(10) + volumeResizeToGiB = int64(20) + bytesPerGiB = int64(1) << 30 +) + +// runVolumeLifecycle exercises create → attach → write → modify → verify → +// detach → delete on a fresh 10 GiB volume against the running Phase 5 +// instance, asserting the resize at both layers: the control-plane record and +// the block device the guest kernel actually sees. +// +// The resize is bracketed by a detach and a reattach because ModifyVolume +// rejects a volume whose instance is running (IncorrectState) — spinifex has no +// elastic-volume path, so the guest cannot witness a live resize and the +// reattach is what makes it observe the new geometry. The sentinel is written +// before the detach and re-read after the reattach, so the same pass proves +// both that capacity grew and that the data already on the volume survived it. +// // Maps to run-e2e.sh ~488–612. func runVolumeLifecycle(t *testing.T, fix *Fixture) { - harness.Phase(t, "Single — Volume Lifecycle (Attach/Detach)") + harness.Phase(t, "Single — Volume Lifecycle (Attach/Resize/Detach)") az := needAZ(t, fix) inst, _ := needInstance(t, fix) instanceID := aws.StringValue(inst.InstanceId) + _, keyPath := needKeyPair(t, fix) // This phase creates + deletes a standalone test volume; do NOT route // through harness.EnsureVolume because the fixture's terminate-on-cleanup // would later try to re-delete a volume this test has just deleted in-line. - harness.Step(t, "create-volume size=10 az=%s", az) + harness.Step(t, "create-volume size=%d az=%s", volumeResizeFromGiB, az) + // e2e:allow-create — the resized volume is the subject under test. createOut, err := fix.AWS.EC2.CreateVolume(&ec2.CreateVolumeInput{ AvailabilityZone: aws.String(az), - Size: aws.Int64(10), + Size: aws.Int64(volumeResizeFromGiB), }) require.NoError(t, err, "create-volume") volumeID := aws.StringValue(createOut.VolumeId) require.NotEmpty(t, volumeID, "CreateVolume returned empty VolumeId") harness.Detail(t, "volume", volumeID) - // Best-effort cleanup if a later assertion fails before the in-line delete. - t.Cleanup(func() { - _, _ = fix.AWS.EC2.DeleteVolume(&ec2.DeleteVolumeInput{VolumeId: aws.String(volumeID)}) - }) + // Cleanup if a later assertion fails before the in-line delete. The volume + // spends most of this phase attached, so a plain DeleteVolume would answer + // VolumeInUse and strand it; RegisterVolumeTeardown force-detaches first and + // tolerates the volume already being gone on the happy path. + harness.RegisterVolumeTeardown(t, fix.AWS, volumeID) harness.WaitForVolumeState(t, fix.AWS, volumeID, "available", harness.WithPoll(500*time.Millisecond)) - const newSize int64 = 20 - harness.Step(t, "modify-volume size=%d", newSize) - _, err = fix.AWS.EC2.ModifyVolume(&ec2.ModifyVolumeInput{ - VolumeId: aws.String(volumeID), - Size: aws.Int64(newSize), - }) - require.NoError(t, err, "modify-volume") - - // Bash polls Volumes[0].Size directly (not Modifications[].State) — replicate. - // Resize is slower than state transitions; allow 5 minutes. - harness.EventuallyErr(t, func() error { - out, err := fix.AWS.EC2.DescribeVolumes(&ec2.DescribeVolumesInput{ - VolumeIds: []*string{aws.String(volumeID)}, - }) - if err != nil { - return fmt.Errorf("describe-volumes: %w", err) - } - if len(out.Volumes) == 0 { - return fmt.Errorf("%s not found", volumeID) - } - if got := aws.Int64Value(out.Volumes[0].Size); got != newSize { - return fmt.Errorf("%s size=%d want=%d", volumeID, got, newSize) - } - return nil - }, 5*time.Minute, 5*time.Second) - harness.Detail(t, "resized_gib", newSize) + tgt := resolveGuestSSHTarget(t, fix, instanceID, keyPath) harness.Step(t, "attach-volume %s -> %s as /dev/sdf", volumeID, instanceID) - _, err = fix.AWS.EC2.AttachVolume(&ec2.AttachVolumeInput{ - VolumeId: aws.String(volumeID), - InstanceId: aws.String(instanceID), - Device: aws.String("/dev/sdf"), - }) - require.NoError(t, err, "attach-volume") - - harness.WaitForVolumeState(t, fix.AWS, volumeID, ec2.VolumeStateInUse, harness.WithPoll(500*time.Millisecond)) + before := harness.GuestDiskSet(t, tgt) + harness.AttachVolumeWait(t, fix.AWS, volumeID, instanceID, "/dev/sdf") + dev := harness.WaitForNewGuestDisk(t, tgt, before, 60*time.Second) + harness.Detail(t, "guest_dev", dev) // Once the volume is in-use, the attachment record should be populated. descAttached, err := fix.AWS.EC2.DescribeVolumes(&ec2.DescribeVolumesInput{ @@ -93,6 +93,118 @@ func runVolumeLifecycle(t *testing.T, fix *Fixture) { "attachment State should be %q", ec2.VolumeAttachmentStateAttached) assert.Equal(t, instanceID, aws.StringValue(att.InstanceId), "Attachment.InstanceId mismatch") + // Pre-resize baseline. Every post-resize size assertion below is expressed + // as growth relative to this number rather than against an absolute 20 GiB, + // so a backend that sizes a volume a hair under the requested GiB doesn't + // flake the test while a resize that lands short still fails it. + preBytes := harness.LsblkDeviceBytes(t, tgt, dev) + harness.Detail(t, "guest_bytes_before", preBytes) + assert.InDeltaf(t, float64(volumeResizeFromGiB*bytesPerGiB), float64(preBytes), float64(bytesPerGiB), + "guest device /dev/%s reports %d bytes, more than 1 GiB from the requested %d GiB", + dev, preBytes, volumeResizeFromGiB) + + harness.Step(t, "write sentinel (%d MiB) to /dev/%s", volumeResizeSentinelMiB, dev) + wantSha := harness.GuestFormatWriteSentinel(t, tgt, dev, volumeResizeLabel, volumeResizeSentinelMiB) + harness.Detail(t, "sha256", wantSha) + + // ModifyVolume refuses an in-use volume, so detach before resizing. + harness.Step(t, "detach-volume %s (ModifyVolume requires the volume be available)", volumeID) + harness.DetachVolumeWait(t, fix.AWS, volumeID) + + harness.Step(t, "modify-volume size=%d", volumeResizeToGiB) + _, err = fix.AWS.EC2.ModifyVolume(&ec2.ModifyVolumeInput{ + VolumeId: aws.String(volumeID), + Size: aws.Int64(volumeResizeToGiB), + }) + require.NoError(t, err, "modify-volume") + + // DescribeVolumesModifications is what Terraform's aws_ebs_volume and the + // EBS CSI driver poll to decide a resize finished, so the record has to + // exist and reach a terminal state — a handler stuck in "modifying", or one + // that returns nothing at all, is a resize no client can observe. + harness.Step(t, "describe-volumes-modifications %s", volumeID) + var mod *ec2.VolumeModification + harness.EventuallyErr(t, func() error { + out, err := fix.AWS.EC2.DescribeVolumesModifications(&ec2.DescribeVolumesModificationsInput{ + VolumeIds: []*string{aws.String(volumeID)}, + }) + if err != nil { + return fmt.Errorf("describe-volumes-modifications: %w", err) + } + if len(out.VolumesModifications) == 0 { + return fmt.Errorf("no modification record for %s yet", volumeID) + } + switch state := aws.StringValue(out.VolumesModifications[0].ModificationState); state { + case ec2.VolumeModificationStateCompleted, ec2.VolumeModificationStateOptimizing: + mod = out.VolumesModifications[0] + return nil + case ec2.VolumeModificationStateFailed: + // Terminal and unrecoverable — retrying only burns the budget. + t.Fatalf("modification for %s reported state=failed: %s", + volumeID, aws.StringValue(out.VolumesModifications[0].StatusMessage)) + return nil + default: + return fmt.Errorf("%s modification state=%q, want completed/optimizing", volumeID, state) + } + }, 2*time.Minute, 2*time.Second) + assert.Equal(t, volumeID, aws.StringValue(mod.VolumeId), "modification names the wrong volume") + assert.Equal(t, volumeResizeToGiB, aws.Int64Value(mod.TargetSize), "modification TargetSize mismatch") + assert.Equal(t, volumeResizeFromGiB, aws.Int64Value(mod.OriginalSize), "modification OriginalSize mismatch") + harness.Detail(t, "modification_state", aws.StringValue(mod.ModificationState), + "target_size", aws.Int64Value(mod.TargetSize)) + + // Control-plane assertion: the KV record carries the new size. This is a + // separate claim from the guest-side one below — it proves the API answers + // correctly, not that anything happened to the block device — and both are + // kept because either can regress without the other. + // Resize is slower than state transitions; allow 5 minutes. + harness.EventuallyErr(t, func() error { + out, err := fix.AWS.EC2.DescribeVolumes(&ec2.DescribeVolumesInput{ + VolumeIds: []*string{aws.String(volumeID)}, + }) + if err != nil { + return fmt.Errorf("describe-volumes: %w", err) + } + if len(out.Volumes) == 0 { + return fmt.Errorf("%s not found", volumeID) + } + if got := aws.Int64Value(out.Volumes[0].Size); got != volumeResizeToGiB { + return fmt.Errorf("%s size=%d want=%d", volumeID, got, volumeResizeToGiB) + } + return nil + }, 5*time.Minute, 5*time.Second) + harness.Detail(t, "resized_gib", volumeResizeToGiB) + + // Guest-side assertion: the whole point of this phase. A backend that + // accepted ModifyVolume, updated KV, and left the underlying viperblock + // volume alone passes everything above and fails here. + harness.Step(t, "reattach-volume %s and assert the guest sees the new size", volumeID) + before = harness.GuestDiskSet(t, tgt) + harness.AttachVolumeWait(t, fix.AWS, volumeID, instanceID, "/dev/sdf") + dev = harness.WaitForNewGuestDisk(t, tgt, before, 60*time.Second) + wantBytes := preBytes + (volumeResizeToGiB-volumeResizeFromGiB)*bytesPerGiB + postBytes := waitForGuestDeviceGrowth(t, tgt, dev, wantBytes, 60*time.Second) + harness.Detail(t, "guest_dev", dev, "guest_bytes_after", postBytes) + + // The resize must not have disturbed what was already on the volume. + gotSha := harness.GuestReadSentinelSha(t, tgt, "/dev/"+dev, volumeResizeLabel) + require.Equalf(t, wantSha, gotSha, "sentinel sha256 mismatch after resize") + + // Grown capacity that no filesystem can consume is not capacity. The + // sentinel helper formats the bare device, so there is no partition table to + // grow first and resize2fs on its own is enough. + harness.Step(t, "resize2fs /dev/%s and re-verify the sentinel", dev) + fsBytes := guestGrowExt4(t, tgt, dev, volumeResizeLabel) + // ext4 spends a few percent of the device on metadata, so the filesystem is + // always smaller than the disk; 90% separates "grew with the device" from + // "still sized for the old 10 GiB". + assert.GreaterOrEqualf(t, fsBytes, postBytes*9/10, + "ext4 on /dev/%s reports %d bytes after resize2fs; device is %d bytes, so the filesystem did not take the new capacity", + dev, fsBytes, postBytes) + harness.Detail(t, "fs_bytes", fsBytes) + gotSha = harness.GuestReadSentinelSha(t, tgt, "/dev/"+dev, volumeResizeLabel) + require.Equalf(t, wantSha, gotSha, "sentinel sha256 mismatch after resize2fs") + // Bash omits --instance-id to exercise the gateway's resolution path. harness.Step(t, "detach-volume %s", volumeID) _, err = fix.AWS.EC2.DetachVolume(&ec2.DetachVolumeInput{ @@ -133,6 +245,78 @@ func runVolumeLifecycle(t *testing.T, fix *Fixture) { }, 2*time.Minute, 2*time.Second) } +// waitForGuestDeviceGrowth polls /dev/ until the guest kernel reports at +// least wantBytes, and returns the size it settled on. +// +// It deliberately does not rescan the device first: whether a resized volume +// presents its new geometry to the guest on its own is the behaviour under +// test, and a rescan up front would paper over exactly the regression this +// poll exists to catch. Only once the poll has timed out does it force a +// rescan, as a diagnostic — that turns "the resize never reached the block +// device" into "it reached the device but the guest needed a kick", which are +// different bugs with different owners. Either way the test fails. +func waitForGuestDeviceGrowth(t *testing.T, tgt harness.SSHTarget, dev string, wantBytes int64, timeout time.Duration) int64 { + t.Helper() + deadline := time.Now().Add(timeout) + var got int64 + for { + got = harness.LsblkDeviceBytes(t, tgt, dev) + if got >= wantBytes { + return got + } + if time.Now().After(deadline) { + break + } + time.Sleep(3 * time.Second) + } + + if out, err := harness.GuestExec(tgt, + fmt.Sprintf("echo 1 | sudo tee /sys/class/block/%s/device/rescan >/dev/null", dev)); err != nil { + t.Logf("diagnostic rescan of /dev/%s failed: %v\n%s", dev, err, out) + } + if after := harness.LsblkDeviceBytes(t, tgt, dev); after >= wantBytes { + t.Fatalf("guest never observed the resize on its own: /dev/%s reported %d bytes (want >= %d) for %s, "+ + "but an explicit rescan surfaced %d — the resize reaches the backend and fails to propagate to the guest", + dev, got, wantBytes, timeout, after) + } + t.Fatalf("guest never observed the resize: /dev/%s reports %d bytes, want >= %d, and an explicit rescan "+ + "did not change that — the resize did not reach the block device", dev, got, wantBytes) + return 0 +} + +// guestGrowExt4 grows the whole-disk ext4 on /dev/ to fill the device and +// returns the mounted filesystem's total size in bytes. The resize is done +// online (mount, then resize2fs) so no offline e2fsck is needed first. The size +// is echoed behind a marker because resize2fs writes its own progress lines to +// the same stream. +func guestGrowExt4(t *testing.T, tgt harness.SSHTarget, dev, label string) int64 { + t.Helper() + mnt := "/mnt/" + label + script := strings.Join([]string{ + fmt.Sprintf("sudo mkdir -p %s", mnt), + fmt.Sprintf("sudo mount /dev/%s %s", dev, mnt), + fmt.Sprintf("sudo resize2fs /dev/%s", dev), + fmt.Sprintf("echo FS_BYTES=$(df -B1 --output=size %s | tail -1 | tr -d ' ')", mnt), + fmt.Sprintf("sudo umount %s", mnt), + }, " && ") + out, err := harness.GuestExec(tgt, script) + if err != nil { + t.Fatalf("guestGrowExt4(%s): %v\n%s", dev, err, out) + } + m := fsBytesRE.FindStringSubmatch(out) + if m == nil { + t.Fatalf("guestGrowExt4(%s): no FS_BYTES marker in guest output:\n%s", dev, out) + } + size, err := strconv.ParseInt(m[1], 10, 64) + if err != nil { + t.Fatalf("guestGrowExt4(%s): parse %q: %v", dev, m[1], err) + } + return size +} + +// fsBytesRE lifts the filesystem size out of guestGrowExt4's marker line. +var fsBytesRE = regexp.MustCompile(`FS_BYTES=(\d+)`) + // runVolumeStatus runs DescribeVolumeStatus against the root volume // and asserts the response references it back. Maps to run-e2e.sh ~614–625. func runVolumeStatus(t *testing.T, fix *Fixture) {