Skip to content

Allow adapter change with restart - #6188

Merged
milan-zededa merged 10 commits into
lf-edge:masterfrom
christoph-zededa:allow_adapter_change
Aug 10, 2026
Merged

Allow adapter change with restart#6188
milan-zededa merged 10 commits into
lf-edge:masterfrom
christoph-zededa:allow_adapter_change

Conversation

@christoph-zededa

@christoph-zededa christoph-zededa commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Description

Changing the set of network adapters of a deployed application instance
(adding or removing a NIC) currently requires purging the application,
which destroys and re-creates its volumes. For applications with large
state this is unnecessarily destructive: the adapter set takes effect
when the domain is (re-)created, so a restart is sufficient.

With this PR, an adapter change arriving without a purge command is
staged by the device while the application keeps running, and is
applied when the application is restarted (e.g. via a restart command
from the controller). Application volumes are preserved.

The main pieces:

  • zedmanager, zedrouter: change app network adapters without a purge
    -- zedmanager no longer classifies a network-adapter change as
    requiring a purge; while the application is running, the new
    AppNetworkConfig is withheld from zedrouter (staged) and published as
    part of the next restart, so the network is never reconfigured
    underneath a running guest. zedrouter's modify path now supports a
    changed adapter set: interface numbers are freed/allocated by set
    difference, and adapters present in both the old and the new set keep
    their allocations.
  • zedrouter: publish status before updating state collectors --
    fixes a latent ordering bug armed by the above: the state collectors
    were re-registered from the not-yet-published AppNetworkStatus, so a
    NIC added via restart never had its IP address attributed and
    reported.
  • hypervisor: don't let stale qmp handler quit a re-created domain
    -- a restart re-creates the domain under the same name and QMP socket
    paths; the torn-down qemu's leftover event handler could quit the
    re-created instance (visible as "unexpected state HALTED" followed by
    the ~10 min boot-retry backoff).
  • evetest -- framework improvements and three tests covering the
    feature (see the validation section below).

Known limitations (WIP):

  • Auto-generated MAC addresses and interface numbers are positional:
    removing or inserting an adapter in the middle of the list shifts
    them for the subsequent adapters. Pin MAC addresses, or add/remove
    adapters only at the end of the list.
  • With EnforceNetworkInterfaceOrder disabled (the default), the
    interface order is derived from ACL IDs, so an ACL-only change that
    alters the first ACL ID is currently mis-detected as an adapter
    change and staged instead of applied immediately; fix pending.
  • There is no controller-visible indication yet that a staged adapter
    change is pending a restart.
  • The window between publishing the staged config and re-creating the
    domain is not fully race-free when the domain is already down at the
    time of the restart; under discussion.

How to test and validate this PR

Automated (evetest, QEMU/amd64):

  • make evetest NAME=TestStagedNICChange -- a NIC added without a
    restart command stays fully inert: no restart, no change in the
    reported adapter set, no VIF/interface-number change, guest
    unchanged, existing port forwardings keep accepting connections while
    the staged one does not -- even while other configuration is
    reconciled in the meantime (a second app is deployed on the shared
    network instance). A restart then applies the staged NIC.
  • make evetest NAME=TestNICCountChange -- add, swap and remove a NIC
    through restarts; the guest and the reported state follow and the app
    volume is preserved throughout. Includes the regression guard for the
    state-collector ordering fix (the reported IP address of the added
    NIC).
  • make evetest NAME=TestAppRestart -- repeated controller-requested
    restarts, as a regression test for the stale-QMP-handler race.

Manual:

  1. Deploy an app instance with one virtual network adapter; write a
    file to its disk.
  2. Add a second network adapter in the controller configuration
    without requesting a purge: the app keeps running unchanged.
  3. Restart the app instance: it comes back with the new NIC, the file
    is still on the disk, and both NICs are reported with their IP
    addresses.

Changelog notes

The set of network adapters of a deployed application instance can be
changed (adapters added or removed) without purging the application:
the change is applied by restarting the application instance,
preserving its volumes.

PR Backports

  • 16.0-stable: No, new feature.
  • 14.5-stable: No, new feature.
  • 13.4-stable: No, new feature.

Checklist

  • I've provided a proper description
  • I've added the proper documentation
  • I've tested my PR on amd64 device
  • I've tested my PR on arm64 device
  • I've written the test verification instructions
  • I've set the proper labels to this PR

And the last but not least:

  • I've checked the boxes above, or I've provided a good reason why
    I didn't check them: the per-agent documentation (pkg/pillar/docs)
    still needs updating and labels are not final while the PR is WIP;
    not tested on arm64 hardware yet.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 97 lines in your changes missing coverage. Please review.
✅ Project coverage is 24.61%. Comparing base (0a4bdce) to head (97b8abd).

Files with missing lines Patch % Lines
pkg/pillar/cmd/zedrouter/appnetwork.go 0.00% 37 Missing ⚠️
pkg/pillar/cmd/zedmanager/handlezedrouter.go 0.00% 36 Missing ⚠️
pkg/pillar/hypervisor/qmp.go 0.00% 15 Missing ⚠️
pkg/pillar/cmd/zedmanager/zedmanager.go 0.00% 7 Missing ⚠️
pkg/pillar/cmd/zedrouter/pubsubhandlers.go 0.00% 1 Missing ⚠️
pkg/pillar/cmd/zedrouter/validation.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6188      +/-   ##
==========================================
+ Coverage   24.28%   24.61%   +0.33%     
==========================================
  Files         512      522      +10     
  Lines       93831    95562    +1731     
==========================================
+ Hits        22786    23525     +739     
- Misses      69244    70049     +805     
- Partials     1801     1988     +187     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread evetest/Makefile Outdated
Comment thread pkg/pillar/cmd/zedmanager/handlezedrouter.go
Comment thread pkg/pillar/cmd/zedrouter/appnetwork.go
@github-actions
github-actions Bot requested a review from milan-zededa July 21, 2026 14:03
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Jul 22, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Jul 27, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
@christoph-zededa christoph-zededa changed the title WIP: Allow adapter change with restart Allow adapter change with restart Jul 27, 2026
@christoph-zededa
christoph-zededa marked this pull request as ready for review July 27, 2026 12:37
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Jul 30, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Jul 30, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Jul 30, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
@christoph-zededa

Copy link
Copy Markdown
Contributor Author

@claude why did codecov/patch fail?

christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Jul 31, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Aug 5, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Aug 7, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Aug 7, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
@christoph-zededa
christoph-zededa force-pushed the allow_adapter_change branch 2 times, most recently from 2755395 to 5409a44 Compare August 7, 2026 14:35
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Aug 7, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
@christoph-zededa
christoph-zededa force-pushed the allow_adapter_change branch 2 times, most recently from 1cc8fdf to 4314efa Compare August 7, 2026 15:06
christoph-zededa added a commit to christoph-zededa/eve that referenced this pull request Aug 7, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Comment thread evetest/devconfig.go Outdated
Comment thread evetest/tests/apps/testsuite_test.go

@milan-zededa milan-zededa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just two minor comments added, overall LGTM, let's start eden tests

christoph-zededa and others added 10 commits August 10, 2026 12:16
Changing the set of network adapters of a running VM app instance - the
number of adapters, or an existing adapter's network, IP, MAC, interface
order or VLAN - used to force a purge, which recreates the boot volume
and destroys the guest's state. zedrouter additionally rejected any
change to the number of adapters outright.

Now such a change no longer purges. It is staged and takes effect on the
next restart of the app instance, which recreates the domain with the
new set of interfaces while preserving its volumes. This commit does not
itself trigger that restart. ACL changes continue to be applied live,
without a restart.

zedmanager:
- quantifyChanges() no longer classifies a network-adapter change as a
  purge; it triggers neither a purge nor an auto-restart. The new
  AppNetworkConfig is still published so zedrouter can prepare the VIFs,
  but the running domain keeps its interfaces until the app is restarted.
- MaybeAddAppNetworkConfig() withholds a network-reconfiguring change
  from zedrouter while the guest is running, and publishes it as part
  of the next restart. adapterChangeNeedsRestart() distinguishes an
  ACL-only change (applied live) from one that must wait for a restart.

zedrouter:
- validateAppNetworkConfigForModify() no longer rejects a changed
  number of adapters.
- checkAppNetworkModifyAppIntfNums() reconciles the per-interface
  numbers as a set difference keyed on (network instance, interface
  index): it frees numbers for removed adapters, allocates them for
  added ones, keeps them for adapters present before and after, and
  tears down a network instance that loses its last reference.

The guest is not hotplugged, and directly-assigned (passthrough)
adapters still require a purge. The restart itself is expected to be
driven by the controller.

Signed-off-by: Christoph Ostarek <christoph@zededa.com>
The harness log prefixes (HARNESS/TEST/BROKER) and the red TEST
FAILURE highlight emitted ANSI escape codes unconditionally, so piping
'make evetest ... | tee log.txt' filled the log file with control
characters.

The test process cannot detect a terminal itself: inside the container
its stdout is always a pipe (go test | tee). Neither can the Makefile
use $(shell [ -t 1 ]) - within $(shell) stdout is make's capture pipe,
never a terminal (which is also why DOCKER_IT never resolved to -it).

Instead, the evetest recipe now tests [ -t 1 ] in the recipe shell,
which sees make's real stdout, and passes the result into the container
as EVETEST_COLOR_OUTPUT. The entrypoint falls back to its own TTY check
when the variable is not set (manual 'docker run -it'), and an
explicitly exported EVETEST_COLOR_OUTPUT always takes precedence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
On a dirty tree, ROOTFS_VERSION embeds the wall-clock minute of each
make invocation (REPO_DIRTY_TAG), so the version computed by
'make evetest' almost never matches the timestamp baked into the image
tag by an earlier 'make eve' run, and the test fails with
"EVE image ... is not available (neither locally nor on Docker Hub)".

When EVETEST_EVE_VERSION is not set and no local EVE image exists for
the computed dirty version, fall back to the version recorded by the
last completed build (dist/<arch>/current, read via 'make
currentversion'), provided it is a dirty build of the same commit and
its Docker image exists locally. Otherwise keep the computed version,
so the existing "build the image first with: make eve" guidance still
applies. An explicitly set EVETEST_EVE_VERSION is used unchanged, as
before.

Also expand the '\#' make-escape through a HASH variable so grep no
longer receives the backslash (newer grep warns "stray \ before #").

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Running 'make evetest' silently used a stale evetest image after
framework changes: ensure-evetest-image only checked that the image
exists, and the tag does not encode content, so you could run tests
without your framework changes. Rebuilding unconditionally before
every run is no good either: it costs ~20s even when fully cached
(buildx --load re-exports the image tarball every time) and is
unnecessary for test-only changes, since tests/, netmodels/ and
matchers/ are bind-mounted into the container at run time.

Instead, build-container bakes a content signature of the framework
sources into an image label, and ensure-evetest-image rebuilds when
the label no longer matches. The signature is computed with git (the
last commit touching the framework paths plus a hash of any
uncommitted changes), so it is content-based - mtime churn from
branch switches or stray files in the build context can never
trigger a rebuild - and a git fsmonitor daemon keeps it fast. Images
built by CI carry the label too, so a pulled image is accepted
without a rebuild exactly when it matches the local sources.
Test-only iterations pay two docker/git invocations (~60ms).

Signed-off-by: Christoph Ostarek <christoph@zededa.com>
with the changes from before, changing the network adapters
(adding/removing) does not require purging anymore - reboot
of the app is enough

Signed-off-by: Christoph Ostarek <christoph@zededa.com>
during runtime

Signed-off-by: Christoph Ostarek <christoph@zededa.com>
qmpEventHandler reacts to SHUTDOWN events by issuing stop+quit on the
executor socket so that qemu (running with -no-shutdown) exits after a
guest power-off. It did this for every SHUTDOWN event, including
host-initiated ones (reason "host-qmp-quit") that are emitted as a
consequence of the teardown path in Delete() already quitting qemu.
Moreover, the stop+quit dial the socket *path* with up to 5 retries
spread over ~15 seconds.

When a domain is restarted without a purge (e.g. BootOrder-style
restart applying a network adapter change), the domain name and hence
the QMP socket paths are reused, and the new qemu instance is created
well within that retry window. The stale handler, still draining the
old instance's SHUTDOWN event, would then connect to the new instance
and terminate it: domainmgr sees "unexpected state HALTED" (or never
finds the QMP socket), marks the boot failed and backs off for 10
minutes. Observed reproducibly with an adapter add/swap/remove test.

Fix by ignoring host-initiated SHUTDOWN events (the quit already
happened) and by using a single dial attempt for the stop+quit issued
from the event handler: the event was just received from a live
instance, so if one attempt fails the instance is gone and retrying
can only ever reach a different one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Add a new tests/apps package for application life-cycle scenarios,
with a first test that exercises the controller-requested application
restart (restart counter bump): a domain restart without purge, which
re-creates the domain under the same name and QMP socket paths within
seconds.

The test deploys a container app, then restarts it three times in a
row, each time asserting that the reported boot time advances, that
the app returns to RUNNING well within the domainmgr boot-retry
backoff (timer.boot.retry, 10 minutes by default), and that the app
is functional again by running a command inside it over SSH.

This serves as a regression guard for the stale-QMP-handler race
fixed in the previous commit, where a leftover qmpEventHandler could
terminate the re-created qemu instance and leave the app in the
10-minute boot backoff.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR lf-edge#6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
"make build-sdn-container" handed the sdn sub-make over to a bare
"linuxkit" from PATH and failed on hosts without a system-wide
linuxkit install, although the repository builds its own copy into
build-tools/bin.

Provide it the same way the top-level Makefile does: point LINUXKIT
at build-tools/bin/linuxkit, prepend that directory to PATH, build
the binary on demand by delegating to the root's "linuxkit" target
(mk/linuxkit.mk) and pass LINUXKIT down to the sdn sub-make, which
so far defaulted to the PATH lookup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
@github-actions
github-actions Bot requested a review from milan-zededa August 10, 2026 10:17
@milan-zededa
milan-zededa merged commit fc297b5 into lf-edge:master Aug 10, 2026
55 of 56 checks passed
milan-zededa pushed a commit that referenced this pull request Aug 10, 2026
A NIC added to (or removed from) a running application without a
restart command is only staged by the device. TestStagedNICChange
verifies the property that makes this staging sound: nothing of the
new adapter set may take effect before the restart, no matter what
other configuration is reconciled in the meantime -- the question
raised in the review of the staged-adapter change (PR #6188).

The test deploys an app with two NICs, then adds a third one to the
controller configuration while reverting the restart-counter bump
made by UpdateApplication, so the change reaches the device without
a restart command. It then repeatedly asserts, before and after
deploying a second app on the shared network instance (which forces
zedrouter to reconcile the very bridge/iptables state the staged
change would touch), that:

- the app keeps running with its baseline boot time (no restart),
- the reported app info still lists only the two original NICs,
- the app's VIF on the shared NI keeps its name (interface-number
  allocations did not move),
- the guest sees only the two original MACs, and
- the ACL state is untouched: the two existing port-forwarding
  rules keep accepting connections while the staged NIC's
  port-forwarding rule (the externally observable part of the
  staged ACLs) does not accept any yet.

A final restart-counter bump must then apply the staged adapter:
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.

The reported (controller-visible) IP address of the newly added NIC
is deliberately not asserted: 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. Volume preservation across the restart is
already covered by TestNICCountChange.

TestNICCountChangeOrderedInterface exercises the same no-purge
adapter addition with an explicitly pinned interface order: an
application is deployed with five NICs, each pinned to its own
interface order, and a sixth NIC is then added with an order that
falls between two of the existing ones. The guest must come back
with six interfaces.

For that, VirtualNetworkAdapter gains an optional InterfaceOrder
field so that a test can pin the order explicitly. When it is left
unset the interface order still defaults to the adapter's position
in the list, leaving the existing tests unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants