From 366736f53f9f7cefc3ccd0ead4d94cd38e1b62f1 Mon Sep 17 00:00:00 2001 From: kacerr Date: Thu, 10 Sep 2026 21:06:54 +0000 Subject: [PATCH] Merge the qemu-guest-agent bootstrap into each Proxy's cloud-config instead of vendor-data In the kind cluster two KVM Proxies with real cloud-configs booted fine but never left Provisioning: the guest agent was not installed, so the provider could not learn the IP. cloud-init merges vendor-data under user-data with its default mergers, which keep the user-data's value for any key both define and never append lists, so a cloud-config with its own `packages:` silently dropped the vendor-data's packages and runcmd. The integration test passed only because its user-data was empty. withGuestAgent now appends qemu-guest-agent to `packages` and the enable command to `runcmd` of the Proxy's own cloud-config (skipping entries already present) and re-renders it. Vendor-data is no longer defaulted; `vendorData` is plain extra data. User-data that is not a cloud-config document (script, #include, MIME) still gets the bootstrap as vendor-data. A cloud-config that is not a YAML mapping, or whose packages/runcmd is not a list, fails the Proxy as ErrPermanent. Validated in-cluster: both Proxies Ready and healthy in two minutes, 204 through squid from the LAN. Docs, sample comment, CHANGELOG and the plan execution log (Run 7) updated. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 18 ++ config/samples/providers-config.yaml | 4 +- docs/kvm-provider.md | 34 +++- .../2026-09-04-1404-kvm-provider.md | 73 +++++++++ internal/provider/config_kvm.go | 7 +- internal/provider/kvm/kvm.go | 9 +- internal/provider/kvm/kvm_test.go | 10 +- internal/provider/kvm/seed.go | 102 +++++++++++- internal/provider/kvm/seed_test.go | 155 +++++++++++++++++- 9 files changed, 382 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 896b720..20989d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## 2026-09-10 21:04 UTC — KVM provider passes in-cluster; guest agent merged into cloud-config + +- Two KVM Proxies with real cloud-configs reach `Ready` from inside the kind cluster + (2 min from apply, IP via the guest agent, 204 through squid from the LAN). +- The `qemu-guest-agent` bootstrap is now merged into each Proxy's cloud-config + (`packages` + `runcmd`) instead of shipped as vendor-data: cloud-init's default + merger drops vendor-data keys the user-data also defines, so any Proxy with its own + `packages:` never got the agent and stayed `Provisioning` forever. `vendorData` is + now plain extra vendor-data with no default; non-cloud-config user-data (scripts, + `#include`) still gets the bootstrap as vendor-data. A cloud-config that is not a + YAML mapping fails the Proxy permanently instead of booting without its config. +- The manager pod mounts a 64 MiB `emptyDir` on `/tmp`: the seed ISO writer stages + files on disk and the root filesystem is read-only. +- Errors already classified with a provider sentinel no longer drop the cached + libvirt SSH connection; only libvirt and transport errors do. +- Key files: `internal/provider/kvm/{seed.go,conn.go,errors.go}`, + `config/manager/manager.yaml`, `docs/kvm-provider.md`. + ## 2026-09-10 10:07 UTC — KVM provider validated on a real hypervisor; UEFI default; libvirt call tracing - `make test-integration-kvm` passes end to end against a libvirt 11.6 / QEMU 10.1 diff --git a/config/samples/providers-config.yaml b/config/samples/providers-config.yaml index f80cca4..d0d7743 100644 --- a/config/samples/providers-config.yaml +++ b/config/samples/providers-config.yaml @@ -30,8 +30,8 @@ providers: knownHostsFile: /etc/proxy-operator/kvm/known_hosts # insecureIgnoreHostKey: true # skip host-key checks (not recommended) # diskSizeGb: 10 # per-VM overlay; cloud-init grows the root fs into it - # vendorData: | # default installs qemu-guest-agent for IP discovery - # #cloud-config + # vendorData: | # extra vendor-data; the qemu-guest-agent bootstrap is merged + # #cloud-config # into each Proxy's cloud-config, not configured here # ... images: # placement.image names a key here (or is an https:// URL directly). diff --git a/docs/kvm-provider.md b/docs/kvm-provider.md index 322d573..a59d1be 100644 --- a/docs/kvm-provider.md +++ b/docs/kvm-provider.md @@ -27,11 +27,13 @@ Per host, once: `edk2-ovmf` on Fedora/RHEL. VMs boot UEFI by default (libvirt picks the OVMF binary itself); a domain start fails with "no firmware found" without it. Only `firmware: bios` images skip this. -6. Optionally, **outbound internet from the VMs**: the default - vendor-data installs `qemu-guest-agent` from the distro mirrors on - first boot. Without it the VM still works — the operator falls back to - the host's ARP table to find the IP, which fills in once the guest - sends traffic. +6. Optionally, **outbound internet from the VMs**: the operator adds + `qemu-guest-agent` to every Proxy's cloud-config (`packages` plus a + `runcmd` that enables it), so it is installed from the distro mirrors + on first boot. Without it the VM still works — the operator falls back + to the host's ARP table to find the IP, which fills in once the guest + sends traffic — but note the ARP table only sees traffic to the host + itself, so on a plain bridge the agent is in practice required. Check from the operator's point of view before wiring it in: @@ -129,7 +131,7 @@ ssh virt@nas1.home.lan ip link show br0 | `knownHostsFile` | unless `insecureIgnoreHostKey` | | OpenSSH `known_hosts` to verify host keys against | | `insecureIgnoreHostKey` | | `false` | skip host-key verification entirely | | `diskSizeGb` | | `10` | capacity of each VM's overlay disk; cloud-init's growpart expands the root fs into it | -| `vendorData` | | installs `qemu-guest-agent` | cloud-init vendor-data placed on every seed next to the Proxy's user-data | +| `vendorData` | | | extra cloud-init vendor-data placed on every seed next to the Proxy's user-data; cloud-init drops any top-level key here that the user-data also defines. The guest-agent bootstrap is not configured here, see *What lands on the host* | | `images..url` | yes per entry | | http(s) URL of a cloud image (qcow2 or raw) | | `images..sha256` | | | expected digest; verified after download, a change triggers a re-download | | `images..format` | | `qcow2` | `qcow2` or `raw` | @@ -174,6 +176,21 @@ console, the guest-agent channel, a virtio RNG and no graphics. Its creation time) — that is how orphan GC recognises its own VMs, since libvirt has no labels. +The seed's `user-data` is the Proxy's `cloudInit` with the guest-agent +bootstrap **merged into the document**: `qemu-guest-agent` appended to +`packages` and `[systemctl, enable, --now, qemu-guest-agent]` to `runcmd` +(neither duplicated if already there; other keys untouched, though the +YAML is re-rendered). It is not shipped as vendor-data because cloud-init +merges vendor-data under user-data with its default mergers, which keep +the user-data's value for any key both define and never append lists — a +cloud-config with its own `packages:` would silently lose the agent and +the Proxy would sit in `Provisioning` forever. Only when the user-data is +not a cloud-config document (a shell script, `#include`, MIME archive) +does the bootstrap go to `vendor-data`, and then only if `vendorData` is +not configured. A cloud-config that does not parse as a YAML mapping, or +whose `packages`/`runcmd` is not a list, fails the Proxy permanently +rather than booting a VM without its config. + ## The image cache The base image is downloaded **by the operator, through libvirt**, into @@ -254,7 +271,10 @@ The provider logs through controller-runtime's logger, named `kvm`, with under *The image cache*. - **IP discovery needs either guest traffic or the agent.** A VM on a bridge with no DHCP never gets an address and stays `Provisioning` - forever; check `virsh console`. + forever; check `virsh console`. The same symptom with a booted, addressed + VM means the guest agent never came up: check `systemctl status + qemu-guest-agent` and `cloud-init status --long` in the guest (a VM + without outbound internet cannot install it). - **Debian 13 cloud images hang in GRUB under BIOS on a headless VM** ([Debian bug #1111240](https://bugs.debian.org/1111240), open, in `grub2`): trixie's image ships `terminal_output gfxterm serial` in its diff --git a/docs/plans-executions/2026-09-04-1404-kvm-provider.md b/docs/plans-executions/2026-09-04-1404-kvm-provider.md index cf3079e..bdfe6b3 100644 --- a/docs/plans-executions/2026-09-04-1404-kvm-provider.md +++ b/docs/plans-executions/2026-09-04-1404-kvm-provider.md @@ -557,3 +557,76 @@ on the host it needs `virsh undefine --nvram` plus its two volumes by hand. The `ListByTag` sweep costs one `virDomainGetMetadata` per domain on the host (21 here, ~0.4 ms each over the tunnel), which is fine at this scale but is the call to watch on a busy hypervisor. + +### Run 7: in-cluster, two defects the integration test could not see + +The e2e pass ran the operator inside the kind cluster +(`egress-proxies-operator:kvm-dev`, built with `make docker-build`, +loaded with `kind load docker-image`, deployed via the kustomize +`config/manager` with the `kvm-home` provider pointing at beelink) and +two Proxies with real cloud-configs: one plain squid, one with a debug +login user. Both defects come from the difference between the test +harness and the real thing, not from libvirt. + +**Read-only root filesystem.** The manager runs with +`readOnlyRootFilesystem: true`, and `iso9660.NewWriter` stages the seed +in a temp directory, so the first Create failed with `creating iso +writer: read-only file system`. Fix: a 64 MiB `emptyDir` on `/tmp` in +`config/manager/manager.yaml`. Side finding: the error is classified +`ErrPermanent` by `writeSeed`, yet `hostPool.call` treated any +non-libvirt error as a transport failure and dropped the SSH connection, +so every retry redialled. `call` now leaves the connection alone for +errors already carrying a provider sentinel (`isClassified`), with a +test. + +**The guest agent never got installed.** Both VMs booted (confirmed on +the console by hand) but sat in `Provisioning` for 25 minutes: the +trace showed `virDomainInterfaceAddresses` from the agent source failing +with `ErrAgentUnresponsive` on every poll and the ARP source empty. The +integration test had passed with `CloudInit: "#cloud-config\n"`; the +real Proxies have `packages: [squid]` and `runcmd:`. cloud-init merges +vendor-data under user-data with its default mergers, which keep the +user-data's value for any key both define and never append lists, so the +vendor-data's `packages`/`runcmd` were dropped wholesale. Confirmed +locally with the dev box's cloud-init 26.1 before changing anything: + +```sh +python3 - <<'EOF' +from cloudinit import mergers +user = {"packages": ["squid"], "runcmd": ["systemctl restart squid"]} +vendor = {"packages": ["qemu-guest-agent"], "runcmd": [["systemctl", "enable", "--now", "qemu-guest-agent"]]} +print(mergers.construct(mergers.default_mergers()).merge(dict(user), dict(vendor))) +EOF +# {'packages': ['squid'], 'runcmd': ['systemctl restart squid']} +``` + +Fix: `withGuestAgent` in `seed.go` merges the bootstrap into the Proxy's +cloud-config itself (append to `packages` and `runcmd`, skip if present, +re-render with `sigs.k8s.io/yaml`), so vendor-data is no longer defaulted +and `vendorData` is plain extra data. User-data that is not a cloud-config +document (script, `#include`, MIME) still gets the bootstrap as +vendor-data since there is nothing to collide with; a cloud-config that +does not parse as a mapping fails the Proxy as `ErrPermanent`. The +rendered document for the squid Proxy passed `cloud-init schema +--config-file`. The logging test's fixture (`#cloud-config` + a bare +sentinel line) is not a mapping and had to become `hostname: `. + +Rebuilt, reloaded, restarted, deleted and recreated both Proxies: both +`Ready`/`HEALTHY True` in 2 min 1 s from `kubectl apply`, IP via the +agent 110 s after `virDomainCreate` (the guest installs squid and the +agent from the mirrors first), and `curl -x http://:3128 +https://www.gstatic.com/generate_204` from the dev box returns 204 +through both. + +Worth noting: the `#cloud-config` round-trip drops comments and reorders +keys alphabetically; semantics are preserved (numbers, booleans and +block scalars survive, checked in tests) but a user diffing the seed +against their CR will see a different document. Vendor-data supplied via +`vendorData` is subject to the same cloud-init merge rule and is now +documented as such. The kind pod also restarted twice during the session +from "Pod sandbox changed" (the node briefly lost the API server), which +is unrelated to the operator. Still uncommitted on purpose: the +`kustomization.yaml` image override (`egress-proxies-operator:kvm-dev`) +and the `beelink` host entries in `config/manager/providers_config.yaml` +and `config/samples/providers-config.yaml`, which are this machine's dev +setup rather than the repo's defaults. diff --git a/internal/provider/config_kvm.go b/internal/provider/config_kvm.go index 61521c7..4a094e8 100644 --- a/internal/provider/config_kvm.go +++ b/internal/provider/config_kvm.go @@ -22,9 +22,10 @@ type KVMConfig struct { // DiskSizeGB is the capacity of each VM's overlay disk. Default 10. The // guest's cloud-init growpart expands the root filesystem into it. DiskSizeGB int64 `json:"diskSizeGb,omitempty"` - // VendorData is cloud-init vendor-data placed on every seed next to the - // Proxy's own user-data. Default: installs and starts qemu-guest-agent so - // the provider can read the VM's IP even before it shows up in ARP. + // VendorData is optional cloud-init vendor-data placed on every seed next + // to the Proxy's own user-data. cloud-init drops any top-level key here + // that the user-data also defines. The qemu-guest-agent bootstrap does + // not live here: the provider merges it into each Proxy's cloud-config. VendorData string `json:"vendorData,omitempty"` // Images are named base images; placement.image names a key here (or is // an http(s) URL directly). The provider downloads each image into a diff --git a/internal/provider/kvm/kvm.go b/internal/provider/kvm/kvm.go index a42e5eb..470fa79 100644 --- a/internal/provider/kvm/kvm.go +++ b/internal/provider/kvm/kvm.go @@ -42,9 +42,6 @@ func withDefaults(cfg provider.KVMConfig) provider.KVMConfig { if cfg.DiskSizeGB == 0 { cfg.DiskSizeGB = defaultDiskSizeGB } - if cfg.VendorData == "" { - cfg.VendorData = defaultVendorData - } images := make(map[string]provider.KVMImage, len(cfg.Images)) for k, img := range cfg.Images { if img.Format == "" { @@ -249,7 +246,11 @@ func (p *Provider) writeSeed(log logr.Logger, api libvirtAPI, pool libvirt.Stora if err := deleteIfExists(api, pool, seedName); err != nil { return libvirt.StorageVol{}, err } - iso, err := buildSeedISO(name, userData, p.cfg.VendorData) + userData, vendorData, err := withGuestAgent(userData, p.cfg.VendorData) + if err != nil { + return libvirt.StorageVol{}, fmt.Errorf("%w: %v", provider.ErrPermanent, err) + } + iso, err := buildSeedISO(name, userData, vendorData) if err != nil { return libvirt.StorageVol{}, fmt.Errorf("%w: %v", provider.ErrPermanent, err) } diff --git a/internal/provider/kvm/kvm_test.go b/internal/provider/kvm/kvm_test.go index 17c7190..7b0e232 100644 --- a/internal/provider/kvm/kvm_test.go +++ b/internal/provider/kvm/kvm_test.go @@ -30,8 +30,8 @@ func TestWithDefaults(t *testing.T) { if cfg.DiskSizeGB != defaultDiskSizeGB { t.Errorf("diskSizeGb = %d, want %d", cfg.DiskSizeGB, defaultDiskSizeGB) } - if cfg.VendorData != defaultVendorData { - t.Errorf("vendorData = %q, want the default", cfg.VendorData) + if cfg.VendorData != "" { + t.Errorf("vendorData = %q, want none: the guest-agent bootstrap is merged into the user-data, not defaulted here", cfg.VendorData) } if cfg.Images["debian"].Format != "qcow2" || cfg.Images["raw"].Format != "raw" { t.Errorf("image formats = %q/%q, want qcow2 default and raw kept", cfg.Images["debian"].Format, cfg.Images["raw"].Format) @@ -187,6 +187,10 @@ func assertCreated(t *testing.T, api *fakeAPI) { if seed.format != formatRaw || len(seed.data) == 0 || int64(len(seed.data)) != seed.capacity { t.Errorf("seed = format %s, %d bytes, capacity %d; want raw with capacity == uploaded size", seed.format, len(seed.data), seed.capacity) } + _, files := readSeed(t, seed.data) + if ud := files["user-data"]; !strings.Contains(ud, "squid") || !strings.Contains(ud, guestAgentPackage) { + t.Errorf("seed user-data = %q, want the Proxy's cloud-config with the guest agent merged in", ud) + } } func TestCreate_freshHost(t *testing.T) { @@ -956,7 +960,7 @@ func TestLogging_verbosityTiers(t *testing.T) { ctx, snapshot := captureContext(tc.verbosity) req := testRequest() const sentinel = "SENTINEL-cloud-init-must-never-be-logged" - req.CloudInit = "#cloud-config\n" + sentinel + "\n" + req.CloudInit = "#cloud-config\nhostname: " + sentinel + "\n" runAllOps(t, ctx, req) diff --git a/internal/provider/kvm/seed.go b/internal/provider/kvm/seed.go index 29bc90b..e2f612d 100644 --- a/internal/provider/kvm/seed.go +++ b/internal/provider/kvm/seed.go @@ -3,28 +3,114 @@ package kvm import ( "bytes" "fmt" + "reflect" "strings" "github.com/kdomanski/iso9660" + "sigs.k8s.io/yaml" ) // seedLabel is the volume label cloud-init's NoCloud datasource looks for. const seedLabel = "cidata" -// defaultVendorData is the cloud-init vendor-data shipped on every seed -// unless the config overrides it. cloud-init merges it under the Proxy's own -// user-data, so stock images grow a guest agent (which lets the provider -// read the VM's IP before ARP sees traffic) without the user's cloud-init -// having to know about it. -const defaultVendorData = `#cloud-config +const ( + cloudConfigHeader = "#cloud-config" + guestAgentPackage = "qemu-guest-agent" +) + +// guestAgentEnable is the runcmd entry that starts the agent once it is +// installed, so the provider can read the VM's IP before ARP sees traffic. +var guestAgentEnable = []any{"systemctl", "enable", "--now", guestAgentPackage} + +// guestAgentVendorData is the same bootstrap as a standalone cloud-config. +// It is only used as vendor-data when the Proxy's user-data is not a +// cloud-config document (a shell script, a MIME archive, an #include), i.e. +// when there are no top-level keys for it to collide with. +const guestAgentVendorData = cloudConfigHeader + ` packages: - qemu-guest-agent runcmd: - [systemctl, enable, --now, qemu-guest-agent] ` -// buildSeedISO renders a NoCloud seed: user-data verbatim, vendor-data as -// given, and a meta-data whose instance-id is the deterministic VM name, so +// withGuestAgent decides where the guest-agent bootstrap goes and returns +// the user-data and vendor-data to put on the seed. +// +// It cannot simply ride along as vendor-data: cloud-init merges vendor-data +// under user-data with its default mergers, which keep the user-data's +// value for any key both define and never append lists. A Proxy whose +// cloud-config has its own `packages:` (most do) would silently lose the +// agent and sit in Provisioning forever. So for a cloud-config user-data +// the bootstrap is merged into the document itself: `qemu-guest-agent` +// appended to `packages`, the enable command to `runcmd`, both skipped +// when already present. Anything the operator cannot parse as a +// cloud-config mapping is an error — cloud-init would reject it too, and a +// VM that boots without its config is harder to diagnose than a Proxy that +// fails fast. +func withGuestAgent(userData, vendorData string) (string, string, error) { + if !isCloudConfig(userData) { + if vendorData == "" { + vendorData = guestAgentVendorData + } + return userData, vendorData, nil + } + var cfg map[string]any + if err := yaml.Unmarshal([]byte(userData), &cfg); err != nil { + return "", "", fmt.Errorf("parsing cloud-config user-data: %w", err) + } + if cfg == nil { + cfg = map[string]any{} + } + pkgs, err := appendUnique(cfg, "packages", guestAgentPackage) + if err != nil { + return "", "", err + } + cfg["packages"] = pkgs + cmds, err := appendUnique(cfg, "runcmd", guestAgentEnable) + if err != nil { + return "", "", err + } + cfg["runcmd"] = cmds + out, err := yaml.Marshal(cfg) + if err != nil { + return "", "", fmt.Errorf("rendering cloud-config user-data: %w", err) + } + return cloudConfigHeader + "\n" + string(out), vendorData, nil +} + +// isCloudConfig reports whether userData is a cloud-config document by +// cloud-init's own rule: the first line is the #cloud-config header. Empty +// user-data counts, so a Proxy without cloud-init still gets the agent. +func isCloudConfig(userData string) bool { + if strings.TrimSpace(userData) == "" { + return true + } + first, _, _ := strings.Cut(userData, "\n") + return strings.TrimSpace(first) == cloudConfigHeader +} + +// appendUnique returns cfg[key] with item appended unless an equal entry +// is already there. A missing key is an empty list; anything but a list is +// an error, since cloud-init expects one there as well. +func appendUnique(cfg map[string]any, key string, item any) ([]any, error) { + var items []any + switch v := cfg[key].(type) { + case nil: + case []any: + items = v + default: + return nil, fmt.Errorf("cloud-config %s: want a list, got %T", key, v) + } + for _, it := range items { + if reflect.DeepEqual(it, item) { + return items, nil + } + } + return append(items, item), nil +} + +// buildSeedISO renders a NoCloud seed: user-data and vendor-data verbatim, +// and a meta-data whose instance-id is the deterministic VM name, so // cloud-init never mistakes a reboot for a new instance. The ISO's volume // descriptor carries the build time, so two builds differ in a few // timestamp bytes; the files and label are identical. diff --git a/internal/provider/kvm/seed_test.go b/internal/provider/kvm/seed_test.go index a28862c..fc97464 100644 --- a/internal/provider/kvm/seed_test.go +++ b/internal/provider/kvm/seed_test.go @@ -4,10 +4,12 @@ import ( "bytes" "io" "maps" + "reflect" "strings" "testing" "github.com/kdomanski/iso9660" + "sigs.k8s.io/yaml" ) func readSeed(t *testing.T, iso []byte) (label string, files map[string]string) { @@ -42,7 +44,7 @@ func readSeed(t *testing.T, iso []byte) (label string, files map[string]string) func TestBuildSeedISO_contents(t *testing.T) { t.Parallel() userData := "#cloud-config\npackages: [squid]\n" - iso, err := buildSeedISO("proxy-abc123def456ghij", userData, defaultVendorData) + iso, err := buildSeedISO("proxy-abc123def456ghij", userData, guestAgentVendorData) if err != nil { t.Fatalf("buildSeedISO() error = %v", err) } @@ -56,7 +58,7 @@ func TestBuildSeedISO_contents(t *testing.T) { if files["user-data"] != userData { t.Errorf("user-data = %q, want verbatim cloud-init", files["user-data"]) } - if files["vendor-data"] != defaultVendorData { + if files["vendor-data"] != guestAgentVendorData { t.Errorf("vendor-data = %q", files["vendor-data"]) } if got, want := files["meta-data"], "instance-id: proxy-abc123def456ghij\nlocal-hostname: proxy-abc123def456ghij\n"; got != want { @@ -89,7 +91,7 @@ func TestBuildSeedISO_deterministicContent(t *testing.T) { func TestBuildSeedISO_emptyUserData(t *testing.T) { t.Parallel() - iso, err := buildSeedISO("proxy-x", "", defaultVendorData) + iso, err := buildSeedISO("proxy-x", "", guestAgentVendorData) if err != nil { t.Fatalf("buildSeedISO() error = %v", err) } @@ -98,3 +100,150 @@ func TestBuildSeedISO_emptyUserData(t *testing.T) { t.Errorf("user-data = %q (present=%v), want an empty file to still exist", body, ok) } } + +// parseCloudConfig is what cloud-init sees: the document under the header. +func parseCloudConfig(t *testing.T, doc string) map[string]any { + t.Helper() + if first, _, _ := strings.Cut(doc, "\n"); first != cloudConfigHeader { + t.Fatalf("first line = %q, want %q", first, cloudConfigHeader) + } + var cfg map[string]any + if err := yaml.Unmarshal([]byte(doc), &cfg); err != nil { + t.Fatalf("merged user-data is not YAML: %v\n%s", err, doc) + } + return cfg +} + +func TestWithGuestAgent_mergesIntoCloudConfig(t *testing.T) { + t.Parallel() + agentCmd := []any{"systemctl", "enable", "--now", "qemu-guest-agent"} + tests := []struct { + name string + userData string + wantPackages []any + wantRuncmd []any + }{ + { + name: "empty document", + userData: "#cloud-config\n", + wantPackages: []any{"qemu-guest-agent"}, + wantRuncmd: []any{agentCmd}, + }, + { + name: "no user-data at all", + userData: "", + wantPackages: []any{"qemu-guest-agent"}, + wantRuncmd: []any{agentCmd}, + }, + { + // The case that motivated this: cloud-init's default merger keeps + // the user-data's lists and drops vendor-data's, so the agent must + // be appended to the user's own packages and runcmd. + name: "user packages and runcmd are kept and extended", + userData: "#cloud-config\npackages:\n - squid\nruncmd:\n - systemctl restart squid\n" + + "write_files:\n - path: /etc/squid/conf.d/x.conf\n content: |\n http_port 3128\n via off\n", + wantPackages: []any{"squid", "qemu-guest-agent"}, + wantRuncmd: []any{"systemctl restart squid", agentCmd}, + }, + { + name: "agent already present is not duplicated", + userData: "#cloud-config\npackages: [qemu-guest-agent]\nruncmd:\n - [systemctl, enable, --now, qemu-guest-agent]\n", + wantPackages: []any{"qemu-guest-agent"}, + wantRuncmd: []any{agentCmd}, + }, + { + name: "header with trailing whitespace", + userData: "#cloud-config \r\npackages: [curl]\n", + wantPackages: []any{"curl", "qemu-guest-agent"}, + wantRuncmd: []any{agentCmd}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + user, vendor, err := withGuestAgent(tc.userData, "") + if err != nil { + t.Fatalf("withGuestAgent() error = %v", err) + } + if vendor != "" { + t.Errorf("vendor-data = %q, want none when the agent is merged into the user-data", vendor) + } + cfg := parseCloudConfig(t, user) + if !reflect.DeepEqual(cfg["packages"], tc.wantPackages) { + t.Errorf("packages = %#v, want %#v", cfg["packages"], tc.wantPackages) + } + if !reflect.DeepEqual(cfg["runcmd"], tc.wantRuncmd) { + t.Errorf("runcmd = %#v, want %#v", cfg["runcmd"], tc.wantRuncmd) + } + }) + } +} + +func TestWithGuestAgent_keepsOtherKeysAndVendorData(t *testing.T) { + t.Parallel() + userData := "#cloud-config\nhostname: px\nwrite_files:\n - path: /etc/x\n content: |\n line one\n line two\n permissions: '0644'\nbootcmd: [echo hi]\n" + user, vendor, err := withGuestAgent(userData, "#cloud-config\nntp: {enabled: true}\n") + if err != nil { + t.Fatalf("withGuestAgent() error = %v", err) + } + if vendor != "#cloud-config\nntp: {enabled: true}\n" { + t.Errorf("vendor-data = %q, want the configured one verbatim", vendor) + } + got := parseCloudConfig(t, user) + delete(got, "packages") + delete(got, "runcmd") + want := parseCloudConfig(t, userData) + if !reflect.DeepEqual(got, want) { + t.Errorf("merged user-data changed the user's keys:\n got %#v\nwant %#v", got, want) + } +} + +func TestWithGuestAgent_nonCloudConfigFallsBackToVendorData(t *testing.T) { + t.Parallel() + tests := []struct { + name string + userData string + vendorData string + wantVendor string + }{ + {"shell script, no vendor-data", "#!/bin/sh\napt-get install -y squid\n", "", guestAgentVendorData}, + {"include, no vendor-data", "#include\nhttps://example/x\n", "", guestAgentVendorData}, + {"shell script, own vendor-data wins", "#!/bin/sh\n", "#cloud-config\nntp: {}\n", "#cloud-config\nntp: {}\n"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + user, vendor, err := withGuestAgent(tc.userData, tc.vendorData) + if err != nil { + t.Fatalf("withGuestAgent() error = %v", err) + } + if user != tc.userData { + t.Errorf("user-data = %q, want it untouched", user) + } + if vendor != tc.wantVendor { + t.Errorf("vendor-data = %q, want %q", vendor, tc.wantVendor) + } + }) + } +} + +func TestWithGuestAgent_rejectsBrokenCloudConfig(t *testing.T) { + t.Parallel() + tests := []struct { + name string + userData string + }{ + {"not yaml", "#cloud-config\npackages: [squid\n"}, + {"not a mapping", "#cloud-config\n- squid\n"}, + {"packages is not a list", "#cloud-config\npackages: squid\n"}, + {"runcmd is not a list", "#cloud-config\nruncmd:\n a: b\n"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if _, _, err := withGuestAgent(tc.userData, ""); err == nil { + t.Errorf("withGuestAgent() = nil error, want one for %q", tc.userData) + } + }) + } +}