Repository navigation
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
049ad54 to
91e113a
Compare
f3bbd67 to
48c0422
Compare
|
Hi @maiqueb are you interested that i will drop the 4th commit and keep the rest ? so far it seems the multus fix is enough (in parallel, i am checking if i can break this assumption locally) |
Yes, please drop the topmost commit. |
Multus has always deleted and recreated /run/multus on daemon start, giving the socket a new inode at the same path. New in v4.3.1 is graceful shutdown: after SIGTERM the old daemon keeps its socket alive for ~10s instead of exiting in ~1s. During a rolling restart that opens a race — the controller bind-mounts the old socket file while it is still alive, then the new daemon deletes and recreates the socket underneath the mount. A file bind mount pins the inode it was made from, and volume mounts belong to the pod, not the container: a container restart does not redo the mount. So the controller keeps probing the deleted socket, its probes fail with `curl: (7)`, and it crash-loops until the pod is deleted. Mount the /run/multus directory instead of the socket file. The directory always reflects the socket that is live right now, so the controller only fails during the short window in which no socket exists, and recovers once the new daemon is listening. Verified on a kind cluster with a rolling Multus restart, as CNAO does: without the fix the controller crash-loops with `curl: (7)` and container restarts never recover; with it the controller restarts once during the brief socket-unavailable window and then stays 1/1 Running. Assisted-by: GLM 5.3 Signed-off-by: Or Shoval <oshoval@redhat.com>
|
rebased and reopen, seems i might managed to simulate the bug, drafting now |
What this PR does / why we need it:
Summary
Mount
/run/multusas a directory in the dynamic-networks controller insteadof mounting
multus.sockdirectly. This prevents the controller from stayingattached to a stale socket when Multus removes and recreates its socket during
a restart.
Problem
The controller's readiness and liveness probes connect to Multus through a
Unix socket. A file bind mount pins the socket inode that existed when the
pod was created. If Multus recreates the socket, the controller's mount still
points to the old inode, and restarting the controller container does not
refresh the pod's volume mount. The probes then fail and the controller can
crashloop.
This behavior was observed while investigating CNAO PR
#2989,
which upgrades Multus to v4.3.1. In the failing workflow, CNAO updates the
Multus configuration and deploys the controller in the same reconciliation.
Two Multus behaviors create the stale-mount window:
FilesystemPreRequirementsremoves and recreates the socket directory at daemon startup, producing a
new socket inode.
(#1525,
SigTermCancelAfter). The old daemon can keep its socket open while thecontroller pod is created and mounts it.
The stale-mount failure happens in this order:
Multus' old pod receives SIGTERM, but in v4.3.1 graceful shutdown keeps its
daemon alive for about 10 seconds.
new controller pod. That mount is pinned to the old socket inode.
FilesystemPreRequirementsremoves the entire/run/multusdirectorywith
os.RemoveAll, recreates it, and then creates a new socket there.instead of the new socket. Its probes fail, and restarting its container
does not refresh the pod's mount.
Change
The controller now mounts the socket's parent directory:
The directory mount stays valid when Multus recreates the socket, so the
controller resolves the current socket at its path.
Validation
On a local k8s-1.36 cluster, a rolling Multus restart reproduced the stale
socket mount on the unmodified controller: probes failed, and restarting the
container did not recover it. With the directory mount, the controller
recovered after Multus recreated the socket and remained ready.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer (optional):
Note:
Seems this repo still use old kind / k8s version via multus-cni, best to bump it on multus-cni,
but not relevant to this PR