Skip to content

pkg/kube: fix multus NAD apply ordering and unreachable retry back-off - #6242

Merged
eriknordmark merged 2 commits into
lf-edge:masterfrom
eriknordmark:kube-multus-apply-order
Aug 4, 2026
Merged

pkg/kube: fix multus NAD apply ordering and unreachable retry back-off#6242
eriknordmark merged 2 commits into
lf-edge:masterfrom
eriknordmark:kube-multus-apply-order

Conversation

@eriknordmark

@eriknordmark eriknordmark commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Description

Two small independent fixes to how cluster-init.sh applies the multus manifest.

The NetworkAttachmentDefinition CRD and an instance of it go in one apply. pkg/kube/multus-daemonset.yaml declares the CRD and the singleton network-instance-attachment object of that kind, and apply_multus_cni feeds the whole file to one kubectl apply. kubectl does not wait for a CRD to be established before creating a custom resource of that kind, so the instance can lose the race and fail with no matches for kind "NetworkAttachmentDefinition" while the CRD and the daemonset apply normally — leaving pods that attach to a network instance with no NAD to reference. Now the CRD is established first and the manifest re-applied to create whatever lost the race; apply is idempotent, so a run that did not hit the race does no extra work. The same commit stops sending kubectl's output to /dev/null on both failure paths, which is what made this slow to identify: the log said only Apply Multus, has failed, jump out now.

I hit this race on a branch carrying #5971 (the Go kube-init port), not on master. Master has the same shape — one manifest holding CRD and CR, one un-gated apply — so the race is available to it, but I have not observed it there. Treat that commit as hardening.

That said, the race is not hypothetical. The Go port in #5971 hit it independently and documents it in kubectlApply: "The retry loop is REQUIRED for manifests that combine a CRD and a custom resource of that CRD in the same file (e.g. multus-daemonset.yaml ships a NetworkAttachmentDefinition CRD plus a NAD instance): kubectl's API-discovery cache is built at process start, so the first apply creates the CRD but errors on the CR with 'no matches for kind'." It solves the same problem a different way — classifying that error as transient and retrying with backoff, rather than waiting for the CRD to be established. Either approach works; the shell path currently does neither.

The multus retry back-off was unreachable. An unconditional continue sat immediately after the apply_multus_cni call, so the block handling a failed apply — a log line plus a ten second sleep — could never run, and a failing apply retried in a tight loop while logging nothing. Moving the continue below that block restores both; the loop still restarts after every attempt, as before.

How to test and validate this PR

Both changes are in the EVE-k cluster bring-up path, so validation is on an EVE-k device.

No regression. Bring up an EVE-k node from scratch and confirm multus still initializes: /var/lib/multus_initialized appears, the log shows Done applying Multus, kubectl get crd network-attachment-definitions.k8s.cni.cncf.io is Established, and kubectl -n eve-kube-app get net-attach-def network-instance-attachment exists. Deploy an app on a network instance and confirm it gets its interface.

First fix. The race is hard to force on demand. The reachable checks are that the NAD is present after bring-up on a node where it previously came up missing, and that a failing apply now logs the object and reason. To exercise the new failure branch, make the CRD unable to establish (e.g. an invalid CRD schema) and confirm bring-up logs Apply Multus, NAD CRD not established: <kubectl output> and retries instead of proceeding as if multus were up.

Second fix. Make apply_multus_cni fail deterministically (e.g. make /etc/multus-daemonset-new.yaml unreadable) and watch the log emit Failed to apply multus cni, wait a while about every ten seconds, where before it spun silently.

shellcheck -x pkg/kube/cluster-init.sh is clean, and each commit is syntax-checked independently.

Changelog notes

Fixed cluster bring-up on Kubernetes-enabled devices sometimes leaving the network-attachment definition missing, which prevented applications from attaching to network instances. Failures while setting up multus are now logged with the underlying reason and retried at a sane interval instead of in a tight loop.

PR Backports

The template's list does not yet include 17.0-stable, which is where these belong.

  • 17.0-stable: To be backported.
  • 16.0-stable: No.
  • 14.5-stable: No.
  • 13.4-stable: No.

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.

No documentation change: no existing doc describes the apply ordering or the retry
cadence. Not run on amd64/arm64 hardware: both changes are architecture-independent
shell in the EVE-k bring-up path, with the device steps above for whoever runs them.

eriknordmark and others added 2 commits July 28, 2026 12:35
Cluster bring-up can leave the singleton network-instance-attachment
object missing, so pods that attach to a network instance have nothing
to reference. The multus manifest declares both the
NetworkAttachmentDefinition CRD and an instance of that kind, and is fed
to a single kubectl apply. kubectl does not wait for a CRD to be
established before creating a custom resource of that kind, so the
instance can lose the race and fail with 'no matches for kind
"NetworkAttachmentDefinition"' while the CRD and the daemonset apply
normally.

Wait for the CRD to be established and apply the manifest again, which
creates whatever lost the race; apply is idempotent, so a run that did
not hit the race does no extra work.

Report what kubectl said as well. Both failure paths discarded stdout and
stderr, so an operator saw only "Apply Multus, has failed, jump out now"
with no indication of which object failed or why.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A failed multus apply retried in a tight loop instead of pausing, and
logged nothing about the failure. An unconditional continue sat directly
after the apply call, which made the block handling that failure -- the
log line and a ten second sleep -- unreachable.

Move the continue below that block so both run. The loop still restarts
after every attempt, successful or not, as it did before.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@eriknordmark eriknordmark added the stable Should be backported to stable release(s) label Jul 28, 2026

@naiming-zededa naiming-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.

LGTM

@eriknordmark
eriknordmark marked this pull request as ready for review July 28, 2026 20:34
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 23.31%. Comparing base (5d69266) to head (72bdad3).
⚠️ Report is 32 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6242      +/-   ##
==========================================
+ Coverage   22.93%   23.31%   +0.37%     
==========================================
  Files         510      520      +10     
  Lines       93473    95189    +1716     
==========================================
+ Hits        21440    22189     +749     
- Misses      70292    71075     +783     
- Partials     1741     1925     +184     

☔ 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.

eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 3, 2026
Record lf-edge#6259 and the two source PRs the table never listed (lf-edge#6240, lf-edge#6242),
note that lf-edge#6063 is now 17 commits, and mark the CDI upload-pod teardown
logging as upstream in master rather than branch-local. Add the lf-edge#6240/lf-edge#6259
test interaction and the rebase-due note: master is 63 commits ahead of the
shared merge-base.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 3, 2026
Record lf-edge#6259 and the two source PRs the table never listed (lf-edge#6240, lf-edge#6242),
note that lf-edge#6063 is now 17 commits, and mark the CDI upload-pod teardown
logging as a pre-merge copy of an upstream commit. Add the lf-edge#6240/lf-edge#6259 test
interaction and the rebase-due note: master is 63 commits ahead of the shared
merge-base.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 3, 2026
Record lf-edge#6259, note that lf-edge#6063 is now 17 commits, refresh lf-edge#6240's head, and mark
the CDI upload-pod teardown logging as a pre-merge copy of an upstream commit.
Spell out why lf-edge#6242 is not replayed here and that its NAD-CRD race is still
open in the Go ApplyMultusCNI. Add the lf-edge#6240/lf-edge#6259 test interaction and the
rebase-due note: master is 63 commits ahead of the shared merge-base.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 3, 2026
Record lf-edge#6259, note that lf-edge#6063 is now 17 commits, refresh lf-edge#6240's head, and mark
the CDI upload-pod teardown logging as a pre-merge copy of an upstream commit.
Spell out why lf-edge#6242 is not replayed here and that its NAD-CRD race is still
open in the Go ApplyMultusCNI. Add the lf-edge#6240/lf-edge#6259 test interaction and the
rebase-due note: master is 63 commits ahead of the shared merge-base.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 3, 2026
The multus manifest declares the NetworkAttachmentDefinition CRD and an
instance of that kind, and both go to a single apply. The apiserver admits
a custom resource only once its CRD is established, so the instance can
lose that race and be rejected with 'no matches for kind
"NetworkAttachmentDefinition"' while the CRD and the daemonset apply
normally. The singleton attachment object is then missing and pods that
attach to a network instance have nothing to reference.

Tolerate the first apply failing, wait for the CRD to report Established,
and apply again to create whatever lost the race; apply is idempotent, so a
run that did not hit the race does no extra work. When the CRD never
establishes, report the first apply's error alongside the wait failure —
that error is the one naming the object that could not be created.

This ports lf-edge#6242 to the Go daemon. That PR fixes the same race in
cluster-init.sh, which this branch deletes, so without the port the race
returns when kube-init replaces the script.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 3, 2026
The multus manifest declares the NetworkAttachmentDefinition CRD and an
instance of that kind, and both go to a single apply. The apiserver admits
a custom resource only once its CRD is established, so the instance can
lose that race and be rejected with 'no matches for kind
"NetworkAttachmentDefinition"' while the CRD and the daemonset apply
normally. The singleton attachment object is then missing and pods that
attach to a network instance have nothing to reference.

Tolerate the first apply failing, wait for the CRD to report Established,
and apply again to create whatever lost the race; apply is idempotent, so a
run that did not hit the race does no extra work. When the CRD never
establishes, report the first apply's error alongside the wait failure —
that error is the one naming the object that could not be created.

This ports lf-edge#6242 to the Go daemon. That PR fixes the same race in
cluster-init.sh, which this branch deletes, so without the port the race
returns when kube-init replaces the script.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 3, 2026
The multus manifest declares the NetworkAttachmentDefinition CRD and an
instance of that kind, and both go to a single apply. The apiserver admits
a custom resource only once its CRD is established, so the instance can
lose that race and be rejected with 'no matches for kind
"NetworkAttachmentDefinition"' while the CRD and the daemonset apply
normally. The singleton attachment object is then missing and pods that
attach to a network instance have nothing to reference.

Tolerate the first apply failing, wait for the CRD to report Established,
and apply again to create whatever lost the race; apply is idempotent, so a
run that did not hit the race does no extra work. When the CRD never
establishes, report the first apply's error alongside the wait failure —
that error is the one naming the object that could not be created.

This ports lf-edge#6242 to the Go daemon. That PR fixes the same race in
cluster-init.sh, which this branch deletes, so without the port the race
returns when kube-init replaces the script.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 4, 2026
The NAD-CRD race lf-edge#6242 fixes in cluster-init.sh is now also fixed in the Go
kube-init daemon, so the note that the port was still open is stale.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 4, 2026
The NAD-CRD race lf-edge#6242 fixes in cluster-init.sh is now also fixed in the Go
kube-init daemon, so the note that the port was still open is stale.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 4, 2026
The NAD-CRD race lf-edge#6242 fixes in cluster-init.sh is now ported to the Go
kube-init daemon and carried on this branch, so the note that the port was
still open is stale.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Aug 4, 2026
The NAD-CRD race lf-edge#6242 fixes in cluster-init.sh is now ported to the Go
kube-init daemon and carried on this branch, so the note that the port was
still open is stale.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@eriknordmark
eriknordmark merged commit c9e902f into lf-edge:master Aug 4, 2026
54 of 55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stable Should be backported to stable release(s)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants