diff --git a/evetest/Makefile b/evetest/Makefile index b9a94b6f134..15c27922068 100644 --- a/evetest/Makefile +++ b/evetest/Makefile @@ -1,15 +1,44 @@ # Copyright (c) 2026 Zededa, Inc. # SPDX-License-Identifier: Apache-2.0 -EVETEST_VERSION := $(shell grep -v '^\#' VERSION | head -n1) +# \# is make's escape for a literal '#'; expand it via a variable so the shell +# does not receive the backslash (newer grep warns about stray '\' before '#'). +HASH := \# +EVETEST_VERSION := $(shell grep -v '^$(HASH)' VERSION | head -n1) EVETEST_ORG ?= lfedge EVETEST_ADAM_VERSION ?= 0.0.81 EVETEST_ADAM_REPO ?= lfedge/adam +# Repository holding built EVE images (mirrors constants.DefaultEVERepo). +EVETEST_EVE_REPO ?= lfedge/eve + +# linuxkit comes from the EVE repo's build tools (see ../Makefile and +# ../mk/linuxkit.mk); it is built on demand via the root's "linuxkit" target +# and handed to the sdn sub-make through LINUXKIT, so it does not have to be +# on PATH. +BUILDTOOLS_BIN := $(abspath $(CURDIR)/../build-tools/bin) +LINUXKIT := $(BUILDTOOLS_BIN)/linuxkit + EVETEST_IMAGE := $(EVETEST_ORG)/evetest:$(EVETEST_VERSION) EVETEST_API_PORT ?= 50021 +# Label on the evetest image recording the framework content signature it was +# built from; used by ensure-evetest-image to detect stale images. +FRAMEWORK_SIG_LABEL := org.lfedge.evetest.framework-sig + +# Framework paths baked into the evetest image. tests/, netmodels/ and +# matchers/ are bind-mounted into the container at run time and testapps/ +# images are built separately, so changes there never require a rebuild. +FRAMEWORK_PATHSPEC := . ':(exclude)tests' ':(exclude)netmodels' ':(exclude)matchers' ':(exclude)testapps' + +# Content signature of the framework sources: the last commit touching them +# plus any uncommitted (staged or unstaged) changes. Git-based, so a git +# fsmonitor daemon keeps it fast; untracked files are not considered until +# they are added to git. Constant outside a git checkout. +FRAMEWORK_SIG = $(shell { git log -1 --format=%H -- $(FRAMEWORK_PATHSPEC); \ + git diff HEAD -- $(FRAMEWORK_PATHSPEC); } 2>/dev/null | sha256sum | cut -d' ' -f1) + PROTOC_VERSION ?= 31.1 PROTOC_GEN_GO_VERSION ?= 1.36.6 PROTOC_GEN_GO_GRPC_VERSION ?= 1.5.1 @@ -57,8 +86,32 @@ endif $(eval EVE_VERSION_ENV :=) ifndef EVETEST_EVE_VERSION ifeq ($(strip $(filter-out false False FALSE 0 f F,$(EVETEST_EVE_LIVE_IMAGE))),) + @# Resolve which EVE version to test. On a dirty tree 'make version' embeds + @# the wall-clock minute of *this* invocation, so it almost never matches + @# the timestamp baked into the image tag by an earlier 'make eve' run. + @# In that case fall back to the version recorded by the last completed + @# build (dist//current, read via 'make currentversion'), provided it + @# is a dirty build of the same commit and its EVE Docker image exists + @# locally. 'make currentversion' reports the full version including the + @# hypervisor/arch suffix; strip it by anchoring on the dirty timestamp. $(eval EVETEST_EVE_VERSION := $(strip $(shell \ - $(MAKE) -s -C $(REPO_ROOT) version 2>/dev/null \ + v="$$($(MAKE) -s -C $(REPO_ROOT) version 2>/dev/null)"; \ + if echo "$$v" | grep -q -e '-dirty-' && \ + ! docker images --format '{{.Tag}}' "$(EVETEST_EVE_REPO)" 2>/dev/null \ + | grep -q "^$$v-"; then \ + base="$${v%%-dirty-*}-dirty-"; \ + cur="$$($(MAKE) -s -C $(REPO_ROOT) currentversion 2>/dev/null \ + | sed -E 's/(-dirty-[0-9]{4}-[0-9]{2}-[0-9]{2}\.[0-9]{2}\.[0-9]{2}).*/\1/')"; \ + if [ -n "$$cur" ] && [ "$${cur#"$$base"}" != "$$cur" ] && \ + docker images --format '{{.Tag}}' "$(EVETEST_EVE_REPO)" 2>/dev/null \ + | grep -q "^$$cur-"; then \ + echo "NOTE: no local EVE image for dirty version $$v;" \ + "falling back to last built version $$cur (from dist/current)." >&2; \ + echo "NOTE: run 'make eve' first if EVE sources changed since that build." >&2; \ + v="$$cur"; \ + fi; \ + fi; \ + echo "$$v" \ ))) $(eval EVE_VERSION_ENV := -e EVETEST_EVE_VERSION=$(EVETEST_EVE_VERSION)) endif @@ -143,6 +196,12 @@ endif echo ""; \ exit 1; \ fi >&2 + @# Colorize framework log output only when stdout is a terminal. The test + @# runs inside the container where stdout is never a TTY (go test pipes it), + @# so the detection must happen here. It also cannot use $(shell [ -t 1 ]) + @# because within $(shell) stdout is make's capture pipe, never a terminal; + @# only a recipe shell sees make's real stdout. + [ -t 1 ] && COLOR=true || COLOR=false; \ docker run --rm $(DOCKER_IT) \ --name evetest-$(EVETEST_API_PORT) \ -p $(EVETEST_API_PORT):$(EVETEST_API_PORT) \ @@ -164,6 +223,7 @@ endif $(DIST_DIR_MOUNT) \ $(GO_CACHE_MOUNT) \ $(DOCKER_CONFIG_MOUNT) \ + -e EVETEST_COLOR_OUTPUT=$$COLOR \ $(ENV_VARS) \ $(EVE_VERSION_ENV) \ $(BROKER_IMAGE_ENV) \ @@ -175,17 +235,24 @@ endif -e EVETEST_HOST_GID=$(EVETEST_HOST_GID) \ $(EVETEST_IMAGE) -# Only builds when the image is missing -- a locally built image is never -# rebuilt just because harness source changed, so after editing evetest code -# run `make build-container` explicitly before `make evetest`, or the run -# silently uses the stale, previously-built harness. +# Ensure a usable evetest image exists locally: pull or build when it is +# missing, and rebuild when the framework signature recorded in the image +# label no longer matches the current sources (an image without the label, +# e.g. one predating this check, is always considered stale). ensure-evetest-image: @if ! docker image inspect $(EVETEST_IMAGE) >/dev/null 2>&1; then \ echo "Docker image $(EVETEST_IMAGE) not found locally, trying to pull..."; \ if ! docker pull $(EVETEST_IMAGE); then \ echo "Pull failed, building image locally..."; \ $(MAKE) build-container; \ + exit 0; \ fi; \ + fi; \ + if [ "$$(docker image inspect \ + -f '{{index .Config.Labels "$(FRAMEWORK_SIG_LABEL)"}}' \ + $(EVETEST_IMAGE) 2>/dev/null)" != "$(FRAMEWORK_SIG)" ]; then \ + echo "evetest framework sources changed, rebuilding $(EVETEST_IMAGE)..."; \ + $(MAKE) build-container; \ fi # Internal target: invoked by proto inside the builder container. @@ -220,6 +287,7 @@ build-container: docker buildx build \ --$(DOCKER_TARGET) \ --platform $(DOCKER_PLATFORM) \ + --label $(FRAMEWORK_SIG_LABEL)=$(FRAMEWORK_SIG) \ --build-arg EVETEST_VERSION=$(EVETEST_VERSION) \ --build-arg EVETEST_ADAM_REPO=$(EVETEST_ADAM_REPO) \ --build-arg EVETEST_ADAM_VERSION=$(EVETEST_ADAM_VERSION) \ @@ -356,5 +424,8 @@ install-cli: cd ./cli && go build -ldflags "-X main.version=$(EVETEST_VERSION)" -o $(GOBIN)/evetest @echo "Installed evetest CLI to $(GOBIN)/evetest" -build-sdn-container: - @$(MAKE) -C sdn build +$(LINUXKIT): + @$(MAKE) -C $(CURDIR)/.. linuxkit + +build-sdn-container: $(LINUXKIT) + @$(MAKE) -C sdn LINUXKIT=$(LINUXKIT) build diff --git a/evetest/README.md b/evetest/README.md index c19213c6ae4..dd91a727551 100644 --- a/evetest/README.md +++ b/evetest/README.md @@ -818,6 +818,7 @@ non-default behavior. | `EVETEST_EVE_FIRMWARE_DIR` | Overrides firmware discovery for a local live image, which otherwise looks for `OVMF*.fd` in `installer/firmware` next to the resolved qcow2 | -- | | `EVETEST_PREFERRED_ARCH` | Preferred CPU architecture (`amd64`, `arm64`) | `amd64` | | `EVETEST_LOG_LEVEL` | Framework log level (`debug`, `info`, `warn`) | `info` | +| `EVETEST_COLOR_OUTPUT` | Colorize framework log output with ANSI escape codes (`true`/`false`) | auto: enabled only when stdout is a terminal, disabled when piped or redirected | | `EVETEST_COLLECT_ARTIFACTS` | Host path for artifacts (logs, collect-info) | -- | | `EVETEST_COLLECT_COVERAGE` | Collect Go coverage profiles (requires `EVETEST_COLLECT_ARTIFACTS` and EVE built with `COVER=y`) | `false` | | `EVETEST_REGISTRY_MIRROR_DOCKER` | Pull-through cache URL(s) for docker.io — one or more comma-separated `[scheme://]host:port[/path]` (IPv6 hosts bracketed, e.g. `http://[fd11::5]:5000`); see `RequireIPv6OnlyRegistryMirrors` | -- | diff --git a/evetest/constants/config.go b/evetest/constants/config.go index 4ec20d466da..70fdfa5e7cc 100644 --- a/evetest/constants/config.go +++ b/evetest/constants/config.go @@ -23,6 +23,13 @@ const ( // This is read by both the evetest container and the broker. LogLevelEnv = "LOG_LEVEL" + // ColorOutputEnv determines whether framework log output is colorized + // with ANSI escape codes. When unset, the Makefile (or, as a fallback, + // the container entrypoint) enables colors only if stdout is attached + // to a terminal, so that piped or redirected output stays free of + // escape codes. + ColorOutputEnv = "COLOR_OUTPUT" + // APIAddressEnv specifies the IP address on which the evetest container exposes // its gRPC API. // This is used by the evetest CLI to connect to a running evetest instance. @@ -382,6 +389,10 @@ func InitViperConfig() { // Logging viper.SetDefault(LogLevelEnv, DefaultLogLevel) + // The Makefile and the entrypoint script override this based on TTY + // presence; the default only applies when the framework is run outside + // the evetest container. + viper.SetDefault(ColorOutputEnv, true) // gRPC API ports and addresses viper.SetDefault(APIAddressEnv, "") diff --git a/evetest/devconfig.go b/evetest/devconfig.go index 4552ee5218d..4793dab64f2 100644 --- a/evetest/devconfig.go +++ b/evetest/devconfig.go @@ -1047,6 +1047,10 @@ func (config ApplicationInstanceConfig) toProto(th *TestHarness, devName string, Id: aclID, }) } + interfaceOrder := uint32(i) + if adapter.InterfaceOrder != nil { + interfaceOrder = *adapter.InterfaceOrder + } appInstConfig.Interfaces = append(appInstConfig.Interfaces, &eveconfig.NetworkAdapter{ Name: adapter.LogicalLabel, @@ -1055,7 +1059,7 @@ func (config ApplicationInstanceConfig) toProto(th *TestHarness, devName string, MacAddress: adapter.MAC.String(), Acls: acls, AccessVlanId: uint32(adapter.AccessVLAN), - InterfaceOrder: uint32(i), + InterfaceOrder: interfaceOrder, }) } } @@ -1397,6 +1401,7 @@ type VirtualNetworkAdapter struct { AccessVLAN uint16 PortFwdRules []PortFwdRule ACLAllowRules []ACLAllowRule + InterfaceOrder *uint32 } func (VirtualNetworkAdapter) isAppNetworkAdapter() {} @@ -2396,18 +2401,19 @@ func (dc *EdgeDeviceConfig) UpdateApplication( if !proto.Equal(app.Fixedresources, newProtoConfig.Fixedresources) { dc.th.t.Fatalf("It is not allowed to change application Fixedresources") } + var needRestart bool var needPurge bool equalAdapter := func(a1, a2 *eveconfig.Adapter) bool { return proto.Equal(a1, a2) } if !generics.EqualSetsFn(app.Adapters, newProtoConfig.Adapters, equalAdapter) { - needPurge = true + needRestart = true } equalNetAdapter := func(a1, a2 *eveconfig.NetworkAdapter) bool { return proto.Equal(a1, a2) } if !generics.EqualSetsFn(app.Interfaces, newProtoConfig.Interfaces, equalNetAdapter) { - needPurge = true + needRestart = true } // The root ref (VolumeRefList[0]) is always left untouched; // buildMountRefs only ever references existing volumes, it does @@ -2420,11 +2426,20 @@ func (dc *EdgeDeviceConfig) UpdateApplication( if !generics.EqualSetsFn(app.VolumeRefList[1:], newMountRefs, equalVolumeRef) { needPurge = true } + // A purge subsumes a restart -- it stops the app, recreates its + // volumes and starts it again, which also applies any adapter + // change that is otherwise staged until the next restart. So when + // both are needed, bumping the purge counter alone is enough. if needPurge { if app.Purge == nil { app.Purge = &eveconfig.InstanceOpsCmd{Counter: 0} } app.Purge.Counter++ + } else if needRestart { + if app.Restart == nil { + app.Restart = &eveconfig.InstanceOpsCmd{Counter: 0} + } + app.Restart.Counter++ } dc.Apps[i].Activate = newProtoConfig.Activate dc.Apps[i].ProfileList = newProtoConfig.ProfileList diff --git a/evetest/entrypoint.sh b/evetest/entrypoint.sh index f5a3af3980d..171e054f8ea 100644 --- a/evetest/entrypoint.sh +++ b/evetest/entrypoint.sh @@ -39,6 +39,20 @@ EOF chmod +x /usr/local/bin/adam-cli +# Fallback when EVETEST_COLOR_OUTPUT was not passed in (the Makefile sets it +# based on whether the host stdout is a terminal): colorize framework log +# output only when stdout is attached to a terminal, i.e. when 'docker run' +# was invoked manually with a pseudo-TTY. This keeps piped or redirected +# output free of ANSI escape codes. +if [ -z "$EVETEST_COLOR_OUTPUT" ]; then + if [ -t 1 ]; then + EVETEST_COLOR_OUTPUT=true + else + EVETEST_COLOR_OUTPUT=false + fi +fi +export EVETEST_COLOR_OUTPUT + # Run go test in background GO_TEST_FLAGS="-v" GO_TEST_OUTPUT_FILE="${EVETEST_ARTIFACT_DIR}/gotest.txt" diff --git a/evetest/harness.go b/evetest/harness.go index 5e9e7cc7e28..15241fb81c1 100644 --- a/evetest/harness.go +++ b/evetest/harness.go @@ -214,10 +214,11 @@ const ( type TestHarness struct { api.UnimplementedEvetestServer - t *T - log *logrus.Logger - userLog *logrus.Logger - brokerLog *logrus.Logger + t *T + log *logrus.Logger + userLog *logrus.Logger + brokerLog *logrus.Logger + colorOutput bool artifactDir string @@ -446,6 +447,15 @@ func removeStaleImageCacheDirs(log *logrus.Logger, imgCacheParent string) { } } +// prefixColor returns the given log-prefix color when colorized output is +// enabled, PrefixColorNone otherwise. +func (th *TestHarness) prefixColor(c logger.PrefixColor) logger.PrefixColor { + if th.colorOutput { + return c + } + return logger.PrefixColorNone +} + // Init initializes the test harness and must be called exactly once per test. // When used inside a test suite, Init may be called multiple times, once per // test case, but only a single harness instance will be created. @@ -508,16 +518,17 @@ func Init(t *testing.T) *T { if err != nil { th.t.Fatalf("Failed to parse log level %q: %v", logLevelStr, err) } + th.colorOutput = viper.GetBool(constants.ColorOutputEnv) th.log = logrus.New() th.log.SetFormatter(&logger.PrefixedFormatter{ Prefix: "HARNESS ", - Color: logger.PrefixColorBlue, + Color: th.prefixColor(logger.PrefixColorBlue), }) th.log.SetLevel(logLevel) th.userLog = logrus.New() th.userLog.SetFormatter(&logger.PrefixedFormatter{ Prefix: "TEST ", - Color: logger.PrefixColorCyan, + Color: th.prefixColor(logger.PrefixColorCyan), }) th.userLog.SetLevel(logLevel) @@ -525,7 +536,7 @@ func Init(t *testing.T) *T { th.brokerLog = logrus.New() th.brokerLog.SetFormatter(&logger.PrefixedFormatter{ Prefix: "BROKER ", - Color: logger.PrefixColorPurple, + Color: th.prefixColor(logger.PrefixColorPurple), }) th.brokerLog.SetLevel(logLevel) diff --git a/evetest/testing.go b/evetest/testing.go index 4c99b394669..ef5dc9217de 100644 --- a/evetest/testing.go +++ b/evetest/testing.go @@ -43,8 +43,12 @@ const ( func (t *T) fail(msg string, now bool) { t.Helper() - // Log the error message with the red color. - t.Log(redColor + "TEST FAILURE: " + msg + resetColor) + // Log the error message, highlighted in red when colors are enabled. + failureMsg := "TEST FAILURE: " + msg + if t.th.colorOutput { + failureMsg = redColor + failureMsg + resetColor + } + t.Log(failureMsg) // Log stacktrace at the point of failure for easier debugging. t.Logf("STACKTRACE:\n%s", debug.Stack()) diff --git a/evetest/tests/apps/restart_test.go b/evetest/tests/apps/restart_test.go new file mode 100644 index 00000000000..2e2a010cb43 --- /dev/null +++ b/evetest/tests/apps/restart_test.go @@ -0,0 +1,224 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Application life-cycle operations tested against the EVE API: +// controller-requested restart of an application instance. + +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" + eveinfo "github.com/lf-edge/eve-api/go/info" + "github.com/lf-edge/eve/evetest" + "github.com/lf-edge/eve/evetest/matchers" + "github.com/lf-edge/eve/evetest/netmodels" + "github.com/lf-edge/eve/pkg/pillar/types" +) + +// TestAppRestart verifies that a controller-requested application restart +// (a bump of the restart counter in AppInstanceConfig, i.e. a domain +// restart *without* purge) brings the application back to the RUNNING +// state, several times in a row. +// +// A restart without purge tears the domain down and re-creates it under +// the same domain name, reusing the same QMP socket paths. That reuse is +// what exposes the stale-QMP-handler race: the torn-down qemu's leftover +// qmpEventHandler reacts to the final host-initiated SHUTDOWN event by +// issuing stop+quit on the executor socket *path*, retrying for ~30s; if +// the re-created qemu re-binds that path within the retry window, the +// handler quits the new instance instead. domainmgr then sees "unexpected +// state HALTED", marks the boot failed and retries only after the ~10 +// minute boot-retry backoff -- so the app does not return within the +// per-restart budget below and the test fails. +// +// IMPORTANT: whether this test actually reproduces the race is timing +// dependent, so it fails only *sometimes*. The race fires only when the +// re-created qemu's QMP becomes reachable *before* the stale handler's +// ~36s stop+quit retry window elapses. On a plain restart the re-create is +// itself largely a race against that window: the teardown spends up to +// ~30s in a QMP-status retry loop (hypervisor Cleanup) before the new qemu +// is even created, so the new instance lands close to the edge of the +// window. Natural variance in that teardown time decides each restart -- +// when it runs short the re-create slips inside the window and the stale +// handler quits the new qemu ("Giving up waiting to connect to QEMU +// Monitor Protocol socket" / "unexpected state HALTED" -> ~10 min +// boot-retry backoff -> this test's per-restart budget expires and it +// fails); when it runs long the handler harmlessly gives up first. The +// restart is repeated many times so that at least one iteration is likely +// to land inside the window. +// +// Note: adding device CPU load does NOT make this more likely -- the retry +// windows are wall-clock sleeps, whereas load only stretches the CPU-bound +// part of the re-create and widens the gap. With the fix in place the test +// always passes (the handler ignores the host-initiated SHUTDOWN). +// +// Because the reproduction is probabilistic, this test is meant to be run +// on demand rather than as part of an unattended suite. A deterministic, +// unit-level test of the handler is a better permanent regression guard. +func TestAppRestart(test *testing.T) { + evetestT := evetest.Init(test) + t := NewGomegaWithT(evetestT) + defer evetest.Close() + + // Define configurable parameters available for the test. + evetest.DefineTestParameters( + evetest.HypervisorParameter(), + ) + + // Get parameter values set for this test execution. + hypervisor := evetest.GetHypervisorParameterValue() + + // Set up the test harness and specify the test prerequisites. + devName := "edge-dev" + requiredDevice := evetest.RequireEdgeDevice{ + Name: devName, + WithHypervisor: hypervisor, + DeviceReusePolicy: evetest.ResetDeviceConfig, + } + requiredNetModel := evetest.RequireNetworkModel{ + NetworkModel: netmodels.SingleEthWithDHCP, + } + evetest.Setup(requiredDevice, requiredNetModel) + device := evetest.GetEdgeDevice(devName) + evetest.Checkpoint("setup-done") + + // Build the device configuration: one mgmt+apps port, one Local NI and + // one container app connected to it. + 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"), + MTU: 1500, + }) + appUUID := devConfig.AddApplication(evetest.ApplicationInstanceConfig{ + DisplayName: "restarted-app", + Activate: true, + Image: evetest.DockerContainer{ + ImageName: "lfedge/evetest-ubuntu-ctr", + Tag: "1.0", + }, + VirtualizationMode: eveconfig.VmMode_HVM, // PV does not work in xen, shim VM fails to start + 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"), + }, + }, + }, + }, + }) + + appUpdates, stopAppWatch := device.WatchAppInfo(appUUID) + defer stopAppWatch() + device.ApplyConfig(devConfig, true, true) + + timeoutExcludingDownload := 5 * time.Minute + device.WaitUntilAppIsRunning(appUUID, timeoutExcludingDownload) + + evetest.Checkpoint("app-deployed") + + // An app reaching RUNNING does not mean it has fully booted -- wait + // until its SSH daemon is reachable through the 2222->22 port-forwarding + // rule before considering the deployment complete. + appAuth := evetest.UsernamePasswordAuth{ + Username: "root", + Password: "testpassword", + } + timeout := 3 * time.Minute + sshTimeout := 20 * time.Second + polling := 3 * time.Second + log := evetest.Logger() + verifyAppOverSSH := func() { + t.Eventually(func(t Gomega) { + log.Infof("Waiting for app SSH daemon to start and become reachable...") + output, _, err := device.RunShellScriptInsideApp(appUUID, appAuth, + "hostname", sshTimeout, 0) + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring(appUUID.String())) + }, timeout, polling).Should(Succeed()) + } + verifyAppOverSSH() + + // Record the baseline boot time; every restart below must advance it. + var appInfo *eveinfo.ZInfoApp + t.Eventually(appUpdates, timeout).Should(Receive(matchers.SatisfyPredicate( + "App is RUNNING and reports its boot time", + func(info *eveinfo.ZInfoApp) bool { + appInfo = info + return info.State == eveinfo.ZSwState_RUNNING && + info.GetBootTime() != nil + }).StopIf(appHasError))) + prevBootTime := appInfo.GetBootTime().AsTime() + + // Restart the application many times in a row. Each restart re-creates + // the domain under the same name and reused QMP socket path; repeating + // it many times increases the chance that natural variance in the + // teardown time dips the re-create below the stale handler's retry + // window and triggers the race (see the test description). + const restartCount = 5 + // A healthy restart completes in well under a minute; only a boot that + // hit the race (and thus the ~10 minute boot-retry backoff) exceeds this + // budget, so a race turns into a prompt failure here rather than a long + // stall. + restartTimeout := 5 * time.Minute + for i := 1; i <= restartCount; i++ { + log.Infof("Restarting app (%d/%d)...", i, restartCount) + device.RebootApplication(appUUID, false, 0) + + t.Eventually(appUpdates, restartTimeout).Should(Receive(matchers.SatisfyPredicate( + fmt.Sprintf("App has restarted (advanced boot time) and is RUNNING (%d/%d)", + i, restartCount), + func(info *eveinfo.ZInfoApp) bool { + appInfo = info + return info.GetBootTime() != nil && + info.GetBootTime().AsTime().After(prevBootTime) && + info.State == eveinfo.ZSwState_RUNNING + }).StopIf(appHasError))) + prevBootTime = appInfo.GetBootTime().AsTime() + + // The restarted app must not just report RUNNING but also be + // functional again. + verifyAppOverSSH() + + evetest.Checkpoint(fmt.Sprintf("app-restarted-%d", i)) + } +} diff --git a/evetest/tests/apps/testsuite_test.go b/evetest/tests/apps/testsuite_test.go index 824822be9f7..80da369eb58 100644 --- a/evetest/tests/apps/testsuite_test.go +++ b/evetest/tests/apps/testsuite_test.go @@ -34,6 +34,9 @@ import ( // version and survives an app restart. // - TestAppLogs -- application stdout is collected and delivered to the // controller, including after the app is stopped and started again. +// - TestAppRestart -- controller-requested restarts (restart counter +// bumps, no purge) bring the app back to RUNNING; regression test for +// a stale QMP handler quitting the re-created domain. func TestAppsSuite(test *testing.T) { evetest.Init(test) defer evetest.Close() @@ -58,5 +61,8 @@ func TestAppsSuite(test *testing.T) { evetest.TestCase{ Test: TestAppLogs, }, + evetest.TestCase{ + Test: TestAppRestart, + }, ) } diff --git a/evetest/tests/networking/nicchange_test.go b/evetest/tests/networking/nicchange_test.go new file mode 100644 index 00000000000..ad55b933a92 --- /dev/null +++ b/evetest/tests/networking/nicchange_test.go @@ -0,0 +1,598 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking_test + +import ( + "fmt" + "net" + "testing" + "time" + + eveconfig "github.com/lf-edge/eve-api/go/config" + "github.com/lf-edge/eve-api/go/evecommon" + eveinfo "github.com/lf-edge/eve-api/go/info" + "github.com/lf-edge/eve/evetest" + "github.com/lf-edge/eve/evetest/matchers" + "github.com/lf-edge/eve/evetest/netmodels" + "github.com/lf-edge/eve/pkg/pillar/types" + "github.com/lf-edge/eve/pkg/pillar/utils/generics" + uuid "github.com/satori/go.uuid" + + // revive:disable:dot-imports + . "github.com/onsi/gomega" +) + +func TestNICCountChangeOrderedInterface(test *testing.T) { + evetestT := evetest.Init(test) + log := evetest.Logger() + t := NewGomegaWithT(evetestT) + defer evetest.Close() + + appAuth := evetest.UsernamePasswordAuth{ + Username: "root", + Password: "testpassword", + } + // Define configurable parameters available for the test. + evetest.DefineTestParameters(evetest.HypervisorParameter()) + + // Get parameter values set for this test execution. + hypervisor := evetest.GetHypervisorParameterValue() + + // Set up the test harness and specify the test prerequisites. + devName := "edge-dev" + requiredDevice := evetest.RequireEdgeDevice{ + Name: devName, + WithHypervisor: hypervisor, + DeviceReusePolicy: evetest.ResetDeviceConfig, + } + requiredNetModel := evetest.RequireNetworkModel{ + NetworkModel: netmodels.SingleEthWithDHCP, + } + evetest.Setup(requiredDevice, requiredNetModel) + devConfig := evetest.NewEdgeDeviceConfig(devName) + evetest.Checkpoint("setup-done") + + dhcpNet := devConfig.AddNetwork( + evetest.DHCPNetworkConfig{ + NetworkType: evecommon.NetworkType_V4Only, + }) + + netAdapters := make([]evetest.AppNetworkAdapter, 0, 3) + for i := range 5 { + netAdapter := addNetwork(i, devConfig, dhcpNet) + ifaceOrder := uint32(i * 10) + netAdapter.InterfaceOrder = &ifaceOrder + netAdapters = append(netAdapters, netAdapter) + } + appConfig := evetest.ApplicationInstanceConfig{ + DisplayName: "container-app", + Activate: true, + Image: evetest.DockerContainer{ + ImageName: "lfedge/evetest-ubuntu-ctr", + Tag: "1.0", + }, + VirtualizationMode: eveconfig.VmMode_HVM, + CPUs: 1, + MemoryBytes: 500 * evetest.MiB, + NetworkAdapters: netAdapters, + EnforceNetIntfOrder: true, + } + appUUID := devConfig.AddApplication(appConfig) + + device := evetest.GetEdgeDevice(devName) + device.ApplyConfig(devConfig, true, true) + + device.WaitUntilAppIsRunning(appUUID, 3*time.Minute) + + t.Eventually(func(t Gomega) { + log.Infof("Waiting for app SSH daemon to start and become reachable...") + stdout, stderr, err := device.RunShellScriptInsideApp(appUUID, appAuth, "ip a", + time.Minute, 0) + t.Expect(err).ToNot(HaveOccurred()) + log.Printf("stdout: \n%s\n", stdout) + log.Printf("stderr: \n%s\n", stderr) + }, 10*time.Minute, 20*time.Second).Should(Succeed()) + + netAdapter := addNetwork(8, devConfig, dhcpNet) + ifaceOrder := uint32(25) + netAdapter.InterfaceOrder = &ifaceOrder + appConfig.NetworkAdapters = append(appConfig.NetworkAdapters, netAdapter) + devConfig.UpdateApplication(appUUID, appConfig) + device.ApplyConfig(devConfig, true, true) + + device.WaitUntilAppIsRunning(appUUID, 5*time.Minute) + + // The added NIC and its position among the others are both properties of + // the same boot, so a single wait covers them: once the guest is back with + // six interfaces, the interface order it enumerated them in is already + // final and needs no further wait of its own. + t.Eventually(func(t Gomega) { + log.Infof("Waiting for the restarted app to come back with the added NIC...") + stdout, stderr, err := device.RunShellScriptInsideApp(appUUID, appAuth, + "ip -o link show", time.Minute, 0) + t.Expect(err).ToNot(HaveOccurred()) + log.Printf("stdout: \n%s\n", stdout) + log.Printf("stderr: \n%s\n", stderr) + t.Expect(stdout).To(ContainSubstring("eth5")) + // The adapter added with interface order 25 falls between the orders + // 20 and 30 of the initial five, so the guest must enumerate it as its + // fourth interface. + t.Expect(stdout).To(MatchRegexp(`eth3:.*` + netAdapter.MAC.String())) + }, 10*time.Minute, 20*time.Second).Should(Succeed()) +} + +func addNetwork(i int, devConfig *evetest.EdgeDeviceConfig, dhcpNet uuid.UUID) evetest.VirtualNetworkAdapter { + mac := net.HardwareAddr{0x2, 0x16, 0x3e, 0x00, 0x00, 0x1 + byte(i)} + gateway := net.IP{10, 11, 12 + byte(i), 1} + dhcpRange := types.IPRange{ + Start: net.IP{10, 11, 12 + byte(i), 2}, + End: net.IP{10, 11, 12 + byte(i), 254}, + } + subnet := net.IPNet{ + IP: net.IP{10, 11, 12 + byte(i), 0}, + Mask: net.IPMask{255, 255, 255, 0}, + } + devConfig.AddNetworkAdapter( + evetest.NetworkAdapterConfig{ + LogicalLabel: fmt.Sprintf("ethernet%d", i), + PhysicalLabel: fmt.Sprintf("eth%d", i), + InterfaceName: fmt.Sprintf("eth%d", i), + NetworkUUID: dhcpNet, + Usage: evecommon.PhyIoMemberUsage_PhyIoUsageMgmtAndApps, + }) + + niuuid := devConfig.AddNetworkInstance(evetest.LocalNetworkInstanceConfig{ + DisplayName: fmt.Sprintf("local-ni%d", i), + Port: fmt.Sprintf("ethernet%d", i), + Subnet: &subnet, + DHCPRange: dhcpRange, + Gateway: gateway, + MTU: 1500, + ForwardLLDP: false, + }) + + interfaceOrder := uint32(i) + netAdapter := evetest.VirtualNetworkAdapter{ + LogicalLabel: fmt.Sprintf("vif%d", i), + NetworkInstanceUUID: niuuid, + MAC: mac, + PortFwdRules: []evetest.PortFwdRule{ + { + Protocol: evetest.NetworkProtocolTCP, + EdgeNodePort: 2222 + uint16(i), + AppPort: 22, + }, + }, + ACLAllowRules: []evetest.ACLAllowRule{ + { + Protocol: evetest.NetworkProtocolAny, + RemoteSubnet: evetest.IPSubnet("0.0.0.0/0"), + }, + }, + InterfaceOrder: &interfaceOrder, + } + + return netAdapter +} + +// TestNICCountChange exercises changing the set of network adapters of a +// running application through restarts (no purge): a NIC is added, the two +// NICs are swapped, the device is rebooted and one NIC is removed again, +// with the app disk preserved throughout. +// +// Network model: SingleEthWithDHCP -- a single management port is all that +// is needed. +// +// Device configuration: one Local NI at first (a second one is added in +// phase 2); a container app starting with one virtual adapter (pinned MAC, +// SSH port forwarding, allow-all ACLs). +// +// Phases: +// 1. Deploy the app with one NIC; wait until its IP address is reported in +// both the app info and the NI status, verify it is reachable over SSH +// and write a file to the app disk (purge canary). +// 2. Add a second Local NI with a second app NIC (the restart counter is +// bumped by UpdateApplication); the guest must see both NICs, both NICs +// must be reported with an IP address and the canary must survive. +// 3. Swap the two adapters in the configuration; the guest's eth0 must +// switch from the first to the second adapter's MAC address. +// 4. Reboot the device; the app must come back with both NICs. +// 5. Remove one adapter (after the swap this is the adapter with the first +// MAC); the app must come back with a single NIC and the canary intact. +// 6. Cleanup: remove the application. +func TestNICCountChange(test *testing.T) { + evetestT := evetest.Init(test) + t := NewGomegaWithT(evetestT) + defer evetest.Close() + + // Define configurable parameters available for the test. + evetest.DefineTestParameters(evetest.HypervisorParameter()) + + // Get parameter values set for this test execution. + hypervisor := evetest.GetHypervisorParameterValue() + + // Set up the test harness and specify the test prerequisites. + devName := "edge-dev" + requiredDevice := evetest.RequireEdgeDevice{ + Name: devName, + WithHypervisor: hypervisor, + DeviceReusePolicy: evetest.ResetDeviceConfig, + } + requiredNetModel := evetest.RequireNetworkModel{ + NetworkModel: netmodels.SingleEthWithDHCP, + } + evetest.Setup(requiredDevice, requiredNetModel) + tc := newNICCountChangeTest(t, devName) + evetest.Checkpoint("setup-done") + + tc.deployAppWithOneNIC() + evetest.Checkpoint("ni-with-app-created") + tc.recordBaseline() + + tc.addSecondNIC() + evetest.Checkpoint("adding another NI to app") + tc.verifyGuestSeesBothNICs() + evetest.Checkpoint("app has two interfaces") + tc.verifyBothNICsReportedWithIP() + evetest.Checkpoint("both NICs reported with IP") + tc.verifyCanary() + evetest.Checkpoint("app disk has not been purged") + + tc.swapNICs() + tc.rebootDeviceAndVerify() + + tc.removeSecondNIC() + evetest.Checkpoint("removing second NI from app") + tc.waitAppBackWithOneNIC() + evetest.Checkpoint("app is back") + tc.verifyGuestSeesOneNIC() + tc.verifyCanary() + evetest.Checkpoint("app disk has not been purged") + + tc.cleanup() +} + +// nicCountChangeTest carries the state shared between the phases of +// TestNICCountChange. +type nicCountChangeTest struct { + t *WithT + device *evetest.EdgeDevice + devConfig *evetest.EdgeDeviceConfig + appConfig evetest.ApplicationInstanceConfig + appUUID uuid.UUID + ni1UUID uuid.UUID + ni2UUID uuid.UUID + appAuth evetest.UsernamePasswordAuth + + timeout time.Duration + sshTimeout time.Duration + polling time.Duration + + appMACs []string // vif0 (ni1), vif1 (ni2, added in phase 2) + appIP net.IP + + appUpdates <-chan *eveinfo.ZInfoApp + stopAppWatch func() + niUpdates <-chan *eveinfo.ZInfoNetworkInstance + stopNIWatch func() +} + +// newNICCountChangeTest builds the base device configuration (network and +// uplink adapter only -- the network instances and the application are added +// by the phases). +func newNICCountChangeTest(t *WithT, devName string) *nicCountChangeTest { + tc := &nicCountChangeTest{ + t: t, + device: evetest.GetEdgeDevice(devName), + appAuth: evetest.UsernamePasswordAuth{ + Username: "root", + Password: "testpassword", + }, + timeout: 5 * time.Minute, + sshTimeout: 20 * time.Second, + polling: 3 * time.Second, + appMACs: []string{ + "02:16:3e:00:00:01", // vif0 (ni1) + "02:16:3e:00:00:02", // vif1 (ni2), added in phase 2 + }, + } + tc.devConfig = evetest.NewEdgeDeviceConfig(devName) + dhcpNet := tc.devConfig.AddNetwork( + evetest.DHCPNetworkConfig{ + NetworkType: evecommon.NetworkType_V4Only, + }) + tc.devConfig.AddNetworkAdapter( + evetest.NetworkAdapterConfig{ + LogicalLabel: "ethernet0", + PhysicalLabel: "eth0", + InterfaceName: "eth0", + NetworkUUID: dhcpNet, + Usage: evecommon.PhyIoMemberUsage_PhyIoUsageMgmtAndApps, + }) + return tc +} + +// runInApp executes a shell script inside the application over SSH. +func (tc *nicCountChangeTest) runInApp(script string) (stdout, stderr string, err error) { + return tc.device.RunShellScriptInsideApp(tc.appUUID, tc.appAuth, script, + tc.sshTimeout, 0) +} + +// deployAppWithOneNIC (phase 1) applies the base device configuration, adds +// the first Local NI with the app connected to it and waits until the app +// is RUNNING. +func (tc *nicCountChangeTest) deployAppWithOneNIC() { + // Apply the initial device configuration, without including any network + // instances for now. + tc.device.ApplyConfig(tc.devConfig, true, true) + + // Create NI with an app connected to it. + tc.ni1UUID = tc.devConfig.AddNetworkInstance(evetest.LocalNetworkInstanceConfig{ + DisplayName: "local-ni1", + 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"), + MTU: 1500, + ForwardLLDP: false, + }) + tc.appConfig = evetest.ApplicationInstanceConfig{ + DisplayName: "container-app", + 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: tc.ni1UUID, + MAC: evetest.MACAddress(tc.appMACs[0]), + PortFwdRules: []evetest.PortFwdRule{ + { + Protocol: evetest.NetworkProtocolTCP, + EdgeNodePort: 2222, + AppPort: 22, + }, + }, + ACLAllowRules: []evetest.ACLAllowRule{ + { + Protocol: evetest.NetworkProtocolAny, + RemoteSubnet: evetest.IPSubnet("0.0.0.0/0"), + }, + }, + }, + }, + } + tc.appUUID = tc.devConfig.AddApplication(tc.appConfig) + + tc.niUpdates, tc.stopNIWatch = tc.device.WatchNetworkInstanceInfo(tc.ni1UUID) + tc.appUpdates, tc.stopAppWatch = tc.device.WatchAppInfo(tc.appUUID) + tc.device.ApplyConfig(tc.devConfig, true, true) + + tc.device.WaitUntilAppIsRunning(tc.appUUID, tc.timeout) +} + +// recordBaseline (phase 1) waits until the app's IP address is reported in +// both the app info and the NI status, verifies the app is reachable over +// SSH (port forwarding) and writes the purge canary to the app disk. +func (tc *nicCountChangeTest) recordBaseline() { + log := evetest.Logger() + + // Wait until application receives IP address from the NI subnet. + var appInfo *eveinfo.ZInfoApp + tc.t.Eventually(tc.appUpdates, tc.timeout).Should(Receive(matchers.SatisfyPredicate( + "App receives IP address", + func(info *eveinfo.ZInfoApp) bool { + appInfo = info + return len(appInfo.Network) == 1 && len(appInfo.Network[0].IPAddrs) == 1 + }).StopIf(appHasError))) + tc.appIP = evetest.IPAddress(appInfo.Network[0].IPAddrs[0]) + tc.stopAppWatch() + + // Confirm that application IP address is (eventually) reported in the + // network instance status. + tc.t.Eventually(tc.niUpdates, tc.timeout).Should(Receive(matchers.SatisfyPredicate( + "App IP is reported inside the NI status", + func(info *eveinfo.ZInfoNetworkInstance) bool { + niInfo := info + if len(niInfo.Vifs) == 0 || len(niInfo.IpAssignments) == 0 { + return false + } + for _, ipAssignment := range niInfo.IpAssignments { + if ipAssignment.MacAddress == tc.appMACs[0] { + return generics.ContainsItem(ipAssignment.IpAddress, tc.appIP.String()) + } + } + return false + }).StopIf(niHasError))) + tc.stopNIWatch() + + log.Infof("Testing port forwarding") + tc.t.Eventually(func(t Gomega) { + log.Infof("Waiting for app SSH daemon to start and become reachable...") + output, _, err := tc.runInApp("ip a") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring("eth0")) + t.Expect(output).To(ContainSubstring(tc.appMACs[0])) + }, tc.timeout, tc.polling).Should(Succeed()) + tc.t.Eventually(func(t Gomega) { + log.Infof("Writing something to the disk to ensure it is not purged") + _, stderrOutput, err := tc.runInApp("echo -n foo > ~/foo.txt") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(stderrOutput).To(BeEmpty()) + }, tc.timeout, tc.polling).Should(Succeed()) +} + +// addSecondNIC (phase 2) adds a second Local NI with a second app NIC. +// UpdateApplication bumps the restart counter, so the device applies the +// change by restarting the application. +func (tc *nicCountChangeTest) addSecondNIC() { + tc.ni2UUID = tc.devConfig.AddNetworkInstance(evetest.LocalNetworkInstanceConfig{ + DisplayName: "local-ni2", + 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"), + }, + }) + tc.appConfig.NetworkAdapters = append(tc.appConfig.NetworkAdapters, + evetest.VirtualNetworkAdapter{ + LogicalLabel: "vif1", + NetworkInstanceUUID: tc.ni2UUID, + MAC: evetest.MACAddress(tc.appMACs[1]), + PortFwdRules: []evetest.PortFwdRule{ + { + Protocol: evetest.NetworkProtocolTCP, + EdgeNodePort: 2224, + AppPort: 22, + }, + }, + ACLAllowRules: []evetest.ACLAllowRule{ + { + Protocol: evetest.NetworkProtocolAny, + RemoteSubnet: evetest.IPSubnet("0.0.0.0/0"), + }, + }, + }) + tc.devConfig.UpdateApplication(tc.appUUID, tc.appConfig) + tc.appUpdates, tc.stopAppWatch = tc.device.WatchAppInfo(tc.appUUID) + tc.device.ApplyConfig(tc.devConfig, true, true) +} + +// verifyGuestSeesBothNICs (phase 2) waits until the restarted app is +// reachable again and the guest sees both NICs. +func (tc *nicCountChangeTest) verifyGuestSeesBothNICs() { + log := evetest.Logger() + tc.t.Eventually(func(t Gomega) { + log.Infof("Waiting for app SSH daemon to start and become reachable after adding another NI...") + output, _, err := tc.runInApp("ip a") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring("eth1")) + t.Expect(output).To(ContainSubstring(tc.appMACs[0])) + t.Expect(output).To(ContainSubstring(tc.appMACs[1])) + }, tc.timeout, tc.polling).Should(Succeed()) +} + +// verifyBothNICsReportedWithIP (phase 2) verifies that both NICs are +// reported to the controller with an IP address. +func (tc *nicCountChangeTest) verifyBothNICsReportedWithIP() { + tc.t.Eventually(tc.appUpdates, tc.timeout).Should(Receive(matchers.SatisfyPredicate( + "Both NICs are reported with an IP address", + func(info *eveinfo.ZInfoApp) bool { + return reportedIPsForMACs(info, tc.appMACs) + }).StopIf(appHasError))) + tc.stopAppWatch() +} + +// verifyCanary (phases 2 and 5) verifies the file written in phase 1 is +// still on the app disk, i.e. the app volume was not purged. +func (tc *nicCountChangeTest) verifyCanary() { + log := evetest.Logger() + tc.t.Eventually(func(t Gomega) { + log.Infof("Reading from disk to check it has not been purged") + output, stderrOutput, err := tc.runInApp("cat ~/foo.txt") + t.Expect(stderrOutput).To(BeEmpty()) + t.Expect(output).To(BeEquivalentTo("foo")) + t.Expect(err).ToNot(HaveOccurred()) + }, tc.timeout, tc.polling).Should(Succeed()) +} + +// swapNICs (phase 3) swaps the two adapters in the configuration (applied +// via another restart) and verifies that the guest's eth0 switches from the +// first to the second adapter's MAC address. +func (tc *nicCountChangeTest) swapNICs() { + log := evetest.Logger() + tc.t.Eventually(func(t Gomega) { + log.Infof("Waiting for app SSH daemon to start and become reachable...") + output, _, err := tc.runInApp("ip a show dev eth0") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring("eth0")) + t.Expect(output).To(ContainSubstring(tc.appMACs[0])) + }, tc.timeout, tc.polling).Should(Succeed()) + + na := tc.appConfig.NetworkAdapters[0] + tc.appConfig.NetworkAdapters[0] = tc.appConfig.NetworkAdapters[1] + tc.appConfig.NetworkAdapters[1] = na + tc.devConfig.UpdateApplication(tc.appUUID, tc.appConfig) + tc.device.ApplyConfig(tc.devConfig, true, true) + + tc.t.Eventually(func(t Gomega) { + log.Infof("Waiting for app SSH daemon to start and become reachable after NI swap...") + output, _, err := tc.runInApp("ip a show dev eth0") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring("eth0")) + t.Expect(output).To(ContainSubstring(tc.appMACs[1])) + }, tc.timeout, tc.polling).Should(Succeed()) +} + +// rebootDeviceAndVerify (phase 4) reboots the device and verifies the app +// comes back with both NICs. +func (tc *nicCountChangeTest) rebootDeviceAndVerify() { + log := evetest.Logger() + tc.device.RequestReboot(true) + tc.t.Eventually(func(t Gomega) { + log.Infof("Waiting for app SSH daemon to start and become reachable after reboot...") + output, _, err := tc.runInApp("ip a") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring("eth0")) + t.Expect(output).To(ContainSubstring("eth1")) + t.Expect(output).To(ContainSubstring(tc.appMACs[0])) + t.Expect(output).To(ContainSubstring(tc.appMACs[1])) + }, tc.timeout, tc.polling).Should(Succeed()) +} + +// removeSecondNIC (phase 5) removes one adapter from the configuration +// (after the swap in phase 3 this is the adapter with the first MAC), +// applied via another restart. +func (tc *nicCountChangeTest) removeSecondNIC() { + log := evetest.Logger() + tc.appConfig.NetworkAdapters = tc.appConfig.NetworkAdapters[:1] + log.Infof("remaining network adapter: %+v", tc.appConfig.NetworkAdapters) + tc.devConfig.UpdateApplication(tc.appUUID, tc.appConfig) + tc.appUpdates, tc.stopAppWatch = tc.device.WatchAppInfo(tc.appUUID) + tc.device.ApplyConfig(tc.devConfig, true, true) +} + +// waitAppBackWithOneNIC (phase 5) waits until the app is reported with a +// single NIC again. +func (tc *nicCountChangeTest) waitAppBackWithOneNIC() { + var appInfo *eveinfo.ZInfoApp + tc.t.Eventually(tc.appUpdates, tc.timeout).Should(Receive(matchers.SatisfyPredicate( + "App receives IP address", + func(info *eveinfo.ZInfoApp) bool { + appInfo = info + return len(appInfo.Network) == 1 && len(appInfo.Network[0].IPAddrs) == 1 + }).StopIf(appHasError))) + tc.appIP = evetest.IPAddress(appInfo.Network[0].IPAddrs[0]) + tc.stopAppWatch() +} + +// verifyGuestSeesOneNIC (phase 5) verifies the guest is left with only the +// remaining adapter's NIC. +func (tc *nicCountChangeTest) verifyGuestSeesOneNIC() { + log := evetest.Logger() + tc.t.Eventually(func(t Gomega) { + log.Infof("Waiting for app SSH daemon to start and become reachable...") + output, _, err := tc.runInApp("ip a") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring("eth0")) + t.Expect(output).ToNot(ContainSubstring("eth1")) + t.Expect(output).To(ContainSubstring(tc.appMACs[1])) + t.Expect(output).ToNot(ContainSubstring(tc.appMACs[0])) + }, tc.timeout, tc.polling).Should(Succeed()) +} + +// cleanup (phase 6) removes the application. +func (tc *nicCountChangeTest) cleanup() { + tc.devConfig.DeleteApplication(tc.appUUID) + tc.device.ApplyConfig(tc.devConfig, false, false) +} diff --git a/evetest/tests/networking/staged_nicchange_test.go b/evetest/tests/networking/staged_nicchange_test.go new file mode 100644 index 00000000000..887fd49b45c --- /dev/null +++ b/evetest/tests/networking/staged_nicchange_test.go @@ -0,0 +1,525 @@ +// Copyright (c) 2026 Zededa, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking_test + +import ( + "net" + "strings" + "testing" + "time" + + eveconfig "github.com/lf-edge/eve-api/go/config" + "github.com/lf-edge/eve-api/go/evecommon" + eveinfo "github.com/lf-edge/eve-api/go/info" + "github.com/lf-edge/eve/evetest" + "github.com/lf-edge/eve/evetest/matchers" + "github.com/lf-edge/eve/evetest/netmodels" + "github.com/lf-edge/eve/pkg/pillar/types" + uuid "github.com/satori/go.uuid" + + // revive:disable:dot-imports + . "github.com/onsi/gomega" +) + +// TestStagedNICChange verifies that a change to the set of network adapters +// of a running application is only *staged* by the device: no part of the +// new adapter set may take effect -- neither in the reported device state +// nor inside the guest -- until the application is restarted, regardless of +// what other configuration is reconciled in the meantime. +// +// Network model: SingleEthWithDHCP -- a single management port is all that +// is needed; the test is about application NIC staging, not uplink topology. +// +// Device configuration: two Local NIs on the single uplink; app1 with two +// virtual adapters (vif0 on ni1, vif1 on ni2, both with pinned MACs, SSH +// port forwarding and allow-all ACLs); later app2 with one adapter on ni2 +// (shared with app1's vif1, so deploying it reprograms the very bridge and +// iptables state that app1's staged change also touches). +// +// Phases: +// 1. Deploy app1, wait until it is RUNNING with both NICs reported with an +// IP address and reachable over SSH; record the boot time and the name +// of app1's VIF on ni2 as the baseline. +// 2. Stage a NIC addition: add vif2 (on ni2, pinned MAC, with an SSH +// port-forwarding rule) to app1 *without* bumping the restart counter, +// by reverting the counter bump made by UpdateApplication directly in +// the device configuration. Wait until the device confirms it has +// processed the new config. +// 3. Soak: repeatedly assert that nothing has changed -- app1 still +// RUNNING with the baseline boot time (no restart), still exactly two +// NICs in the reported app info, app1's VIF set on ni2 unchanged (the +// same single VIF with the baseline name), the guest still sees only +// the two original MACs, and the ACL state is untouched: the two +// original port-forwarding rules keep accepting connections while the +// staged one must not accept any yet. +// 4. Stir reconciliation: deploy app2 on ni2 and wait until it is RUNNING +// and reachable over SSH (proving zedrouter reprogrammed the shared NI +// for the new app). Then repeat the phase-3 soak for app1. +// 5. Restart app1 (restart-counter bump). The staged adapter must now take +// effect: the boot time advances, the guest sees all three MACs with +// an IPv4 address on each NIC and the staged port-forwarding rule +// starts accepting connections. (Volume preservation across such a +// restart is already covered by TestNICCountChange.) +// 6. Cleanup: remove both applications. +func TestStagedNICChange(test *testing.T) { + evetestT := evetest.Init(test) + t := NewGomegaWithT(evetestT) + defer evetest.Close() + + // Define configurable parameters available for the test. + evetest.DefineTestParameters(evetest.HypervisorParameter()) + + // Get parameter values set for this test execution. + hypervisor := evetest.GetHypervisorParameterValue() + + // Set up the test harness and specify the test prerequisites. + devName := "edge-dev" + requiredDevice := evetest.RequireEdgeDevice{ + Name: devName, + WithHypervisor: hypervisor, + DeviceReusePolicy: evetest.ResetDeviceConfig, + } + requiredNetModel := evetest.RequireNetworkModel{ + NetworkModel: netmodels.SingleEthWithDHCP, + } + evetest.Setup(requiredDevice, requiredNetModel) + tc := newStagedNICChangeTest(t, devName) + evetest.Checkpoint("setup-done") + + tc.deployApp1() + evetest.Checkpoint("app1-deployed") + tc.recordBaseline() + evetest.Checkpoint("baseline-recorded") + + tc.stageNICAddition() + evetest.Checkpoint("nic-add-staged") + tc.assertNothingAppliedEarly("right after staging") + evetest.Checkpoint("staged-change-inert") + + tc.deployUnrelatedApp() + evetest.Checkpoint("unrelated-app-deployed") + tc.assertNothingAppliedEarly("after deploying another app on the shared NI") + evetest.Checkpoint("staged-change-still-inert") + + tc.restartApp1() + evetest.Checkpoint("app1-restarted") + tc.verifyStagedNICApplied() + evetest.Checkpoint("staged-change-applied-on-restart") + + tc.cleanup() +} + +// stagedNICChangeTest carries the state shared between the phases of +// TestStagedNICChange. +type stagedNICChangeTest struct { + t *WithT + device *evetest.EdgeDevice + devConfig *evetest.EdgeDeviceConfig + app1Config evetest.ApplicationInstanceConfig + app1UUID uuid.UUID + app2UUID uuid.UUID + ni2UUID uuid.UUID + appAuth evetest.UsernamePasswordAuth + + timeout time.Duration + sshTimeout time.Duration + polling time.Duration + soak time.Duration + soakPolling time.Duration + + app1MACs []string // vif0 (ni1), vif1 (ni2), vif2 (ni2, staged) + app2MAC string + allowAll []evetest.ACLAllowRule + + baselineBootTime time.Time + baselineVifName string +} + +// newStagedNICChangeTest builds the base device configuration (uplink, two +// Local NIs) and the initial two-NIC configuration of app1. +func newStagedNICChangeTest(t *WithT, devName string) *stagedNICChangeTest { + tc := &stagedNICChangeTest{ + t: t, + device: evetest.GetEdgeDevice(devName), + appAuth: evetest.UsernamePasswordAuth{ + Username: "root", + Password: "testpassword", + }, + timeout: 5 * time.Minute, + sshTimeout: 20 * time.Second, + polling: 3 * time.Second, + // The soak must span at least one info-publish interval so that the + // repeated assertions run against state reported after the staging. + soak: 90 * time.Second, + soakPolling: 10 * time.Second, + app1MACs: []string{ + "02:16:3e:00:00:01", // vif0 (ni1) + "02:16:3e:00:00:02", // vif1 (ni2) + "02:16:3e:00:00:03", // vif2 (ni2), staged in phase 2 + }, + app2MAC: "02:16:3e:00:00:04", + allowAll: []evetest.ACLAllowRule{ + { + Protocol: evetest.NetworkProtocolAny, + RemoteSubnet: evetest.IPSubnet("0.0.0.0/0"), + }, + }, + } + + tc.devConfig = evetest.NewEdgeDeviceConfig(devName) + dhcpNet := tc.devConfig.AddNetwork( + evetest.DHCPNetworkConfig{ + NetworkType: evecommon.NetworkType_V4Only, + }) + tc.devConfig.AddNetworkAdapter( + evetest.NetworkAdapterConfig{ + LogicalLabel: "ethernet0", + PhysicalLabel: "eth0", + InterfaceName: "eth0", + NetworkUUID: dhcpNet, + Usage: evecommon.PhyIoMemberUsage_PhyIoUsageMgmtAndApps, + }) + ni1UUID := tc.devConfig.AddNetworkInstance(evetest.LocalNetworkInstanceConfig{ + DisplayName: "local-ni1", + 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"), + MTU: 1500, + }) + tc.ni2UUID = tc.devConfig.AddNetworkInstance(evetest.LocalNetworkInstanceConfig{ + DisplayName: "local-ni2", + 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"), + MTU: 1500, + }) + + tc.app1Config = evetest.ApplicationInstanceConfig{ + DisplayName: "staged-app", + 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: ni1UUID, + MAC: evetest.MACAddress(tc.app1MACs[0]), + PortFwdRules: []evetest.PortFwdRule{ + { + Protocol: evetest.NetworkProtocolTCP, + EdgeNodePort: 2222, + AppPort: 22, + }, + }, + ACLAllowRules: tc.allowAll, + }, + evetest.VirtualNetworkAdapter{ + LogicalLabel: "vif1", + NetworkInstanceUUID: tc.ni2UUID, + MAC: evetest.MACAddress(tc.app1MACs[1]), + PortFwdRules: []evetest.PortFwdRule{ + { + Protocol: evetest.NetworkProtocolTCP, + EdgeNodePort: 2224, + AppPort: 22, + }, + }, + ACLAllowRules: tc.allowAll, + }, + }, + } + return tc +} + +// runInApp executes a shell script inside the given application over SSH. +func (tc *stagedNICChangeTest) runInApp(appUUID uuid.UUID, + script string) (stdout, stderr string, err error) { + return tc.device.RunShellScriptInsideApp(appUUID, tc.appAuth, script, + tc.sshTimeout, 0) +} + +// deployApp1 (phase 1) deploys app1 with its two initial NICs and waits +// until it is RUNNING with both NICs reported with an IP address. The boot +// time of this first boot becomes the baseline that the staged phases must +// not disturb. +func (tc *stagedNICChangeTest) deployApp1() { + tc.app1UUID = tc.devConfig.AddApplication(tc.app1Config) + appUpdates, stopAppWatch := tc.device.WatchAppInfo(tc.app1UUID) + defer stopAppWatch() + tc.device.ApplyConfig(tc.devConfig, true, true) + tc.device.WaitUntilAppIsRunning(tc.app1UUID, tc.timeout) + + var appInfo *eveinfo.ZInfoApp + tc.t.Eventually(appUpdates, tc.timeout).Should(Receive(matchers.SatisfyPredicate( + "App is RUNNING with both NICs reported and boot time known", + func(info *eveinfo.ZInfoApp) bool { + appInfo = info + if info.State != eveinfo.ZSwState_RUNNING || info.GetBootTime() == nil { + return false + } + return reportedIPsForMACs(info, tc.app1MACs[:2]) + }).StopIf(appHasError))) + tc.baselineBootTime = appInfo.GetBootTime().AsTime() +} + +// recordBaseline (phase 1) verifies app1 is reachable over SSH and records +// the name of app1's VIF on ni2, which must not change while the NIC +// addition is staged. +func (tc *stagedNICChangeTest) recordBaseline() { + log := evetest.Logger() + tc.t.Eventually(func(t Gomega) { + log.Infof("Waiting for app1 SSH daemon to become reachable...") + output, _, err := tc.runInApp(tc.app1UUID, "ip a") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring(tc.app1MACs[0])) + t.Expect(output).To(ContainSubstring(tc.app1MACs[1])) + }, tc.timeout, tc.polling).Should(Succeed()) + tc.t.Eventually(func(t Gomega) { + var vifCount int + tc.baselineVifName, vifCount = tc.app1VifsOnNI2() + t.Expect(vifCount).To(Equal(1)) + t.Expect(tc.baselineVifName).ToNot(BeEmpty()) + }, tc.timeout, tc.polling).Should(Succeed()) +} + +// app1VifsOnNI2 returns the name of app1's VIF with the vif1 MAC address as +// reported by ni2, together with the total count of app1 VIFs on ni2. +func (tc *stagedNICChangeTest) app1VifsOnNI2() (vifName string, count int) { + niInfo := tc.device.GetNetworkInstanceInfo(tc.ni2UUID) + if niInfo == nil { + return "", 0 + } + for _, vif := range niInfo.Vifs { + if vif.AppID != tc.app1UUID.String() { + continue + } + count++ + if vif.MacAddress == tc.app1MACs[1] { + vifName = vif.VifName + } + } + return vifName, count +} + +// stageNICAddition (phase 2) adds vif2 to app1 in the controller +// configuration without a restart command, so that the device can only +// stage the change. +func (tc *stagedNICChangeTest) stageNICAddition() { + tc.app1Config.NetworkAdapters = append(tc.app1Config.NetworkAdapters, + evetest.VirtualNetworkAdapter{ + LogicalLabel: "vif2", + NetworkInstanceUUID: tc.ni2UUID, + MAC: evetest.MACAddress(tc.app1MACs[2]), + // The port-forwarding rule makes the staged ACL state externally + // observable: the DNAT rule for port 2228 must not be programmed + // while the change is staged and must start working after the + // restart. + PortFwdRules: []evetest.PortFwdRule{ + { + Protocol: evetest.NetworkProtocolTCP, + EdgeNodePort: 2228, + AppPort: 22, + }, + }, + ACLAllowRules: tc.allowAll, + }) + // UpdateApplication bumps the restart counter whenever the adapter set + // changes; revert the bump directly in the device configuration so that + // the NIC addition reaches the device without a restart command and + // therefore stays staged. + var app1Proto *eveconfig.AppInstanceConfig + for _, app := range tc.devConfig.Apps { + if app.Uuidandversion.Uuid == tc.app1UUID.String() { + app1Proto = app + break + } + } + tc.t.Expect(app1Proto).ToNot(BeNil()) + restartCounterBefore := app1Proto.GetRestart().GetCounter() + tc.devConfig.UpdateApplication(tc.app1UUID, tc.app1Config) + app1Proto.Restart.Counter = restartCounterBefore + // waitUntilConfirmed=true: return only after the device reports the new + // config as processed by zedagent, so the soaks that follow run against + // a device that already holds the staged change. + tc.device.ApplyConfig(tc.devConfig, true, true) +} + +// assertNothingAppliedEarly (phases 3 and 4) asserts repeatedly, over a +// period longer than the info-publish interval, that neither the reported +// device state nor the guest shows any trace of the staged adapter. +func (tc *stagedNICChangeTest) assertNothingAppliedEarly(phase string) { + log := evetest.Logger() + log.Infof("Asserting the staged NIC change has no effect (%s)...", phase) + tc.t.Consistently(func(t Gomega) { + info := tc.device.GetAppInfo(tc.app1UUID) + t.Expect(info).ToNot(BeNil()) + t.Expect(info.State).To(Equal(eveinfo.ZSwState_RUNNING), + "app must stay RUNNING while the change is staged") + t.Expect(info.GetBootTime()).ToNot(BeNil()) + t.Expect(info.GetBootTime().AsTime().Equal(tc.baselineBootTime)).To(BeTrue(), + "boot time must not advance while the change is staged") + t.Expect(info.Network).To(HaveLen(2), + "only the two original NICs may be reported while the change is staged") + vifName, vifCount := tc.app1VifsOnNI2() + t.Expect(vifCount).To(Equal(1), + "no additional app1 VIF may appear on ni2 while the change is staged") + t.Expect(vifName).To(Equal(tc.baselineVifName), + "app1's VIF on ni2 must keep its name while the change is staged") + output, _, err := tc.runInApp(tc.app1UUID, "ip a") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring(tc.app1MACs[0])) + t.Expect(output).To(ContainSubstring(tc.app1MACs[1])) + t.Expect(output).ToNot(ContainSubstring(tc.app1MACs[2]), + "the staged NIC must not appear inside the guest") + t.Expect(tc.devicePortReachable("2222")).To(BeTrue(), + "port forwarding of an existing adapter must keep working "+ + "while the change is staged") + t.Expect(tc.devicePortReachable("2224")).To(BeTrue(), + "port forwarding of an existing adapter must keep working "+ + "while the change is staged") + t.Expect(tc.devicePortReachable("2228")).To(BeFalse(), + "the staged NIC's port-forwarding ACL must not be programmed "+ + "while the change is staged") + }, tc.soak, tc.soakPolling).Should(Succeed()) +} + +// devicePortReachable reports whether a TCP connection can be established +// to the given port on any of the device's uplink IP addresses. +func (tc *stagedNICChangeTest) devicePortReachable(port string) bool { + for _, ip := range tc.device.GetDeviceIPAddress("ethernet0") { + conn, err := net.DialTimeout("tcp", + net.JoinHostPort(ip.String(), port), 3*time.Second) + if err == nil { + err := conn.Close() + if err != nil { + panic(err) + } + return true + } + } + return false +} + +// deployUnrelatedApp (phase 4) deploys app2 on the NI shared with app1's +// vif1 to force zedrouter to reconcile the very bridge/iptables state that +// app1's staged change also touches. +func (tc *stagedNICChangeTest) deployUnrelatedApp() { + log := evetest.Logger() + app2Config := evetest.ApplicationInstanceConfig{ + DisplayName: "reconciler-app", + 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: tc.ni2UUID, + MAC: evetest.MACAddress(tc.app2MAC), + PortFwdRules: []evetest.PortFwdRule{ + { + Protocol: evetest.NetworkProtocolTCP, + EdgeNodePort: 2226, + AppPort: 22, + }, + }, + ACLAllowRules: tc.allowAll, + }, + }, + } + tc.app2UUID = tc.devConfig.AddApplication(app2Config) + tc.device.ApplyConfig(tc.devConfig, true, true) + tc.device.WaitUntilAppIsRunning(tc.app2UUID, tc.timeout) + // app2 being reachable over SSH proves zedrouter reprogrammed the shared + // NI (DNAT and filtering rules) while app1's change was staged. + tc.t.Eventually(func(t Gomega) { + log.Infof("Waiting for app2 SSH daemon to become reachable...") + _, _, err := tc.runInApp(tc.app2UUID, "true") + t.Expect(err).ToNot(HaveOccurred()) + }, tc.timeout, tc.polling).Should(Succeed()) +} + +// restartApp1 (phase 5) restarts app1 via a restart-counter bump and waits +// until it is RUNNING again with an advanced boot time. +func (tc *stagedNICChangeTest) restartApp1() { + appUpdates, stopAppWatch := tc.device.WatchAppInfo(tc.app1UUID) + defer stopAppWatch() + tc.device.RebootApplication(tc.app1UUID, false, 0) + tc.t.Eventually(appUpdates, tc.timeout).Should(Receive(matchers.SatisfyPredicate( + "App has restarted (advanced boot time) and is RUNNING", + func(info *eveinfo.ZInfoApp) bool { + return info.GetBootTime() != nil && + info.GetBootTime().AsTime().After(tc.baselineBootTime) && + info.State == eveinfo.ZSwState_RUNNING + }).StopIf(appHasError))) +} + +// verifyStagedNICApplied (phase 5) verifies that the restart applied the +// staged adapter: the guest must see all three MACs with an IPv4 address +// on each NIC. +// +// Note: the reported (controller-visible) IP address of the newly added NIC +// is deliberately not asserted here: zedrouter currently updates the +// state-collecting machinery before publishing the new AppNetworkStatus, so +// the new NIC's IP is not attributed until that ordering is fixed. Extend +// the assertion to reportedIPsForMACs(info, tc.app1MACs) once it is. +func (tc *stagedNICChangeTest) verifyStagedNICApplied() { + log := evetest.Logger() + tc.t.Eventually(func(t Gomega) { + log.Infof("Waiting for app1 to come back with all three NICs...") + output, _, err := tc.runInApp(tc.app1UUID, "ip a") + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(output).To(ContainSubstring(tc.app1MACs[0])) + t.Expect(output).To(ContainSubstring(tc.app1MACs[1])) + t.Expect(output).To(ContainSubstring(tc.app1MACs[2]), + "the restart must apply the staged NIC") + // Each NIC must obtain an IPv4 address from its NI subnet + // (10.11.12.0/24 or 10.11.13.0/24). + t.Expect(strings.Count(output, "inet 10.11.")).To(BeNumerically(">=", 3)) + t.Expect(tc.devicePortReachable("2228")).To(BeTrue(), + "the restart must apply the staged NIC's port-forwarding ACL") + }, tc.timeout, tc.polling).Should(Succeed()) +} + +// cleanup (phase 6) removes both applications. +func (tc *stagedNICChangeTest) cleanup() { + tc.devConfig.DeleteApplication(tc.app1UUID) + tc.devConfig.DeleteApplication(tc.app2UUID) + tc.device.ApplyConfig(tc.devConfig, false, false) +} + +// reportedIPsForMACs reports whether info contains, for every MAC address in +// macs, a network entry with at least one assigned IP address. +func reportedIPsForMACs(info *eveinfo.ZInfoApp, macs []string) bool { + for _, mac := range macs { + var found bool + for _, network := range info.Network { + if network.MacAddr == mac && len(network.IPAddrs) > 0 { + found = true + break + } + } + if !found { + return false + } + } + return true +} diff --git a/evetest/tests/networking/testsuite_test.go b/evetest/tests/networking/testsuite_test.go index 08c16d83346..848595a2d51 100644 --- a/evetest/tests/networking/testsuite_test.go +++ b/evetest/tests/networking/testsuite_test.go @@ -315,6 +315,15 @@ func TestDeviceConnectivitySuite(test *testing.T) { // - TestAccessVLANs -- VLAN-aware Switch NI (stub scenario). // - TestNetworkAdapterPassthrough -- direct adapter assignment to an // app (stub scenario; needs broker QEMU flag tweak). +// - TestStagedNICChange -- a NIC added to a running app without a +// restart command stays staged (no effect on device or guest) until +// the app is restarted. +// - TestNICCountChange -- a NIC is added, the two NICs are swapped and +// one is removed again, each applied through a restart (no purge); the +// guest, the reported state and the app volume follow. +// - TestNICCountChangeOrderedInterface -- the same no-purge NIC addition +// with explicitly pinned interface orders: the added adapter must be +// enumerated by the guest at the position its order dictates. func TestApplicationConnectivitySuite(test *testing.T) { evetest.Init(test) defer evetest.Close() @@ -407,6 +416,15 @@ func TestApplicationConnectivitySuite(test *testing.T) { evetest.TestCase{ Test: TestNetworkAdapterPassthrough, }, + evetest.TestCase{ + Test: TestStagedNICChange, + }, + evetest.TestCase{ + Test: TestNICCountChange, + }, + evetest.TestCase{ + Test: TestNICCountChangeOrderedInterface, + }, ) } diff --git a/pkg/pillar/cmd/zedmanager/handlezedrouter.go b/pkg/pillar/cmd/zedmanager/handlezedrouter.go index e9bca5e9eb7..0128f8dd806 100644 --- a/pkg/pillar/cmd/zedmanager/handlezedrouter.go +++ b/pkg/pillar/cmd/zedmanager/handlezedrouter.go @@ -6,11 +6,21 @@ package zedmanager import ( "bytes" "reflect" + "slices" "github.com/lf-edge/eve/pkg/pillar/types" ) // MaybeAddAppNetworkConfig ensures we have an AppNetworkConfig +// +// A change to the application's network adapters (their number, their order, +// or any field of an existing adapter other than ACLs) reconfigures the +// network stack (bridge, dnsmasq, VIFs). While the application is running such +// a change is withheld from +// zedrouter (stageNetwork below) so the network is not reconfigured underneath +// the running guest; it is published instead on the next restart (or when the +// app is being started), so zedrouter reconfigures the network as part of the +// (re)start. ACL and other field changes are applied live. func MaybeAddAppNetworkConfig(ctx *zedmanagerContext, aiConfig types.AppInstanceConfig, aiStatus *types.AppInstanceStatus) { @@ -22,13 +32,19 @@ func MaybeAddAppNetworkConfig(ctx *zedmanagerContext, effectiveActivate := effectiveActivateCombined(aiConfig, ctx) changed := false + stageNetwork := false m := lookupAppNetworkConfig(ctx, key) if m != nil { log.Functionf("appNetwork config already exists for %s", key) - if len(aiConfig.AppNetAdapterList) != len(m.AppNetAdapterList) { - log.Errorln("Unsupported: Changed number of AppNetAdapter for ", - aiConfig.UUIDandVersion) - return + if !reflect.DeepEqual(m.AppNetAdapterList, aiConfig.AppNetAdapterList) { + log.Functionf("MaybeAddAppNetworkConfig: AppNetAdapters changed "+ + "from %+v to %+v for %s", m.AppNetAdapterList, + aiConfig.AppNetAdapterList, aiConfig.UUIDandVersion) + changed = true + if adapterChangeNeedsRestart(m.AppNetAdapterList, + aiConfig.AppNetAdapterList) { + stageNetwork = true + } } if m.Activate != effectiveActivate { log.Functionf("MaybeAddAppNetworkConfig Activate changed from %v to %v", @@ -53,41 +69,60 @@ func MaybeAddAppNetworkConfig(ctx *zedmanagerContext, log.Functionf("MaybeAddAppNetworkConfig: CipherBlockStatus.CipherData changed") changed = true } - for i, new := range aiConfig.AppNetAdapterList { - old := m.AppNetAdapterList[i] - if !reflect.DeepEqual(new.ACLs, old.ACLs) { - log.Functionf("Under ACLs changed from %v to %v", - old.ACLs, new.ACLs) - changed = true - break - } - } } else { log.Tracef("appNetwork config add for %s", key) changed = true } - if changed { - nc := types.AppNetworkConfig{ - UUIDandVersion: aiConfig.UUIDandVersion, - DisplayName: aiConfig.DisplayName, - Activate: effectiveActivate, - GetStatsIPAddr: aiConfig.CollectStatsIPAddr, - CloudInitUserData: aiConfig.CloudInitUserData, - CipherBlockStatus: aiConfig.CipherBlockStatus, - MetaDataType: aiConfig.MetaDataType, - DeploymentType: aiConfig.DeploymentType, // can not be dynamically changed - } - nc.AppNetAdapterList = make([]types.AppNetAdapterConfig, - len(aiConfig.AppNetAdapterList)) - for i, ulc := range aiConfig.AppNetAdapterList { - ul := &nc.AppNetAdapterList[i] - *ul = ulc - } - publishAppNetworkConfig(ctx, &nc) + if !changed { + log.Functionf("MaybeAddAppNetworkConfig done (no change) for %s", key) + return } + // Stage network-reconfiguring changes while the guest is running; they are + // applied on the next restart (when RestartInprogress is set). If the app is + // not currently activated it is being started, so there is no running guest + // to protect and the change is applied immediately. + if stageNetwork && aiStatus.Activated && + aiStatus.RestartInprogress == types.NotInprogress { + log.Noticef("MaybeAddAppNetworkConfig(%s): network adapter change staged; "+ + "will be applied when the application is restarted", key) + return + } + nc := types.AppNetworkConfig{ + UUIDandVersion: aiConfig.UUIDandVersion, + DisplayName: aiConfig.DisplayName, + Activate: effectiveActivate, + GetStatsIPAddr: aiConfig.CollectStatsIPAddr, + CloudInitUserData: aiConfig.CloudInitUserData, + CipherBlockStatus: aiConfig.CipherBlockStatus, + MetaDataType: aiConfig.MetaDataType, + DeploymentType: aiConfig.DeploymentType, // can not be dynamically changed + } + nc.AppNetAdapterList = slices.Clone(aiConfig.AppNetAdapterList) + publishAppNetworkConfig(ctx, &nc) log.Functionf("MaybeAddAppNetworkConfig done for %s", key) } +// adapterChangeNeedsRestart reports whether the difference between the old and +// the new adapter list cannot be applied to a running application. Only ACL +// changes are applied live by zedrouter; any other difference (the number of +// adapters, or an adapter's name, network, IP, MAC, interface order, VLAN, ...) +// reconfigures the network stack and therefore has to wait until the +// application is restarted. Note that the adapter lists are sorted by +// IntfOrder (see zedagent's parseAppNetAdapterConfig), so a pure reordering +// also shows up as a difference here, as it must: it changes the order in +// which the guest enumerates its NICs. +func adapterChangeNeedsRestart(oldList, newList []types.AppNetAdapterConfig) bool { + withoutACLs := func(adapters []types.AppNetAdapterConfig) []types.AppNetAdapterConfig { + stripped := make([]types.AppNetAdapterConfig, len(adapters)) + copy(stripped, adapters) + for i := range stripped { + stripped[i].ACLs = nil + } + return stripped + } + return !reflect.DeepEqual(withoutACLs(oldList), withoutACLs(newList)) +} + func lookupAppNetworkConfig(ctx *zedmanagerContext, key string) *types.AppNetworkConfig { pub := ctx.pubAppNetworkConfig diff --git a/pkg/pillar/cmd/zedmanager/zedmanager.go b/pkg/pillar/cmd/zedmanager/zedmanager.go index e92752d06ea..0b57db3000b 100644 --- a/pkg/pillar/cmd/zedmanager/zedmanager.go +++ b/pkg/pillar/cmd/zedmanager/zedmanager.go @@ -1587,11 +1587,16 @@ func handleDelete(ctx *zedmanagerContext, key string, log.Functionf("handleDelete done for %s", status.DisplayName) } -// Returns needRestart, needPurge, plus a string for each. -// If there is a change to the disks, adapters, or network interfaces -// it returns needPurge. -// If there is a change to the CPU etc resources it returns needRestart -// Changes to ACLs don't result in either being returned. +// Returns needPurge, needRestart, plus a reason string for each. +// needPurge (recreates volumes) is returned for a change to the volumes or the +// IoAdapterList. +// needRestart (recreates the domain only) is returned for a change to CPU/memory +// and other FixedResources. +// A change to the network adapters - their number, or an existing adapter's +// network/IP/MAC - returns neither: the new adapter set is staged but only +// applied when the user restarts the application (we do not auto-restart or +// hotplug the guest). +// Changes to ACLs don't result in either being returned (applied live). func quantifyChanges(config types.AppInstanceConfig, oldConfig types.AppInstanceConfig, status types.AppInstanceStatus) (bool, bool, string, string) { @@ -1626,32 +1631,36 @@ func quantifyChanges(config types.AppInstanceConfig, oldConfig types.AppInstance str := fmt.Sprintf("number of AppNetAdapter changed from %d to %d", len(oldConfig.AppNetAdapterList), len(config.AppNetAdapterList)) - log.Function(str) - needPurge = true - purgeReason += str + "\n" + // Changing the number of network adapters does not recreate volumes + // (so it is not a purge) and is deliberately NOT auto-applied: we set + // neither needPurge nor needRestart. The new AppNetworkConfig is not + // handed to zedrouter either while the guest runs (see + // MaybeAddAppNetworkConfig), so the network is not reconfigured + // underneath it; the config is published on the next restart, when the + // domain is re-created with the new set of VIFs. + log.Functionf("%s - will be applied on next application restart", str) } else { + // A change to an existing adapter's network/IP/MAC is staged like a + // change to the number of adapters above: it does not recreate volumes + // (not a purge), is not auto-applied, and is withheld from zedrouter + // until the application is restarted. ACL changes are applied live by + // zedrouter and need neither purge nor restart. for i, uc := range config.AppNetAdapterList { old := oldConfig.AppNetAdapterList[i] if old.AppMacAddr.String() != uc.AppMacAddr.String() { - str := fmt.Sprintf("AppMacAddr changed from %v to %v", + log.Functionf("AppMacAddr changed from %v to %v - "+ + "will be applied on next application restart", old.AppMacAddr, uc.AppMacAddr) - log.Function(str) - needPurge = true - purgeReason += str + "\n" } if !old.AppIPAddr.Equal(uc.AppIPAddr) { - str := fmt.Sprintf("AppIPAddr changed from %v to %v", + log.Functionf("AppIPAddr changed from %v to %v - "+ + "will be applied on next application restart", old.AppIPAddr, uc.AppIPAddr) - log.Function(str) - needPurge = true - purgeReason += str + "\n" } if old.Network != uc.Network { - str := fmt.Sprintf("Network changed from %v to %v", + log.Functionf("Network changed from %v to %v - "+ + "will be applied on next application restart", old.Network, uc.Network) - log.Function(str) - needPurge = true - purgeReason += str + "\n" } if !cmp.Equal(old.ACLs, uc.ACLs) { log.Functionf("FYI ACLs changed: %v", diff --git a/pkg/pillar/cmd/zedrouter/appnetwork.go b/pkg/pillar/cmd/zedrouter/appnetwork.go index 2130b2c02c3..a0497b75c4d 100644 --- a/pkg/pillar/cmd/zedrouter/appnetwork.go +++ b/pkg/pillar/cmd/zedrouter/appnetwork.go @@ -215,9 +215,8 @@ func (z *zedrouter) doCopyAppNetworkConfigToStatus( status.AppNetAdapterList = make([]types.AppNetAdapterStatus, ulcount) for i, netConfig := range config.AppNetAdapterList { // Preserve previous VIF status unless it was moved to another network. - // Note that adding or removing VIF is not currently supported - // (such change would be rejected by config validation methods, - // see zedrouter/validation.go). + // Adding or removing a VIF is supported; the new/removed adapters + // simply have no matching previous status to preserve here. if i < len(prevNetStatus) && prevNetStatus[i].Network == netConfig.Network { status.AppNetAdapterList[i] = prevNetStatus[i] } @@ -263,6 +262,7 @@ func (z *zedrouter) checkAndRecreateAppNetworks(niID uuid.UUID) { appNetStatus.ClearError() changedErr = true } + wasAwaiting := appNetStatus.AwaitNetworkInstance if changedErr || appNetStatus.AwaitNetworkInstance != awaitNetworkInstance { appNetStatus.AwaitNetworkInstance = awaitNetworkInstance z.publishAppNetworkStatus(&appNetStatus) @@ -275,6 +275,11 @@ func (z *zedrouter) checkAndRecreateAppNetworks(niID uuid.UUID) { // with conflicting port forwarding rules could have been deployed during this // time, which would necessitate preventing the activation of this app's network. z.handleAppNetworkCreate(nil, appNetConfig.Key(), *appNetConfig) + } else if wasAwaiting && !appNetStatus.HasError() && + !appNetStatus.AwaitNetworkInstance && appNetStatus.Activated { + // Apply the config change that handleAppNetworkModify dropped while + // the network instance was not ready yet. + z.handleAppNetworkModify(nil, appNetConfig.Key(), *appNetConfig, *appNetConfig) } z.log.Functionf("checkAndRecreateAppNetworks(%v) done for %s", niID, appNetConfig.DisplayName) @@ -311,15 +316,20 @@ func (z *zedrouter) doUpdateActivatedAppNetwork(oldConfig, newConfig types.AppNe z.log.Functionf("Updated activated application network %s (%s)", newConfig.UUIDandVersion.UUID, newConfig.DisplayName) - // Update state data collecting parameters. - z.checkAppContainerStatsCollecting(&newConfig, status) - z.updateVIFsForStateCollecting(&oldConfig, &newConfig) - // Update app network status as well as status of connected network instances. z.processAppConnReconcileStatus(appConnRecStatus, status) z.reloadStatusOfAssignedIPs(status) z.publishAppNetworkStatus(status) z.updateNIStatusAfterAppNetworkActivate(status) + + // Update state data collecting parameters. This must come after + // publishAppNetworkStatus: getArgsForNIStateCollecting builds the + // collector's VIF list from the *published* AppNetworkStatus, so + // registering earlier re-registers the collectors with the pre-modify + // adapter list -- an added NIC never gets its IP attributed and a + // removed one is watched forever. Same order as doActivateAppNetwork. + z.checkAppContainerStatsCollecting(&newConfig, status) + z.updateVIFsForStateCollecting(&oldConfig, &newConfig) } func (z *zedrouter) doInactivateAppNetwork(config types.AppNetworkConfig, @@ -356,73 +366,76 @@ func (z *zedrouter) doInactivateAppNetwork(config types.AppNetworkConfig, z.publishAppNetworkStatus(status) } -// Check if any references to network instances have changed and potentially update -// allocated application interface numbers. -// Adds errors to status if there is a failure. +// Reconcile the per-interface numbers allocated for the application's VIFs with +// the (possibly changed) set of AppNetAdapters in the new config. An interface is +// identified by the network instance it connects to together with the per-network +// interface index (IfIdx). Numbers are freed for adapters that were removed (or +// moved to a different network/index) and allocated for newly added adapters; +// adapters present in both the old and new set keep their already-allocated number. +// Adds errors to status if an allocation fails. func (z *zedrouter) checkAppNetworkModifyAppIntfNums(config types.AppNetworkConfig, status *types.AppNetworkStatus) { + appID := config.UUIDandVersion.UUID + + type intfRef struct { + network uuid.UUID + ifIdx uint32 + } - // Check if any AppNetAdapter have changes to the Networks they use + // Interfaces required by the new config. + newRefs := make(map[intfRef]types.AppNetAdapterConfig) for i := range config.AppNetAdapterList { - adapterConfig := &config.AppNetAdapterList[i] - adapterStatus := &status.AppNetAdapterList[i] - if adapterConfig.Network == adapterStatus.Network { + adapterConfig := config.AppNetAdapterList[i] + newRefs[intfRef{adapterConfig.Network, adapterConfig.IfIdx}] = adapterConfig + } + // Interfaces for which a number is currently allocated (based on status). + oldRefs := make(map[intfRef]struct{}) + for i := range status.AppNetAdapterList { + adapterStatus := status.AppNetAdapterList[i] + oldRefs[intfRef{adapterStatus.Network, adapterStatus.IfIdx}] = struct{}{} + } + + // Free numbers for interfaces that are no longer present. + affectedNIs := make(map[uuid.UUID]struct{}) + for ref := range oldRefs { + if _, stillUsed := newRefs[ref]; stillUsed { continue } - z.log.Functionf( - "checkAppNetworkModifyAppIntfNums(%v) for %s: change from %s to %s", - config.UUIDandVersion, config.DisplayName, - adapterStatus.Network, adapterConfig.Network) - // update the reference to the network instance - err := z.doAppNetworkModifyAppIntfNum( - status.UUIDandVersion.UUID, adapterConfig, adapterStatus) - if err != nil { - err = fmt.Errorf("failed to modify appIntfNum: %v", err) - z.log.Errorf( - "checkAppNetworkModifyAppIntfNums(%v/%v): %v", + if err := z.freeAppIntfNum(ref.network, appID, ref.ifIdx); err != nil { + z.log.Error(err) + // Continue anyway, try to (de)allocate as many as possible. + } + affectedNIs[ref.network] = struct{}{} + } + + // Allocate numbers for newly added interfaces. + for ref, adapterConfig := range newRefs { + if _, alreadyAllocated := oldRefs[ref]; alreadyAllocated { + continue + } + withStaticIP := adapterConfig.AppIPAddr != nil + if err := z.allocateAppIntfNum( + ref.network, appID, ref.ifIdx, withStaticIP); err != nil { + err = fmt.Errorf("failed to allocate appIntfNum: %v", err) + z.log.Errorf("checkAppNetworkModifyAppIntfNums(%v/%v): %v", config.UUIDandVersion.UUID, config.DisplayName, err) z.addAppNetworkError(status, "checkAppNetworkModifyAppIntfNums", err) // Continue anyway... } } -} - -// handle a change to the network UUID for one AppNetAdapterConfig. -// Assumes the caller has checked that such a change is present. -// Release the current appIntfNum and acquire appIntfNum on the new network instance. -func (z *zedrouter) doAppNetworkModifyAppIntfNum(appID uuid.UUID, - adapterConfig *types.AppNetAdapterConfig, - adapterStatus *types.AppNetAdapterStatus) error { - - newNetworkID := adapterConfig.Network - oldNetworkID := adapterStatus.Network - newIfIdx := adapterConfig.IfIdx - oldIfIdx := adapterStatus.IfIdx - - // Try to release the app number on the old network. - err := z.freeAppIntfNum(oldNetworkID, appID, oldIfIdx) - if err != nil { - z.log.Error(err) - // Continue anyway... - } - - // Allocate an app number on the new network. - withStaticIP := adapterConfig.AppIPAddr != nil - err = z.allocateAppIntfNum(newNetworkID, appID, newIfIdx, withStaticIP) - if err != nil { - z.log.Error(err) - return err - } - // Did the freeAppIntfNum release any last reference from app to NI? - netstatus := z.lookupNetworkInstanceStatus(oldNetworkID.String()) - if netstatus != nil { + // Freeing a number may have removed the last reference from the app to a + // network instance, which then may be deleted or inactivated. + for niID := range affectedNIs { + netstatus := z.lookupNetworkInstanceStatus(niID.String()) + if netstatus == nil { + continue + } if z.maybeDelOrInactivateNetworkInstance(netstatus) { - z.log.Functionf("Deleted/Inactivated NI %s as a result of moving app %s "+ - "to another network %s", oldNetworkID, appID, newNetworkID) + z.log.Functionf("Deleted/Inactivated NI %s as a result of "+ + "removing interface(s) of app %s", niID, appID) } } - return nil } // For app already deployed (before node reboot), keep using the same MAC address diff --git a/pkg/pillar/cmd/zedrouter/pubsubhandlers.go b/pkg/pillar/cmd/zedrouter/pubsubhandlers.go index 8d86ed5ff68..7ada68aaad7 100644 --- a/pkg/pillar/cmd/zedrouter/pubsubhandlers.go +++ b/pkg/pillar/cmd/zedrouter/pubsubhandlers.go @@ -541,8 +541,11 @@ func (z *zedrouter) handleAppNetworkCreate(ctxArg interface{}, key string, z.log.Functionf("handleAppNetworkCreate(%s) done for %s", key, config.DisplayName) } -// handleAppNetworkModify cannot handle any change. -// For example, the number of AppNetAdapters can not be changed. +// handleAppNetworkModify applies a change to an existing application network. +// This includes changing the number of AppNetAdapters: the corresponding VIFs +// are added or removed inside the network stack by the reconciler. The guest is +// not hotplugged; zedmanager purges (recreates) the application domain so that it +// comes up with the new set of interfaces. func (z *zedrouter) handleAppNetworkModify(ctxArg interface{}, key string, configArg interface{}, oldConfigArg interface{}) { newConfig := configArg.(types.AppNetworkConfig) @@ -574,7 +577,7 @@ func (z *zedrouter) handleAppNetworkModify(ctxArg interface{}, key string, }() // Check for unsupported/invalid changes. - if err := z.validateAppNetworkConfigForModify(newConfig, oldConfig); err != nil { + if err := z.validateAppNetworkConfigForModify(newConfig); err != nil { z.log.Errorf("handleAppNetworkModify(%v): validation failed: %v", newConfig.UUIDandVersion.UUID, err) z.addAppNetworkError(status, "handleAppNetworkModify", err) diff --git a/pkg/pillar/cmd/zedrouter/validation.go b/pkg/pillar/cmd/zedrouter/validation.go index cc2792933b2..f86c92009d5 100644 --- a/pkg/pillar/cmd/zedrouter/validation.go +++ b/pkg/pillar/cmd/zedrouter/validation.go @@ -209,17 +209,13 @@ func (z *zedrouter) validateAppNetworkConfig(appNetConfig types.AppNetworkConfig } func (z *zedrouter) validateAppNetworkConfigForModify( - newConfig types.AppNetworkConfig, oldConfig types.AppNetworkConfig) error { - // XXX What about changing the number of interfaces as part of an inactive/active - // transition? - // XXX We could allow the addition of interfaces if the domU would find out through - // some hotplug event. - // But deletion is hard. - // For now don't allow any adds or deletes. - if len(newConfig.AppNetAdapterList) != len(oldConfig.AppNetAdapterList) { - return fmt.Errorf("changing number of AppNetAdapters (for %s) is unsupported", - newConfig.UUIDandVersion) - } + newConfig types.AppNetworkConfig) error { + // Changing the number of AppNetAdapters is supported: VIFs are added or + // removed inside the network stack by the reconciler, and zedmanager + // purges (recreates) the application domain so the guest comes up with the + // new set of interfaces. We do not hotplug interfaces into a running guest. + // checkAppNetworkModifyAppIntfNums() (de)allocates the per-interface + // numbers for the added/removed adapters. return z.validateAppNetworkConfig(newConfig) } diff --git a/pkg/pillar/hypervisor/qmp.go b/pkg/pillar/hypervisor/qmp.go index aff88386203..dbd9c624ad0 100644 --- a/pkg/pillar/hypervisor/qmp.go +++ b/pkg/pillar/hypervisor/qmp.go @@ -110,6 +110,27 @@ func buildQMPCommand(execute string, arguments interface{}) ([]byte, error) { return cmd, nil } +// execStopOnce and execQuitOnce issue the command with a single dial attempt +// instead of the usual retry loop. They are meant for qmpEventHandler, which +// reacts to an event just received from a live qemu instance: if a single +// attempt fails, that instance is already gone. Retrying the dial would be +// actively harmful there -- a domain restarted without purge reuses the same +// domain name and hence the same socket path, so a retried command can +// connect to the freshly re-created qemu instance and terminate it instead. +func execStopOnce(socket string) error { + cmd := `{ "execute": "stop" }` + logrus.Debugf("executing QMP command once: %s", cmd) + _, err := execRawCmd(socket, cmd, false) + return err +} + +func execQuitOnce(socket string) error { + cmd := `{ "execute": "quit" }` + logrus.Debugf("executing QMP command once: %s", cmd) + _, err := execRawCmd(socket, cmd, false) + return err +} + func execVNCPassword(socket string, password string) error { vncSetPwd, err := buildQMPCommand("change-vnc-password", map[string]string{"password": password}) @@ -267,11 +288,22 @@ func qmpEventHandler(listenerSocket, executorSocket, domainName string) { } switch event.Event { case "SHUTDOWN": + // qemu runs with -no-shutdown: only a guest-initiated shutdown + // (power-off, panic) needs a host-side quit so the containerd + // task exits. A host-initiated one (e.g. reason "host-qmp-quit" + // from the teardown path) means qemu is already quitting -- + // reacting to it can instead kill a new qemu instance that was + // just re-created under the same domain name and socket path + // (restart without purge). + if guest, ok := event.Data["guest"].(bool); ok && !guest { + logrus.Infof("qmpEventHandler: ignoring host-initiated SHUTDOWN (%v) on socket: %s", event.Data, listenerSocket) + continue + } logrus.Infof("qmpEventHandler: Received event: %s event details: %v. Calling quit on socket: %s", event.Event, event.Data, executorSocket) - if err := execStop(executorSocket); err != nil { + if err := execStopOnce(executorSocket); err != nil { logrus.Errorf("qmpEventHandler: Exception while stopping domain with socket: %s. %s", executorSocket, err.Error()) } - if err := execQuit(executorSocket); err != nil { + if err := execQuitOnce(executorSocket); err != nil { logrus.Errorf("qmpEventHandler: Exception while quitting domain with socket: %s. %s", executorSocket, err.Error()) } case "STOP":