diff --git a/evetest/README.md b/evetest/README.md index c19213c6ae4..1afe3d77631 100644 --- a/evetest/README.md +++ b/evetest/README.md @@ -399,7 +399,11 @@ Use the CLI to inspect state, then run `evetest continue` to resume. - **Reuse existing package-level helpers** before writing new ones. Each test package has shared helpers for common patterns — for example the networking package has `getDevicePort`, `getCurrentDPC`, `appHasError`, `niHasError`. Check the other - `_test.go` files in the package before duplicating logic. + `_test.go` files in the package before duplicating logic. When a package + accumulates enough shared helpers to need files of their own, name each file for + the state it observes — not for the test that first needed it — and record the + layout in the package comment in that package's `testsuite_test.go`; see + `tests/apps/` for a worked example. Check that comment before adding a helper. - **Do not mutate shared global state.** If a test needs to modify a package-level variable (e.g. a network model defined in `evetest/netmodels/`), operate on a deep diff --git a/evetest/edgecluster.go b/evetest/edgecluster.go index 10582d9709c..e1f370500c1 100644 --- a/evetest/edgecluster.go +++ b/evetest/edgecluster.go @@ -164,13 +164,27 @@ func clusterNodeReady(info *eveinfo.ZInfoKubeCluster, nodeName string) bool { // It watches ZInfoKubeCluster updates from all devices and returns the device // whose cluster info reports the app (matched by display name) in EveApps // or EveVmApps with a non-empty NodeName. -func (ec *EdgeCluster) FindDeviceHostingApp( - appUUID uuid.UUID, timeout time.Duration) *EdgeDevice { +// +// Pass excludeDevNames to wait for the app to move OFF the named devices, which +// is what a failover test needs. Without it, this can return a device that no +// longer hosts the app: each device's last published cluster info is consulted +// first, and that snapshot can predate the event the caller is waiting for - +// most visibly when the excluded device is powered off, since its own stale +// snapshot still names it as the host and it publishes nothing further. +// Excluded devices are skipped as a source of cluster info and are never +// returned, so the answer can only come from a device that is still up. +func (ec *EdgeCluster) FindDeviceHostingApp(appUUID uuid.UUID, + timeout time.Duration, excludeDevNames ...string) *EdgeDevice { ec.checkDevices("FindDeviceHostingApp") ctx, cancel := context.WithTimeout(ec.th.ctx, timeout) defer cancel() appUUIDStr := appUUID.String() + excluded := make(map[string]bool, len(excludeDevNames)) + for _, name := range excludeDevNames { + excluded[name] = true + } + // Look up the app display name from the first device's config. var appDisplayName string for _, dev := range ec.devices { @@ -190,24 +204,29 @@ func (ec *EdgeCluster) FindDeviceHostingApp( appUUID) } - // First check already published cluster info from all devices. - for _, dev := range ec.devices { - if info := dev.GetClusterInfo(); info != nil { - if nodeName := findAppNodeName(info, appDisplayName); nodeName != "" { - for _, d := range ec.devices { - if d.devName == nodeName { - return d - } - } - ec.th.t.Fatalf("Node %q reports hosting app %q, but no matching "+ - "device was found in cluster %q", - nodeName, appUUID, ec.clusterName) + // hostingNode returns the node the info reports as hosting the app, unless + // that node is excluded - an excluded node is what the caller is waiting for + // the app to leave, so reporting it is never the answer. + hostingNode := func(info *eveinfo.ZInfoKubeCluster) string { + nodeName := findAppNodeName(info, appDisplayName) + if nodeName == "" || excluded[nodeName] { + return "" + } + return nodeName + } + + // deviceByName maps a reported node name back to a cluster device. + deviceByName := func(nodeName string) *EdgeDevice { + for _, dev := range ec.devices { + if dev.devName == nodeName { + return dev } } + ec.th.t.Fatalf("Node %q reports hosting app %q, but no matching device "+ + "was found in cluster %q", nodeName, appUUID, ec.clusterName) + return nil } - // Subscribe to cluster info from all devices and wait for the app - // to appear with a node name. type result struct { nodeName string } @@ -216,9 +235,17 @@ func (ec *EdgeCluster) FindDeviceHostingApp( subCtx, subCancel := context.WithCancel(ctx) defer subCancel() + // Subscribe before taking the cached snapshot below, so an update landing + // between the two is not missed. for _, dev := range ec.devices { + if excluded[dev.devName] { + // A device the app must move off is not a trustworthy source: if it + // is powered off it publishes nothing, and its last snapshot is + // stale by definition. + continue + } updates, stop := dev.WatchClusterInfo() - go func(dev *EdgeDevice, updates <-chan *eveinfo.ZInfoKubeCluster, stop func()) { + go func(updates <-chan *eveinfo.ZInfoKubeCluster, stop func()) { defer stop() for { select { @@ -226,8 +253,7 @@ func (ec *EdgeCluster) FindDeviceHostingApp( if !ok { return } - nodeName := findAppNodeName(info, appDisplayName) - if nodeName != "" { + if nodeName := hostingNode(info); nodeName != "" { select { case resultCh <- result{nodeName: nodeName}: default: @@ -238,21 +264,31 @@ func (ec *EdgeCluster) FindDeviceHostingApp( return } } - }(dev, updates, stop) + }(updates, stop) + } + + // Then check the cluster info each still-eligible device has already + // published, so the common case does not have to wait for a fresh message. + for _, dev := range ec.devices { + if excluded[dev.devName] { + continue + } + if info := dev.GetClusterInfo(); info != nil { + if nodeName := hostingNode(info); nodeName != "" { + return deviceByName(nodeName) + } + } } select { case res := <-resultCh: subCancel() - // Map node name back to an EdgeDevice. - for _, dev := range ec.devices { - if dev.devName == res.nodeName { - return dev - } - } - ec.th.t.Fatalf("Node %q reports hosting app %q, but no matching device "+ - "was found in cluster %q", res.nodeName, appUUID, ec.clusterName) + return deviceByName(res.nodeName) case <-ctx.Done(): + if len(excludeDevNames) > 0 { + ec.th.t.Fatalf("Timed out waiting for app %q to move off %v in cluster %q", + appUUID, excludeDevNames, ec.clusterName) + } ec.th.t.Fatalf("Timed out waiting for app %q to be scheduled in cluster %q", appUUID, ec.clusterName) } @@ -261,7 +297,7 @@ func (ec *EdgeCluster) FindDeviceHostingApp( // findAppNodeName checks if the given cluster info contains the app // (by display name) in EveApps or EveVmApps with a non-empty NodeName. -// Kubernetes adds a hash suffix to the display name (e.g. "my-app-584dbd8fnx"), +// Kubernetes adds a hash suffix to the display name (e.g. "my-app-abcde12345"), // so we match by prefix with a "-" separator. func findAppNodeName(info *eveinfo.ZInfoKubeCluster, appDisplayName string) string { prefix := appDisplayName + "-" diff --git a/evetest/edgedevice.go b/evetest/edgedevice.go index 4ed12183b3e..2c3a0cb2bb1 100644 --- a/evetest/edgedevice.go +++ b/evetest/edgedevice.go @@ -50,6 +50,11 @@ func GetEdgeDevice(devName string) *EdgeDevice { return &EdgeDevice{th: th, devName: devName} } +// Name returns the device name. +func (d *EdgeDevice) Name() string { + return d.devName +} + // GetAllEdgeDevices returns handles for all EdgeDevices currently known to the // test th. func GetAllEdgeDevices() (devices []*EdgeDevice) { diff --git a/evetest/tests/apps/appstate_test.go b/evetest/tests/apps/appstate_test.go new file mode 100644 index 00000000000..cdc08a7649b --- /dev/null +++ b/evetest/tests/apps/appstate_test.go @@ -0,0 +1,110 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Pillar's own view of the app instance: pubsub publications and persisted state +// keyed by app UUID. +// +// Rule for this file: one reader per (agent, topic) question. Tests do not call +// evetest.ReadPublication or ReadAllPublications directly - they call a named +// reader here, so the knowledge of which agent publishes what, and how a +// transient read failure is reported, lives in one place. + +package apps_test + +import ( + "strconv" + + "github.com/lf-edge/eve/evetest" + "github.com/lf-edge/eve/pkg/pillar/types" + uuid "github.com/satori/go.uuid" +) + +// appPublication returns the publication of type T that belongs to appUUID. +// +// Every pillar status type keyed by app instance has a Key() returning the app +// UUID string (types.AppInstanceStatus.Key, types.DomainStatus.Key), which is +// what the constraint expresses. evetest.ReadAllPublications derives the pubsub +// topic name from T itself, so instantiating this is all a new reader needs. +// Note that types.VolumeStatus does NOT satisfy the intent here even though it +// has a Key(): its key is "#", not an app UUID - see +// soleVolumeStatus in appvolumes_test.go for why volumes cannot be attributed to +// an app this way at all. +func appPublication[T interface{ Key() string }]( + dev *evetest.EdgeDevice, agent string, appUUID uuid.UUID) (item T, found bool) { + pubs, err := evetest.ReadAllPublications[T](dev, agent, false) + if err != nil { + // Transient: a publication can vanish between being listed and being + // copied. Report not-found and let the caller's Eventually retry. + evetest.Logger().Warnf("appPublication: reading %s publications: %v", agent, err) + return item, false + } + for _, pub := range pubs { + if pub.Key() == appUUID.String() { + return pub, true + } + } + return item, false +} + +// appPurgePhase returns the app's state and purge phase as zedmanager itself +// publishes them. This is what distinguishes "the purge finished" from "the purge +// is wedged": a purge parked on a VolumeRefStatus removal volumemgr will never +// confirm stays in DownloadAndVerify indefinitely and never even requests the new +// volume. That code is in zedmanager, so the failure mode is not specific to a +// hypervisor. +func appPurgePhase(dev *evetest.EdgeDevice, appUUID uuid.UUID) ( + state types.SwState, purge types.Inprogress, found bool) { + status, found := appPublication[types.AppInstanceStatus](dev, "zedmanager", appUUID) + if !found { + return state, purge, false + } + return status.State, status.PurgeInprogress, true +} + +// appDomainStatus returns domainmgr's published DomainStatus for the app. There +// is at most one, because DomainStatus is keyed by app UUID - which is exactly +// why it cannot be used to count workload generations (see listAppVMIRS and +// listKVMDomainDirs in appworkload_test.go). It is authoritative for the +// domain's id, name and attached disks. +func appDomainStatus( + dev *evetest.EdgeDevice, appUUID uuid.UUID) (types.DomainStatus, bool) { + return appPublication[types.DomainStatus](dev, "domainmgr", appUUID) +} + +// purgeCounter reads the persisted purge counter zedmanager keeps for appUUID +// (pkg/pillar/types.UuidToNum, NumType "purgeCmdCounter"). This counter is +// exactly what a reboot mid-purge can corrupt - advancing the purge phase +// before the old generation is actually gone - and it is not republished +// anywhere in the EVE API, so it is read from the persisted pubsub state. +// +// found is false while the file does not exist, which is the expected state +// before an app's first purge. +func purgeCounter( + dev *evetest.EdgeDevice, appUUID uuid.UUID) (counter uint32, found bool) { + var rec types.UuidToNum + if err := evetest.ReadPublication( + dev, "zedmanager", true, appUUID.String(), &rec); err != nil { + // Absent before the app's first purge, which is expected; a transient + // read failure lands here too and the caller's retry absorbs it. + return 0, false + } + return uint32(rec.Number), true +} + +// inprogressName gives types.Inprogress a readable form for failure messages. +// The type has no String method of its own. +func inprogressName(p types.Inprogress) string { + switch p { + case types.NotInprogress: + return "NotInprogress" + case types.DownloadAndVerify: + return "DownloadAndVerify" + case types.BringDown: + return "BringDown" + case types.RecreateVolumes: + return "RecreateVolumes" + case types.BringUp: + return "BringUp" + } + return "Inprogress(" + strconv.Itoa(int(p)) + ")" +} diff --git a/evetest/tests/apps/appvolumes_test.go b/evetest/tests/apps/appvolumes_test.go new file mode 100644 index 00000000000..f168229ddd8 --- /dev/null +++ b/evetest/tests/apps/appvolumes_test.go @@ -0,0 +1,163 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// The app's disk, in all three forms it takes: volumemgr's own VolumeStatus, the +// PVC on eve-k, and the file or snapshot directory under /persist on kvm/xen. +// +// Rule for this file: readers and invariants about STORAGE. The "no artifact +// exists that EVE no longer references" checks live here rather than with the +// purge assertions on purpose - they are true after a delete, a restart or a +// snapshot rollback too, so a future non-purge test must be able to use them +// unchanged. +// +// Two scope caveats apply to everything in this file, because types.VolumeStatus +// carries no app UUID (the join is AppInstanceStatus.VolumeRefStatusList): +// +// - Device-scoped, not app-scoped. These helpers describe every volume on the +// node. They are only valid while the suite deploys exactly one app; the +// moment a second app appears they must filter through VolumeRefStatusList. +// - Single-node only. assertNoOrphanedPVCs compares cluster-wide `kubectl get +// pvc` output against ONE node's publications, which in a replicated-storage +// cluster generates false failures - which is why +// purge_during_failover_test.go documents skipping volume checks entirely. + +package apps_test + +import ( + "path" + + // revive:disable:dot-imports + . "github.com/onsi/gomega" + + "github.com/lf-edge/eve/evetest" + "github.com/lf-edge/eve/pkg/pillar/types" +) + +// volumeDirs mirrors types.VolumeClearDirName and types.VolumeEncryptedDirName. +// Which one a given volume lands in depends on VolumeStatus.Encrypted, so both +// are listed and a missing directory is not an error. +var volumeDirs = []string{"/persist/clear/volumes", "/persist/vault/volumes"} + +// soleVolumeStatus returns the node's one and only published VolumeStatus, +// failing the assertion if there is not exactly one. Named "sole" rather than +// "single" as a reminder that the scope is the whole device: see this file's +// header for when that stops being valid. +func soleVolumeStatus(g Gomega, dev *evetest.EdgeDevice) types.VolumeStatus { + vols, err := evetest.ReadAllPublications[types.VolumeStatus](dev, "volumemgr", false) + g.Expect(err).ToNot(HaveOccurred(), "reading volumemgr's VolumeStatus publications") + g.Expect(vols).To(HaveLen(1), + "expected exactly one volume, found %d - a second one would mean the old "+ + "generation's disk was never torn down", len(vols)) + if len(vols) == 0 { + return types.VolumeStatus{} + } + return vols[0] +} + +// listPVCNames returns the names of every PVC in the namespace EVE runs app +// workloads in, regardless of which app or generation it belongs to - see +// assertNoOrphanedPVCs, which is what attributes them. +func listPVCNames(dev *evetest.EdgeDevice) []string { + list, ok := kubectlListItems(dev, "pvc") + if !ok { + return nil + } + var names []string + for _, item := range list.Items { + names = append(names, item.Metadata.Name) + } + return names +} + +// assertNoOrphanedPVCs cross-checks every PVC actually present in the cluster +// against the PVC name volumemgr's own current VolumeStatus publications would +// produce (VolumeStatus.GetPVCName): "-pvc-". A PVC that +// exists in Kubernetes but that no published VolumeStatus would name is orphaned +// - Kubernetes still has the disk, but EVE no longer references it. +// +// This is strictly stronger than a volume count: the stale-generation sweep in +// hypervisor/kubevirt.go reconciles the VMIRS/ReplicaSet and its pods only, not +// PVCs, so a purge that otherwise completes cleanly can still leave a stale +// generation's disk behind indefinitely and nothing else here would catch it. +func assertNoOrphanedPVCs(g Gomega, dev *evetest.EdgeDevice) { + vols, err := evetest.ReadAllPublications[types.VolumeStatus](dev, "volumemgr", false) + g.Expect(err).ToNot(HaveOccurred(), "reading volumemgr's VolumeStatus publications") + expected := make(map[string]bool, len(vols)) + for _, v := range vols { + expected[v.GetPVCName()] = true + } + for _, name := range listPVCNames(dev) { + g.Expect(expected).To(HaveKey(name), + "PVC %q exists in the cluster but no currently published VolumeStatus "+ + "would produce it - orphaned disk left behind by a stale generation", name) + } +} + +// listVolumeArtifacts returns the full path of every entry in EVE's volume +// directories. Entries can be files (a qcow2 or raw disk) or directories (a +// container volume's snapshot mount), so this does not filter by type - see +// assertNoStaleVolumeArtifacts, which attributes them. +func listVolumeArtifacts(dev *evetest.EdgeDevice) []string { + var paths []string + for _, dir := range volumeDirs { + for _, name := range listDirEntries(dev, dir) { + paths = append(paths, path.Join(dir, name)) + } + } + return paths +} + +// assertNoStaleVolumeArtifacts is assertNoOrphanedPVCs' counterpart for a local +// hypervisor: it cross-checks every artifact on disk against the path each +// published VolumeStatus would produce (VolumeStatus.PathName). An artifact that +// exists but that no live VolumeStatus names is a stale generation's disk - the +// storage is still consumed, but EVE no longer references it. +// +// This carries the most weight on the kvm path. The duplicate-workload defects +// are structurally impossible there (no cluster-side object outlives the node), so +// a purge that goes wrong on kvm shows up as a disk nobody owns rather than as a +// second running domain. +func assertNoStaleVolumeArtifacts(g Gomega, dev *evetest.EdgeDevice) { + vols, err := evetest.ReadAllPublications[types.VolumeStatus](dev, "volumemgr", false) + g.Expect(err).ToNot(HaveOccurred(), "reading volumemgr's VolumeStatus publications") + expected := make(map[string]bool, len(vols)) + for _, v := range vols { + expected[v.PathName()] = true + } + for _, p := range listVolumeArtifacts(dev) { + g.Expect(expected).To(HaveKey(p), + "volume artifact %q exists on disk but no currently published "+ + "VolumeStatus would produce it - stale generation's disk left behind", p) + } +} + +// assertVolumeArtifactGone asserts a specific volume path no longer exists. +// Unlike assertNoStaleVolumeArtifacts, which needs volumemgr's publications to be +// trustworthy, this names the exact artifact the purge was supposed to replace. +func assertVolumeArtifactGone(g Gomega, dev *evetest.EdgeDevice, target string) { + exists, ok := pathExists(dev, target) + g.Expect(ok).To(BeTrue(), "could not check whether %q still exists", target) + g.Expect(exists).To(BeFalse(), + "the pre-purge volume %q must be deleted once the purge completes", target) +} + +// A note on why the assertions above are meaningful, because they depend on +// something outside this package: +// +// The volume's generation key changes across a purge ONLY because +// evetest.PurgeApplication bumps the generationCount of every volume the app +// references, on the VolumeRef entries and on the matching Volume entries, the +// way a real controller does. Per the API, a change to that field "indicates that +// the mutated volume needs to be purged and built from scratch. This is a +// generalization of the purge command for an application instance." +// +// Bumping only the app's purge counter would NOT recreate the volume: zedmanager +// matches volume refs on VolumeRefConfig.Key() = +// "##" +// (cmd/zedmanager/handlevolumemgr.go:218), so with an unchanged generation the +// existing reference still matches at cmd/zedmanager/updatestatus.go:406, nothing +// is removed or added, RecreateVolumes only clears VerifyOnly, and the app +// restarts on its old disk. If PurgeApplication ever regresses to bumping just +// the app counter, every assertion in this file about a changed generation or a +// vanished artifact becomes unsatisfiable rather than merely weaker - so they are +// also the tripwire for that regression. diff --git a/evetest/tests/apps/appworkload_test.go b/evetest/tests/apps/appworkload_test.go new file mode 100644 index 00000000000..1666cbb838f --- /dev/null +++ b/evetest/tests/apps/appworkload_test.go @@ -0,0 +1,106 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Where and whether the app is actually running, as the hypervisor itself sees +// it: VMIRS objects on eve-k, qemu domain state directories on kvm/xen. +// +// Rule for this file: readers that enumerate the app's WORKLOAD instances. This +// is the one place a stale generation is observable, because pillar's own +// DomainStatus is keyed by app UUID and so can only ever describe one +// (appstate_test.go). Kube readers first, local-hypervisor readers second; the +// access mechanism itself lives in deviceaccess_test.go. +// +// Split trigger: if a xen-specific reader is ever needed, break this into +// kubeworkload_test.go and localdomain_test.go. Two backends sharing one file is +// deliberate while callers choose between them purely by hypervisor and a +// reviewer needs both in view at once. + +package apps_test + +import ( + "path" + "sort" + "strconv" + "strings" + + "github.com/lf-edge/eve/evetest" + uuid "github.com/satori/go.uuid" +) + +const ( + // appDomainNameLabel holds the owning app's DomainName, + // "..". See the eveLabelKey constant in + // hypervisor/kubevirt.go. + // + // EVE puts this label in the VMIRS spec.selector.matchLabels and in the VMI + // template, but not in the VMIRS metadata.labels. Read the selector. Pillar + // attributes a VMIRS the same way, in sweepStaleGenerations. + appDomainNameLabel = "App-Domain-Name" + + // kvmDomainStateDir mirrors hypervisor/kvm.go's kvmStateDir: qemu gets one + // directory per domain, holding that domain's pidfile. EVE bind-mounts /run + // into the pillar container, so this path is readable from dom0 - the same + // assumption evetest.ReadAllPublications already makes for /run/. + kvmDomainStateDir = "/run/hypervisor/kvm" +) + +// listAppVMIRS returns the names of every VMIRS (any generation) that belongs to +// appUUID. It matches the prefix "." on the App-Domain-Name selector +// label, because the label value also carries a version and an appnum that do +// not matter here. Names are sorted, so a caller can compare the whole set. +// +// found is false if the list could not be read. An empty list then means "no +// VMIRS", and not "the device did not answer". +func listAppVMIRS( + dev *evetest.EdgeDevice, appUUID uuid.UUID) (names []string, found bool) { + list, ok := kubectlListItems(dev, "vmirs") + if !ok { + return nil, false + } + prefix := appUUID.String() + "." + for _, item := range list.Items { + if strings.HasPrefix( + item.Spec.Selector.MatchLabels[appDomainNameLabel], prefix) { + names = append(names, item.Metadata.Name) + } + } + sort.Strings(names) + return names, true +} + +// listKVMDomainDirs returns the qemu per-domain state directories belonging to +// appUUID, found by prefix on the domain name ("..", see +// types.DomainConfig.GetTaskName). +// +// Note the asymmetry with listAppVMIRS: a kvm domain name carries no purge +// counter, so two generations of the same app at the same version would share one +// directory name and be indistinguishable here. That is also why the +// duplicate-generation defect cannot take this shape on kvm at all. What this +// does catch is a directory left behind for a different version/appnum, and any +// directory for an app that should be gone entirely. +func listKVMDomainDirs(dev *evetest.EdgeDevice, appUUID uuid.UUID) []string { + prefix := appUUID.String() + "." + var names []string + for _, name := range listDirEntries(dev, kvmDomainStateDir) { + if strings.HasPrefix(name, prefix) { + names = append(names, name) + } + } + return names +} + +// kvmDomainPid reads the pid qemu wrote for domainName. found is false if the +// pidfile is absent or unparsable. +func kvmDomainPid( + dev *evetest.EdgeDevice, domainName string) (pid int, found bool) { + contents, found := readDeviceFile( + dev, path.Join(kvmDomainStateDir, domainName, "pid")) + if !found { + return 0, false + } + pid, err := strconv.Atoi(strings.TrimSpace(contents)) + if err != nil || pid <= 0 { + return 0, false + } + return pid, true +} diff --git a/evetest/tests/apps/deviceaccess_test.go b/evetest/tests/apps/deviceaccess_test.go new file mode 100644 index 00000000000..151b75a0e48 --- /dev/null +++ b/evetest/tests/apps/deviceaccess_test.go @@ -0,0 +1,208 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Raw access plumbing: the only place in this package that shells out to a +// device. +// +// Rule for this file: nothing here knows what an app, a volume or a purge is, +// and conversely no other file in the package may call RunShellScript directly - +// add a primitive here and call it instead. That keeps the count of ad-hoc SSH +// invocations bounded and gives one place to fix quoting, timeouts and +// error-vs-absent semantics. +// +// Every reader here is deliberately NON-FATAL: it warns and reports "not found" +// rather than failing the test, so callers can retry inside Eventually while a +// device is still converging. The framework's own readers now return errors +// too, so the two behave alike; these exist for paths the framework does not +// cover, such as kubectl and the hypervisor state directories. +// +// Promotion trigger: when a second suite needs kubectl access, move +// kubectlListItems to an EdgeDevice method in evetest/edgedevice.go. Do not copy +// it. It is kept local for now because tests/cluster has no kubectl calls at all, +// so the shape is unsettled after two consumers, and because the framework has no +// non-fatal read family to fit it into yet. + +package apps_test + +import ( + "encoding/json" + "sort" + "strings" + "time" + + "github.com/lf-edge/eve/evetest" +) + +const ( + // sshCmdTimeout bounds a single kubectl/cat/ls invocation run over SSH. + sshCmdTimeout = 20 * time.Second + + // eveKubeAppNamespace is the Kubernetes namespace EVE runs app workloads in; + // both VMIRS objects and their PVCs live there. See + // pkg/pillar/kubeapi.EVEKubeNameSpace. + eveKubeAppNamespace = "eve-kube-app" +) + +// kubeItemList is the minimal shape needed from any `kubectl get +// -o json`: a name per item, the item's own labels, the selector labels, and +// its status phase. A VMIRS is attributed by its selector, which is the only +// place EVE puts the App-Domain-Name label; a PVC has neither label field but +// does have Status.Phase. +type kubeItemList struct { + Items []struct { + Metadata struct { + Name string `json:"name"` + Labels map[string]string `json:"labels"` + } `json:"metadata"` + Spec struct { + Selector struct { + MatchLabels map[string]string `json:"matchLabels"` + } `json:"selector"` + } `json:"spec"` + Status struct { + Phase string `json:"phase"` + } `json:"status"` + } `json:"items"` +} + +// kubectlListItems lists one Kubernetes resource type from the EVE app +// namespace. found is false, with a warning logged, if the device is +// unreachable, k3s is not up, or the output does not parse - all of which are +// transient states a caller inside Eventually should retry rather than fail on. +// +// Reaching into Kubernetes at all is a deliberate exception to the framework +// guideline "assert against the EVE API, not internal state" (README "Writing +// Tests -> Guidelines"): there is no EVE-API-exposed signal for "how many +// generations of this app's workload exist" - the cluster-status topic zedkube +// publishes carries only the single name of the desired generation. Until that +// gap is closed, this is the only vantage point from which a stale generation +// surviving a purge is observable at all. +func kubectlListItems( + dev *evetest.EdgeDevice, resource string) (list kubeItemList, found bool) { + stdout, stderr, err := dev.RunShellScript( + "eve exec kube kubectl -n "+eveKubeAppNamespace+" get "+resource+" -o json", + sshCmdTimeout, 0) + if err != nil { + evetest.Logger().Warnf( + "kubectlListItems: kubectl get %s failed: %v (stderr: %s)", + resource, err, stderr) + return list, false + } + if err := json.Unmarshal([]byte(stdout), &list); err != nil { + evetest.Logger().Warnf( + "kubectlListItems: failed to parse kubectl %s output: %v", resource, err) + return list, false + } + return list, true +} + +// kubeEventList is the minimal shape needed from `kubectl get events -o json`: +// the reason, the human-readable message, and the name of the object the +// event is about. +type kubeEventList struct { + Items []struct { + Reason string `json:"reason"` + Message string `json:"message"` + InvolvedObject struct { + Name string `json:"name"` + } `json:"involvedObject"` + } `json:"items"` +} + +// kubectlEvents lists every event in the EVE app namespace. found is false on +// the same conditions as kubectlListItems. +func kubectlEvents(dev *evetest.EdgeDevice) (list kubeEventList, found bool) { + stdout, stderr, err := dev.RunShellScript( + "eve exec kube kubectl -n "+eveKubeAppNamespace+" get events -o json", + sshCmdTimeout, 0) + if err != nil { + evetest.Logger().Warnf( + "kubectlEvents: kubectl get events failed: %v (stderr: %s)", err, stderr) + return list, false + } + if err := json.Unmarshal([]byte(stdout), &list); err != nil { + evetest.Logger().Warnf( + "kubectlEvents: failed to parse kubectl events output: %v", err) + return list, false + } + return list, true +} + +// restartCSIProvisioner deletes Longhorn's csi-provisioner pod(s), forcing a +// fresh leader election and cache. See the REMOVE ME note atop +// longhorn_provisioner_workaround_test.go for why this exists. +func restartCSIProvisioner(dev *evetest.EdgeDevice) { + if _, stderr, err := dev.RunShellScript( + "eve exec kube kubectl -n longhorn-system delete pod -l app=csi-provisioner", + sshCmdTimeout, 0); err != nil { + evetest.Logger().Warnf("restartCSIProvisioner: delete failed: %v (stderr: %s)", err, stderr) + } +} + +// listDirEntries returns the sorted names of the entries in a directory on the +// device. A missing directory yields an empty list, not an error: several of the +// directories inspected by this suite only exist once a particular hypervisor or +// storage backend has created them. +func listDirEntries(dev *evetest.EdgeDevice, dir string) []string { + stdout, _, err := dev.RunShellScript( + "ls -1 "+dir+" 2>/dev/null || true", sshCmdTimeout, 0) + if err != nil { + evetest.Logger().Warnf("listDirEntries: ls %s failed: %v", dir, err) + return nil + } + var names []string + for _, line := range strings.Split(strings.TrimSpace(stdout), "\n") { + if name := strings.TrimSpace(line); name != "" { + names = append(names, name) + } + } + sort.Strings(names) + return names +} + +// readDeviceFile reads a file off the device. found is false if the file does not +// exist or is empty, which callers use to express "not published yet" without +// failing the test. +func readDeviceFile( + dev *evetest.EdgeDevice, path string) (contents string, found bool) { + stdout, _, err := dev.RunShellScript( + "cat "+path+" 2>/dev/null || true", sshCmdTimeout, 0) + if err != nil || strings.TrimSpace(stdout) == "" { + return "", false + } + return stdout, true +} + +// syncDisks flushes the device's filesystem caches. +// +// Needed before a hard power-off. EVE unpacks an app's container image layers +// without fsyncing them and containerd marks the snapshots Committed regardless, +// so pulling power inside the writeback window leaves the extracted layers as a +// complete directory tree of zero-length files - measured at ~150MB before a +// power-off and ~3MB after. Nothing ever re-extracts them, every later volume +// built from that image is hollow, and the app boot-loops while EVE reports it +// RUNNING. That is an EVE durability bug in its own right; syncing here keeps it +// out of tests whose subject is something else. +func syncDisks(dev *evetest.EdgeDevice) { + if _, _, err := dev.RunShellScript("sync", sshCmdTimeout, 0); err != nil { + evetest.Logger().Warnf("syncDisks: sync failed: %v", err) + } +} + +// pathExists reports whether a path exists on the device. ok is false if the +// check itself could not be run, so a caller can tell "the file is gone" apart +// from "the device did not answer". +func pathExists(dev *evetest.EdgeDevice, target string) (exists, ok bool) { + stdout, _, err := dev.RunShellScript( + "test -e "+target+" && echo present || echo gone", sshCmdTimeout, 0) + if err != nil { + return false, false + } + switch strings.TrimSpace(stdout) { + case "present": + return true, true + case "gone": + return false, true + } + return false, false +} diff --git a/evetest/tests/apps/fixtures_test.go b/evetest/tests/apps/fixtures_test.go new file mode 100644 index 00000000000..70fe0572987 --- /dev/null +++ b/evetest/tests/apps/fixtures_test.go @@ -0,0 +1,145 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Inputs to the harness, and suite-wide tunables. +// +// Rule for this file: if a helper takes no *evetest.EdgeDevice and reads nothing +// off a device, it belongs here. Everything here describes what to build before a +// test runs; nothing here observes a running system. + +package apps_test + +import ( + "time" + + eveconfig "github.com/lf-edge/eve-api/go/config" + "github.com/lf-edge/eve/evetest" + uuid "github.com/satori/go.uuid" +) + +// Timeouts for this suite, in one place so they can be reviewed together. +// +// Sizing principle: Eventually returns as soon as its condition holds, so a +// generous timeout costs nothing when the system behaves and only delays a +// genuine failure. Each of these is set from an observed duration with real +// headroom, never tuned to just-barely-pass. If one of them starts expiring, +// the answer is to find out what changed - not to raise the number. +const ( + // assertPollInterval is how often the end-state assertions re-check. + assertPollInterval = 5 * time.Second + + // appReadyTimeout bounds WaitUntilAppIsRunning. Excludes image download, + // which the framework accounts for separately. + appReadyTimeout = 10 * time.Minute + + // purgeCompleteTimeout bounds PurgeApplication(waitUntilPurged=true). + purgeCompleteTimeout = 5 * time.Minute + + // baselineTimeout bounds reading the pre-purge state. Everything it reads is + // already published by the time the app reports RUNNING, so this only + // absorbs publication lag. + baselineTimeout = 2 * time.Minute + + // purgeEndStateTimeout bounds the assertions about the NEW generation: + // counter advanced, purge phase finished, app running, one volume, one + // workload. Observed to settle within about a minute of the app coming back. + purgeEndStateTimeout = 5 * time.Minute + + // storageReclaimTimeout bounds removal of the OLD generation's disk, which + // is a separate concern on a separate clock: the purge is complete once the + // app runs on the new generation, and volumemgr reclaims the previous + // artifact afterwards - observed at roughly five minutes, far short of the + // one-hour VdiskGCTime but far longer than the purge itself. + storageReclaimTimeout = 15 * time.Minute + + // storageReclaimPollInterval is deliberately coarse: each poll shells out to + // the device, and nothing is expected to change for minutes. + storageReclaimPollInterval = 15 * time.Second + + // clusterReadyTimeout bounds a single eve-k node becoming Ready. k3s and + // Longhorn take minutes to come up, and an app deployed before that sits in + // INITIAL - burning the app-ready budget on something that is not the app. + // Measured at about six minutes on a 4-vCPU node; every other test in this + // package allows twenty. + clusterReadyTimeout = 20 * time.Minute + + // clusterFormationTimeout bounds a multi-node cluster forming, which is + // slower than one node joining itself. + clusterFormationTimeout = 30 * time.Minute + + // failoverTimeout bounds KubeVirt rescheduling a replica after its node is + // powered off. Dominated by the node-not-ready detection, not by the pod. + failoverTimeout = 10 * time.Minute +) + +// purgeDeviceRequirements returns the RequireEdgeDevice used by every test in +// this suite: a node on the requested hypervisor, always created fresh so a +// previous test's purge counters or workload generations can never leak into +// this one - the invariants asserted here are precisely about what generations +// exist, so a warm/reused device would make a false pass indistinguishable +// from a true one. +// +// On Kubevirt, grub options cap dom0/eve/ctrd vcpus to speed up cluster +// formation, mirroring tests/cluster/cluster_test.go's +// clusterDeviceRequirements. They are omitted on the other hypervisors, where +// there is no cluster to form. +func purgeDeviceRequirements(devName string, withTPM bool, + filesystem evetest.Filesystem, hv evetest.Hypervisor) evetest.RequireEdgeDevice { + req := evetest.RequireEdgeDevice{ + Name: devName, + WithTPM: withTPM, + WithHypervisor: hv, + DeviceReusePolicy: evetest.CreateFromScratchWithLiveImage, + WithFilesystem: filesystem, + } + if hv == evetest.HypervisorKubevirt { + req.WithGrubOptions = []string{ + "set_global hv_dom0_cpu_settings \"dom0_max_vcpus=4\"", + "set_global hv_eve_cpu_settings \"eve_max_vcpus=3\"", + "set_global hv_ctrd_cpu_settings \"ctrd_max_vcpus=3\"", + } + } + return req +} + +// vmShimApplication returns the ApplicationInstanceConfig for the app used +// throughout this suite: the standard evetest-ubuntu-ctr container image run +// with VirtualizationMode=HVM. On eve-k, HVM (rather than the +// container-native NOHYPER default) makes domainmgr's kube path create a +// VMIRS (hypervisor/kubevirt.go CreateReplicaVMIConfig) instead of a plain +// pod, so this "shim VM" is the cheapest fixture that exercises the +// VMIRS-lifecycle code this suite is testing. On kvm/xen the same config +// yields an ordinary qemu domain, which is what makes the two comparable. +func vmShimApplication( + displayName string, niUUID uuid.UUID) evetest.ApplicationInstanceConfig { + return evetest.ApplicationInstanceConfig{ + DisplayName: displayName, + Activate: true, + Image: evetest.DockerContainer{ + ImageName: "lfedge/evetest-ubuntu-ctr", + Tag: "1.0", + }, + VirtualizationMode: eveconfig.VmMode_HVM, + CPUs: 1, + MemoryBytes: 500 * evetest.MiB, + NetworkAdapters: []evetest.AppNetworkAdapter{ + evetest.VirtualNetworkAdapter{ + LogicalLabel: "vif0", + NetworkInstanceUUID: niUUID, + PortFwdRules: []evetest.PortFwdRule{ + { + Protocol: evetest.NetworkProtocolTCP, + EdgeNodePort: 2222, + AppPort: 22, + }, + }, + ACLAllowRules: []evetest.ACLAllowRule{ + { + Protocol: evetest.NetworkProtocolAny, + RemoteSubnet: evetest.IPSubnet("0.0.0.0/0"), + }, + }, + }, + }, + } +} diff --git a/evetest/tests/apps/longhorn_provisioner_workaround_test.go b/evetest/tests/apps/longhorn_provisioner_workaround_test.go new file mode 100644 index 00000000000..02e8105b203 --- /dev/null +++ b/evetest/tests/apps/longhorn_provisioner_workaround_test.go @@ -0,0 +1,160 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// ============================================================================ +// REMOVE ME +// ============================================================================ +// +// This file works around an infra bug in Longhorn's CSI provisioner. It is +// not related to anything this suite tests. Delete this file and the one +// call to waitForAppRunningMitigatingPVCStall (grep for it, in +// purge_during_failover_test.go) once Longhorn no longer needs a pod restart +// to recover from a stuck PVC. +// +// Observed symptom: a PVC stays Pending indefinitely. Its ProvisioningFailed +// events alternate "volume ... not found" (404) and "volume ... already +// exists" (500) - the provisioner created the Longhorn backend volume once, +// lost track of that success in its own cache, and keeps retrying against a +// name it no longer recognises. +// +// Confirmed live, on the current Longhorn version, with no EVE or pillar +// change involved: deleting the csi-provisioner pod forces a fresh leader +// election and cache, and the very next retry succeeds +// (ProvisioningSucceeded within minutes). +// +// Only wired into TestVMAppPurgeDuringFailover: that is the one test this has +// actually been observed to fail, and restarting a cluster-wide Longhorn pod +// is not something to do reflexively from every test that creates a volume. +// +// Scope note: this deletes a cluster-wide Longhorn pod. Safe here because +// each purge test gets its own freshly-created device/cluster +// (purgeDeviceRequirements), so nothing else is using it. Do not call this +// against a shared or long-lived cluster. +package apps_test + +import ( + "strings" + "time" + + "github.com/lf-edge/eve/evetest" +) + +// pvcStallSignature is the substring both ends of the failure oscillation +// share. "not found" alone is not distinctive enough to key on - a PVC can +// legitimately report "not found" for a moment during normal provisioning. +const pvcStallSignature = "already exists" + +// pvcStallCheckInterval is how often waitForAppRunningMitigatingPVCStall +// polls for the stuck-PVC signature while waitFn is blocked. +const pvcStallCheckInterval = 30 * time.Second + +// pvcStallThreshold is how long a PVC must show the signature, continuously, +// before this mitigation acts. A PVC that clears - becomes Bound, or simply +// stops matching the signature - before this elapses is a normal, if slow, +// provisioning retry; passing through and leaving it alone is the point. +const pvcStallThreshold = 2 * time.Minute + +// waitForAppRunningMitigatingPVCStall wraps waitFn - normally +// cluster.WaitUntilAppIsRunning - with a background watcher that restarts +// Longhorn's csi-provisioner if a PVC shows the stuck-PVC signature for at +// least pvcStallThreshold while waitFn is blocked. +// +// waitFn still Fatalf's on its own timeout exactly as it would unwrapped; +// this only gives the provisioner a chance to recover before that timeout. +// kubeDev is any device that can reach the cluster's kubectl - for a cluster +// test, any member node. +func waitForAppRunningMitigatingPVCStall(kubeDev *evetest.EdgeDevice, waitFn func()) { + stop := make(chan struct{}) + done := make(chan struct{}) + go func() { + defer close(done) + ticker := time.NewTicker(pvcStallCheckInterval) + defer ticker.Stop() + firstSeenStalled := map[string]time.Time{} + kicked := false + for { + select { + case <-stop: + return + case <-ticker.C: + if kicked { + // One restart per wait is enough. Retrying it on every + // tick would fight a genuinely slow (but healthy) + // provision with unnecessary leader-election churn. + continue + } + now := time.Now() + stalled := stalledPVCNames(kubeDev) + // A PVC no longer in the stalled set recovered on its own - + // forget it, so a later, unrelated stall starts its own + // clock rather than inheriting an old one. + for name := range firstSeenStalled { + if !stalled[name] { + delete(firstSeenStalled, name) + } + } + for name := range stalled { + if _, tracked := firstSeenStalled[name]; !tracked { + firstSeenStalled[name] = now + } + } + for name, since := range firstSeenStalled { + if now.Sub(since) < pvcStallThreshold { + continue + } + evetest.Logger().Warnf( + "waitForAppRunningMitigatingPVCStall: PVC %q stuck for over %s, "+ + "restarting csi-provisioner - see the REMOVE ME note in "+ + "longhorn_provisioner_workaround_test.go", name, pvcStallThreshold) + restartCSIProvisioner(kubeDev) + kicked = true + break + } + } + } + }() + waitFn() + close(stop) + <-done +} + +// stalledPVCNames returns the names of PVCs that are currently Pending and +// have a ProvisioningFailed event carrying pvcStallSignature. Checking events +// against PVCs still Pending, rather than events alone, means a PVC that has +// since become Bound is never counted, even though its old events persist +// for a while - that is exactly the "passes through once bound" behaviour +// this mitigation is meant to have. +// +// Empty on any read failure - a transient kubectl error must never +// contribute to the stall clock. +func stalledPVCNames(dev *evetest.EdgeDevice) map[string]bool { + stalled := map[string]bool{} + + pvcs, found := kubectlListItems(dev, "pvc") + if !found { + return stalled + } + pending := make(map[string]bool) + for _, item := range pvcs.Items { + if item.Status.Phase == "Pending" { + pending[item.Metadata.Name] = true + } + } + if len(pending) == 0 { + return stalled + } + + events, found := kubectlEvents(dev) + if !found { + return stalled + } + for _, ev := range events.Items { + if ev.Reason != "ProvisioningFailed" || !pending[ev.InvolvedObject.Name] { + continue + } + if strings.Contains(ev.Message, pvcStallSignature) { + stalled[ev.InvolvedObject.Name] = true + } + } + return stalled +} diff --git a/evetest/tests/apps/purge_after_power_cycle_test.go b/evetest/tests/apps/purge_after_power_cycle_test.go new file mode 100644 index 00000000000..14ade26f667 --- /dev/null +++ b/evetest/tests/apps/purge_after_power_cycle_test.go @@ -0,0 +1,263 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package apps_test + +import ( + "testing" + + // revive:disable:dot-imports + . "github.com/onsi/gomega" + + "github.com/lf-edge/eve-api/go/evecommon" + "github.com/lf-edge/eve/evetest" + "github.com/lf-edge/eve/evetest/netmodels" + "github.com/lf-edge/eve/pkg/pillar/types" +) + +// TestVMAppPurgeAfterPowerCycle exercises a purge issued while the device is +// powered off, on the hypervisor selected by the HYPERVISOR parameter. The +// sequence is identical for every hypervisor; only the end-state assertions +// differ, because what a "generation" of a workload is differs. +// +// Kubevirt (the defect, HYPERVISOR=kubevirt - the default for this test) +// --------------------------------------------------------------------- +// /run is tmpfs, so a reboot drops DomainConfig/DomainStatus while the VMIRS +// survives in etcd and re-creates its own replica; zedmanager re-detects the +// purge from the persisted counter, but the teardown check consults only /run, +// finds nothing, concludes "no domain to halt", and advances the counter - +// which, on a build with the underlying bug, creates a second VMIRS alongside +// the first. Unlike a same-node replica restart (a narrow race), this +// reproduces deterministically every time. +// +// kvm/xen (the control, HYPERVISOR=kvm) +// ------------------------------------- +// The duplicate cannot occur. A domain here is a local qemu process: the +// power-off destroys it, nothing re-creates a replica while the node is down, +// and no object outlives the node for a later purge to lose track of. The +// domain name carries no purge counter either (".."), so +// two generations could not coexist under distinct names even in principle. +// +// Running the kvm variant against a build without the fixes is therefore +// expected to PASS, and that pass is the useful result: it localises the +// duplicate-generation defect to the kube path. What the kvm variant still +// covers is everything in the purge that is not hypervisor-specific: +// +// 1. The purge must complete. zedmanager parks the outgoing VolumeRefStatus +// and waits for volumemgr to confirm the delete; if the reboot wiped +// volumemgr's own state for that reference before it was asked again, the +// confirmation never arrives and the app stays in DownloadAndVerify for +// ever, never requesting the new volume. That code is in zedmanager and is +// shared by every hypervisor, so a FAILURE of the kvm variant on master is +// the interesting outcome - it means that wedge reproduces off the kube +// path too. +// 2. The old generation's disk must be gone, and no volume artifact may be left +// that no live VolumeStatus names - the kvm-shaped form of a purge going +// wrong is a disk nobody owns, not a second running domain. +// 3. kvm must not regress from the kube fixes, which change shared zedmanager +// code and restate DomainStatus.DomainId's meaning as a cross-hypervisor +// invariant ("zero if and only if the domain is confirmed absent" - for +// kvm, the qemu pid). +// +// Network model +// ------------- +// - netmodels.SingleEthWithDHCP -- one mgmt+app port, SDN DNS, controller +// reachable. +// +// Device configuration +// -------------------- +// - purgeDeviceRequirements (fixtures_test.go) on the selected hypervisor: +// always created fresh, so no prior generation or purge counter can leak +// in; default ext4 (configurable via FILESYSTEM). +// - SystemAdapter on eth0 (DHCP, mgmt+app). +// - One Local NI "local-ni" (10.11.13.0/24, a distinct subnet from the other +// tests in this suite) and one shim-VM app (vmShimApplication). The same +// app fixture serves both variants: a container image with +// VirtualizationMode=HVM runs in a VMIRS on eve-k and in a qemu domain on +// kvm, which is what makes the two results comparable. +// +// Test parameters +// --------------- +// - HYPERVISOR (kubevirt|kvm|xen). Note the default is **kubevirt**, not the +// framework-wide kvm default: an unqualified run of this test should +// exercise the path the defect is on. Set EVETEST_HYPERVISOR=kvm for the +// control. +// - TPM via evetest.TPMParameter(). +// - FILESYSTEM (ext4|zfs, defaults to ext4) via evetest.FilesystemParameter(). +// +// Phases +// ------ +// 1. setup-done -> app-is-running: bring the node up, deploy the app, wait +// for RUNNING. +// 2. baseline-recorded: capture the volume's generation key and on-disk path, +// and read the persisted purge counter. +// 3. device-powered-off: sync, then EdgeDevice.PowerOff() (hard power-off +// through the broker, no reboot to bring it back on its own). The sync +// matters - see syncDisks in deviceaccess_test.go. +// 4. purge-issued-while-down: PurgeApplication(waitUntilPurged=false) - +// bumps the purge counter and pushes config to the controller; this must +// succeed even though the device cannot fetch it yet (see +// EdgeDevice.ApplyConfig - a push with both wait flags false never depends +// on device reachability). +// 5. device-powered-on -> app-is-running-again: EdgeDevice.PowerOn(true) +// waits for the device to boot, then WaitUntilAppIsRunning waits for the +// purge to complete. A purge wedged on a volume-ref removal never gets +// past this point, so the wait is itself an assertion. +// 6. End-state assertions: assertPurgeCompleted for what every hypervisor +// must satisfy (counter advanced, purge phase finished, one volume at a new +// generation), then assertKubePurgeEndState or +// assertLocalDomainPurgeEndState for what only that hypervisor can be +// asked. On Kubevirt the decisive one is "exactly one VMIRS, named for the +// NEW generation" - precisely what the bug violates, since both +// generations can otherwise survive indefinitely, each reporting ready. +// +// Suite placement +// --------------- +// - TestVMAppPurgeSuite, as two variants (kubevirt and kvm). +func TestVMAppPurgeAfterPowerCycle(test *testing.T) { + evetestT := evetest.Init(test) + t := NewGomegaWithT(evetestT) + defer evetest.Close() + + evetest.DefineTestParameters( + // Same key as evetest.HypervisorParameter(), but defaulting to + // Kubevirt rather than kvm: this test exists for the kube path, and + // kvm is its control. + evetest.TestParameterDefinition{ + Key: evetest.HypervisorParameterKey, + DefaultValue: evetest.HypervisorKubevirt, + Description: evetest.TestParameterDescription{ + Summary: "Hypervisor to purge the app on. Kubevirt is where the " + + "duplicate-generation defect lives; kvm/xen act as the control.", + Default: "kubevirt", + AllowedValues: "kubevirt|kvm|xen", + }, + }, + evetest.TPMParameter(), + evetest.FilesystemParameter(), + ) + hypervisor := evetest.GetHypervisorParameterValue() + withTPM := evetest.GetTPMParameterValue() + filesystem := evetest.GetFilesystemParameterValue() + + devName := "edge-dev" + requiredDevice := purgeDeviceRequirements(devName, withTPM, filesystem, hypervisor) + requiredNetModel := evetest.RequireNetworkModel{ + NetworkModel: netmodels.SingleEthWithDHCP, + } + evetest.Setup(requiredDevice, requiredNetModel) + evetest.Checkpoint("setup-done") + + devConfig := evetest.NewEdgeDeviceConfig(devName) + dhcpNet := devConfig.AddNetwork( + evetest.DHCPNetworkConfig{ + NetworkType: evecommon.NetworkType_V4Only, + }) + devConfig.AddNetworkAdapter( + evetest.NetworkAdapterConfig{ + LogicalLabel: "ethernet0", + PhysicalLabel: "eth0", + InterfaceName: "eth0", + NetworkUUID: dhcpNet, + Usage: evecommon.PhyIoMemberUsage_PhyIoUsageMgmtAndApps, + }) + + niUUID := devConfig.AddNetworkInstance(evetest.LocalNetworkInstanceConfig{ + DisplayName: "local-ni", + Port: "ethernet0", + Subnet: evetest.IPSubnet("10.11.13.0/24"), + DHCPRange: types.IPRange{ + Start: evetest.IPAddress("10.11.13.2"), + End: evetest.IPAddress("10.11.13.254"), + }, + Gateway: evetest.IPAddress("10.11.13.1"), + EnableFlowlog: true, + MTU: 1500, + ForwardLLDP: false, + }) + const appDisplayName = "purge-app" + appUUID := devConfig.AddApplication(vmShimApplication(appDisplayName, niUUID)) + + device := evetest.GetEdgeDevice(devName) + device.ApplyConfig(devConfig, true, true) + log := evetest.Logger() + log.Infof("Submitted config with application UUID=%v", appUUID) + if hypervisor == evetest.HypervisorKubevirt { + // k3s and Longhorn must be up before the app can start; without this the + // app sits in INITIAL and consumes the app-ready budget. + device.WaitForClusterNodeIsReady(clusterReadyTimeout) + } + evetest.Checkpoint("config-applied") + + device.WaitUntilAppIsRunning(appUUID, appReadyTimeout) + evetest.Checkpoint("app-is-running") + + // Baseline: the volume generation key and its on-disk path, both of which the + // purge must replace, plus the counter it must advance. Unlike + // TestVMAppPurgeBaseline there is no domain-identity check, because the power + // cycle recreates the domain by itself and a changed DomainId would prove + // nothing about the purge. + baselineVolGen := "" + baselineVolPath := "" + t.Eventually(func(g Gomega) { + vol := soleVolumeStatus(g, device) + baselineVolGen = vol.Key() + baselineVolPath = vol.PathName() + }, baselineTimeout, assertPollInterval).Should(Succeed()) + baselineCounter, _ := purgeCounter(device, appUUID) + log.Infof("Baseline volume %q at %q, purge counter %d", + baselineVolGen, baselineVolPath, baselineCounter) + evetest.Checkpoint("baseline-recorded") + + // Flush before pulling power. Without this, the image layers unpacked + // moments ago are still dirty and the power-off discards them - see + // syncDisks. The subject of this test is the purge, not unpack durability. + syncDisks(device) + + log.Infof("Powering off device %q", devName) + device.PowerOff() + evetest.Checkpoint("device-powered-off") + + device.PurgeApplication(appUUID, false, 0) + evetest.Checkpoint("purge-issued-while-down") + + log.Infof("Powering device %q back on", devName) + device.PowerOn(true) + if hypervisor == evetest.HypervisorKubevirt { + // The reboot took k3s down with it, so the node has to become Ready + // again before the purged app can be scheduled. + device.WaitForClusterNodeIsReady(clusterReadyTimeout) + } + evetest.Checkpoint("device-powered-on") + + device.WaitUntilAppIsRunning(appUUID, appReadyTimeout) + evetest.Checkpoint("app-is-running-again") + + wantCounter := baselineCounter + 1 + t.Eventually(func(g Gomega) { + assertPurgeCompleted(g, device, appUUID, wantCounter, baselineVolGen) + if hypervisor == evetest.HypervisorKubevirt { + assertKubePurgeEndState(g, device, appUUID, appDisplayName, wantCounter) + } else { + assertLocalDomainPurgeEndState(g, device, appUUID, baselineVolGen, + hypervisor) + } + }, purgeEndStateTimeout, assertPollInterval).Should(Succeed()) + evetest.Checkpoint("purge-verified") + + // The old generation's storage is reclaimed on its own schedule, minutes + // after the purge itself completes - see assertOldVolumeReclaimed. + // + // On Kubevirt, sweepStaleGenerations (hypervisor/kubevirt.go) deletes the + // old VMIRS and its pod but not its PVC. volumemgr's own periodic GC + // (gcPVCs) reclaims that PVC instead, on the vdiskGCTime/10 ticker - up to + // a few minutes, comfortably inside storageReclaimTimeout. + t.Eventually(func(g Gomega) { + if hypervisor == evetest.HypervisorKubevirt { + assertNoOrphanedPVCs(g, device) + } else { + assertOldVolumeReclaimed(g, device, baselineVolPath) + } + }, storageReclaimTimeout, storageReclaimPollInterval).Should(Succeed()) + evetest.Checkpoint("old-storage-reclaimed") +} diff --git a/evetest/tests/apps/purge_assertions_test.go b/evetest/tests/apps/purge_assertions_test.go new file mode 100644 index 00000000000..857ba6924a4 --- /dev/null +++ b/evetest/tests/apps/purge_assertions_test.go @@ -0,0 +1,189 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// End-state assertions for the purge topic. +// +// Rule for this file: assertions that are meaningless outside a purge - anything +// taking a purge counter, or asserting a generation transition. The litmus test +// is "would this still make sense in a test that never purges anything?" If yes, +// it belongs in the subject file for whatever it observes (appstate, +// appworkload, appvolumes), not here. + +package apps_test + +import ( + "fmt" + "strconv" + + // revive:disable:dot-imports + . "github.com/onsi/gomega" + + "github.com/lf-edge/eve/evetest" + "github.com/lf-edge/eve/pkg/pillar/base" + "github.com/lf-edge/eve/pkg/pillar/types" + uuid "github.com/satori/go.uuid" +) + +// assertPurgeCompleted asserts everything a completed purge must satisfy on every +// hypervisor, so one test sequence can be driven against eve-k and kvm with only +// the closing detector differing (assertKubePurgeEndState or +// assertLocalDomainPurgeEndState). +// +// The purge-phase check is the one that catches a purge which never finishes, as +// opposed to one which finishes wrongly - see appPurgePhase for that failure mode. +// +// The volume-generation check holds because evetest.PurgeApplication bumps the +// referenced volumes' generationCount alongside the app's purge counter, the way +// a controller does - see the note in appvolumes_test.go, which also explains why +// bumping only the app counter would make this assertion unsatisfiable. +func assertPurgeCompleted(g Gomega, dev *evetest.EdgeDevice, appUUID uuid.UUID, + wantCounter uint32, baselineVolGen string) { + newCounter, found := purgeCounter(dev, appUUID) + g.Expect(found).To(BeTrue(), "expected the purge counter file to exist") + g.Expect(newCounter).To(Equal(wantCounter), + "persisted purge counter must advance by exactly one") + + state, purge, found := appPurgePhase(dev, appUUID) + g.Expect(found).To(BeTrue(), "expected a published AppInstanceStatus for the app") + g.Expect(purge).To(Equal(types.NotInprogress), + "the purge must be finished, not parked in %s", inprogressName(purge)) + // domainmgr's own diagnosis goes in the message: SwState alone cannot + // distinguish "never started" from a guest that boots and then powers itself + // off, which is what a climbing TriedCount with BootFailed set looks like. + domDiag := "" + if dom, ok := appDomainStatus(dev, appUUID); ok { + domDiag = fmt.Sprintf(" (DomainStatus BootFailed=%v TriedCount=%d Error=%q)", + dom.BootFailed, dom.TriedCount, dom.Error) + } + g.Expect(state).To(Equal(types.RUNNING), + "expected the app to be RUNNING after the purge, got %v%s", state, domDiag) + + // Exactly one volume, so the old generation's disk was not left alongside + // the new one - and it is the new generation, so the disk really was rebuilt. + g.Expect(soleVolumeStatus(g, dev).Key()).NotTo(Equal(baselineVolGen), + "the app's volume must be a new generation, not the pre-purge one") +} + +// assertDomainReplaced asserts the workload the app runs in is a different one +// than before the purge, identified by DomainId: the qemu pid on kvm/xen, and on +// eve-k a value derived from the VMIRS's own metadata.uid. A purge must stop the +// old domain and start a new one, so an unchanged id means the purge advanced its +// counter without doing the work. +// +// Only useful where nothing ELSE restarted the domain in the meantime - a reboot +// changes the id by itself, so a test that power-cycles the device cannot draw any +// conclusion from this. +func assertDomainReplaced(g Gomega, dev *evetest.EdgeDevice, appUUID uuid.UUID, + baselineDomainID int) { + domStatus, found := appDomainStatus(dev, appUUID) + g.Expect(found).To(BeTrue(), "expected a published DomainStatus for the app") + g.Expect(domStatus.DomainId).NotTo(BeZero(), + "DomainId must identify a live domain; zero means 'confirmed absent'") + g.Expect(domStatus.DomainId).NotTo(Equal(baselineDomainID), + "the domain must have been replaced by the purge, but DomainId is still %d "+ + "- the purge counter advanced without the domain being recreated", + baselineDomainID) +} + +// assertExactlyOneVMIRSAtGeneration is the primary end-state detector this suite +// is built around: after a purge to newCounter there must be exactly one VMIRS for +// the app, and it must be named for the NEW generation - not the old one (a +// stalled purge leaves the old generation's VMIRS alone) and not both (the old +// generation's VMIRS surviving alongside a newly created one). +func assertExactlyOneVMIRSAtGeneration( + g Gomega, dev *evetest.EdgeDevice, appUUID uuid.UUID, appDisplayName string, + newCounter uint32) { + // base.GetAppKubeNameWithPurge would be the exact match for this (name + "-" + + // purge counter), but it is newer than the pillar module version currently + // pinned by evetest's go.mod, so the suffix is appended here instead - see + // base.GetAppKubeNameWithPurge's own implementation for why this is exactly + // equivalent. + wantName := base.GetAppKubeName(appDisplayName, appUUID) + "-" + + strconv.FormatUint(uint64(newCounter), 10) + names, found := listAppVMIRS(dev, appUUID) + g.Expect(found).To(BeTrue(), + "could not list VMIRS objects; k3s may still be starting") + g.Expect(names).To(HaveLen(1), + "expected exactly one VMIRS for the app, found %v", names) + if len(names) == 1 { + g.Expect(names[0]).To(Equal(wantName), + "the surviving VMIRS must be the NEW generation %q, not a stale one", wantName) + } +} + +// assertKubePurgeEndState is the eve-k end-state detector: the decisive "exactly +// one VMIRS, and it is the new generation" check. +// +// Reclaiming the old generation's PVC is NOT checked here, for the same reason +// the local path does not check its disk here - see assertOldVolumeReclaimed. +// Callers run assertNoOrphanedPVCs separately, on the slower clock. +func assertKubePurgeEndState(g Gomega, dev *evetest.EdgeDevice, appUUID uuid.UUID, + appDisplayName string, newCounter uint32) { + assertExactlyOneVMIRSAtGeneration(g, dev, appUUID, appDisplayName, newCounter) +} + +// assertLocalDomainPurgeEndState is assertKubePurgeEndState's counterpart for a +// hypervisor whose workload is a local process rather than a cluster object. +// +// Two things differ from the kube case. The first is what CANNOT be asserted: +// there is no "exactly one generation" check, because a kvm domain name carries no +// purge counter (see listKVMDomainDirs) - which is also why the +// duplicate-generation defect cannot take this shape on kvm at all. +// +// The second is reclaiming the old disk, which does NOT belong here: volumemgr +// removes the previous generation's artifact on its own clock, observed at +// roughly five minutes after the purge completed, so a check for it inside the +// purge's own end-state window fails on timing rather than on substance. See +// assertOldVolumeReclaimed, which the caller runs separately with a timeout +// suited to that. +func assertLocalDomainPurgeEndState(g Gomega, dev *evetest.EdgeDevice, + appUUID uuid.UUID, baselineVolGen string, hv evetest.Hypervisor) { + domStatus, found := appDomainStatus(dev, appUUID) + g.Expect(found).To(BeTrue(), "expected a published DomainStatus for the app") + g.Expect(domStatus.DomainId).NotTo(BeZero(), + "DomainId must be the live domain's pid; zero means 'confirmed absent'") + + // Which generation the running domain is attached to. DomainStatus.PurgeCounter + // would say this directly, but it is newer than the pillar module version + // evetest's go.mod pins; each disk's VolumeKey carries the same information + // (it is VolumeStatus.Key) and is available in the pinned version. + var domainVolKeys []string + for _, disk := range domStatus.DiskStatusList { + if disk.VolumeKey != "" { + domainVolKeys = append(domainVolKeys, disk.VolumeKey) + } + } + g.Expect(domainVolKeys).NotTo(ContainElement(baselineVolGen), + "the running domain must not still be attached to the pre-purge volume") + + if hv != evetest.HypervisorKVM { + // The remaining assertions read qemu's own per-domain state directory, + // which only the kvm backend writes. + return + } + domainDirs := listKVMDomainDirs(dev, appUUID) + g.Expect(domainDirs).To(HaveLen(1), + "expected exactly one qemu domain state directory for the app, found %v", + domainDirs) + if len(domainDirs) != 1 { + return + } + g.Expect(domainDirs[0]).To(Equal(domStatus.DomainName), + "the domain directory must be the one DomainStatus names") + pid, ok := kvmDomainPid(dev, domainDirs[0]) + g.Expect(ok).To(BeTrue(), "expected a readable pidfile for %q", domainDirs[0]) + g.Expect(pid).To(Equal(domStatus.DomainId), + "DomainStatus.DomainId must be the pid qemu wrote for the domain") +} + +// assertOldVolumeReclaimed asserts the previous generation's disk is eventually +// removed. Deliberately separate from the purge's end state: the purge is +// complete once the app runs on the new generation, while reclaiming the old +// artifact happens minutes later on volumemgr's own schedule (well before the +// one-hour VdiskGCTime, but far outside the purge window). Give it its own, +// longer Eventually. +func assertOldVolumeReclaimed( + g Gomega, dev *evetest.EdgeDevice, baselineVolPath string) { + assertVolumeArtifactGone(g, dev, baselineVolPath) + assertNoStaleVolumeArtifacts(g, dev) +} diff --git a/evetest/tests/apps/purge_baseline_test.go b/evetest/tests/apps/purge_baseline_test.go new file mode 100644 index 00000000000..ff6bac7770a --- /dev/null +++ b/evetest/tests/apps/purge_baseline_test.go @@ -0,0 +1,167 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package apps_test + +import ( + "testing" + + // revive:disable:dot-imports + . "github.com/onsi/gomega" + + "github.com/lf-edge/eve-api/go/evecommon" + "github.com/lf-edge/eve/evetest" + "github.com/lf-edge/eve/evetest/netmodels" + "github.com/lf-edge/eve/pkg/pillar/types" +) + +// TestVMAppPurgeBaseline exercises a plain purge of a healthy, undisturbed +// app: exactly one VMIRS must exist both before and after, and it must be +// named for the new purge generation afterward. It is used as the control +// case for the other tests in this suite (TestVMAppPurgeAfterPowerCycle, +// TestVMAppPurgeDuringFailover) and as a general regression guard for the +// purge path. +// +// Network model +// ------------- +// - netmodels.SingleEthWithDHCP -- one mgmt+app port, SDN DNS, controller +// reachable. +// +// Device configuration +// -------------------- +// - purgeDeviceRequirements (fixtures_test.go), called with +// HypervisorKubevirt - this test needs a cluster and is not run on any +// other hypervisor; +// DeviceReusePolicy=CreateFromScratchWithLiveImage (a stale VMIRS +// generation or purge counter from a prior test must never leak into +// this one), default ext4 (configurable via FILESYSTEM), grub options +// capping dom0/eve/ctrd vcpus for faster cluster formation. +// - SystemAdapter on eth0 (DHCP, mgmt+app). +// - One Local NI "local-ni" (10.11.12.0/24) and one shim-VM app +// (vmShimApplication in fixtures_test.go: evetest-ubuntu-ctr:1.0, +// VirtualizationMode=HVM, so it runs as a VMIRS). +// +// Test parameters +// --------------- +// - TPM via evetest.TPMParameter(). +// - FILESYSTEM (ext4|zfs, defaults to ext4) via evetest.FilesystemParameter(). +// +// Phases +// ------ +// 1. setup-done -> app-is-running: bring up the single-node eve-k cluster, +// deploy the app, wait for RUNNING. +// 2. baseline-recorded: capture the volume's generation key, the running +// domain's DomainId, and the persisted purge counter (0 or absent). +// 3. purge-issued -> purge-complete: device.PurgeApplication(waitUntilPurged +// =true) - this exercises the framework's own PURGING/HALTING -> RUNNING +// wait, i.e. the "normal" zedmanager state-machine path. +// 4. End-state assertions: exactly one VMIRS, named for the NEW generation; +// persisted purge counter advanced by exactly one; purge phase back to +// NotInprogress with the app RUNNING; exactly one volume; a DomainId +// different from the baseline, i.e. the domain really was recreated; no PVC +// in the cluster left orphaned (unreferenced by any current VolumeStatus); +// and a volume generation key different from the baseline, i.e. the disk was +// rebuilt (which holds because PurgeApplication bumps the referenced +// volumes' generationCount the way a controller does - see the note in +// appvolumes_test.go). +// +// Suite placement +// --------------- +// - TestVMAppPurgeSuite (all tests in this suite are pinned to Kubevirt). +func TestVMAppPurgeBaseline(test *testing.T) { + evetestT := evetest.Init(test) + t := NewGomegaWithT(evetestT) + defer evetest.Close() + + evetest.DefineTestParameters( + evetest.TPMParameter(), + evetest.FilesystemParameter(), + ) + withTPM := evetest.GetTPMParameterValue() + filesystem := evetest.GetFilesystemParameterValue() + + devName := "edge-dev" + requiredDevice := purgeDeviceRequirements(devName, withTPM, filesystem, + evetest.HypervisorKubevirt) + requiredNetModel := evetest.RequireNetworkModel{ + NetworkModel: netmodels.SingleEthWithDHCP, + } + evetest.Setup(requiredDevice, requiredNetModel) + evetest.Checkpoint("setup-done") + + devConfig := evetest.NewEdgeDeviceConfig(devName) + dhcpNet := devConfig.AddNetwork( + evetest.DHCPNetworkConfig{ + NetworkType: evecommon.NetworkType_V4Only, + }) + devConfig.AddNetworkAdapter( + evetest.NetworkAdapterConfig{ + LogicalLabel: "ethernet0", + PhysicalLabel: "eth0", + InterfaceName: "eth0", + NetworkUUID: dhcpNet, + Usage: evecommon.PhyIoMemberUsage_PhyIoUsageMgmtAndApps, + }) + + niUUID := devConfig.AddNetworkInstance(evetest.LocalNetworkInstanceConfig{ + DisplayName: "local-ni", + Port: "ethernet0", + Subnet: evetest.IPSubnet("10.11.12.0/24"), + DHCPRange: types.IPRange{ + Start: evetest.IPAddress("10.11.12.2"), + End: evetest.IPAddress("10.11.12.254"), + }, + Gateway: evetest.IPAddress("10.11.12.1"), + EnableFlowlog: true, + MTU: 1500, + ForwardLLDP: false, + }) + const appDisplayName = "purge-app" + appUUID := devConfig.AddApplication(vmShimApplication(appDisplayName, niUUID)) + + device := evetest.GetEdgeDevice(devName) + device.ApplyConfig(devConfig, true, true) + log := evetest.Logger() + log.Infof("Submitted config with application UUID=%v", appUUID) + // k3s and Longhorn must be up before the app can start; without this the + // app sits in INITIAL and consumes the app-ready budget. + device.WaitForClusterNodeIsReady(clusterReadyTimeout) + evetest.Checkpoint("config-applied") + + device.WaitUntilAppIsRunning(appUUID, appReadyTimeout) + evetest.Checkpoint("app-is-running") + + // The volume generation and the domain's identity before the purge. Nothing + // else restarts the app in this test, so a DomainId that has not changed + // afterwards means the purge advanced its counter without recreating the + // domain. + baselineVolGen := "" + baselineDomainID := 0 + t.Eventually(func(g Gomega) { + baselineVolGen = soleVolumeStatus(g, device).Key() + domStatus, found := appDomainStatus(device, appUUID) + g.Expect(found).To(BeTrue(), "expected a published DomainStatus for the app") + g.Expect(domStatus.DomainId).NotTo(BeZero(), "expected a live DomainId") + baselineDomainID = domStatus.DomainId + }, baselineTimeout, assertPollInterval).Should(Succeed()) + baselineCounter, _ := purgeCounter(device, appUUID) + evetest.Checkpoint("baseline-recorded") + + device.PurgeApplication(appUUID, true, purgeCompleteTimeout) + evetest.Checkpoint("purge-complete") + + wantCounter := baselineCounter + 1 + t.Eventually(func(g Gomega) { + assertPurgeCompleted(g, device, appUUID, wantCounter, baselineVolGen) + assertDomainReplaced(g, device, appUUID, baselineDomainID) + assertKubePurgeEndState(g, device, appUUID, appDisplayName, wantCounter) + }, purgeEndStateTimeout, assertPollInterval).Should(Succeed()) + evetest.Checkpoint("purge-verified") + + // The old generation's PVC is reclaimed after the purge completes, not as + // part of it - see assertOldVolumeReclaimed for why this gets its own clock. + t.Eventually(func(g Gomega) { + assertNoOrphanedPVCs(g, device) + }, storageReclaimTimeout, storageReclaimPollInterval).Should(Succeed()) + evetest.Checkpoint("old-storage-reclaimed") +} diff --git a/evetest/tests/apps/purge_during_failover_test.go b/evetest/tests/apps/purge_during_failover_test.go new file mode 100644 index 00000000000..6f9ba0c7765 --- /dev/null +++ b/evetest/tests/apps/purge_during_failover_test.go @@ -0,0 +1,213 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package apps_test + +import ( + "fmt" + "testing" + "time" + + // revive:disable:dot-imports + . "github.com/onsi/gomega" + + eveconfig "github.com/lf-edge/eve-api/go/config" + "github.com/lf-edge/eve-api/go/evecommon" + "github.com/lf-edge/eve/evetest" + "github.com/lf-edge/eve/evetest/netmodels" + "github.com/lf-edge/eve/pkg/pillar/types" +) + +// TestVMAppPurgeDuringFailover exercises a purge issued after the app's +// designated node has failed over: the app's designated node is powered +// off, KubeVirt reschedules the replica onto a different node, and a purge +// is then issued while the designated node is still down. The purge must +// not wait on the dead node: gating the teardown on the app's designated +// node, or on where a replica currently happens to be scheduled, would +// deadlock exactly this case, because neither signal is both durable and +// liveness-aware on its own. +// +// Network model +// ------------- +// - netmodels.SeparateClusterPort -- six ports (two per device): eth0 +// ports share a management+app SDN bridge with DHCP and controller +// reachability; eth1 ports share a separate cluster-only bridge used +// for inter-node K3s traffic. +// +// Device configuration +// -------------------- +// - Three purgeDeviceRequirements (fixtures_test.go) devices, called with +// HypervisorKubevirt - same fresh-image/vcpu-cap setup as the other +// Kubevirt tests in this suite, +// following tests/cluster/cluster_test.go's TestThreeNodesCluster +// topology. +// - ClusterConfig (REPLICATED_STORAGE) with three ClusterNode entries on +// 10.244.244.0/24; node 1 is the bootstrap node. +// - One Local NI "local-ni" (10.11.14.0/24) and one shim-VM app +// (vmShimApplication) with DesignatedNodeName=devName[0] (node 1) and +// Affinity=PREFERRED. +// +// Test parameters +// --------------- +// - TPM via evetest.TPMParameter(). +// - FILESYSTEM (ext4|zfs, defaults to ext4) via evetest.FilesystemParameter(). +// +// Phases +// ------ +// 1. setup-done -> nodes-are-ready: bring up the three-node cluster. +// 2. app-is-deployed: deploy the app; assert it is in fact running on its +// preferred (DNID) node, node 1, while node 1 is healthy. +// 3. dnid-node-powered-off: EdgeDevice.PowerOff() on node 1. +// 4. failed-over: EdgeCluster.FindDeviceHostingApp with node 1 excluded waits +// for KubeVirt to reschedule the replica onto node 2 or node 3. The +// exclusion matters: without it the powered-off node's own stale cluster +// info still names it as the host and would be returned immediately. +// 5. purge-complete: EdgeCluster.PurgeApplication(waitUntilPurged=true) - +// this bumps the purge counter on every device (including the +// powered-off node 1 - EdgeDevice.ApplyConfig's push does not require +// device reachability) but waits only on the node actually hosting the +// app; there is no exclusive gate on the delete itself. +// 6. End-state assertion: exactly one VMIRS, named for the NEW generation, +// observed from the node that now hosts the app. Unlike the other two +// tests in this suite, this one makes no volume or guest-level assertion +// (VolumeStatus is a per-node ephemeral publication and its +// clustered/replicated-storage semantics across a node failover have +// not been established for this suite; the app's forwarded SSH port on +// the new host has not been either). +// 7. dnid-node-powered-on: power node 1 back on so cluster teardown does +// not have to reason about an already-off device. +// +// Suite placement +// --------------- +// - TestVMAppPurgeSuite (all tests in this suite are pinned to Kubevirt). +func TestVMAppPurgeDuringFailover(test *testing.T) { + evetestT := evetest.Init(test) + t := NewGomegaWithT(evetestT) + defer evetest.Close() + + evetest.DefineTestParameters( + evetest.TPMParameter(), + evetest.FilesystemParameter(), + ) + withTPM := evetest.GetTPMParameterValue() + filesystem := evetest.GetFilesystemParameterValue() + + var requiredDevices [3]evetest.Requirement + var devName [3]string + for i := 0; i < 3; i++ { + devName[i] = fmt.Sprintf("edge-dev%d", i+1) + requiredDevices[i] = purgeDeviceRequirements(devName[i], withTPM, filesystem, + evetest.HypervisorKubevirt) + } + requiredNetModel := evetest.RequireNetworkModel{ + NetworkModel: netmodels.SeparateClusterPort, + } + var requirements []evetest.Requirement + requirements = append(requirements, requiredDevices[:]...) + requirements = append(requirements, requiredNetModel) + evetest.Setup(requirements...) + evetest.Checkpoint("setup-done") + + var nodes [3]evetest.ClusterNode + for i := 0; i < 3; i++ { + clusterIP := evetest.IPAddressWithPrefix(fmt.Sprintf("10.244.244.%d/24", i+2)) + nodes[i] = evetest.ClusterNode{ + DevName: devName[i], + ClusterIP: clusterIP, + ClusterInterface: "ethernet1", + BootstrapNode: i == 0, + } + } + clusterConfig := evetest.NewEdgeClusterConfig( + eveconfig.ClusterType_CLUSTER_TYPE_REPLICATED_STORAGE, + nodes[:]..., + ) + + dhcpNet := clusterConfig.AddNetwork( + evetest.DHCPNetworkConfig{ + NetworkType: evecommon.NetworkType_V4Only, + }) + noIPNet := clusterConfig.AddNetwork(evetest.NoIPNetworkConfig{}) + clusterConfig.AddNetworkAdapter( + evetest.NetworkAdapterConfig{ + LogicalLabel: "ethernet0", + PhysicalLabel: "eth0", + InterfaceName: "eth0", + NetworkUUID: dhcpNet, + Usage: evecommon.PhyIoMemberUsage_PhyIoUsageMgmtAndApps, + }) + clusterConfig.AddNetworkAdapter( + evetest.NetworkAdapterConfig{ + LogicalLabel: "ethernet1", + PhysicalLabel: "eth1", + InterfaceName: "eth1", + NetworkUUID: noIPNet, + Usage: evecommon.PhyIoMemberUsage_PhyIoUsageShared, + }) + + cluster := evetest.NewEdgeCluster("purge-failover-cluster") + cluster.ApplyConfig(clusterConfig, true, true) + evetest.Checkpoint("initial-config-applied") + + cluster.WaitUntilNodesAreReady(clusterFormationTimeout) + evetest.Checkpoint("nodes-are-ready") + + niUUID := clusterConfig.AddNetworkInstance(evetest.LocalNetworkInstanceConfig{ + DisplayName: "local-ni", + Port: "ethernet0", + Subnet: evetest.IPSubnet("10.11.14.0/24"), + DHCPRange: types.IPRange{ + Start: evetest.IPAddress("10.11.14.2"), + End: evetest.IPAddress("10.11.14.254"), + }, + Gateway: evetest.IPAddress("10.11.14.1"), + EnableFlowlog: true, + MTU: 1500, + ForwardLLDP: false, + }) + const appDisplayName = "purge-app" + appUUID := clusterConfig.AddApplication(evetest.ClusterApplicationInstanceConfig{ + ApplicationInstanceConfig: vmShimApplication(appDisplayName, niUUID), + DesignatedNodeName: devName[0], + Affinity: eveconfig.AffinityType_AFFINITY_TYPE_PREFERRED, + }) + cluster.ApplyConfig(clusterConfig, true, true) + log := evetest.Logger() + log.Infof("Submitted config with application UUID=%v, DNID node=%q", appUUID, devName[0]) + evetest.Checkpoint("app-config-is-submitted") + + // Any cluster member can reach kubectl; devName[0] is as good as any for + // the mitigation's own queries while the wait below is blocked. + kubeDev := evetest.GetEdgeDevice(devName[0]) + waitForAppRunningMitigatingPVCStall(kubeDev, func() { + cluster.WaitUntilAppIsRunning(appUUID, appReadyTimeout) + }) + evetest.Checkpoint("app-is-deployed") + + initialHost := cluster.FindDeviceHostingApp(appUUID, time.Minute) + t.Expect(initialHost.Name()).To(Equal(devName[0]), + "app should have been scheduled onto its preferred (DNID) node while it is healthy") + + dnidDevice := evetest.GetEdgeDevice(devName[0]) + baselineCounter, _ := purgeCounter(dnidDevice, appUUID) + + log.Infof("Powering off DNID node %q to force a failover", devName[0]) + dnidDevice.PowerOff() + evetest.Checkpoint("dnid-node-powered-off") + + failoverHost := cluster.FindDeviceHostingApp(appUUID, failoverTimeout, devName[0]) + log.Infof("App failed over to device %q", failoverHost.Name()) + evetest.Checkpoint("failed-over") + + cluster.PurgeApplication(appUUID, true, purgeCompleteTimeout) + evetest.Checkpoint("purge-complete") + + wantCounter := baselineCounter + 1 + t.Eventually(func(g Gomega) { + assertExactlyOneVMIRSAtGeneration(g, failoverHost, appUUID, appDisplayName, wantCounter) + }, purgeEndStateTimeout, assertPollInterval).Should(Succeed()) + + log.Infof("Powering DNID node %q back on", devName[0]) + dnidDevice.PowerOn(true) + evetest.Checkpoint("dnid-node-powered-on") +} diff --git a/evetest/tests/apps/testsuite_test.go b/evetest/tests/apps/testsuite_test.go index 824822be9f7..37afa0e151e 100644 --- a/evetest/tests/apps/testsuite_test.go +++ b/evetest/tests/apps/testsuite_test.go @@ -1,6 +1,48 @@ // Copyright (c) 2026 Zededa, Inc. // SPDX-License-Identifier: Apache-2.0 +// Package apps_test holds the EVE application-lifecycle tests. +// +// Helper layout. helpers_test.go holds the general app-lifecycle helpers shared +// by the whole package (device name, image and NI constants, app SSH auth, +// addLocalNI, singleVIFWithSSH, deleteAppAndWait, waitForAppSSH). The purge +// tests add helpers of their own, in files named for the STATE THEY OBSERVE +// rather than for the test that first needed them, each owning both its readers +// and its invariants: +// +// fixtures_test.go purge-suite device requirements and app config +// deviceaccess_test.go the only place that shells out to a device: kubectl, +// cat, ls, test -e +// appstate_test.go pillar's own view of the app - pubsub and persisted +// state keyed by app UUID +// appworkload_test.go where the app is running as the hypervisor sees it - +// VMIRS objects, qemu domain state directories +// appvolumes_test.go the app's disk in all three forms - VolumeStatus, +// PVC, file under /persist - and the storage invariants +// _assertions_test.go +// assertions meaningless outside that topic +// longhorn_provisioner_workaround_test.go +// REMOVE ME once resolved - works around a Longhorn +// CSI-provisioner bug unrelated to this suite +// +// Where does a new helper go? +// +// Q0 Is it generally useful to any app test? +// Yes -> helpers_test.go +// Q1 Does it touch a device at all? +// No -> fixtures_test.go +// Q2 Does it shell out (ssh/kubectl/cat/ls)? +// Yes -> the raw call goes in deviceaccess_test.go; the caller goes to Q3 +// Q3 Would it still make sense in a test that never purges anything? +// Yes -> the subject file for what it observes +// No -> _assertions_test.go +// +// A new test scenario adds one __test.go and no helper file. A +// new topic (restart, delete, snapshot) adds its tests plus at most one +// _assertions_test.go and reuses the subject files unchanged. A new +// observation subject adds one app_test.go. A new ACCESS MECHANISM - +// another kubectl verb, virsh, a new path - adds no file at all; extend +// deviceaccess_test.go. package apps_test import ( @@ -60,3 +102,53 @@ func TestAppsSuite(test *testing.T) { }, ) } + +// TestVMAppPurgeSuite groups the app-purge tests: a plain purge +// (TestVMAppPurgeBaseline), a purge issued while the device is powered off +// (TestVMAppPurgeAfterPowerCycle - a deterministic reproduction of a +// reboot-during-purge bug), and a purge issued after the app's designated node +// has failed over (TestVMAppPurgeDuringFailover). +// +// It is separate from TestAppsSuite because these tests assert on which +// generation of a workload exists, so each one needs a device created from +// scratch (see purgeDeviceRequirements) rather than the warm device TestAppsSuite +// reuses across its subtests. +// +// The baseline and failover tests need a cluster and so are pinned to Kubevirt +// (eve-k). The power-cycle test runs twice, once per hypervisor: Kubevirt is +// where the duplicate-generation defect lives, and kvm is the control that +// localises it there - see that test's own comment for what each variant can +// and cannot assert. +func TestVMAppPurgeSuite(test *testing.T) { + evetest.Init(test) + defer evetest.Close() + + evetest.DefineTestParameters( + evetest.TPMParameter(), + evetest.FilesystemParameter(), + ) + + evetest.RunTestSuite( + evetest.TestCase{Test: TestVMAppPurgeBaseline}, + evetest.TestCase{ + Test: TestVMAppPurgeAfterPowerCycle, + Variants: []evetest.TestVariant{ + { + Name: "TestVMAppPurgeAfterPowerCycleKubevirt", + Parameters: []evetest.TestParameterValue{ + {Key: evetest.HypervisorParameterKey, + Value: evetest.HypervisorKubevirt}, + }, + }, + { + Name: "TestVMAppPurgeAfterPowerCycleKVM", + Parameters: []evetest.TestParameterValue{ + {Key: evetest.HypervisorParameterKey, + Value: evetest.HypervisorKVM}, + }, + }, + }, + }, + evetest.TestCase{Test: TestVMAppPurgeDuringFailover}, + ) +} diff --git a/pkg/pillar/cmd/volumemgr/initialvolumestatus.go b/pkg/pillar/cmd/volumemgr/initialvolumestatus.go index b15c2d9083e..b6f8e18be29 100644 --- a/pkg/pillar/cmd/volumemgr/initialvolumestatus.go +++ b/pkg/pillar/cmd/volumemgr/initialvolumestatus.go @@ -109,7 +109,7 @@ func gcObjects(ctx *volumemgrContext, dirName string) { } locations = append(locations, filepath.Join(dirName, location.Name())) } - gcVolumes(ctx, locations) + gcVolumes(ctx, locations, getVolumeStatusByLocation) log.Tracef("gcObjects(%s) Done", dirName) } @@ -122,30 +122,72 @@ func gcDatasets(ctx *volumemgrContext, dataset string) { dataset, err) return } - gcVolumes(ctx, locations) + gcVolumes(ctx, locations, getVolumeStatusByLocation) log.Tracef("gcDatasets(%s) Done", dataset) } -func gcVolumes(ctx *volumemgrContext, locations []string) { - for _, location := range locations { - tempVolumeStatus, err := getVolumeStatusByLocation(location) +// getPVCList is kubeapi.GetPVCList behind a package-level var, so a test can +// supply a fixed PVC list without a real Kubernetes API server. See +// hypervisor/kubevirt.go's newKubevirtClient for the same pattern. +var getPVCList = kubeapi.GetPVCList + +// gcPVCs is gcObjects' counterpart for PVC-backed volumes: it garbage +// collects any PVC with no currently-published VolumeStatus. The hypervisor's +// own purge sweep (kubevirt.go's sweepStaleGenerations) deletes a stale +// generation's VMIRS and pod but not its PVC; this periodic pass is what +// reclaims it instead. Call only when base.IsHVTypeKube(). +func gcPVCs(ctx *volumemgrContext) { + log.Tracef("gcPVCs") + names, err := getPVCList(log) + if err != nil { + log.Errorf("gcPVCs: GetPVCList failed: %v", err) + return + } + gcVolumes(ctx, names, getVolumeStatusByPVC) + log.Tracef("gcPVCs Done") +} + +// resolveVolumeStatus turns one GC candidate - a file/dataset path or a PVC +// name - into the VolumeStatus it would have if it were still in use. +type resolveVolumeStatus func(candidate string) (*types.VolumeStatus, error) + +// volumesToReap resolves each candidate and returns those with no +// currently-published VolumeStatus - the ones gcVolumes will destroy. +// +// Split out from gcVolumes so this decision is testable on its own. For +// PVC-backed volumes, destroying is a real Kubernetes API call; this +// function makes none. +func volumesToReap(ctx *volumemgrContext, candidates []string, + resolve resolveVolumeStatus) []*types.VolumeStatus { + var reap []*types.VolumeStatus + for _, candidate := range candidates { + vs, err := resolve(candidate) if err != nil { - log.Errorf("gcVolumes: getVolumeStatusByLocation '%s' failed: %v", - location, err) + log.Errorf("volumesToReap: resolve %q failed: %v", candidate, err) continue } - // Do not GC a replicated volume. - if tempVolumeStatus != nil && tempVolumeStatus.IsReplicated { + // Do not GC a replicated volume: it may be tracked here only because + // this node is a failover candidate, not because it is unused. + if vs != nil && vs.IsReplicated { continue } - vs := ctx.LookupVolumeStatus(tempVolumeStatus.Key()) - if vs == nil { - log.Functionf("gcVolumes: Found unused volume %s. Deleting it.", - location) - if _, err := volumehandlers.GetVolumeHandler(log, ctx, tempVolumeStatus).DestroyVolume(); err != nil { - log.Errorf("gcVolumes: destroyVolume '%s' failed: %v", - location, err) - } + if ctx.LookupVolumeStatus(vs.Key()) == nil { + reap = append(reap, vs) + } + } + return reap +} + +// gcVolumes destroys every candidate volumesToReap finds unused. candidates +// may be filesystem paths, ZFS dataset names, or PVC names; resolve is what +// tells them apart. +func gcVolumes(ctx *volumemgrContext, candidates []string, resolve resolveVolumeStatus) { + for _, vs := range volumesToReap(ctx, candidates, resolve) { + log.Functionf("gcVolumes: Found unused volume %s. Deleting it.", + vs.FileLocation) + if _, err := volumehandlers.GetVolumeHandler(log, ctx, vs).DestroyVolume(); err != nil { + log.Errorf("gcVolumes: destroyVolume '%s' failed: %v", + vs.FileLocation, err) } } } diff --git a/pkg/pillar/cmd/volumemgr/initialvolumestatus_test.go b/pkg/pillar/cmd/volumemgr/initialvolumestatus_test.go new file mode 100644 index 00000000000..d65427ed60d --- /dev/null +++ b/pkg/pillar/cmd/volumemgr/initialvolumestatus_test.go @@ -0,0 +1,130 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package volumemgr + +import ( + "fmt" + "testing" + + "github.com/lf-edge/eve/pkg/pillar/base" + "github.com/lf-edge/eve/pkg/pillar/types" + uuid "github.com/satori/go.uuid" + "github.com/stretchr/testify/assert" +) + +func TestGetVolumeStatusByPVC(t *testing.T) { + id := uuid.Must(uuid.NewV4()) + pvcName := fmt.Sprintf("%s-pvc-3", id.String()) + + vs, err := getVolumeStatusByPVC(pvcName) + assert.NoError(t, err) + assert.Equal(t, id, vs.VolumeID) + assert.Equal(t, int64(3), vs.GenerationCounter) + assert.Equal(t, pvcName, vs.FileLocation) + // getPVCList/getVolumeStatusByPVC cannot recover LocalGenerationCounter + // from the PVC name alone, so GetPVCName must round-trip against the + // GenerationCounter this parsed - otherwise volumesToReap would compute + // the wrong Key() and reap a live volume. + assert.Equal(t, pvcName, vs.GetPVCName()) +} + +func TestGetVolumeStatusByPVCInvalid(t *testing.T) { + cases := []string{ + "not-a-uuid-pvc-3", + "11111111-1111-1111-1111-111111111111-pvc-notanumber", + "11111111-1111-1111-1111-111111111111", // no "-pvc-" separator + } + for _, name := range cases { + _, err := getVolumeStatusByPVC(name) + assert.Error(t, err, "expected an error for PVC name %q", name) + } +} + +// TestVolumesToReapSkipsLiveVolume pins the safety property this whole +// change depends on: a volume this node still tracks - including one it +// tracks only because it is a failover candidate for an app running +// elsewhere in the cluster, not because it is unused - must never be +// reaped, regardless of what resolve() reconstructs from its name alone. +func TestVolumesToReapSkipsLiveVolume(t *testing.T) { + ctx := initStatusCtx(t) + live := types.VolumeStatus{ + VolumeID: uuid.Must(uuid.NewV4()), + GenerationCounter: 0, + } + publishVolumeStatus(&ctx, &live) + + pvcName := live.GetPVCName() + reap := volumesToReap(&ctx, []string{pvcName}, getVolumeStatusByPVC) + assert.Empty(t, reap, "a currently-published volume must never be reaped") +} + +// TestVolumesToReapIncludesUnknownVolume is the positive case: a candidate +// with no published VolumeStatus at all is exactly what this pass exists to +// find. +func TestVolumesToReapIncludesUnknownVolume(t *testing.T) { + ctx := initStatusCtx(t) + orphanID := uuid.Must(uuid.NewV4()) + pvcName := fmt.Sprintf("%s-pvc-0", orphanID.String()) + + reap := volumesToReap(&ctx, []string{pvcName}, getVolumeStatusByPVC) + assert.Len(t, reap, 1) + assert.Equal(t, orphanID, reap[0].VolumeID) +} + +// TestVolumesToReapSkipsReplicated pins the defence-in-depth check: even if +// a resolver ever reconstructs IsReplicated=true for a candidate with no +// published VolumeStatus, it must still not be reaped. getVolumeStatusByPVC +// itself can never produce this today (a PVC name carries no such bit), but +// gcVolumes is shared with the file/dataset paths, where the same combination +// is possible. +func TestVolumesToReapSkipsReplicated(t *testing.T) { + ctx := initStatusCtx(t) + resolve := func(string) (*types.VolumeStatus, error) { + return &types.VolumeStatus{ + VolumeID: uuid.Must(uuid.NewV4()), + IsReplicated: true, + }, nil + } + + reap := volumesToReap(&ctx, []string{"whatever"}, resolve) + assert.Empty(t, reap, "a replicated volume must never be reaped") +} + +// TestVolumesToReapSkipsUnresolvable confirms one bad candidate - a PVC name +// this node cannot parse - is logged and skipped rather than stopping the +// whole pass or reaping something by mistake. +func TestVolumesToReapSkipsUnresolvable(t *testing.T) { + ctx := initStatusCtx(t) + orphanID := uuid.Must(uuid.NewV4()) + goodName := fmt.Sprintf("%s-pvc-0", orphanID.String()) + + reap := volumesToReap(&ctx, + []string{"not-a-valid-pvc-name", goodName}, getVolumeStatusByPVC) + assert.Len(t, reap, 1) + assert.Equal(t, orphanID, reap[0].VolumeID) +} + +// TestGcPVCsSkipsLiveVolumes exercises gcPVCs end to end through the +// swappable getPVCList seam. Every name it returns matches a published +// VolumeStatus, so volumesToReap's result is empty and gcVolumes never +// reaches DestroyVolume - the real Kubernetes delete call, which this test +// does not want to invoke. +func TestGcPVCsSkipsLiveVolumes(t *testing.T) { + ctx := initStatusCtx(t) + live := types.VolumeStatus{ + VolumeID: uuid.Must(uuid.NewV4()), + GenerationCounter: 1, + } + publishVolumeStatus(&ctx, &live) + + origGetPVCList := getPVCList + getPVCList = func(*base.LogObject) ([]string, error) { + return []string{live.GetPVCName()}, nil + } + t.Cleanup(func() { getPVCList = origGetPVCList }) + + assert.NotPanics(t, func() { gcPVCs(&ctx) }) + assert.NotNil(t, ctx.LookupVolumeStatus(live.Key()), + "gcPVCs must not have touched the still-published volume") +} diff --git a/pkg/pillar/cmd/volumemgr/volumemgr.go b/pkg/pillar/cmd/volumemgr/volumemgr.go index 496ed38dbc5..ddfca4a7214 100644 --- a/pkg/pillar/cmd/volumemgr/volumemgr.go +++ b/pkg/pillar/cmd/volumemgr/volumemgr.go @@ -854,6 +854,9 @@ func Run(ps *pubsub.PubSub, loggerArg *logrus.Logger, logArg *base.LogObject, ar gcDatasets(&ctx, types.VolumeEncryptedZFSDataset) gcDatasets(&ctx, types.VolumeClearZFSDataset) } + if base.IsHVTypeKube() { + gcPVCs(&ctx) + } if !ctx.initGced { gcUnusedInitObjects(&ctx) ctx.initGced = true diff --git a/pkg/pillar/cmd/zedmanager/updatestatus.go b/pkg/pillar/cmd/zedmanager/updatestatus.go index 1bc853521d6..504ba044e3d 100644 --- a/pkg/pillar/cmd/zedmanager/updatestatus.go +++ b/pkg/pillar/cmd/zedmanager/updatestatus.go @@ -417,11 +417,23 @@ func doInstall(ctx *zedmanagerContext, MaybeRemoveVolumeRefConfig(ctx, config.UUIDandVersion.UUID, vrs.VolumeID, vrs.GenerationCounter, vrs.LocalGenerationCounter) if !vrs.PendingAdd { - vrs.PendingAdd = true - // Keep in VolumeRefStatus until we get an update - // from volumemgr - newVrs = append(newVrs, *vrs) - removed = true + if lookupVolumeRefStatus(ctx, vrs.Key()) == nil { + // volumemgr has no live VolumeRefStatus for this key + // at all, so there is nothing to wait for: either it + // was already deleted, or (e.g. after a reboot wiped + // ephemeral state) it was never republished this + // boot to begin with, in which case no delete event + // will ever arrive. Drop it now instead of parking + // on PendingAdd forever. + log.Functionf("No volumemgr VolumeRefStatus for %s; dropping immediately", + vrs.Key()) + } else { + vrs.PendingAdd = true + // Keep in VolumeRefStatus until we get an update + // from volumemgr + newVrs = append(newVrs, *vrs) + removed = true + } } } log.Functionf("purge inactive (%s) volumeRefStatus from %d to %d", diff --git a/pkg/pillar/cmd/zedmanager/updatestatus_test.go b/pkg/pillar/cmd/zedmanager/updatestatus_test.go new file mode 100644 index 00000000000..56df0073a4c --- /dev/null +++ b/pkg/pillar/cmd/zedmanager/updatestatus_test.go @@ -0,0 +1,160 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package zedmanager + +import ( + "testing" + + "github.com/lf-edge/eve/pkg/pillar/base" + "github.com/lf-edge/eve/pkg/pillar/pubsub" + "github.com/lf-edge/eve/pkg/pillar/types" + uuid "github.com/satori/go.uuid" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" +) + +// newDoInstallTestContext builds a zedmanagerContext wired up with just +// enough pub/sub plumbing for doInstall: an (empty) DomainConfig +// publication - no domain owns any volume - zedmanager's own outgoing +// VolumeRefConfig publication, and a VolumeRefStatus subscription seeded +// with whatever volumemgr is pretending to have live status for. +func newDoInstallTestContext(t *testing.T, liveVolumeRefStatus []types.VolumeRefStatus) *zedmanagerContext { + t.Helper() + logger := logrus.StandardLogger() + log = base.NewSourceLogObject(logger, agentName, 0) + ps := pubsub.New(pubsub.NewMemoryDriver(), logger, log) + + pubDomainConfig, err := ps.NewPublication(pubsub.PublicationOptions{ + AgentName: agentName, + TopicType: types.DomainConfig{}, + }) + assert.NoError(t, err) + + pubVolumeRefConfig, err := ps.NewPublication(pubsub.PublicationOptions{ + AgentName: agentName, + TopicType: types.VolumeRefConfig{}, + }) + assert.NoError(t, err) + + volumemgrPub, err := ps.NewPublication(pubsub.PublicationOptions{ + AgentName: "volumemgr", + TopicType: types.VolumeRefStatus{}, + }) + assert.NoError(t, err) + for _, vrs := range liveVolumeRefStatus { + assert.NoError(t, volumemgrPub.Publish(vrs.Key(), vrs)) + } + + subVolumeRefStatus, err := ps.NewSubscription(pubsub.SubscriptionOptions{ + AgentName: "volumemgr", + MyAgentName: agentName, + TopicImpl: types.VolumeRefStatus{}, + Persistent: true, + }) + assert.NoError(t, err) + assert.NoError(t, subVolumeRefStatus.Activate()) + + return &zedmanagerContext{ + pubDomainConfig: pubDomainConfig, + pubVolumeRefConfig: pubVolumeRefConfig, + subVolumeRefStatus: subVolumeRefStatus, + } +} + +// TestDoInstallDropsStaleVolumeRefWithNoLiveStatus is the regression test +// for the reboot-purge wedge: a VolumeRefStatus entry that the current +// AppInstanceConfig no longer wants, and that volumemgr has no live status +// for at all (never republished this boot, or already deleted), must be +// dropped immediately instead of parked on PendingAdd waiting for a delete +// event that can never arrive - which previously left doInstall returning +// early forever, never even requesting the newly desired volume. +func TestDoInstallDropsStaleVolumeRefWithNoLiveStatus(t *testing.T) { + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + staleVolumeID := uuid.Must(uuid.FromString("22222222-2222-2222-2222-222222222222")) + newVolumeID := uuid.Must(uuid.FromString("33333333-3333-3333-3333-333333333333")) + + // volumemgr has nothing live for the stale volume at all. + ctx := newDoInstallTestContext(t, nil) + + // zedmanager itself still has a live VolumeRefConfig request out for the + // stale volume (as it would across a reboot, if that publication is what + // survived while volumemgr's own side did not) - doInstall must still + // unpublish it, even though it drops the status entry immediately. + staleVrc := types.VolumeRefConfig{VolumeID: staleVolumeID, AppUUID: appUUID, VerifyOnly: true} + assert.NoError(t, ctx.pubVolumeRefConfig.Publish(staleVrc.Key(), staleVrc)) + + config := types.AppInstanceConfig{ + UUIDandVersion: types.UUIDandVersion{UUID: appUUID, Version: "1"}, + VolumeRefConfigList: []types.VolumeRefConfig{ + {VolumeID: newVolumeID, AppUUID: appUUID, VerifyOnly: true}, + }, + } + status := &types.AppInstanceStatus{ + UUIDandVersion: config.UUIDandVersion, + PurgeInprogress: types.DownloadAndVerify, + VolumeRefStatusList: []types.VolumeRefStatus{ + { + VolumeID: staleVolumeID, + AppUUID: appUUID, + State: types.LOADED, + PendingAdd: false, + VerifyOnly: true, + }, + }, + } + + doInstall(ctx, config, status) + + for _, vrs := range status.VolumeRefStatusList { + assert.NotEqual(t, staleVolumeID, vrs.VolumeID, + "stale VolumeRefStatus must be dropped once volumemgr has no live status for it") + } + if assert.Len(t, status.VolumeRefStatusList, 1) { + assert.Equal(t, newVolumeID, status.VolumeRefStatusList[0].VolumeID) + } + staleVrcAfter, _ := ctx.pubVolumeRefConfig.Get(staleVrc.Key()) + assert.Nil(t, staleVrcAfter, + "the stale VolumeRefConfig request to volumemgr must still be unpublished") +} + +// TestDoInstallWaitsForStaleVolumeRefWithLiveStatus pins the other half of +// the same invariant: if volumemgr still has a live VolumeRefStatus for the +// stale ref, doInstall must not drop it out from under an in-flight async +// removal - it should mark it PendingAdd and wait for volumemgr's delete, +// exactly as before this fix. +func TestDoInstallWaitsForStaleVolumeRefWithLiveStatus(t *testing.T) { + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + staleVolumeID := uuid.Must(uuid.FromString("22222222-2222-2222-2222-222222222222")) + newVolumeID := uuid.Must(uuid.FromString("33333333-3333-3333-3333-333333333333")) + + staleVrs := types.VolumeRefStatus{ + VolumeID: staleVolumeID, + AppUUID: appUUID, + State: types.LOADED, + PendingAdd: false, + VerifyOnly: true, + } + // volumemgr still reports this volume as live. + ctx := newDoInstallTestContext(t, []types.VolumeRefStatus{staleVrs}) + + config := types.AppInstanceConfig{ + UUIDandVersion: types.UUIDandVersion{UUID: appUUID, Version: "1"}, + VolumeRefConfigList: []types.VolumeRefConfig{ + {VolumeID: newVolumeID, AppUUID: appUUID, VerifyOnly: true}, + }, + } + status := &types.AppInstanceStatus{ + UUIDandVersion: config.UUIDandVersion, + PurgeInprogress: types.DownloadAndVerify, + VolumeRefStatusList: []types.VolumeRefStatus{staleVrs}, + } + + doInstall(ctx, config, status) + + if assert.Len(t, status.VolumeRefStatusList, 1) { + assert.Equal(t, staleVolumeID, status.VolumeRefStatusList[0].VolumeID) + assert.True(t, status.VolumeRefStatusList[0].PendingAdd, + "must be marked pending removal while waiting for volumemgr's delete") + } +} diff --git a/pkg/pillar/docs/zedkube.md b/pkg/pillar/docs/zedkube.md index 48feb1a8969..f6a3730ecc3 100644 --- a/pkg/pillar/docs/zedkube.md +++ b/pkg/pillar/docs/zedkube.md @@ -112,7 +112,51 @@ This collection is specific for the kubernetes status and stats. Although EVE ha ### Handle Domain Apps Status in domainmgr -When the application is launched and managed in KubeVirt mode, the Kubernetes cluster is provisioned for this application, being a VMI (Virtual Machine Instance) replicaSet object or a Pod replicaSet object. It uses a declarative approach to manage the desired state of the applications. The configurations are saved in the Kubernetes database for the Kubernetes controller to use to ensure the objects eventually achieve the correct state if possible. Any particular VMI/Pod state of a domain may not be in working condition at the time when EVE domainmgr checks. In the domainmgr code running in KubeVirt mode, if it can not contact the Kubernetes API server to query about the application, or if the application itself has not be started yet in the cluster, the kubervirt.go will return the 'Unknown' status back. It will keep a 'Unknown' status starting timestamp per application. If the 'Unknown' status lasts longer then 5 minutes, the status functions in kubevirt.go will return 'Halting' status back to domainmgr. The timestamp will be cleared once it can get the application status from the kubernetes. +When the application is launched and managed in KubeVirt mode, the Kubernetes cluster is provisioned for this application, being a VMI (Virtual Machine Instance) replicaSet object or a Pod replicaSet object. It uses a declarative approach to manage the desired state of the applications. The configurations are saved in the Kubernetes database for the Kubernetes controller to use to ensure the objects eventually achieve the correct state if possible. Any particular VMI/Pod state of a domain may not be in working condition at the time when EVE domainmgr checks. + +`hypervisor/kubevirt.go`'s `Info` reports back to domainmgr on every poll. It is the only place that decides `DomainId` and `SwState` for a kube app, and every downstream consumer - `doInactivate`'s teardown gates, `doCleanup`'s success test, `verifyStatus` - trusts what it returns without re-deriving anything itself. The contract: + +> **`DomainId` is zero if and only if the VMIRS (or, for a NOHYPER app, its plain container ReplicaSet) is confirmed absent.** It is never zero merely because the answer is unknown or unattributable. + +`Info` confirms existence directly with a `Get` on the object by name, rather than inferring it from where a replica happens to be running: + +| Situation | `DomainId` | `SwState` | +| --- | --- | --- | +| object found, a replica is running on this node | derived from the object's UID | `RUNNING` (or whatever its mapped phase is) | +| object found, no replica attributable to any node yet (mid-restart, mid-failover) | derived from the object's UID | `SCHEDULING` | +| object found, replica reports an unmapped phase | derived from the object's UID | `SCHEDULING` (escalates to `HALTING` only after 30+ minutes stuck unmapped) | +| object found, running on a *different* node | derived from the object's UID | `UNKNOWN` (deliberately not `SCHEDULING` - see below) | +| **object confirmed absent (`Get` -> `NotFound`)** | **`0`** | `HALTED` | +| existence could not be confirmed (API unreachable, or any other error) | the caller's last-known `DomainId`, unchanged | `UNKNOWN` | + +The "running on a different node" row is deliberately not `SCHEDULING`: that would send domainmgr's rescheduling logic into a path that expects a boot in progress, and an app that is healthy on another node would report `BOOTING` forever. The id changing from 0 to a real, derived value is the fix that row needs; the `SwState` itself is left as `UNKNOWN`, matching what a generic id-only change already does downstream. + +The derived, non-zero id (`workloadID` in `hypervisor/kubevirt.go`) is an FNV-1a hash of the object's `metadata.uid`, falling back to hashing its Kubernetes name before the object exists (there is no UID to read yet between `Create` and `Start`). It plays the same role kvm/xen give a qemu pid - a `DomainId` change signals a new generation - but it is never used as a cross-app key, so a hash collision between two different apps' ids is harmless. + +### Naming, purge generations, and why they collide + +Every kube app object - the VMIRS (or plain ReplicaSet, for a NOHYPER app), and its virt-launcher/app pods - is named `--` (`base.GetAppKubeNameWithPurge`), where `` is the first 5 characters of the app's UUID and `` is the sum of the controller's purge and local-purge counters. The counter is embedded in the name specifically so that a purge's old and new generations never collide on the object name in the Kubernetes API - without it, a new VMIRS could not be created while the old one, even if already deleted, was still terminating. + +The object *name* is therefore unique per generation, but almost everything a generation configures is derived deterministically from the app's config and is **not** generation-scoped: MAC addresses, the RWO disk backing the VM, and pod interface (veth) names are all the same across every generation of the same app. Two generations that are ever briefly both alive - even just the old one's pod still tearing down while the new one's pod comes up - collide on all of these, which is what produces failures like `cannot set "eve-bridge" interface name to "podif2": interface name already exists`. This is why a purge must never let two generations exist at once, and why confirming a deleted generation is *actually* gone (not just its own object, but its pods) has to happen before the next generation is created, not just before the next generation is deleted. + +### The stale-generation sweep + +`Setup`/`Start` run an unconditional sweep before creating each app's object: + +1. List every VMIRS (or plain ReplicaSet) in the cluster and filter to this app's, by the `App-Domain-Name` label prefix (`.`) on the VMIRS's `Spec.Selector.MatchLabels` (a plain ReplicaSet carries the same label directly on `ObjectMeta.Labels`, since its own selector has nothing UUID-scoped to filter on). +2. Select every generation whose trailing purge-counter suffix is strictly less than the one about to be created. Name inequality, not label or version comparison, is what matters here: a non-purge config edit bumps the config version embedded in the label without renaming the object, so a version-only rule would misclassify the current generation as stale. +3. Delete each stale generation, then confirm it is actually gone - the object *and* any of its pods - before moving on. Deleting the object returns quickly, but Kubernetes' garbage collection of the pod it owns is asynchronous and can take tens of seconds; proceeding to create the next generation as soon as the object itself is gone is exactly the race described above. +4. Only once every stale generation is confirmed gone does `Start` create the new one. If confirming absence times out, `Start` fails outright rather than risk creating a second generation alongside one that might still exist. + +Running this sweep unconditionally, on every `Start`, matters specifically for a reboot mid-purge: `/run` is tmpfs, so a reboot drops domainmgr's own state, and zedmanager's purge bookkeeping lives in `/persist` and is keyed by a counter, not by "does an old generation still exist" - a purge that was in progress across a reboot is not always re-detected as a purge on the next boot. The sweep doesn't depend on detecting a purge at all: it just enumerates what actually exists in the cluster and reconciles against the counter it is about to create, every time. + +The sweep does not touch the stale generation's PVC. It reconciles VMIRS/ReplicaSet objects and their pods only, so a disk left behind by a stale generation is not reclaimed by this mechanism at all; that gap is still open. + +### Why the sweep alone does not guarantee a stuck purge recovers + +The sweep only runs when `Start` is actually called for the app - it is not a periodic or independently-triggered check. If zedmanager itself never gets far enough to ask domainmgr to bring the app back up, `Start` is never invoked and the sweep never gets a chance to run, no matter how correct it is on its own terms. + +This is not hypothetical: a purge issued while the device is off can leave zedmanager (`cmd/zedmanager/updatestatus.go`) waiting indefinitely on a `VolumeRefStatus` deletion from volumemgr that confirms a stale volume reference is gone, before it will move on to requesting the new generation's volume. If volumemgr's own state for that reference was also wiped by the same reboot, and it never gets asked about that reference again this boot, no such deletion will ever arrive - zedmanager stays parked in its purge-download phase forever, and `domainmgr`'s `Start` for the new generation is never called. `doInstall` now checks whether volumemgr has any live status for the stale reference at all before deciding to wait on it, and drops it immediately if not, so this no longer depends on an event that might never come. The two fixes are complementary: this one gets zedmanager to actually call `Start` again after a stalled removal; the sweep is what makes that `Start` safe once it happens. ## External Boot Image Migration diff --git a/pkg/pillar/hypervisor/kubevirt.go b/pkg/pillar/hypervisor/kubevirt.go index 80c69e70359..25f377fcdd0 100644 --- a/pkg/pillar/hypervisor/kubevirt.go +++ b/pkg/pillar/hypervisor/kubevirt.go @@ -15,7 +15,6 @@ import ( "fmt" "io" "math" - "math/rand" "net" "net/http" "os" @@ -59,6 +58,30 @@ const ( unknownToHaltMinutes = 30 // If VMI is unknown for 30 minutes, return halt state ) +// newKubevirtClient and newK8sClient wrap the kubevirt/k8s client +// constructors as package-level vars so unit tests can swap in a fake +// client. kubecli's documented mock hook only covers +// GetKubevirtClientFromClientConfig, while this file calls +// GetKubevirtClientFromRESTConfig directly; wrapping it here is what makes +// it swappable at all. Tests that reassign these must not run with +// t.Parallel. +// newK8sClient is declared against the kubernetes.Interface, not +// kubernetes.NewForConfig's own concrete *kubernetes.Clientset return type, +// specifically so tests can substitute k8s.io/client-go/kubernetes/fake's +// Clientset (which implements the interface but is not that concrete type). +var ( + newKubevirtClient = kubecli.GetKubevirtClientFromRESTConfig + newK8sClient = func(c *rest.Config) (kubernetes.Interface, error) { + return kubernetes.NewForConfig(c) + } + // getKubeConfig wraps kubeapi.GetKubeConfig, which a couple of call + // sites (getVMIStatus, GetDomsCPUMem) read directly instead of going + // through ctx.kubeConfig/getConfig. Wrapping it here for the same + // swap-in-tests reason as the two vars above - without it, those call + // sites always try to read the real kubeconfig file from disk. + getKubeConfig = kubeapi.GetKubeConfig +) + // MetaDataType is a type for different Domain types // We only support ReplicaSet for VMI and Pod for now. type MetaDataType int @@ -75,7 +98,7 @@ const ( type vmiMetaData struct { repPod *appsv1.ReplicaSet // Handle to the replicaSetof pod repVMI *v1.VirtualMachineInstanceReplicaSet // Handle to the replicaSet of VMI - domainID int // DomainID understood by domainmgr in EVE + domainID int // Cached types.DomainStatus.DomainId - see workloadID and its doc comment mtype MetaDataType // switch on is ReplicaSet, Pod or is VMI name string // Display-Name(all lower case) + first 5 bytes of domainName cputotal uint64 // total CPU in NS so far @@ -140,6 +163,7 @@ var stateMap = map[string]types.SwState{ "suspended": types.PAUSED, "Pending": types.PENDING, "Scheduling": types.SCHEDULING, + "Scheduled": types.SCHEDULING, "Failed": types.FAILED, "Halting": types.HALTING, "Succeeded": types.SCHEDULING, @@ -259,8 +283,38 @@ func (ctx kubevirtContext) Name() string { return KubevirtHypervisorName } +// kubevirtTask wraps kubevirtContext with the DomainStatus that produced +// it, so that a Task's identity can be derived from the status instead of +// depending only on the domainName string a caller happens to pass to each +// method. Embedding kubevirtContext means every types.Task method not +// overridden here is served, unchanged, by the existing domainName-keyed +// implementation. +type kubevirtTask struct { + kubevirtContext + status *types.DomainStatus +} + func (ctx kubevirtContext) Task(status *types.DomainStatus) types.Task { - return ctx + return kubevirtTask{ctx, status} +} + +// kubeName derives the Kubernetes object name for t.status's app and purge +// generation. Setup/Start populate vmiList using this exact derivation (see +// CreateReplicaVMIConfig/CreateReplicaPodConfig), so it always names the +// same object a freshly-run Setup/Start would. +func (t kubevirtTask) kubeName() string { + return base.GetAppKubeNameWithPurge(t.status.DisplayName, + t.status.UUIDandVersion.UUID, t.status.PurgeCounter) +} + +// metaType mirrors the NOHYPER check in Setup: a NOHYPER app runs as a +// plain container ReplicaSet (IsMetaReplicaPod); anything else runs as a +// VMI ReplicaSet (IsMetaReplicaVMI). +func (t kubevirtTask) metaType() MetaDataType { + if t.status.VirtualizationMode == types.NOHYPER { + return IsMetaReplicaPod + } + return IsMetaReplicaVMI } // uuidPrefixOfDomainName returns the "uuid." prefix from a domainName of the @@ -356,7 +410,7 @@ func (ctx kubevirtContext) CreateReplicaVMIConfig(domainName string, config type return err } - kvClient, err := kubecli.GetKubevirtClientFromRESTConfig(ctx.kubeConfig) + kvClient, err := newKubevirtClient(ctx.kubeConfig) if err != nil { logrus.Errorf("couldn't get the kubernetes client API config: %v", err) return err @@ -739,10 +793,14 @@ func (ctx kubevirtContext) CreateReplicaVMIConfig(domainName string, config type } } meta := vmiMetaData{ - repVMI: replicaSet, - name: kubeName, - mtype: IsMetaReplicaVMI, - domainID: int(rand.Uint32()), + repVMI: replicaSet, + name: kubeName, + mtype: IsMetaReplicaVMI, + // The VMIRS doesn't exist yet (Create runs before Start creates it), + // so there is no UID to hash; workloadID falls back to kubeName. A + // real, stable id is captured from Start's own Create response and + // re-derived on every subsequent Info call. + domainID: workloadID("", kubeName), memOverhead: uint64(overhead), sriovVFs: sriovVFs, } @@ -757,6 +815,199 @@ func (ctx kubevirtContext) CreateReplicaVMIConfig(domainName string, config type return nil } +// sweepConfirmRetries/sweepConfirmInterval bound how long +// sweepStaleGenerations will wait for a deleted stale generation to +// actually disappear (object and pods) before giving up. This reuses +// Start's own pre-existing 5x10s retry budget (see the Create retry loop +// below) rather than inventing a new one: an unreachable API must not +// block activation indefinitely. +// +// sweepConfirmInterval is a var, not a const, solely so unit tests can +// shrink it and exercise the timeout path without a real 50-second wait. +const sweepConfirmRetries = 5 + +var sweepConfirmInterval = 10 * time.Second + +// appUUIDFromDomainName extracts the app UUID from a domainName of the +// form "uuid.version.appnum". +func appUUIDFromDomainName(domainName string) (uuid.UUID, error) { + prefix := uuidPrefixOfDomainName(domainName) + return uuid.FromString(strings.TrimSuffix(prefix, ".")) +} + +// confirmVMIRSGone waits for a deleted VMIRS - and its virt-launcher pod - +// to actually disappear from the cluster. Deleting the VMIRS returns +// quickly, but Kubernetes' garbage collection of the owned VMI and pod is +// asynchronous and can take tens of seconds; since generations share veth +// names and the RWO disk, proceeding to create the next generation as soon +// as the VMIRS object itself is gone races the old pod's own network +// teardown - the exact "interface name already exists" sandbox failure +// this is guarding against. +func confirmVMIRSGone(kubeconfig *rest.Config, name, domainNameLabel string) error { + virtClient, err := newKubevirtClient(kubeconfig) + if err != nil { + return err + } + podclientset, err := newK8sClient(kubeconfig) + if err != nil { + return err + } + + for retry := 0; ; retry++ { + getCtx, getCancel := apiCtx() + _, getErr := virtClient.ReplicaSet(kubeapi.EVEKubeNameSpace). + Get(getCtx, name, metav1.GetOptions{}) + getCancel() + objGone := errors.IsNotFound(getErr) + if getErr != nil && !objGone { + return fmt.Errorf("confirmVMIRSGone(%s): %w", name, getErr) + } + + podsGone := true + if domainNameLabel != "" { + listCtx, listCancel := apiCtx() + pods, listErr := podclientset.CoreV1().Pods(kubeapi.EVEKubeNameSpace). + List(listCtx, metav1.ListOptions{ + LabelSelector: "kubevirt.io=virt-launcher," + eveLabelKey + "=" + domainNameLabel, + }) + listCancel() + if listErr != nil { + return fmt.Errorf("confirmVMIRSGone(%s): %w", name, listErr) + } + podsGone = len(pods.Items) == 0 + } + + if objGone && podsGone { + logrus.Infof("confirmVMIRSGone(%s): confirmed gone", name) + return nil + } + if retry >= sweepConfirmRetries-1 { + return fmt.Errorf("timed out waiting for stale VMIRS %s to be gone "+ + "(object gone:%t pods gone:%t)", name, objGone, podsGone) + } + logrus.Infof("confirmVMIRSGone(%s): not yet gone (object gone:%t pods gone:%t), retrying in %v", + name, objGone, podsGone, sweepConfirmInterval) + time.Sleep(sweepConfirmInterval) + } +} + +// confirmPodReplicaSetGone is confirmVMIRSGone's analogue for a NOHYPER +// app's plain container ReplicaSet. Its pods are labeled "app=" +// (CreateReplicaPodConfig), which already is generation-specific, so no +// separate domainName label is needed to scope the pod check. +func confirmPodReplicaSetGone(kubeconfig *rest.Config, name string) error { + clientset, err := newK8sClient(kubeconfig) + if err != nil { + return err + } + + for retry := 0; ; retry++ { + getCtx, getCancel := apiCtx() + _, getErr := clientset.AppsV1().ReplicaSets(kubeapi.EVEKubeNameSpace). + Get(getCtx, name, metav1.GetOptions{}) + getCancel() + objGone := errors.IsNotFound(getErr) + if getErr != nil && !objGone { + return fmt.Errorf("confirmPodReplicaSetGone(%s): %w", name, getErr) + } + + listCtx, listCancel := apiCtx() + pods, listErr := clientset.CoreV1().Pods(kubeapi.EVEKubeNameSpace). + List(listCtx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("app=%s", name), + }) + listCancel() + if listErr != nil { + return fmt.Errorf("confirmPodReplicaSetGone(%s): %w", name, listErr) + } + podsGone := len(pods.Items) == 0 + + if objGone && podsGone { + logrus.Infof("confirmPodReplicaSetGone(%s): confirmed gone", name) + return nil + } + if retry >= sweepConfirmRetries-1 { + return fmt.Errorf("timed out waiting for stale ReplicaSet %s to be gone "+ + "(object gone:%t pods gone:%t)", name, objGone, podsGone) + } + logrus.Infof("confirmPodReplicaSetGone(%s): not yet gone (object gone:%t pods gone:%t), retrying in %v", + name, objGone, podsGone, sweepConfirmInterval) + time.Sleep(sweepConfirmInterval) + } +} + +// sweepStaleGenerations deletes every VMIRS/ReplicaSet belonging to this +// app whose purge counter is strictly less than desiredCounter, and +// confirms each one is actually gone - object and pods - before returning. +// It runs unconditionally on every Start, because a reboot mid-purge means +// no purge is ever *detected* on the boot path that recreates the domain +// (zedmanager sees only /run, which the reboot emptied); only an +// unconditional sweep catches that case. A confirm-absence failure here +// fails Start outright - the caller must never proceed to create the new +// generation while an old one may still exist. +func (ctx kubevirtContext) sweepStaleGenerations( + appUUID uuid.UUID, desiredName string, desiredCounter uint32, mtype MetaDataType) error { + // Do not depend on the caller for kubeConfig. The receiver is a value, + // so a getConfig call up the stack can fill in a different copy. + if err := getConfig(&ctx); err != nil { + return err + } + kubeconfig := ctx.kubeConfig + + if mtype == IsMetaReplicaPod { + clientset, err := newK8sClient(kubeconfig) + if err != nil { + return err + } + listCtx, listCancel := apiCtx() + list, err := clientset.AppsV1().ReplicaSets(kubeapi.EVEKubeNameSpace). + List(listCtx, metav1.ListOptions{}) + listCancel() + if err != nil { + return err + } + for _, name := range stalePodReplicaSetNames(list.Items, appUUID, desiredName, desiredCounter) { + logrus.Infof("sweepStaleGenerations: tearing down stale ReplicaSet %s before creating %s", + name, desiredName) + if err := StopReplicaPodContainer(kubeconfig, name); err != nil { + return fmt.Errorf("sweepStaleGenerations: failed to delete %s: %w", name, err) + } + if err := confirmPodReplicaSetGone(kubeconfig, name); err != nil { + return fmt.Errorf("sweepStaleGenerations: %w", err) + } + } + return nil + } + + virtClient, err := newKubevirtClient(kubeconfig) + if err != nil { + return err + } + listCtx, listCancel := apiCtx() + list, err := virtClient.ReplicaSet(kubeapi.EVEKubeNameSpace).List(listCtx, metav1.ListOptions{}) + listCancel() + if err != nil { + return err + } + domainNameByName := make(map[string]string, len(list.Items)) + for _, vmirs := range list.Items { + if vmirs.Spec.Selector != nil { + domainNameByName[vmirs.GetName()] = vmirs.Spec.Selector.MatchLabels[eveLabelKey] + } + } + for _, name := range staleVMIRSNames(list.Items, appUUID, desiredName, desiredCounter) { + logrus.Infof("sweepStaleGenerations: tearing down stale VMIRS %s before creating %s", + name, desiredName) + if err := StopReplicaVMI(kubeconfig, name); err != nil { + return fmt.Errorf("sweepStaleGenerations: failed to delete %s: %w", name, err) + } + if err := confirmVMIRSGone(kubeconfig, name, domainNameByName[name]); err != nil { + return fmt.Errorf("sweepStaleGenerations: %w", err) + } + } + return nil +} + func (ctx kubevirtContext) Start(domainName string) error { logrus.Debugf("Starting Kubevirt domain %s", domainName) @@ -777,6 +1028,18 @@ func (ctx kubevirtContext) Start(domainName string) error { } logrus.Infof("Starting Kubevirt domain %s, devicename nodename %d nodeName:%s vmis:%v", domainName, len(ctx.nodeNameMap), nodeName, vmis) + appUUID, err := appUUIDFromDomainName(domainName) + if err != nil { + return logError("Start domain %s: could not parse app UUID: %v", domainName, err) + } + desiredCounter, ok := trailingCounter(vmis.name) + if !ok { + return logError("Start domain %s: could not parse purge counter from %s", domainName, vmis.name) + } + if err := ctx.sweepStaleGenerations(appUUID, vmis.name, desiredCounter, vmis.mtype); err != nil { + return logError("Start domain %s: stale-generation sweep failed: %v", domainName, err) + } + // Start the Pod ReplicaSet if vmis.mtype == IsMetaReplicaPod { err := StartReplicaPodContiner(ctx, vmis) @@ -787,7 +1050,7 @@ func (ctx kubevirtContext) Start(domainName string) error { // Start the VMI ReplicaSet repvmi := vmis.repVMI - virtClient, err := kubecli.GetKubevirtClientFromRESTConfig(kubeconfig) + virtClient, err := newKubevirtClient(kubeconfig) if err != nil { logrus.Errorf("couldn't get the kubernetes client API config: %v", err) return err @@ -795,8 +1058,11 @@ func (ctx kubevirtContext) Start(domainName string) error { // Create the VMI ReplicaSet, retrying on transient kube API server errors. const maxRetries = 5 + var created *v1.VirtualMachineInstanceReplicaSet for retries := maxRetries; ; retries-- { - _, err = virtClient.ReplicaSet(kubeapi.EVEKubeNameSpace).Create(context.Background(), repvmi, metav1.CreateOptions{}) + createCtx, createCancel := apiCtx() + created, err = virtClient.ReplicaSet(kubeapi.EVEKubeNameSpace).Create(createCtx, repvmi, metav1.CreateOptions{}) + createCancel() if err == nil { break } @@ -804,6 +1070,10 @@ func (ctx kubevirtContext) Start(domainName string) error { // VMI could have been already started, for example failover from // other node. An interrupted DetachUtilVmirsReplicaReset can leave // the existing object scaled to 0 replicas; ensure it is not. + // + // The response on this branch carries no object, so the placeholder + // id from Setup stands until the next Info call re-derives it from a + // fresh Get. logrus.Warnf("VMI replicaset %v already exists", repvmi) if ensureErr := ensureVmirsReplicas(virtClient, vmis.name); ensureErr != nil { return logError("Start domain %s: failed to ensure vmirs %s replicas: %v", domainName, vmis.name, ensureErr) @@ -817,15 +1087,34 @@ func (ctx kubevirtContext) Start(domainName string) error { logrus.Errorf("Start VMI replicaset failed, retrying (%d left): %v", retries-1, err) time.Sleep(10 * time.Second) } + if created != nil { + newID := workloadID(string(created.UID), vmis.name) + if newID != vmis.domainID { + logrus.Infof("Start(%s): domainID changed from %d to %d", domainName, vmis.domainID, newID) + vmis.domainID = newID + } + } logrus.Infof("Started Kubevirt domain replicaset %s, VMI replicaset %s", domainName, vmis.name) // Start() returns as soon as VMIRS is created; cluster drives VMI scheduling. return nil } -// Create is no-op for kubevirt, just return the domainID we already have. +// Create is a no-op for kubevirt: it just returns an id for the domain that +// Setup already built (and Start is about to create the VMIRS/pod for). +// +// This runs before the object exists in the cluster - Start creates it +// afterwards - so if vmiList already has an entry (the normal case; Setup +// just populated it) its cached id is returned unchanged. If not, config +// carries everything needed to derive the exact same kubeName-based +// fallback id Setup would have used, so this never has to dereference a +// missing map entry. func (ctx kubevirtContext) Create(domainName string, cfgFilename string, config *types.DomainConfig) (int, error) { - return ctx.vmiList[domainName].domainID, nil + if vmis, ok := ctx.vmiList[domainName]; ok { + return vmis.domainID, nil + } + kubeName := base.GetAppKubeNameWithPurge(config.DisplayName, config.UUIDandVersion.UUID, config.PurgeCounter) + return workloadID("", kubeName), nil } // podListToSchedulingState derives the node-placement decision from a @@ -833,13 +1122,24 @@ func (ctx kubevirtContext) Create(domainName string, cfgFilename string, config // signal for node assignment; Status.Phase is deliberately not used because a // pod in Init:0/1 carries Phase="Pending" even after it has been bound to a // node by the scheduler. -func podListToSchedulingState(pods []k8sv1.Pod, nodeName string) (onMe bool, anyNode bool, err error) { +// +// scheduledOnNone means "no non-terminating replica is bound to any node" - +// i.e. the first non-terminating pod's own Spec.NodeName is empty. This is +// the same definition the VMI-list path (replicaVmiScheduledOnMe) uses for +// its own scheduledOnNone; a bound-but-elsewhere pod (Spec.NodeName set to +// something other than nodeName) is neither onMe nor scheduledOnNone. +func podListToSchedulingState(pods []k8sv1.Pod, nodeName string) (onMe bool, scheduledOnNone bool, err error) { for _, p := range pods { if p.ObjectMeta.DeletionTimestamp != nil { continue } - return p.Spec.NodeName == nodeName, true, nil + onMe = p.Spec.NodeName == nodeName + scheduledOnNone = p.Spec.NodeName == "" + logrus.Infof("podListToSchedulingState: pod %s Spec.NodeName:%q onMe:%t scheduledOnNone:%t", + p.ObjectMeta.Name, p.Spec.NodeName, onMe, scheduledOnNone) + return onMe, scheduledOnNone, nil } + logrus.Infof("podListToSchedulingState: no non-terminating pod among %d", len(pods)) return false, false, fmt.Errorf("unhandled scheduling state") } @@ -850,7 +1150,7 @@ func (ctx kubevirtContext) replicaVmiScheduledOnMe(vmirsName string) (scheduledO return false, false, err } - virtClient, err := kubecli.GetKubevirtClientFromRESTConfig(ctx.kubeConfig) + virtClient, err := newKubevirtClient(ctx.kubeConfig) if err != nil { logrus.Errorf("couldn't get the kubernetes client API config: %v", err) return false, false, err @@ -893,16 +1193,23 @@ func (ctx kubevirtContext) vmiScheduledOnMeFromVmirs(virtClient kubecli.Kubevirt } if vmi.Status.NodeName == "" { // Not scheduled yet, move on + logrus.Infof("vmiScheduledOnMeFromVmirs(%s): vmi %s not yet scheduled anywhere", + vmirs.Name, vmi.ObjectMeta.Name) continue } - return (vmi.Status.NodeName == nodeName), false, nil + onMe := vmi.Status.NodeName == nodeName + logrus.Infof("vmiScheduledOnMeFromVmirs(%s): vmi %s scheduled on %q onMe:%t", + vmirs.Name, vmi.ObjectMeta.Name, vmi.Status.NodeName, onMe) + return onMe, false, nil } // Intentional fallback to looking at a virt-launcher pod // One or both VMI objects may be either terminating or not scheduled to any node. + logrus.Infof("vmiScheduledOnMeFromVmirs(%s): no attributable vmi among %d, "+ + "falling back to pod list", vmirs.Name, len(vmis.Items)) } // No VMI, look for a Virt-launcher pod instead, it will start earlier - podclientset, err := kubernetes.NewForConfig(ctx.kubeConfig) + podclientset, err := newK8sClient(ctx.kubeConfig) if err != nil { return false, false, fmt.Errorf("no kube config") } @@ -910,6 +1217,7 @@ func (ctx kubevirtContext) vmiScheduledOnMeFromVmirs(virtClient kubecli.Kubevirt LabelSelector: "kubevirt.io=virt-launcher," + appDomainNameSelector, }) if len(vlPods.Items) == 0 { + logrus.Infof("vmiScheduledOnMeFromVmirs(%s): no virt-launcher pod found", vmirs.Name) return false, false, nil } return podListToSchedulingState(vlPods.Items, nodeName) @@ -927,7 +1235,7 @@ func (ctx kubevirtContext) replicaPodScheduledOnMe(rsName string) (onMe bool, sc return false, false, fmt.Errorf("Failed to get nodeName") } - podclientset, err := kubernetes.NewForConfig(ctx.kubeConfig) + podclientset, err := newK8sClient(ctx.kubeConfig) if err != nil { return false, false, fmt.Errorf("no kube config") } @@ -1001,14 +1309,12 @@ func (ctx kubevirtContext) Stop(domainName string, force bool) error { } kubeconfig := ctx.kubeConfig - keyToDelete := domainName vmis, ok := ctx.vmiList[domainName] if !ok { if stale, oldKey := ctx.lookupVMIByUUIDPrefix(domainName); stale != nil { logrus.Warnf("Stop: domainName %s not in vmiList; using stale entry under %s", domainName, oldKey) vmis = stale - keyToDelete = oldKey } else { return logError("domain %s failed to get vmlist", domainName) } @@ -1036,10 +1342,9 @@ func (ctx kubevirtContext) Stop(domainName string, force bool) error { ctx.clearSRIOVAdminMACs(vmis) - delete(ctx.vmiList, keyToDelete) - - delete(ctx.prevDomainMetric, keyToDelete) - + // The vmiList entry must outlive Stop: Info() needs it to read the guest's + // phase, and without it can only report SCHEDULING while domainmgr polls + // for the domain to stop. Delete() and Cleanup() own the removal. return nil } @@ -1065,6 +1370,11 @@ func (ctx kubevirtContext) Delete(domainName string) (result error) { } onMe, scheduledOnNone, err := ctx.scheduledOnMe(vmis.mtype, vmis.name) + if err != nil { + // A failed lookup reports onMe=false, which is indistinguishable + // from the "scheduled elsewhere" case below. + return err + } if !onMe && !scheduledOnNone { // Not scheduled on me, but is scheduled elsewhere. return nil @@ -1102,7 +1412,7 @@ func (ctx kubevirtContext) Delete(domainName string) (result error) { // StopReplicaVMI stops the VMI ReplicaSet func StopReplicaVMI(kubeconfig *rest.Config, repVmiName string) error { - virtClient, err := kubecli.GetKubevirtClientFromRESTConfig(kubeconfig) + virtClient, err := newKubevirtClient(kubeconfig) if err != nil { logrus.Errorf("couldn't get the kubernetes client API config: %v", err) return err @@ -1120,7 +1430,7 @@ func StopReplicaVMI(kubeconfig *rest.Config, repVmiName string) error { logrus.Infof("Stop VMI Replicaset, Domain already deleted: %v", repVmiName) return nil } - logrus.Errorf("Stop VMI Replicaset error %v\n", err) + logrus.Errorf("Stop VMI Replicaset %s error %v", repVmiName, err) return err } @@ -1206,29 +1516,111 @@ func ensureVmirsReplicas(virtClient kubecli.KubevirtClient, vmiRsName string) er ensureVmirsReplicaRetries, vmiRsName) } -func (ctx kubevirtContext) Info(domainName string) (int, types.SwState, error) { +// apiCtx bounds a single Kubernetes API request with kubeapi.KubeAPITimeout, +// the budget the rest of pillar already uses - see getVmirs just below, and the +// kubeapi and cmd/zedkube packages. It exists as a helper because several of +// these calls sit inside retry loops, where a deferred cancel would hold every +// context until the loop finished. +// +// This matters more here than elsewhere: domainmgr's runHandler calls Info on a +// 9-30s ticker and Start synchronously, and zedbox's watchdog reboots the device +// if that handler stops checking in. A request that blocks forever on an +// unhealthy API server therefore takes the node down. With a deadline it fails +// instead, which Info reports as UNKNOWN with the caller's id preserved - +// exactly the contract documented on types.DomainStatus.DomainId. +func apiCtx() (context.Context, context.CancelFunc) { + return context.WithTimeout(context.Background(), kubeapi.KubeAPITimeout()) +} + +// replicaSetUID confirms existence of the app's VMIRS (or, for a NOHYPER +// app, its plain ReplicaSet) directly via Get, rather than inferring +// existence from replica placement, and returns its metadata.uid. +// +// err is the raw API error. Callers must check errors.IsNotFound(err) to +// distinguish "confirmed absent" from "could not confirm" (e.g. the API is +// unreachable) - only the former may ever produce the zero id. +func (t kubevirtTask) replicaSetUID(kubeName string) (uid string, err error) { + if err := getConfig(&t.kubevirtContext); err != nil { + return "", err + } + if t.metaType() == IsMetaReplicaPod { + clientset, err := newK8sClient(t.kubeConfig) + if err != nil { + return "", err + } + getCtx, getCancel := apiCtx() + rs, err := clientset.AppsV1().ReplicaSets(kubeapi.EVEKubeNameSpace). + Get(getCtx, kubeName, metav1.GetOptions{}) + getCancel() + if err != nil { + return "", err + } + return string(rs.UID), nil + } + virtClient, err := newKubevirtClient(t.kubeConfig) + if err != nil { + return "", err + } + getCtx, getCancel := apiCtx() + vmirs, err := virtClient.ReplicaSet(kubeapi.EVEKubeNameSpace). + Get(getCtx, kubeName, metav1.GetOptions{}) + getCancel() + if err != nil { + return "", err + } + return string(vmirs.UID), nil +} +// Info's contract: DomainId is zero if and only if the VMIRS (or plain +// ReplicaSet, for a NOHYPER app) is confirmed absent (Get -> NotFound). +// Every other outcome - found, found-but-unattributed, found-with-an- +// unmapped-phase, or the existence check itself failing - returns a +// non-zero id, because a zero id is what tells domainmgr's doInactivate/ +// doCleanup that a teardown already happened. +func (t kubevirtTask) Info(domainName string) (int, types.SwState, error) { logrus.Debugf("Info called for Domain: %s", domainName) - nodeName, ok := ctx.nodeNameMap["nodename"] + nodeName, ok := t.nodeNameMap["nodename"] if !ok { return 0, types.BROKEN, logError("Failed to get nodeName") } - var res string - var err error - err = getConfig(&ctx) - if err != nil { - return 0, types.BROKEN, err + // Fill in kubeConfig here. The receiver is a value, so the getConfig + // call in replicaSetUID fills in a copy and leaves t.kubeConfig nil. + // A nil *rest.Config panics newKubevirtClient below. + if err := getConfig(&t.kubevirtContext); err != nil { + return t.status.DomainId, types.UNKNOWN, logError( + "Info(%s): can not get kubeconfig: %v", domainName, err) } - vmis, ok := ctx.vmiList[domainName] + + kubeName := t.kubeName() + + uid, err := t.replicaSetUID(kubeName) + if err != nil { + if errors.IsNotFound(err) { + logrus.Infof("Info(%s): %s confirmed absent", domainName, kubeName) + return 0, types.HALTED, nil + } + // Existence could not be confirmed one way or the other (API + // unreachable, or some other error): never produce the "confirmed + // absent" token while the answer is unknown, so keep whatever id + // the caller already had. + logrus.Infof("Info(%s): existence check for %s failed: %v", domainName, kubeName, err) + return t.status.DomainId, types.UNKNOWN, err + } + id := workloadID(uid, kubeName) + + // Note: unlike Stop/Delete/Cleanup, a miss here does not fall back to + // lookupVMIByUUIDPrefix's stale entry. That fallback's cached name can + // belong to an older generation than the kubeName whose existence was + // just confirmed above, which would make the phase-reading calls below + // silently query the wrong object. Since existence no longer depends on + // vmiList at all, a miss here can safely just hold (SCHEDULING) rather + // than risk that mismatch. + vmis, ok := t.vmiList[domainName] if !ok { - if stale, oldKey := ctx.lookupVMIByUUIDPrefix(domainName); stale != nil { - logrus.Warnf("Info: domainName %s not in vmiList; using stale entry under %s", - domainName, oldKey) - vmis = stale - } else { - return 0, types.HALTED, logError("info domain %s failed to get vmlist", domainName) - } + logrus.Infof("Info(%s): %s exists but there is no local vmiList entry for it", + domainName, kubeName) + return id, types.SCHEDULING, nil } // Check for a stranded VMIRS before the scheduledOnMe/!onMe short-circuit @@ -1236,69 +1628,127 @@ func (ctx kubevirtContext) Info(domainName string) (int, types.SwState, error) { // onMe would be false and this domain would otherwise be reported as // UNKNOWN forever instead of recovering. The VMIRS fetched here is reused // for the scheduling check below instead of fetching it a second time. + // + // Note the object queried is kubeName, derived from DomainStatus, not + // vmis.name from the cache: vmiList can still name an older generation + // after a purge, and checking that one's replica count would report on the + // wrong object. var vmirs *v1.VirtualMachineInstanceReplicaSet var virtClient kubecli.KubevirtClient if vmis.mtype == IsMetaReplicaVMI { var verr error - virtClient, verr = kubecli.GetKubevirtClientFromRESTConfig(ctx.kubeConfig) + virtClient, verr = newKubevirtClient(t.kubeConfig) if verr != nil { return 0, types.BROKEN, logError("couldn't get the kubernetes client API config: %v", verr) } var rerr error - vmirs, rerr = getVmirs(virtClient, vmis.name) + vmirs, rerr = getVmirs(virtClient, kubeName) if rerr != nil { + if errors.IsNotFound(rerr) { + // Present at the existence check above and gone by this Get, + // so this is the same confirmed-absent answer arriving one + // call later. Returning here also skips the fall-through's + // own Get, which can only reach the same conclusion. + logrus.Infof("Info(%s): %s confirmed absent after the existence check", + domainName, kubeName) + return 0, types.HALTED, nil + } if isK3sUnreachable(rerr) { - return 0, types.UNKNOWN, nil + // Unknown, not absent: hold the caller's id rather than + // emitting the "confirmed gone" token. + return t.status.DomainId, types.UNKNOWN, nil } // Non-unreachable Get failure: fall through to scheduledOnMe's own // independent fetch attempt below, same as before this reuse was // added -- vmirs stays nil so the generic dispatch runs. vmirs = nil } else if vmirsStranded(vmirsDesiredReplicas(vmirs)) { - return 0, types.HALTED, logError("domain %s vmirs %s scaled to 0 replicas", domainName, vmis.name) + // The object exists, so the id stays non-zero (zero means + // confirmed absent, see types.DomainStatus.DomainId); HALTED is + // what tells domainmgr the workload is down and to recreate it, + // which Start's ensureVmirsReplicas then repairs. + return id, types.HALTED, logError("domain %s vmirs %s scaled to 0 replicas", + domainName, kubeName) } } - var onMe bool + var onMe, scheduledOnNone bool if vmirs != nil { - onMe, _, err = ctx.vmiScheduledOnMeFromVmirs(virtClient, vmirs) + onMe, scheduledOnNone, err = t.vmiScheduledOnMeFromVmirs(virtClient, vmirs) } else { - onMe, _, err = ctx.scheduledOnMe(vmis.mtype, vmis.name) + onMe, scheduledOnNone, err = t.scheduledOnMe(vmis.mtype, kubeName) } if err != nil { + if errors.IsNotFound(err) { + // Backstop for the lookups that do their own Get: the NOHYPER + // dispatch and the fall-through above. Absence is absence + // wherever it is observed. + logrus.Infof("Info(%s): %s confirmed absent during the scheduling lookup", + domainName, kubeName) + return 0, types.HALTED, nil + } if isK3sUnreachable(err) { - return 0, types.UNKNOWN, nil + logrus.Infof("Info(%s): k3s unreachable while determining scheduled node: %v", + domainName, err) + return t.status.DomainId, types.UNKNOWN, err } - return 0, types.BROKEN, logError("Failed to determine scheduled node: %s", err) + logrus.Infof("Info(%s): failed to determine scheduled node for %s: %v", + domainName, kubeName, err) + return id, types.SCHEDULING, err } if !onMe { - return 0, types.UNKNOWN, nil + if scheduledOnNone { + // Exists, but no non-terminating replica is bound to any node + // yet (mid-restart, mid-failover) - this is the case that used + // to fall through to a false zero via !onMe. + logrus.Infof("Info(%s): %s exists but is not yet scheduled anywhere", domainName, kubeName) + return id, types.SCHEDULING, nil + } + // Bound to a different node: healthy elsewhere, just not here. + // Returning SCHEDULING here would send verifyStatus into its + // rescheduling arm and make an app that is healthy on another node + // report BOOTING forever, so this stays UNKNOWN - only the id + // changes from 0 to a real, non-zero value. + logrus.Infof("Info(%s): %s scheduled on a different node", domainName, kubeName) + return id, types.UNKNOWN, nil } + var res string if vmis.mtype == IsMetaReplicaPod { - res, err = InfoReplicaSetContainer(ctx, vmis) + res, err = InfoReplicaSetContainer(t.kubevirtContext, vmis) } else { res, err = getVMIStatus(vmis, nodeName) } if err != nil { if isK3sUnreachable(err) { - return 0, types.UNKNOWN, nil - } - return 0, types.BROKEN, logError("domain %s failed to get info: %v", domainName, err) - } - - if effectiveDomainState, matched := stateMap[res]; !matched { - // Received undefined state in our map, return UNKNOWN instead - retStatus, err := checkAndReturnStatus(vmis, true) + logrus.Infof("Info(%s): k3s unreachable while getting status: %v", domainName, err) + return t.status.DomainId, types.UNKNOWN, err + } + logrus.Infof("Info(%s): failed to get status for %s: %v", domainName, kubeName, err) + return id, types.SCHEDULING, err + } + + effectiveDomainState, matched := stateMap[res] + if !matched { + // Received an undefined phase string. Object presence is already + // confirmed, so the default is SCHEDULING (held), not the old + // HALTING-by-default (which zeroed DomainId via domainmgr's own + // HALTED/HALTING branch and skipped a teardown that had not + // actually happened). checkAndReturnStatus's own 30+ minute + // escalation to "Halting" for a workload stuck unmapped that long + // is unrelated to this and is preserved unchanged. + retStatus, statusErr := checkAndReturnStatus(vmis, true) logrus.Infof("domain %s reported to be in unexpected state %s", domainName, res) - effectiveDomainState = types.HALTING - if retStatus == "Unknown" { - effectiveDomainState = types.UNKNOWN + effectiveDomainState = types.SCHEDULING + if retStatus != "Unknown" { + effectiveDomainState = types.HALTING } - return vmis.domainID, effectiveDomainState, err - } else { - return vmis.domainID, effectiveDomainState, err + logrus.Infof("Info(%s): unmapped state %q -> %s, id:%d", domainName, res, + effectiveDomainState.String(), id) + return id, effectiveDomainState, statusErr } + logrus.Infof("Info(%s): state %q -> %s, id:%d", domainName, res, effectiveDomainState.String(), id) + return id, effectiveDomainState, nil } func (ctx kubevirtContext) Cleanup(domainName string) error { @@ -1312,16 +1762,28 @@ func (ctx kubevirtContext) Cleanup(domainName string) error { } var err error + key := domainName vmis, ok := ctx.vmiList[domainName] if !ok { if stale, oldKey := ctx.lookupVMIByUUIDPrefix(domainName); stale != nil { logrus.Warnf("Cleanup: domainName %s not in vmiList; using stale entry under %s", domainName, oldKey) vmis = stale + key = oldKey } else { return logError("cleanup domain %s failed to get vmlist", domainName) } } + + // Cleanup is the unconditional tail of doInactivate, whereas Delete only + // runs while DomainId is still set, so the bookkeeping is released here. + // Deferred so a Cleanup that fails partway still releases it rather than + // leaking the entry for the lifetime of the agent. + defer func() { + delete(ctx.vmiList, key) + delete(ctx.prevDomainMetric, key) + }() + if vmis.mtype == IsMetaReplicaPod { _, err = InfoReplicaSetContainer(ctx, vmis) if err == nil { @@ -1358,12 +1820,12 @@ func convertToKubernetesFormat(b int) string { func getVMIStatus(vmis *vmiMetaData, nodeName string) (string, error) { repVmiName := vmis.name - kubeconfig, err := kubeapi.GetKubeConfig() + kubeconfig, err := getKubeConfig() if err != nil { return "", logError("couldn't get the Kube Config: %v", err) } - virtClient, err := kubecli.GetKubevirtClientFromRESTConfig(kubeconfig) + virtClient, err := newKubevirtClient(kubeconfig) if err != nil { return "", logError("couldn't get the Kube client Config: %v", err) @@ -1388,12 +1850,21 @@ func getVMIStatus(vmis *vmiMetaData, nodeName string) (string, error) { return retStatus, err2 } - // Use the first VMI in the list + // Use the first non-terminating VMI in the list. Skipping terminating + // copies matters when a replica is being replaced on the same node: the + // old (terminating) and new VMI briefly coexist with the same + // GenerateName and NodeName, and picking whichever the API happened to + // list first - rather than the live one - is what made a confirm-absence + // wait latch onto the object that was already on its way out instead of + // the one it actually needed to watch. var nonLocalStatus string var targetVMI *v1.VirtualMachineInstance for _, vmi := range vmiList.Items { logrus.Debugf("getVMIStatus: repVmi:%s nodeName:%s vmiList vmi.ObjectMeta.Name:%s vmi.Status.NodeName:%s vmi.ObjectMeta.DeletionTimestamp:%v vmi.Status.Phase:%s", repVmiName, nodeName, vmi.ObjectMeta.Name, vmi.Status.NodeName, vmi.ObjectMeta.DeletionTimestamp, vmi.Status.Phase) + if vmi.ObjectMeta.DeletionTimestamp != nil { + continue + } if vmi.Status.NodeName == nodeName { if vmi.GenerateName == repVmiName { targetVMI = &vmi @@ -1761,6 +2232,12 @@ func (ctx kubevirtContext) CreateReplicaPodConfig(domainName string, config type ObjectMeta: metav1.ObjectMeta{ Name: kubeName, Namespace: kubeapi.EVEKubeNameSpace, + // Mirrors the VMIRS path's own top-level label (CreateReplicaVMIConfig): + // lets a stale-generation sweep find every ReplicaSet belonging to this + // app, across purge generations, by UUID prefix alone. + Labels: map[string]string{ + eveLabelKey: domainName, + }, }, Spec: appsv1.ReplicaSetSpec{ Replicas: repNum, @@ -1858,10 +2335,11 @@ func (ctx kubevirtContext) CreateReplicaPodConfig(domainName string, config type } } meta := vmiMetaData{ - repPod: replicaSet, - mtype: IsMetaReplicaPod, - name: kubeName, - domainID: int(rand.Uint32()), + repPod: replicaSet, + mtype: IsMetaReplicaPod, + name: kubeName, + // See the identical comment in CreateReplicaVMIConfig. + domainID: workloadID("", kubeName), memOverhead: uint64(overhead), } ctx.evictStaleVMIByUUIDPrefix(domainName) @@ -1939,7 +2417,7 @@ func StartReplicaPodContiner(ctx kubevirtContext, vmis *vmiMetaData) error { if err != nil { return err } - clientset, err := kubernetes.NewForConfig(ctx.kubeConfig) + clientset, err := newK8sClient(ctx.kubeConfig) if err != nil { logrus.Errorf("StartReplicaPodContiner: can't get clientset %v", err) return err @@ -1951,8 +2429,16 @@ func StartReplicaPodContiner(ctx kubevirtContext, vmis *vmiMetaData) error { if !errors.IsAlreadyExists(err) { logrus.Errorf("StartReplicaPodContiner: replicaset create failed: %v", err) return err - } else { - opStr = "already exists" + } + // The response on this branch carries no object, so the placeholder id + // from Setup stands until the next Info call re-derives it from a fresh Get. + opStr = "already exists" + } else if result != nil { + newID := workloadID(string(result.UID), vmis.name) + if newID != vmis.domainID { + logrus.Infof("StartReplicaPodContiner(%s): domainID changed from %d to %d", + rep.ObjectMeta.Name, vmis.domainID, newID) + vmis.domainID = newID } } @@ -2004,7 +2490,7 @@ func InfoReplicaSetContainer(ctx kubevirtContext, vmis *vmiMetaData) (string, er if err != nil { return "", err } - podclientset, err := kubernetes.NewForConfig(ctx.kubeConfig) + podclientset, err := newK8sClient(ctx.kubeConfig) if err != nil { return "", logError("InfoReplicaSetContainer: couldn't get the pod Config: %v", err) } @@ -2056,7 +2542,7 @@ func checkReplicaPodMetrics(ctx kubevirtContext, res map[string]types.DomainMetr return } kubeconfig := ctx.kubeConfig - podclientset, err := kubernetes.NewForConfig(kubeconfig) + podclientset, err := newK8sClient(kubeconfig) if err != nil { logrus.Errorf("checkReplicaPodMetrics: can not get pod client %v", err) return @@ -2174,7 +2660,7 @@ func getPodMetrics(clientset *metricsv.Clientset, pod k8sv1.Pod, vmis *vmiMetaDa // StopReplicaPodContainer stops the ReplicaSet pod func StopReplicaPodContainer(kubeconfig *rest.Config, repName string) error { - clientset, err := kubernetes.NewForConfig(kubeconfig) + clientset, err := newK8sClient(kubeconfig) if err != nil { logrus.Errorf("StopReplicaPodContainer: can't get clientset %v", err) return err @@ -2206,7 +2692,7 @@ func InfoPodContainer(ctx kubevirtContext, podName string) (string, error) { if err != nil { return "", err } - podclientset, err := kubernetes.NewForConfig(ctx.kubeConfig) + podclientset, err := newK8sClient(ctx.kubeConfig) if err != nil { return "", logError("InfoPodContainer: couldn't get the pod Config: %v", err) } @@ -2259,7 +2745,7 @@ func calculateMemoryUsagePercent(usedMemory, allocatedMemory int64) float64 { func getConfig(ctx *kubevirtContext) error { if ctx.kubeConfig == nil { - kubeconfig, err := kubeapi.GetKubeConfig() + kubeconfig, err := getKubeConfig() if err != nil { logrus.Error("getConfig: can not get kubeconfig") return err diff --git a/pkg/pillar/hypervisor/kubevirt_identity.go b/pkg/pillar/hypervisor/kubevirt_identity.go new file mode 100644 index 00000000000..9d021cf47f6 --- /dev/null +++ b/pkg/pillar/hypervisor/kubevirt_identity.go @@ -0,0 +1,37 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package hypervisor + +import "hash/fnv" + +// workloadID derives a stable, non-zero identifier for a kube workload from +// its cluster object UID, falling back to its Kubernetes name when the UID +// is not yet known (e.g. before the object has been created - Create runs +// before Start creates the VMIRS, so there is no UID to read yet). +// +// This is the kube-mode analogue of a pid: DomainId is only ever compared +// against the same app's own previous value, or against zero, never used +// as a cross-app key, so a hash collision between two different apps is +// harmless. +// +// 0 is reserved exclusively for "confirmed absent" (see Info's contract), +// so a hash that happens to land on exactly 0 is mapped to 1. +func workloadID(uid, kubeName string) int { + key := uid + if key == "" { + key = kubeName + } + h := fnv.New64a() + _, _ = h.Write([]byte(key)) + // Mask to 63 bits so the result is always a non-negative int (int is + // 64-bit on every platform EVE builds for), then guard the one masked + // value that lands on zero. + id := int64(h.Sum64() & 0x7FFFFFFFFFFFFFFF) + if id == 0 { + return 1 + } + return int(id) +} diff --git a/pkg/pillar/hypervisor/kubevirt_identity_test.go b/pkg/pillar/hypervisor/kubevirt_identity_test.go new file mode 100644 index 00000000000..1f7231ba69a --- /dev/null +++ b/pkg/pillar/hypervisor/kubevirt_identity_test.go @@ -0,0 +1,48 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package hypervisor + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestWorkloadID(t *testing.T) { + t.Run("deterministic per UID", func(t *testing.T) { + id1 := workloadID("11111111-1111-1111-1111-111111111111", "myapp-a1b2c-1") + id2 := workloadID("11111111-1111-1111-1111-111111111111", "myapp-a1b2c-1") + assert.Equal(t, id1, id2) + }) + + t.Run("differs across UIDs", func(t *testing.T) { + id1 := workloadID("11111111-1111-1111-1111-111111111111", "myapp-a1b2c-1") + id2 := workloadID("22222222-2222-2222-2222-222222222222", "myapp-a1b2c-1") + assert.NotEqual(t, id1, id2) + }) + + t.Run("never zero", func(t *testing.T) { + // A broad sweep of inputs, including ones hand-picked to be more + // likely to collide with a masking edge case, none of which may + // ever produce exactly 0. + inputs := []string{"", "0", "a", "myapp-a1b2c-1", "11111111-1111-1111-1111-111111111111"} + for _, in := range inputs { + assert.NotZero(t, workloadID(in, "fallback-name")) + assert.NotZero(t, workloadID("", in)) + } + }) + + t.Run("falls back to kubeName when UID is unavailable", func(t *testing.T) { + idFromUID := workloadID("11111111-1111-1111-1111-111111111111", "myapp-a1b2c-1") + idFromNameAsUID := workloadID("myapp-a1b2c-1", "unused") + idFromFallback := workloadID("", "myapp-a1b2c-1") + assert.NotEqual(t, idFromUID, idFromNameAsUID, + "a UID and an unrelated kubeName must not coincidentally hash the same input") + assert.Equal(t, idFromNameAsUID, idFromFallback, + "an empty UID must fall back to hashing kubeName, i.e. the exact same input "+ + "as passing kubeName in the UID slot") + }) +} diff --git a/pkg/pillar/hypervisor/kubevirt_info_test.go b/pkg/pillar/hypervisor/kubevirt_info_test.go new file mode 100644 index 00000000000..94747737938 --- /dev/null +++ b/pkg/pillar/hypervisor/kubevirt_info_test.go @@ -0,0 +1,208 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package hypervisor + +import ( + "testing" + + "github.com/lf-edge/eve/pkg/pillar/types" + uuid "github.com/satori/go.uuid" + "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + v1 "kubevirt.io/api/core/v1" + "kubevirt.io/client-go/kubecli" +) + +// newInfoTestTask builds a kubevirtTask ready to call Info/replicaSetUID +// against a mocked kubevirt client, with an arbitrary non-zero +// status.DomainId sentinel so tests can tell whether Info preserved it. +func newInfoTestTask(t *testing.T, lastKnownID int) (kubevirtTask, *types.DomainStatus) { + t.Helper() + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + status := &types.DomainStatus{ + UUIDandVersion: types.UUIDandVersion{UUID: appUUID}, + DisplayName: "myapp", + DomainName: "myapp." + appUUID.String(), + PurgeCounter: 1, + DomainId: lastKnownID, + } + status.VirtualizationMode = types.HVM // -> IsMetaReplicaVMI + + var ctx kubevirtContext + ctx.nodeNameMap = map[string]string{"nodename": "node1"} + + // Leave ctx.kubeConfig nil, as domainmgr does, and stub only the + // kubeconfig read. Info must then call getConfig itself. An earlier + // version of this helper set kubeConfig here and hid a nil-pointer + // panic in Info. + swapGetKubeConfig(t) + + return ctx.Task(status).(kubevirtTask), status +} + +// TestInfoContract pins the main invariant in Info's contract (see its doc +// comment in kubevirt.go): a zero DomainId means the VMIRS is confirmed +// absent, and nothing else. Every other outcome must return a non-zero id. +// +// It covers the two rows the existence check decides alone (NotFound, and +// unreachable) plus the stranded-VMIRS row. The remaining "found" rows need +// VMI and pod listing as well, so the evetest purge tests cover those. +func TestInfoContract(t *testing.T) { + const lastKnownID = 918273645 + + t.Run("NotFound is the only case that returns zero", func(t *testing.T) { + task, _ := newInfoTestTask(t, lastKnownID) + + ctrl := gomock.NewController(t) + mockClient := kubecli.NewMockKubevirtClient(ctrl) + mockRS := kubecli.NewMockReplicaSetInterface(ctrl) + mockClient.EXPECT().ReplicaSet(gomock.Any()).Return(mockRS).AnyTimes() + mockRS.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return( + nil, apierrors.NewNotFound( + schema.GroupResource{Group: "kubevirt.io", Resource: "virtualmachineinstancereplicasets"}, + task.kubeName())) + swapKubevirtClient(t, mockClient) + + id, state, err := task.Info(task.status.DomainName) + assert.NoError(t, err) + assert.Equal(t, types.HALTED, state) + assert.Zero(t, id) + }) + + t.Run("an unreachable API never returns zero and keeps the last known id", func(t *testing.T) { + task, _ := newInfoTestTask(t, lastKnownID) + + ctrl := gomock.NewController(t) + mockClient := kubecli.NewMockKubevirtClient(ctrl) + mockRS := kubecli.NewMockReplicaSetInterface(ctrl) + mockClient.EXPECT().ReplicaSet(gomock.Any()).Return(mockRS).AnyTimes() + mockRS.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return( + nil, assert.AnError) + swapKubevirtClient(t, mockClient) + + id, state, err := task.Info(task.status.DomainName) + assert.Error(t, err) + assert.Equal(t, types.UNKNOWN, state) + assert.Equal(t, lastKnownID, id, "must preserve the caller's last known id, not fabricate a new one") + assert.NotZero(t, id) + }) + + // The two rows above return before Info builds a client. This row is the + // shortest path to that line, which panicked on a device. HALTED with a + // non-zero id tells domainmgr to recreate the workload. + t.Run("a stranded VMIRS is HALTED with a non-zero id", func(t *testing.T) { + task, _ := newInfoTestTask(t, lastKnownID) + domainName := task.status.DomainName + task.vmiList = map[string]*vmiMetaData{ + domainName: {mtype: IsMetaReplicaVMI, name: task.kubeName()}, + } + + noReplicas := int32(0) + stranded := &v1.VirtualMachineInstanceReplicaSet{ + ObjectMeta: metav1.ObjectMeta{Name: task.kubeName(), UID: "stranded-uid"}, + Spec: v1.VirtualMachineInstanceReplicaSetSpec{Replicas: &noReplicas}, + } + + ctrl := gomock.NewController(t) + mockClient := kubecli.NewMockKubevirtClient(ctrl) + mockRS := kubecli.NewMockReplicaSetInterface(ctrl) + mockClient.EXPECT().ReplicaSet(gomock.Any()).Return(mockRS).AnyTimes() + // Two Gets: the existence check in replicaSetUID, then getVmirs. + mockRS.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()). + Return(stranded, nil).AnyTimes() + swapKubevirtClient(t, mockClient) + + id, state, err := task.Info(domainName) + assert.Error(t, err, "a stranded VMIRS is logged as an error") + assert.Equal(t, types.HALTED, state) + assert.NotZero(t, id, "the object exists, so zero would falsely mean confirmed-absent") + }) +} + +// TestInfoVmirsDeletedMidCall covers a VMIRS that is present for Info's +// existence check and deleted before the Get that follows it. Absence +// observed at the second Get must produce the same answer as absence +// observed at the first - HALTED, zero id, no error - rather than the +// SCHEDULING-with-an-error that a NotFound would otherwise fall through to. +// That error matters beyond the state it carries: waitForDomainGone treats +// any error from Info as "the domain is gone" and stops waiting. +func TestInfoVmirsDeletedMidCall(t *testing.T) { + const lastKnownID = 918273645 + task, status := newInfoTestTask(t, lastKnownID) + status.DomainName = "11111111-1111-1111-1111-111111111111.1.1" + task.vmiList = map[string]*vmiMetaData{ + status.DomainName: {mtype: IsMetaReplicaVMI, name: task.kubeName()}, + } + + ctrl := gomock.NewController(t) + mockClient := kubecli.NewMockKubevirtClient(ctrl) + mockRS := kubecli.NewMockReplicaSetInterface(ctrl) + mockClient.EXPECT().ReplicaSet(gomock.Any()).Return(mockRS).AnyTimes() + + notFound := apierrors.NewNotFound( + schema.GroupResource{Group: "kubevirt.io", Resource: "virtualmachineinstancereplicasets"}, + task.kubeName()) + gomock.InOrder( + // The existence check finds it... + mockRS.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return( + &v1.VirtualMachineInstanceReplicaSet{ + ObjectMeta: metav1.ObjectMeta{Name: task.kubeName(), UID: "some-uid"}, + }, nil), + // ...and it is gone from here on. Left unbounded rather than pinned + // to a single call so this asserts the answer Info returns, not how + // many times it asks. + mockRS.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, notFound).AnyTimes(), + ) + swapKubevirtClient(t, mockClient) + + id, state, err := task.Info(status.DomainName) + assert.NoError(t, err, "a confirmed-absent domain is not an error") + assert.Equal(t, types.HALTED, state) + assert.Zero(t, id) +} + +// TestInfoUnreachableKeepsLastID is a focused restatement of the second +// case in TestInfoContract: a range of different last-known ids must all +// survive an existence-check failure unchanged. +func TestInfoUnreachableKeepsLastID(t *testing.T) { + for _, lastKnownID := range []int{1, 42, 918273645} { + task, _ := newInfoTestTask(t, lastKnownID) + + ctrl := gomock.NewController(t) + mockClient := kubecli.NewMockKubevirtClient(ctrl) + mockRS := kubecli.NewMockReplicaSetInterface(ctrl) + mockClient.EXPECT().ReplicaSet(gomock.Any()).Return(mockRS).AnyTimes() + mockRS.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, assert.AnError) + swapKubevirtClient(t, mockClient) + + id, state, err := task.Info(task.status.DomainName) + assert.Error(t, err) + assert.Equal(t, types.UNKNOWN, state) + assert.Equal(t, lastKnownID, id) + } +} + +// TestCreateReturnsNonZero pins the pre-Start sequencing invariant: Create +// runs before the VMIRS exists, so vmiList has no entry for it yet, but +// Create must still return a non-zero id derived from config - never a nil +// dereference (the map access this replaced) and never zero. +func TestCreateReturnsNonZero(t *testing.T) { + var ctx kubevirtContext // zero value: nil vmiList + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + config := &types.DomainConfig{ + UUIDandVersion: types.UUIDandVersion{UUID: appUUID}, + DisplayName: "myapp", + PurgeCounter: 1, + } + + id, err := ctx.Create("some-domain-name", "", config) + assert.NoError(t, err) + assert.NotZero(t, id) +} diff --git a/pkg/pillar/hypervisor/kubevirt_staleness.go b/pkg/pillar/hypervisor/kubevirt_staleness.go new file mode 100644 index 00000000000..0b6969652f3 --- /dev/null +++ b/pkg/pillar/hypervisor/kubevirt_staleness.go @@ -0,0 +1,120 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package hypervisor + +import ( + "strconv" + "strings" + + uuid "github.com/satori/go.uuid" + appsv1 "k8s.io/api/apps/v1" + v1 "kubevirt.io/api/core/v1" +) + +// staleVMIRSGeneration reports whether vmirs is a superseded generation of +// the given app's VMIRS: it carries the app's App-Domain-Name label prefix +// ("."), its object name differs from the desired generation's name, +// and its trailing purge-counter suffix parses as strictly less than +// desiredCounter. +// +// The label lives on Spec.Selector.MatchLabels (and the pod template), +// not on the VMIRS object's own ObjectMeta.Labels - CreateReplicaVMIConfig +// never sets that field at all - so this reads the selector, not +// vmirs.GetLabels(). +// +// Name inequality, not label or version comparison, is what distinguishes +// "this generation" from "an older one": a non-purge config edit bumps the +// label's embedded config version without renaming the object, so a +// version-only rule would misclassify the current generation as stale. +// +// An unparsable or missing counter suffix is never considered stale, since +// there is nothing to safely compare it against. +func staleVMIRSGeneration( + vmirs v1.VirtualMachineInstanceReplicaSet, appUUID uuid.UUID, + desiredName string, desiredCounter uint32) bool { + var label string + if vmirs.Spec.Selector != nil { + label = vmirs.Spec.Selector.MatchLabels[eveLabelKey] + } + if !strings.HasPrefix(label, appUUID.String()+".") { + return false + } + name := vmirs.GetName() + if name == desiredName { + return false + } + counter, ok := trailingCounter(name) + if !ok { + return false + } + return counter < desiredCounter +} + +// stalePodReplicaSetGeneration is staleVMIRSGeneration's analogue for a +// NOHYPER app's plain container ReplicaSet. Unlike the VMIRS case, this +// object's own ObjectMeta.Labels does carry the App-Domain-Name label - +// see CreateReplicaPodConfig - because there is no pre-existing convention +// on this type to follow instead. +func stalePodReplicaSetGeneration( + rs appsv1.ReplicaSet, appUUID uuid.UUID, desiredName string, desiredCounter uint32) bool { + label := rs.GetLabels()[eveLabelKey] + if !strings.HasPrefix(label, appUUID.String()+".") { + return false + } + name := rs.GetName() + if name == desiredName { + return false + } + counter, ok := trailingCounter(name) + if !ok { + return false + } + return counter < desiredCounter +} + +// stalePodReplicaSetNames is staleVMIRSNames's ReplicaSet-Pod analogue. +func stalePodReplicaSetNames( + list []appsv1.ReplicaSet, appUUID uuid.UUID, desiredName string, desiredCounter uint32) []string { + var names []string + for _, rs := range list { + if stalePodReplicaSetGeneration(rs, appUUID, desiredName, desiredCounter) { + names = append(names, rs.GetName()) + } + } + return names +} + +// trailingCounter extracts the purge-counter suffix from a name of the form +// "-" (see base.GetAppKubeNameWithPurge), i.e. the +// digits after the last hyphen. ok is false if there is no hyphen or the +// suffix is not a valid non-negative integer. +func trailingCounter(name string) (counter uint32, ok bool) { + idx := strings.LastIndex(name, "-") + if idx < 0 || idx == len(name)-1 { + return 0, false + } + n, err := strconv.ParseUint(name[idx+1:], 10, 32) + if err != nil { + return 0, false + } + return uint32(n), true +} + +// staleVMIRSNames returns the names of every VMIRS in vmirsList that is a +// stale generation of appUUID's app, per staleVMIRSGeneration. Pure and +// side-effect free; callers are responsible for actually enumerating +// vmirsList from the cluster and for deleting whatever it returns. +func staleVMIRSNames( + vmirsList []v1.VirtualMachineInstanceReplicaSet, appUUID uuid.UUID, + desiredName string, desiredCounter uint32) []string { + var names []string + for _, vmirs := range vmirsList { + if staleVMIRSGeneration(vmirs, appUUID, desiredName, desiredCounter) { + names = append(names, vmirs.GetName()) + } + } + return names +} diff --git a/pkg/pillar/hypervisor/kubevirt_staleness_test.go b/pkg/pillar/hypervisor/kubevirt_staleness_test.go new file mode 100644 index 00000000000..f33bc62deef --- /dev/null +++ b/pkg/pillar/hypervisor/kubevirt_staleness_test.go @@ -0,0 +1,170 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package hypervisor + +import ( + "testing" + + uuid "github.com/satori/go.uuid" + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "kubevirt.io/api/core/v1" +) + +// mkVMIRS builds a fixture matching what CreateReplicaVMIConfig actually +// creates: the App-Domain-Name label lives on Spec.Selector.MatchLabels, +// not on the object's own ObjectMeta.Labels (which CreateReplicaVMIConfig +// never sets). +func mkVMIRS(name, domainNameLabel string) v1.VirtualMachineInstanceReplicaSet { + return v1.VirtualMachineInstanceReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: v1.VirtualMachineInstanceReplicaSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{eveLabelKey: domainNameLabel}, + }, + }, + } +} + +// mkPodReplicaSet builds a fixture matching CreateReplicaPodConfig, which +// (unlike the VMIRS case) does set the App-Domain-Name label directly on +// ObjectMeta.Labels. +func mkPodReplicaSet(name, domainNameLabel string) appsv1.ReplicaSet { + return appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{eveLabelKey: domainNameLabel}, + }, + } +} + +func TestStaleGenerationPredicate(t *testing.T) { + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + otherUUID := uuid.Must(uuid.FromString("22222222-2222-2222-2222-222222222222")) + domainName := appUUID.String() + ".1.0" + const desiredName = "myapp-a1b2c-2" + const desiredCounter = uint32(2) + + tests := []struct { + name string + vmirs v1.VirtualMachineInstanceReplicaSet + wantOK bool + }{ + { + name: "older generation of this app is stale", + vmirs: mkVMIRS("myapp-a1b2c-1", domainName), + wantOK: true, + }, + { + name: "much older generation of this app is stale", + vmirs: mkVMIRS("myapp-a1b2c-0", domainName), + wantOK: true, + }, + { + name: "the desired generation itself is never stale", + vmirs: mkVMIRS(desiredName, domainName), + wantOK: false, + }, + { + name: "a differently-named object at the same counter is never stale (not strictly less)", + vmirs: mkVMIRS("myapp-old-2", domainName), + wantOK: false, + }, + { + name: "a newer generation is never stale", + vmirs: mkVMIRS("myapp-a1b2c-3", domainName), + wantOK: false, + }, + { + name: "a different app's older-numbered generation is ignored", + vmirs: mkVMIRS("otherapp-x9y8z-1", otherUUID.String()+".1.0"), + wantOK: false, + }, + { + name: "unparsable (non-numeric) trailing suffix is never stale", + vmirs: mkVMIRS("myapp-a1b2c-abc", domainName), + wantOK: false, + }, + { + name: "name with no hyphen is never stale", + vmirs: mkVMIRS("myappa1b2c1", domainName), + wantOK: false, + }, + { + name: "empty App-Domain-Name label is ignored", + vmirs: mkVMIRS("myapp-a1b2c-1", ""), + wantOK: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := staleVMIRSGeneration(tc.vmirs, appUUID, desiredName, desiredCounter) + assert.Equal(t, tc.wantOK, got) + }) + } +} + +func TestStaleVMIRSNames(t *testing.T) { + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + otherUUID := uuid.Must(uuid.FromString("22222222-2222-2222-2222-222222222222")) + domainName := appUUID.String() + ".1.0" + const desiredName = "myapp-a1b2c-2" + + list := []v1.VirtualMachineInstanceReplicaSet{ + mkVMIRS("myapp-a1b2c-0", domainName), + mkVMIRS("myapp-a1b2c-1", domainName), + mkVMIRS(desiredName, domainName), + mkVMIRS("otherapp-x9y8z-0", otherUUID.String()+".1.0"), + } + + got := staleVMIRSNames(list, appUUID, desiredName, 2) + assert.ElementsMatch(t, []string{"myapp-a1b2c-0", "myapp-a1b2c-1"}, got) +} + +func TestStalePodReplicaSetNames(t *testing.T) { + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + otherUUID := uuid.Must(uuid.FromString("22222222-2222-2222-2222-222222222222")) + domainName := appUUID.String() + ".1.0" + const desiredName = "myapp-a1b2c-2" + + list := []appsv1.ReplicaSet{ + mkPodReplicaSet("myapp-a1b2c-0", domainName), + mkPodReplicaSet("myapp-a1b2c-1", domainName), + mkPodReplicaSet(desiredName, domainName), + mkPodReplicaSet("otherapp-x9y8z-0", otherUUID.String()+".1.0"), + } + + got := stalePodReplicaSetNames(list, appUUID, desiredName, 2) + assert.ElementsMatch(t, []string{"myapp-a1b2c-0", "myapp-a1b2c-1"}, got) +} + +func TestTrailingCounter(t *testing.T) { + tests := []struct { + name string + wantCounter uint32 + wantOK bool + }{ + {name: "myapp-a1b2c-2", wantCounter: 2, wantOK: true}, + {name: "myapp-a1b2c-0", wantCounter: 0, wantOK: true}, + {name: "myapp-a1b2c-", wantOK: false}, + {name: "myapp-a1b2c-abc", wantOK: false}, + {name: "noseparator", wantOK: false}, + {name: "", wantOK: false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + counter, ok := trailingCounter(tc.name) + assert.Equal(t, tc.wantOK, ok) + if tc.wantOK { + assert.Equal(t, tc.wantCounter, counter) + } + }) + } +} diff --git a/pkg/pillar/hypervisor/kubevirt_stopreplicavmi_test.go b/pkg/pillar/hypervisor/kubevirt_stopreplicavmi_test.go new file mode 100644 index 00000000000..f16ee1f0d22 --- /dev/null +++ b/pkg/pillar/hypervisor/kubevirt_stopreplicavmi_test.go @@ -0,0 +1,89 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package hypervisor + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" + "kubevirt.io/client-go/kubecli" +) + +// swapKubevirtClient overrides newKubevirtClient to always return client, +// restoring the original constructor when the test ends. Tests using this +// must not run with t.Parallel, since the override is a shared +// package-level var. +// +// The override rejects a nil *rest.Config, because the real constructor +// panics on one. A stub that ignores its argument hides a caller that never +// filled in kubeConfig. +func swapKubevirtClient(t *testing.T, client kubecli.KubevirtClient) { + t.Helper() + orig := newKubevirtClient + newKubevirtClient = func(cfg *rest.Config) (kubecli.KubevirtClient, error) { + if cfg == nil { + t.Errorf("newKubevirtClient called with a nil *rest.Config: " + + "the caller did not populate kubeConfig (getConfig on a value " + + "receiver fills in a copy); this panics in production") + } + return client, nil + } + t.Cleanup(func() { newKubevirtClient = orig }) +} + +// TestStopReplicaVMIErrorHandling pins the fix to the inverted IsNotFound +// check in StopReplicaVMI: success and NotFound must both return nil, and +// only a real API error should be returned (and logged as an error). +func TestStopReplicaVMIErrorHandling(t *testing.T) { + tests := []struct { + name string + delErr error + wantErr bool + }{ + { + name: "success", + delErr: nil, + wantErr: false, + }, + { + name: "not found is not an error", + delErr: apierrors.NewNotFound( + schema.GroupResource{Group: "kubevirt.io", Resource: "virtualmachineinstancereplicasets"}, + "myapp-a1b2c-1"), + wantErr: false, + }, + { + name: "a real API error is returned", + delErr: apierrors.NewInternalError(assert.AnError), + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + mockClient := kubecli.NewMockKubevirtClient(ctrl) + mockRS := kubecli.NewMockReplicaSetInterface(ctrl) + mockClient.EXPECT().ReplicaSet(gomock.Any()).Return(mockRS).AnyTimes() + mockRS.EXPECT(). + Delete(gomock.Any(), "myapp-a1b2c-1", gomock.Any()). + Return(tc.delErr) + + swapKubevirtClient(t, mockClient) + + err := StopReplicaVMI(&rest.Config{}, "myapp-a1b2c-1") + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/pillar/hypervisor/kubevirt_sweep_test.go b/pkg/pillar/hypervisor/kubevirt_sweep_test.go new file mode 100644 index 00000000000..01804f1464a --- /dev/null +++ b/pkg/pillar/hypervisor/kubevirt_sweep_test.go @@ -0,0 +1,136 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package hypervisor + +import ( + "testing" + "time" + + uuid "github.com/satori/go.uuid" + "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/rest" + v1 "kubevirt.io/api/core/v1" + "kubevirt.io/client-go/kubecli" +) + +// swapK8sClientNoPods overrides newK8sClient to return a fake clientset +// with no pods in it, i.e. every pod-list confirm-absence check reports +// "gone" immediately. Sufficient for sweep tests, which are exercising the +// VMIRS/ReplicaSet object side of confirm-absence, not pod teardown timing. +// It asserts a non-nil config for the reason given on swapKubevirtClient. +func swapK8sClientNoPods(t *testing.T) { + t.Helper() + orig := newK8sClient + fakeClientset := fake.NewSimpleClientset() + newK8sClient = func(cfg *rest.Config) (kubernetes.Interface, error) { + if cfg == nil { + t.Errorf("newK8sClient called with a nil *rest.Config: " + + "the caller did not populate kubeConfig") + } + return fakeClientset, nil + } + t.Cleanup(func() { newK8sClient = orig }) +} + +// swapSweepConfirmInterval shrinks the confirm-absence retry interval for +// the duration of a test, so a deliberately-never-gone timeout test does +// not have to wait out the real ~50s budget. +func swapSweepConfirmInterval(t *testing.T, d time.Duration) { + t.Helper() + orig := sweepConfirmInterval + sweepConfirmInterval = d + t.Cleanup(func() { sweepConfirmInterval = orig }) +} + +// TestSweepDeletesOnlyOlderGenerations pins the sweep's core selection +// invariant: given a mix of older, current, and another app's generations, +// only the strictly-older generations of the target app are deleted - the +// current generation and the other app's generation must survive untouched. +func TestSweepDeletesOnlyOlderGenerations(t *testing.T) { + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + otherUUID := uuid.Must(uuid.FromString("22222222-2222-2222-2222-222222222222")) + domainName := appUUID.String() + ".1.0" + const desiredName = "myapp-a1b2c-2" + const desiredCounter = uint32(2) + + list := &v1.VirtualMachineInstanceReplicaSetList{ + Items: []v1.VirtualMachineInstanceReplicaSet{ + mkVMIRS("myapp-a1b2c-0", domainName), + mkVMIRS("myapp-a1b2c-1", domainName), + mkVMIRS(desiredName, domainName), + mkVMIRS("otherapp-x9y8z-1", otherUUID.String()+".1.0"), + }, + } + notFoundErr := apierrors.NewNotFound( + schema.GroupResource{Group: "kubevirt.io", Resource: "virtualmachineinstancereplicasets"}, "") + + ctrl := gomock.NewController(t) + mockClient := kubecli.NewMockKubevirtClient(ctrl) + mockRS := kubecli.NewMockReplicaSetInterface(ctrl) + mockClient.EXPECT().ReplicaSet(gomock.Any()).Return(mockRS).AnyTimes() + mockRS.EXPECT().List(gomock.Any(), gomock.Any()).Return(list, nil) + + deleted := map[string]bool{} + mockRS.EXPECT().Delete(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ interface{}, name interface{}, _ interface{}) error { + deleted[name.(string)] = true + return nil + }).Times(2) + // Confirm-absence: object already gone by the time we check. + mockRS.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, notFoundErr).Times(2) + + swapKubevirtClient(t, mockClient) + swapK8sClientNoPods(t) + + var ctx kubevirtContext + ctx.kubeConfig = &rest.Config{} + + err := ctx.sweepStaleGenerations(appUUID, desiredName, desiredCounter, IsMetaReplicaVMI) + assert.NoError(t, err) + assert.Equal(t, map[string]bool{"myapp-a1b2c-0": true, "myapp-a1b2c-1": true}, deleted) +} + +// TestSweepConfirmAbsenceTimeoutFails: if a stale generation is deleted but +// never actually confirmed gone, the sweep must return an error (which +// Start propagates, refusing to create the new +// generation) rather than give up silently or hang indefinitely. +func TestSweepConfirmAbsenceTimeoutFails(t *testing.T) { + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + domainName := appUUID.String() + ".1.0" + + list := &v1.VirtualMachineInstanceReplicaSetList{ + Items: []v1.VirtualMachineInstanceReplicaSet{ + mkVMIRS("myapp-a1b2c-1", domainName), + }, + } + + ctrl := gomock.NewController(t) + mockClient := kubecli.NewMockKubevirtClient(ctrl) + mockRS := kubecli.NewMockReplicaSetInterface(ctrl) + mockClient.EXPECT().ReplicaSet(gomock.Any()).Return(mockRS).AnyTimes() + mockRS.EXPECT().List(gomock.Any(), gomock.Any()).Return(list, nil) + mockRS.EXPECT().Delete(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + // The object is never confirmed gone: every Get keeps succeeding (still there). + mockRS.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&v1.VirtualMachineInstanceReplicaSet{}, nil). + Times(sweepConfirmRetries) + + swapKubevirtClient(t, mockClient) + swapK8sClientNoPods(t) + swapSweepConfirmInterval(t, time.Millisecond) + + var ctx kubevirtContext + ctx.kubeConfig = &rest.Config{} + + err := ctx.sweepStaleGenerations(appUUID, "myapp-a1b2c-2", 2, IsMetaReplicaVMI) + assert.Error(t, err) +} diff --git a/pkg/pillar/hypervisor/kubevirt_task_test.go b/pkg/pillar/hypervisor/kubevirt_task_test.go new file mode 100644 index 00000000000..f41670398f6 --- /dev/null +++ b/pkg/pillar/hypervisor/kubevirt_task_test.go @@ -0,0 +1,63 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package hypervisor + +import ( + "testing" + + "github.com/lf-edge/eve/pkg/pillar/base" + "github.com/lf-edge/eve/pkg/pillar/types" + uuid "github.com/satori/go.uuid" + "github.com/stretchr/testify/assert" +) + +// TestTaskDerivesKubeName constructs a Task from a DomainStatus whose +// DisplayName/PurgeCounter match no vmiList entry, and asserts that the +// kubeName it derives is exactly what CreateReplicaVMIConfig/ +// CreateReplicaPodConfig would have named the same generation. +func TestTaskDerivesKubeName(t *testing.T) { + var ctx kubevirtContext // zero value: nil vmiList, no entries whatsoever + + appUUID := uuid.Must(uuid.FromString("11111111-1111-1111-1111-111111111111")) + status := &types.DomainStatus{ + UUIDandVersion: types.UUIDandVersion{UUID: appUUID}, + DisplayName: "myapp", + PurgeCounter: 3, + } + + task := ctx.Task(status) + kt, ok := task.(kubevirtTask) + assert.True(t, ok, "Task() must return a kubevirtTask") + + // The derivation must not depend on any vmiList entry existing. + _, found := kt.vmiList[kt.kubeName()] + assert.False(t, found) + + want := base.GetAppKubeNameWithPurge("myapp", appUUID, 3) + assert.Equal(t, want, kt.kubeName()) +} + +func TestTaskMetaType(t *testing.T) { + var ctx kubevirtContext + + tests := []struct { + name string + mode types.VmMode + want MetaDataType + }{ + {name: "NOHYPER runs as a plain container ReplicaSet", mode: types.NOHYPER, want: IsMetaReplicaPod}, + {name: "HVM runs as a VMI ReplicaSet", mode: types.HVM, want: IsMetaReplicaVMI}, + {name: "PV (zero value) runs as a VMI ReplicaSet", mode: types.PV, want: IsMetaReplicaVMI}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + status := &types.DomainStatus{} + status.VirtualizationMode = tc.mode + kt := ctx.Task(status).(kubevirtTask) + assert.Equal(t, tc.want, kt.metaType()) + }) + } +} diff --git a/pkg/pillar/hypervisor/kubevirt_test.go b/pkg/pillar/hypervisor/kubevirt_test.go index 36690f15593..164e90435f0 100644 --- a/pkg/pillar/hypervisor/kubevirt_test.go +++ b/pkg/pillar/hypervisor/kubevirt_test.go @@ -56,41 +56,44 @@ func TestPodListToSchedulingState(t *testing.T) { } tests := []struct { - name string - pods []corev1.Pod - wantOnMe bool - wantAnyNode bool - wantErr bool + name string + pods []corev1.Pod + wantOnMe bool + wantScheduledOnNone bool + wantErr bool }{ { - name: "Pending phase with NodeName set (Init:0/1) on this node", - pods: []corev1.Pod{mkPod(node, corev1.PodPending)}, - wantOnMe: true, - wantAnyNode: true, + name: "Pending phase with NodeName set (Init:0/1) on this node", + pods: []corev1.Pod{mkPod(node, corev1.PodPending)}, + wantOnMe: true, + // Bound to a node (this one), so nothing about "scheduled on none" applies. + wantScheduledOnNone: false, }, { - name: "Pending phase with NodeName set (Init:0/1) on another node", - pods: []corev1.Pod{mkPod("other-node", corev1.PodPending)}, - wantOnMe: false, - wantAnyNode: true, + name: "Pending phase with NodeName set (Init:0/1) on another node", + pods: []corev1.Pod{mkPod("other-node", corev1.PodPending)}, + wantOnMe: false, + // Bound to a node, just not this one - neither onMe nor scheduledOnNone. + wantScheduledOnNone: false, }, { - name: "Running pod on this node", - pods: []corev1.Pod{mkPod(node, corev1.PodRunning)}, - wantOnMe: true, - wantAnyNode: true, + name: "Running pod on this node", + pods: []corev1.Pod{mkPod(node, corev1.PodRunning)}, + wantOnMe: true, + wantScheduledOnNone: false, }, { - name: "Running pod on another node", - pods: []corev1.Pod{mkPod("other-node", corev1.PodRunning)}, - wantOnMe: false, - wantAnyNode: true, + name: "Running pod on another node", + pods: []corev1.Pod{mkPod("other-node", corev1.PodRunning)}, + wantOnMe: false, + wantScheduledOnNone: false, }, { - name: "Pending pod with NodeName empty (not yet scheduled)", - pods: []corev1.Pod{mkPod("", corev1.PodPending)}, - wantOnMe: false, - wantAnyNode: true, + name: "Pending pod with NodeName empty (not yet scheduled)", + pods: []corev1.Pod{mkPod("", corev1.PodPending)}, + wantOnMe: false, + // Not bound to any node at all. + wantScheduledOnNone: true, }, { name: "No pods", @@ -110,14 +113,14 @@ func TestPodListToSchedulingState(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - onMe, anyNode, err := podListToSchedulingState(tc.pods, node) + onMe, scheduledOnNone, err := podListToSchedulingState(tc.pods, node) if tc.wantErr { assert.Error(t, err) return } assert.NoError(t, err) assert.Equal(t, tc.wantOnMe, onMe) - assert.Equal(t, tc.wantAnyNode, anyNode) + assert.Equal(t, tc.wantScheduledOnNone, scheduledOnNone) }) } } diff --git a/pkg/pillar/hypervisor/kubevirt_waitforvmi_test.go b/pkg/pillar/hypervisor/kubevirt_waitforvmi_test.go new file mode 100644 index 00000000000..57ffaa39cbb --- /dev/null +++ b/pkg/pillar/hypervisor/kubevirt_waitforvmi_test.go @@ -0,0 +1,85 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +//go:build k + +package hypervisor + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + v1 "kubevirt.io/api/core/v1" + "kubevirt.io/client-go/kubecli" +) + +// swapGetKubeConfig overrides getKubeConfig (the wrapper around +// kubeapi.GetKubeConfig) to return a dummy, non-nil *rest.Config instead of +// reading the real kubeconfig file from disk, restoring the original when +// the test ends. +func swapGetKubeConfig(t *testing.T) { + t.Helper() + orig := getKubeConfig + getKubeConfig = func() (*rest.Config, error) { + return &rest.Config{}, nil + } + t.Cleanup(func() { getKubeConfig = orig }) +} + +// TestWaitForVMITargetsCorrectObject is the regression test for the +// terminating-replica confusion seen in the field: when a same-node +// replica restart briefly leaves both the outgoing and incoming VMI +// matching the same GenerateName and NodeName, getVMIStatus (the primitive +// waitForVMI polls) must report the live one's phase, not the terminating +// one's - previously the loop picked whichever the API happened to list +// first. +func TestWaitForVMITargetsCorrectObject(t *testing.T) { + const nodeName = "node1" + const repVmiName = "myapp-a1b2c-1" + + now := metav1.Now() + terminating := v1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myapp-a1b2c-1bbbbb", + GenerateName: repVmiName, + DeletionTimestamp: &now, + }, + Status: v1.VirtualMachineInstanceStatus{ + NodeName: nodeName, + Phase: v1.Running, + }, + } + live := v1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myapp-a1b2c-1ccccc", + GenerateName: repVmiName, + }, + Status: v1.VirtualMachineInstanceStatus{ + NodeName: nodeName, + Phase: v1.Scheduling, + }, + } + + ctrl := gomock.NewController(t) + mockClient := kubecli.NewMockKubevirtClient(ctrl) + mockVMI := kubecli.NewMockVirtualMachineInstanceInterface(ctrl) + mockClient.EXPECT().VirtualMachineInstance(gomock.Any()).Return(mockVMI).AnyTimes() + // Deliberately list the terminating copy first, matching what the + // field trace showed: picking by list order rather than skipping + // terminating entries is exactly the bug this pins. + mockVMI.EXPECT().List(gomock.Any(), gomock.Any()).Return(&v1.VirtualMachineInstanceList{ + Items: []v1.VirtualMachineInstance{terminating, live}, + }, nil) + + swapKubevirtClient(t, mockClient) + swapGetKubeConfig(t) + + vmis := &vmiMetaData{name: repVmiName} + state, err := getVMIStatus(vmis, nodeName) + assert.NoError(t, err) + assert.Equal(t, "Scheduling", state, + "must report the live replica's phase, not the terminating one's") +} diff --git a/pkg/pillar/types/domainmgrtypes.go b/pkg/pillar/types/domainmgrtypes.go index c404aa14190..ff5054148ba 100644 --- a/pkg/pillar/types/domainmgrtypes.go +++ b/pkg/pillar/types/domainmgrtypes.go @@ -394,6 +394,19 @@ type DomainStatus struct { PendingModify bool PendingDelete bool DomainName string // Name of Xen domain + // DomainId identifies the running domain, with hypervisor-specific meaning: + // for xen/kvm it is the underlying qemu process's pid; for kubevirt (HV=k) + // it is a value derived from the app's current VMIRS/ReplicaSet identity + // (see hypervisor/kubevirt.go's workloadID), since there is no pid. + // + // The one invariant every hypervisor backend must uphold, and every + // consumer relies on: DomainId is zero if and only if the domain is + // confirmed not present (no process for xen/kvm; VMIRS/ReplicaSet + // confirmed absent for kubevirt). It must never be zero merely because + // the answer is unknown or unattributable - doInactivate's teardown + // gates and doCleanup's success test both key on zero meaning "already + // gone", so a zero returned for any other reason skips a teardown that + // never happened and reports it as done. DomainId int BootTime time.Time DiskStatusList []DiskStatus