From 6891b49aac99258d9db67231ef105f4e3e2d0cee Mon Sep 17 00:00:00 2001 From: ErenAri Date: Thu, 2 Jul 2026 14:55:56 +0300 Subject: [PATCH] fix(vm): fail fast when cidata profiles lack cloud-localds The silent vvfat config-drive fallback cost two full publish-lane runs to diagnose: RHEL-family/Amazon/Oracle/SUSE guests never boot from it on some hosts (0-byte serial), so the failure surfaced 15 minutes later per target as an opaque "wait for SSH: not ready" infra error. seedDeliveryForProfile now returns an error when a profile needs a cidata seed ISO and cloud-localds is not installed, carrying the fix in the message: install cloud-image-utils, or explicitly opt back into the legacy fallback with BPFCOMPAT_ALLOW_VVFAT_SEED=1 (new env var, cataloged in internal/envref + docs/env-reference.md). When the opt-in is used, the run note warns what to do if the guest never reaches SSH. Tests: fail-fast error carries all three actionable hints; opt-in restores vvfat; ubuntu/debian NoCloud-net unaffected; CoreOS Ignition path unchanged. Co-Authored-By: Claude Opus 4.8 --- docs/env-reference.md | 1 + internal/envref/envref.go | 5 ++++ internal/vm/ignition_test.go | 4 +-- internal/vm/qemu.go | 41 ++++++++++++++++++++++----- internal/vm/qemu_test.go | 54 ++++++++++++++++++++++++++++-------- 5 files changed, 85 insertions(+), 20 deletions(-) diff --git a/docs/env-reference.md b/docs/env-reference.md index 1a5c2d0..f19d8b1 100644 --- a/docs/env-reference.md +++ b/docs/env-reference.md @@ -131,6 +131,7 @@ Generated by `bpfcompat env --markdown`. Do not edit by hand. |---|---|---| | `BPFCOMPAT_AARCH64_UEFI_CODE` | /usr/share/AAVMF/AAVMF_CODE.fd | Path to the aarch64 UEFI firmware CODE image (pflash). aarch64 `virt` has no built-in firmware, so VM boots need it; install qemu-efi-aarch64 or point this at your distro's edk2 build. Ignored on x86_64. | | `BPFCOMPAT_AARCH64_UEFI_VARS` | /usr/share/AAVMF/AAVMF_VARS.fd | Path to the aarch64 UEFI vars template (pflash). A per-VM writable copy is staged from it so guest NVRAM writes don't touch the shared template. Ignored on x86_64. | +| `BPFCOMPAT_ALLOW_VVFAT_SEED` | false | Re-enable the legacy vvfat config-drive seed for RHEL-family/Amazon/Oracle/SUSE profiles when cloud-localds is missing. Off by default: those guests are known not to boot from the vvfat fallback on some hosts (0-byte serial, then an SSH timeout), so seed selection fails fast with an install hint instead. Prefer installing cloud-image-utils (provides cloud-localds). | | `BPFCOMPAT_ENABLE_RHCOS` | false | Enable the RHEL CoreOS (rhcos) profile. RHCOS boots via the same Ignition path as Fedora CoreOS, but its image ships with an OpenShift release rather than a public URL. Stage the image with `make rhcos-image` and set this to 1/true once it is present; left off, rhcos stays unsupported so it is never claimed runnable without a real image. | ## Validator diff --git a/internal/envref/envref.go b/internal/envref/envref.go index 1c644dd..2f53883 100644 --- a/internal/envref/envref.go +++ b/internal/envref/envref.go @@ -299,6 +299,11 @@ var catalog = []Var{ Category: "VM Runner", Description: "Enable the RHEL CoreOS (rhcos) profile. RHCOS boots via the same Ignition path as Fedora CoreOS, but its image ships with an OpenShift release rather than a public URL. Stage the image with `make rhcos-image` and set this to 1/true once it is present; left off, rhcos stays unsupported so it is never claimed runnable without a real image.", }, + { + Name: "BPFCOMPAT_ALLOW_VVFAT_SEED", Default: "false", + Category: "VM Runner", + Description: "Re-enable the legacy vvfat config-drive seed for RHEL-family/Amazon/Oracle/SUSE profiles when cloud-localds is missing. Off by default: those guests are known not to boot from the vvfat fallback on some hosts (0-byte serial, then an SSH timeout), so seed selection fails fast with an install hint instead. Prefer installing cloud-image-utils (provides cloud-localds).", + }, { Name: "BPFCOMPAT_AARCH64_UEFI_CODE", Default: "/usr/share/AAVMF/AAVMF_CODE.fd", Category: "VM Runner", diff --git a/internal/vm/ignition_test.go b/internal/vm/ignition_test.go index 0c538a8..1e82395 100644 --- a/internal/vm/ignition_test.go +++ b/internal/vm/ignition_test.go @@ -48,8 +48,8 @@ func TestIsCoreOSIgnitionDistro(t *testing.T) { } func TestSeedDeliveryForCoreOS(t *testing.T) { - if got := seedDeliveryForProfile(Profile{Distro: "fedora-coreos"}); got != seedDeliveryIgnition { - t.Errorf("fedora-coreos seed delivery = %q, want %q", got, seedDeliveryIgnition) + if got, err := seedDeliveryForProfile(Profile{Distro: "fedora-coreos"}); err != nil || got != seedDeliveryIgnition { + t.Errorf("fedora-coreos seed delivery = %q (err %v), want %q", got, err, seedDeliveryIgnition) } } diff --git a/internal/vm/qemu.go b/internal/vm/qemu.go index 0d6291d..7d648c6 100644 --- a/internal/vm/qemu.go +++ b/internal/vm/qemu.go @@ -237,7 +237,11 @@ func ExecuteProfile(ctx context.Context, req ExecutionRequest) (result Execution return } - seedMode := seedDeliveryForProfile(req.Profile) + seedMode, err := seedDeliveryForProfile(req.Profile) + if err != nil { + result.InfraError = err.Error() + return + } seedDir := filepath.Join(vmRunDir, "seed") seedURL := "" seedImagePath := "" @@ -272,7 +276,7 @@ func ExecuteProfile(ctx context.Context, req ExecutionRequest) (result Execution } result.Notes = append(result.Notes, "seed delivery: local NoCloud config drive image (cloud-localds)") case seedDeliveryNoCloudConfigFS: - result.Notes = append(result.Notes, "seed delivery: local NoCloud config drive (vvfat, label=cidata)") + result.Notes = append(result.Notes, "seed delivery: local NoCloud config drive (vvfat, label=cidata) — legacy fallback enabled via BPFCOMPAT_ALLOW_VVFAT_SEED; if this guest never reaches SSH, install cloud-image-utils instead") default: seedSrv, err := startSeedServer(seedDir) if err != nil { @@ -937,17 +941,40 @@ func needsCIDATASeed(profile Profile) bool { return strings.EqualFold(strings.TrimSpace(profile.ID), "rhel-8-4.18") } -func seedDeliveryForProfile(profile Profile) seedDeliveryMode { +func seedDeliveryForProfile(profile Profile) (seedDeliveryMode, error) { if isCoreOSIgnitionDistro(profile) { - return seedDeliveryIgnition + return seedDeliveryIgnition, nil } if needsCIDATASeed(profile) { if commandAvailable("cloud-localds") { - return seedDeliveryNoCloudConfigDrive + return seedDeliveryNoCloudConfigDrive, nil + } + // The vvfat config-drive fallback is known NOT to boot RHEL-family/ + // Amazon/Oracle/SUSE guests on some hosts (0-byte serial, SSH + // timeout) — and because the guest simply never comes up, the + // failure surfaces 15 minutes later as an opaque SSH timeout. + // Fail fast with the actionable fix instead; the legacy fallback + // stays reachable behind an explicit opt-in. + if allowVVFATSeedFallback() { + return seedDeliveryNoCloudConfigFS, nil } - return seedDeliveryNoCloudConfigFS + return "", fmt.Errorf("profile %s needs a cloud-init cidata seed ISO but cloud-localds is not installed; "+ + "install cloud-image-utils (Debian/Ubuntu: apt-get install cloud-image-utils), "+ + "or set BPFCOMPAT_ALLOW_VVFAT_SEED=1 to try the legacy vvfat config-drive fallback "+ + "(known not to boot RHEL-family/Amazon/Oracle/SUSE guests on some hosts)", profile.ID) + } + return seedDeliveryNoCloudNet, nil +} + +// allowVVFATSeedFallback reports whether the operator explicitly re-enabled +// the legacy vvfat config-drive seed for cidata profiles when cloud-localds +// is missing. +func allowVVFATSeedFallback() bool { + switch strings.ToLower(strings.TrimSpace(os.Getenv("BPFCOMPAT_ALLOW_VVFAT_SEED"))) { + case "1", "true", "yes", "on": + return true } - return seedDeliveryNoCloudNet + return false } func commandAvailable(name string) bool { diff --git a/internal/vm/qemu_test.go b/internal/vm/qemu_test.go index 6bae266..d21b5b7 100644 --- a/internal/vm/qemu_test.go +++ b/internal/vm/qemu_test.go @@ -395,23 +395,55 @@ func TestParseFirecrackerMarkedOutput(t *testing.T) { } func TestSeedDeliveryForProfile(t *testing.T) { - if got := seedDeliveryForProfile(Profile{ID: "rhel-8-4.18"}); got != seedDeliveryNoCloudConfigDrive && got != seedDeliveryNoCloudConfigFS { - t.Fatalf("expected rhel-8-4.18 seed delivery %q or %q, got %q", seedDeliveryNoCloudConfigDrive, seedDeliveryNoCloudConfigFS, got) + if got, err := seedDeliveryForProfile(Profile{ID: "ubuntu-22.04-5.15", Distro: "ubuntu"}); err != nil || got != seedDeliveryNoCloudNet { + t.Fatalf("expected default seed delivery %q, got %q (err %v)", seedDeliveryNoCloudNet, got, err) } - if got := seedDeliveryForProfile(Profile{ID: "ubuntu-22.04-5.15", Distro: "ubuntu"}); got != seedDeliveryNoCloudNet { - t.Fatalf("expected default seed delivery %q, got %q", seedDeliveryNoCloudNet, got) - } - if got := seedDeliveryForProfile(Profile{ID: "debian-12-6.1", Distro: "debian"}); got != seedDeliveryNoCloudNet { - t.Fatalf("expected debian seed delivery %q, got %q", seedDeliveryNoCloudNet, got) + if got, err := seedDeliveryForProfile(Profile{ID: "debian-12-6.1", Distro: "debian"}); err != nil || got != seedDeliveryNoCloudNet { + t.Fatalf("expected debian seed delivery %q, got %q (err %v)", seedDeliveryNoCloudNet, got, err) } // EL-family / Amazon / SUSE must use the CIDATA disk seed (SMBIOS-net is - // ignored by their cloud-init). + // ignored by their cloud-init). With cloud-localds present that is the + // seed ISO; without it, selection must error rather than fall back. for _, distro := range []string{"almalinux", "rocky", "rhel", "centos-stream", "oracle", "amazon-linux", "sles", "opensuse"} { - got := seedDeliveryForProfile(Profile{ID: distro + "-x", Distro: distro}) - if got != seedDeliveryNoCloudConfigDrive && got != seedDeliveryNoCloudConfigFS { - t.Fatalf("expected CIDATA seed for distro %q, got %q", distro, got) + got, err := seedDeliveryForProfile(Profile{ID: distro + "-x", Distro: distro}) + if err != nil { + continue // host without cloud-localds: covered by the fail-fast test below + } + if got != seedDeliveryNoCloudConfigDrive { + t.Fatalf("expected CIDATA seed ISO for distro %q, got %q", distro, got) + } + } +} + +// Without cloud-localds on PATH, cidata profiles must fail fast with the +// actionable install hint — the vvfat fallback boots to a 0-byte serial on +// some hosts and only surfaces as an SSH timeout much later. +func TestSeedDeliveryFailsFastWithoutCloudLocalds(t *testing.T) { + t.Setenv("PATH", t.TempDir()) // no cloud-localds resolvable + t.Setenv("BPFCOMPAT_ALLOW_VVFAT_SEED", "") + + _, err := seedDeliveryForProfile(Profile{ID: "almalinux-8-4.18", Distro: "almalinux"}) + if err == nil { + t.Fatal("expected error for cidata profile without cloud-localds") + } + for _, want := range []string{"cloud-localds", "cloud-image-utils", "BPFCOMPAT_ALLOW_VVFAT_SEED"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("error %q missing actionable hint %q", err.Error(), want) } } + + // Explicit opt-in re-enables the legacy vvfat fallback. + t.Setenv("BPFCOMPAT_ALLOW_VVFAT_SEED", "1") + got, err := seedDeliveryForProfile(Profile{ID: "almalinux-8-4.18", Distro: "almalinux"}) + if err != nil || got != seedDeliveryNoCloudConfigFS { + t.Fatalf("expected vvfat fallback with opt-in, got %q (err %v)", got, err) + } + + // Ubuntu/Debian (NoCloud net seed) are unaffected by the missing tool. + got, err = seedDeliveryForProfile(Profile{ID: "ubuntu-22.04-5.15", Distro: "ubuntu"}) + if err != nil || got != seedDeliveryNoCloudNet { + t.Fatalf("expected NoCloud net seed for ubuntu, got %q (err %v)", got, err) + } } func TestMapFixupArgs(t *testing.T) {