Skip to content

[ipam/eni] Wait for ENI interface in the CNI plugin before configuring routes - #675

Closed
jaredledvina wants to merge 1 commit into
mainfrom
jared.ledvina/fixup-eni-ifindex-race
Closed

[ipam/eni] Wait for ENI interface in the CNI plugin before configuring routes#675
jaredledvina wants to merge 1 commit into
mainfrom
jared.ledvina/fixup-eni-ifindex-race

Conversation

@jaredledvina

@jaredledvina jaredledvina commented Jul 24, 2026

Copy link
Copy Markdown
Member

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read [Submitting a pull request]
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section [Developer’s Certificate of Origin]
  • Provide a title or release-note blurb suitable for the release notes.
  • Disclose use of machine learning models (including LLMs and other generative AI)
    in accordance with the [Cilium AI Policy], and indicate the rating using
    [AI Influence Level].
    Example: "This PR was prepared with AIL:3. I personally checked X."
  • Thanks for contributing!

This PR was prepared with AIL:3. I personally checked the logic, tests, and build/CI results.

This PR was prepared with AIL:3. I personally checked the logic, tests, and build/CI results.

In AWS ENI IPAM mode, ENI attachment is asynchronous: the operator can report an
ENI as attached (and cilium-agent can hand out an IP from it) before the ENI's
netlink interface is visible on the node. Any code that resolves the ENI's
ifindex by MAC address immediately afterwards can lose that race and fail with
interface with MAC ... not found.

Upstream cilium#47295 has since merged and fixes the cilium-agent side
of this, in daemon/infraendpoints/infra_ip_allocation.go. It does not touch
the cilium-cni plugin, which is a separate binary with its own callers of
RoutingInfo.Configure(), and those are still exposed:

  • interfaceAdd in plugins/cilium-cni/cmd/interface.go configures ip rules and
    routes on every CNI ADD. When it loses the race, CNI ADD fails outright with
    unable to setup interface datapath: unable to install ip rules and routes: unable to find ifindex for interface MAC: interface with MAC ... not found
    and the pod is left in ContainerCreating.
  • the IPAMENI branch of Cmd.Add in plugins/cilium-cni/cmd/cmd.go resolves
    the master MAC via ifindexFromMac and only logs on failure, leaving
    ep.ParentInterfaceIndex at 0. That silently breaks IPv4 masquerade reply
    routing (nodeport_snat_fwd_ipv4) for the endpoint.

Upstream's guard cannot be reused as-is from the plugin: its waitForENI is an
unexported method on infraIPAllocator, unreachable from a separate binary.

This PR therefore:

  • Extracts the wait into an exported WaitForENIInterface(ctx, mac.MAC) in
    pkg/datapath/linux/routing/routing.go, so both binaries can share one
    implementation, and reduces the agent's waitForENI to a net.ParseMAC plus
    a delegation to it.
  • Applies the guard to both exposed CNI plugin paths above.
  • Adds tests in pkg/datapath/linux/routing/routing_test.go covering interface
    already present, interface appearing after a delay (the race being guarded),
    timeout, and context cancellation, plus a case for the new invalid-MAC error
    in daemon/infraendpoints.

The ingress-path hunk this PR originally carried has been dropped: upstream's
merged version is strictly better, because it also gates on IPAMAlibabaCloud
(the ingress Configure() runs in that mode too) and logs at Error rather
than Warn.

One item for reviewer attention. The shared backoff uses Steps: 7
(~15.75s worst case before jitter) where upstream's is Steps: 5 (~3.75s). The
longer window is intended for CNI ADD, where a real ENI attach can exceed a few
seconds and kubelet retries anyway, but because the constant is now shared it
also applies to the agent's router and ingress paths. Confirm that is wanted, or
say so and I will scope the longer backoff to the plugin only.

Verified: gofmt, GOOS=linux go build, GOOS=linux go vet and
golangci-lint (v2.12.2, repo pin) are clean on the three touched packages.
The new routing tests are TestPrivileged* and need Linux netns plus root, so
they have not run locally — they need CI or a Linux VM.

References: cilium#47295 (merged, agent side), cilium#41954,
cilium#45414

ipam/eni: wait for the ENI's netlink interface to appear before the cilium-cni plugin configures ip rules and routes, avoiding transient "interface with MAC ... not found" failures that could leave pods stuck in ContainerCreating.

@jaredledvina
jaredledvina force-pushed the jared.ledvina/fixup-eni-ifindex-race branch 7 times, most recently from c0a0a2d to fe893a6 Compare July 24, 2026 20:54
Signed-off-by: Jared Ledvina <jared.ledvina@datadoghq.com>
@jaredledvina
jaredledvina force-pushed the jared.ledvina/fixup-eni-ifindex-race branch from fe893a6 to b9037bc Compare July 28, 2026 15:57
@jaredledvina jaredledvina changed the title [ipam/eni] Wait for ENI interface before configuring datapath routes [ipam/eni] Wait for ENI interface in the CNI plugin before configuring routes Jul 28, 2026
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.

1 participant