Update NRI to latest (0.12.5) by MikeZappa87 · Pull Request #14453 · containerd/containerd

MikeZappa87 · 2026-10-22T17:22:46Z

Waiting on NRI tag release. I will see the work involved to add a e2e/integration test around this new behavior as well.

cc: @aojea @mikebrow

Copilot

🔵 Needs a closer look

The lifecycle failure-handling changes need maintainer validation, and the dependency update still awaits its upstream release tag.

No open issues 🎉

What changed in this PR

Updates containerd’s NRI dependency to a newer upstream snapshot while awaiting a release tag, bringing configurable plugin failure handling.

Changes:

Adds failure policies controlling request failure and plugin retention after timeouts.
Updates the API, generated serialization, plugin stub, and lifecycle handling.
Replaces the semantic-version dependency with local parsing and comparison logic.

File	Description
vendor/modules.txt	Records the updated NRI version.
vendor/github.com/containerd/nri/README.md	Documents failure policies and timeout behavior.
vendor/github.com/containerd/nri/pkg/version/version.go	Implements local semantic-version handling.
vendor/github.com/containerd/nri/pkg/stub/stub.go	Allows plugins to declare failure policies.
vendor/github.com/containerd/nri/pkg/api/api.proto	Defines failure-policy protocol fields.
vendor/github.com/containerd/nri/pkg/api/api_vtproto.pb.go	Adds failure-policy serialization support.
vendor/github.com/containerd/nri/pkg/adaptation/plugin.go	Applies policies to lifecycle request failures.
vendor/github.com/containerd/nri/pkg/adaptation/api.go	Exposes the failure-policy type alias.
go.sum	Adds checksums for the NRI snapshot.
go.mod	Updates the NRI dependency requirement.

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The upstream release tag remains pending, and lifecycle failure-handling changes need confirmed integration results and human review.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The lifecycle failure-handling changes need runtime validation and maintainer review, and the upstream release tag remains pending.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Lifecycle failure and connection-retention changes need human review supported by live integration results that were not verified here.

1 open issue

Register sandbox cleanup before asserting expected run failure · New

_{Review effort: Balanced}

MikeZappa87 · 2026-10-23T01:07:21Z

@aojea

containerd/internal/cri/server/sandbox_stop.go

Line 101 in 57d315c

err := c.nri.StopPodSandbox(ctx, &sandbox)

If the StopPodSandbox NRI rpc returns error it doesn't block which is different behavior than the RunPodSandbox which stops the process.

Copilot

🔵 Needs a closer look

Lifecycle timeout and disconnection changes need human confirmation with the privileged integration suite, whose results remain unverified.

No open issues 🎉

1 resolved since last review

Register sandbox cleanup before asserting expected run failure

_{Review effort: Balanced}

MikeZappa87 · 2026-10-23T01:16:32Z

🔵 Needs a closer look

Lifecycle timeout and disconnection changes need human confirmation with the privileged integration suite, whose results remain unverified.

No open issues 🎉

1 resolved since last review

Register sandbox cleanup before asserting expected run failure

Review effort: Balanced

integration test results:

--- PASS: TestNriPluginFailurePolicy (9.95s)
--- PASS: TestNriPluginFailurePolicy/default_policy_drops_plugin_and_proceeds (2.50s)
--- PASS: TestNriPluginFailurePolicy/fail_request_and_drop_plugin (2.32s)
--- PASS: TestNriPluginFailurePolicy/keep_plugin_and_proceed (2.63s)
--- PASS: TestNriPluginFailurePolicy/fail_request_and_keep_plugin_connected (2.50s)
PASS

aojea · 2026-10-23T11:02:01Z

failures are legit, sending patch to NRI, there was a latent bug that before worked by accident caused by the lazy cleanup done via defer r.removeClosedPlugins():

When a plugin disconnected while idle (p.closed = true), the next CRI request still called p.impl.() on the closed p.rpcc and got ttrpc.ErrClosed.
Because handleRequestError always returned nil on ttrpc.ErrClosed, the error was ignored and defer r.removeClosedPlugins() cleaned up the plugin after the RPC finished.

With FailRequest: true, calling p.impl.() on a plugin that NRI already knows is closed (p.isClosed() == true via WithOnClose) turns a prior idle disconnect into a ttrpc: closed error on the next CRI request.

When a plugin disconnects while no request is in progress, the adaptation marks it closed but keeps it in the plugin list. The list is only cleaned up at the end of the next request. That next request is still relayed to the closed plugin and fails with "ttrpc: closed". Before failure policies, that error was always swallowed and the request proceeded. With fail_request, it fails the request for a plugin that is already gone. The same happens for validators, independent of any policy: the next CreateContainer fails with "validator plugin ... failed: ttrpc: closed". In both cases the failure happens exactly once, because the same request then removes the plugin. Skip closed plugins when relaying a request, in the lifecycle loops and in the validators loop. The check is in the adaptation and not in the plugin relay methods on purpose: in CreateContainer, the plugin list reported to the validators is built before the relay. A closed plugin must not be reported as present, so that the required plugins check of the default validator rejects the container instead of passing it. This was found by the containerd integration tests when updating NRI to v0.12.4 (containerd/containerd#14453). A test stops a fail_request plugin, and the first RunPodSandbox of the next test failed with "plugin 00-timeout-plugin failed to handle RunPodSandbox request: ttrpc: closed": https://github.com/containerd/containerd/actions/runs/39886249632/job/120339658097 Tests cover each of the 13 lifecycle requests as the first request after an idle disconnect, a validator that disconnects while idle, a plugin that disconnects during a request before its turn, and the required plugins check catching a plugin that disconnected while idle. Signed-off-by: Antonio Ojea <aojea@google.com>

MikeZappa87 · 2026-10-23T14:34:26Z

@aojea @mikebrow this branch has nri pointed at main to verify behavior before tagging NRI again

Copilot

🔵 Needs a closer look

The temporary fork dependency and lifecycle failure-handling changes need maintainer validation against the final upstream release.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The lifecycle changes need maintainer review and integration validation, particularly the unresolved idle-disconnect request failure.

No open issues 🎉

_{Review effort: Balanced}

aojea · 2026-10-23T15:14:36Z

I think this is not getting the latest fix go get github.com/containerd/nri@5df40b41f7a269412ea74505265b9142c4d4ecb5 go mod tidy && go mod vendor

MikeZappa87 · 2026-10-23T15:21:00Z

I think this is not getting the latest fix `go get github.com/containerd/nri@5df40b41f7a269412ea74505265b9142c4d4ecb5

go mod tidy && go mod vendor`

I'll fix when I get back to my desk. I did @main

Copilot

🔵 Needs a closer look

The prerelease update changes lifecycle failure propagation and connection handling, warranting maintainer confirmation of integration coverage and release readiness.

No open issues 🎉

_{Review effort: Balanced}

aojea · 2026-10-23T17:30:54Z

Fixed 👏

mikebrow · 2026-10-23T18:42:43Z

https://github.com/containerd/nri/releases/tag/v0.12.5

Signed-off-by: Michael Zappa <michael.zappa@gmail.com>

Copilot

🟡 Changes recommended

Retaining an unresponsive plugin connection can leave transport writes blocked indefinitely while holding the shared NRI lifecycle lock.

No open issues 🎉

_{Review effort: Balanced}

mikebrow

LGTM

aojea · 2026-10-24T19:15:36Z

LGTM

kubernetes-prow Bot added the do-not-merge/work-in-progress label Oct 22, 2026

github-project-automation Bot added this to Pull Request Review Oct 22, 2026

Copilot AI balanced review requested due to automatic review settings October 22, 2026 17:22

github-project-automation Bot moved this to Needs Triage in Pull Request Review Oct 22, 2026

kubernetes-prow Bot added the size/XXL label Oct 22, 2026

MikeZappa87 mentioned this pull request Oct 22, 2026

Failure policy containerd/nri#329

Merged

Copilot started reviewing on behalf of MikeZappa87 October 22, 2026 17:26

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

MikeZappa87 force-pushed the mzappa/nriversionbump branch from d111543 to 1770f27 Compare October 22, 2026 17:51

Copilot AI balanced review requested due to automatic review settings October 22, 2026 17:51

Copilot started reviewing on behalf of MikeZappa87 October 22, 2026 17:51

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

MikeZappa87 force-pushed the mzappa/nriversionbump branch 2 times, most recently from 5a64b10 to a8508c8 Compare October 22, 2026 19:55

Copilot AI balanced review requested due to automatic review settings October 22, 2026 19:55

Copilot started reviewing on behalf of MikeZappa87 October 22, 2026 19:56

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 23, 2026 00:10

MikeZappa87 force-pushed the mzappa/nriversionbump branch from a8508c8 to 6c0f2b9 Compare October 23, 2026 00:10

MikeZappa87 marked this pull request as ready for review October 23, 2026 00:10

MikeZappa87 changed the title ~~[WIP] Update NRI to latest (Waiting on NRI tag)~~ Update NRI to latest (0.12.4) Oct 23, 2026

Copilot started reviewing on behalf of MikeZappa87 October 23, 2026 00:11

View session

kubernetes-prow Bot removed the do-not-merge/work-in-progress label Oct 23, 2026