From ffb328661d27d2f3ef1a2aa263cefaf13dc70919 Mon Sep 17 00:00:00 2001 From: kacerr Date: Thu, 10 Sep 2026 07:40:23 +0000 Subject: [PATCH] Parse KVM domain metadata by local name: libvirt returns it without a namespace Run 3 on the real host got past Create and failed in Get: libvirt 11.6 selects the element by our URI in DomainGetMetadata but hands it back with no namespace declaration, and the parser required the URI on the element. The lookup already guarantees the namespace, so match on the local name only. The fake now strips the declaration the way libvirt does, and a new test pins the bare form. Execution log updated. Co-Authored-By: Claude Fable 5.1 --- .../2026-09-04-1404-kvm-provider.md | 19 ++++++++++++++++++- internal/provider/kvm/fake_test.go | 6 +++++- internal/provider/kvm/metadata.go | 18 ++++++++++++------ internal/provider/kvm/metadata_test.go | 15 +++++++++++++++ 4 files changed, 50 insertions(+), 8 deletions(-) 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 e9dc2bb..18f61b3 100644 --- a/docs/plans-executions/2026-09-04-1404-kvm-provider.md +++ b/docs/plans-executions/2026-09-04-1404-kvm-provider.md @@ -81,7 +81,9 @@ The metadata element is written with a default namespace marshal. libvirt may hand it back prefixed; `parseInstanceMetadata` is tested against both spellings since `encoding/xml` matches on the namespace URI. Whether libvirt accepts the default-namespace form on `DomainDefineXML` -is confirmed in Step 6 against a real host, not here. +is confirmed in Step 6 against a real host, not here. (Step 7: it does, +but `DomainGetMetadata` returns the element with no namespace at all; the +parser now matches on local name only.) Checked before relying on `github.com/kdomanski/iso9660` for the seed: its filename mangling lowercases and keeps `-`, so `user-data`/`meta-data`/ @@ -445,3 +447,18 @@ paths the fake reports. Worth noting: the first two runs left the base image and its marker in the pool as designed, so later runs skip the 435 MB download. + +### Run 3: `parsing domain metadata: expected element in name space … but have no name space` + +With file-backed disks, Create returned in 1 s (image cached) and the +domain started. Get then failed to parse the metadata: libvirt 11.6 finds +the element by our URI but returns it as a bare `` with no +namespace declaration at all. Step 2 had tested the default-namespace and +prefixed spellings and left "does libvirt accept the default-namespace +form" for the real host; it does, it just does not give it back that way. + +Fix: `parseInstanceMetadata` matches on the local name only, since the +lookup already selected the element by URI. The fake now strips the +declaration when storing metadata, so the provider's Get and ListByTag +tests parse the shape libvirt really returns, and a new +`TestParseInstanceMetadata_libvirtBareForm` pins it. diff --git a/internal/provider/kvm/fake_test.go b/internal/provider/kvm/fake_test.go index 02c52e4..2e0928d 100644 --- a/internal/provider/kvm/fake_test.go +++ b/internal/provider/kvm/fake_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "slices" + "strings" "sync" libvirt "github.com/digitalocean/go-libvirt" @@ -151,8 +152,11 @@ func (f *fakeAPI) DomainDefineXML(raw string) (libvirt.Domain, error) { if err != nil { return libvirt.Domain{}, lvError(libvirt.ErrXMLError, "%v", err) } + // libvirt hands the element back without its namespace declaration + // (seen on 11.6); mimic that so Get/ListByTag parse the real shape. + stored := strings.Replace(string(meta), ` xmlns="`+metadataURI+`"`, "", 1) existing := f.domains[d.Name] - nd := &fakeDomain{name: d.Name, xml: raw, metadata: string(meta)} + nd := &fakeDomain{name: d.Name, xml: raw, metadata: stored} if existing != nil { nd.running = existing.running nd.addrs = existing.addrs diff --git a/internal/provider/kvm/metadata.go b/internal/provider/kvm/metadata.go index 55ab4e7..7eeb110 100644 --- a/internal/provider/kvm/metadata.go +++ b/internal/provider/kvm/metadata.go @@ -31,15 +31,21 @@ func newInstanceMetadata(uid string, createdAt time.Time) instanceMetadata { } // parseInstanceMetadata decodes the element as DomainGetMetadata returns it. -// libvirt may re-serialise it with a namespace prefix rather than the default -// namespace it was defined with; encoding/xml matches on namespace URI, so -// both spellings decode. +// libvirt selects the element by namespace URI but serialises it without the +// declaration (observed on libvirt 11.6: a bare ), and other +// versions may keep a default or prefixed form. The URI is already checked +// by the lookup, so only the local name is matched here. func parseInstanceMetadata(s string) (instanceMetadata, error) { - var m instanceMetadata - if err := xml.Unmarshal([]byte(s), &m); err != nil { + var raw struct { + XMLName xml.Name `xml:"instance"` + Managed string `xml:"managed"` + UID string `xml:"uid"` + CreatedAt string `xml:"createdAt"` + } + if err := xml.Unmarshal([]byte(s), &raw); err != nil { return instanceMetadata{}, fmt.Errorf("parsing domain metadata: %w", err) } - return m, nil + return instanceMetadata{Managed: raw.Managed, UID: raw.UID, CreatedAt: raw.CreatedAt}, nil } func (m instanceMetadata) managed() bool { diff --git a/internal/provider/kvm/metadata_test.go b/internal/provider/kvm/metadata_test.go index f3ab94f..d7727f2 100644 --- a/internal/provider/kvm/metadata_test.go +++ b/internal/provider/kvm/metadata_test.go @@ -45,6 +45,21 @@ func TestParseInstanceMetadata_libvirtPrefixedForm(t *testing.T) { } } +func TestParseInstanceMetadata_libvirtBareForm(t *testing.T) { + t.Parallel() + // What libvirt 11.6 actually returns from DomainGetMetadata: the element + // selected by our URI, serialised without any namespace declaration. + in := `trueuid-3` + + `2026-09-10T07:30:00Z` + m, err := parseInstanceMetadata(in) + if err != nil { + t.Fatalf("parseInstanceMetadata() error = %v", err) + } + if !m.managed() || m.UID != "uid-3" || m.createdAt().Minute() != 30 { + t.Errorf("parsed = %+v", m) + } +} + func TestParseInstanceMetadata_notManaged(t *testing.T) { t.Parallel() tests := []struct {