cri: reject CreateContainer when sandbox is not running#13654
Conversation
Before this fix, CreateContainer would proceed even if the sandbox
had already stopped or was in an unknown state. This could result in
containers being created in an unusable sandbox, leading to confusing
errors downstream.
StartContainer already guards with the same check:
if sandbox.Status.Get().State != sandboxstore.StateReady {
return nil, fmt.Errorf("sandbox container %q is not running", ...)
}
Apply the identical guard in CreateContainer, immediately after the
sandbox state is known, so that callers receive a clear error instead
of a partial container object.
Fixes #13599
Signed-off-by: crawfordxx <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR updates the CRI CreateContainer path to enforce the same sandbox-running guard already used by StartContainer, ensuring CreateContainer fails when the target pod sandbox is not in StateReady (running), per CRI expectations.
Changes:
- Add a
StateReadyguard inCreateContainerto reject requests targeting non-running sandboxes with a clear error. - Align the sandbox store import naming (
sandboxstore) and update the internal request struct field type accordingly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if sandbox.Status.Get().State != sandboxstore.StateReady { | ||
| return nil, fmt.Errorf("sandbox container %q is not running", sandboxID) | ||
| } |
|
/cherry-pick release/2.3 |
|
@AkihiroSuda: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@AkihiroSuda: new pull request created: #13668 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@AkihiroSuda: new pull request created: #13669 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
What does this PR do?
CreateContainercurrently proceeds without checking whether the sandbox is in a running (StateReady) state. If the sandbox has already stopped or is in an unknown state, the call silently creates a container shell that is unusable.StartContaineralready has the correct guard (added in the existing code):This PR applies the identical check in
CreateContainer, right after the sandbox state is resolved, so callers receive an actionable error instead of a dangling container.Why is it needed?
The CRI specification requires
CreateContainerto fail when the target sandbox is not running. Thecri-toolstest at kubernetes-sigs/cri-tools#2126 exercises this exact case and currently fails against containerd.Fixes #13599
Testing
container_create_test.gounit tests continue to pass (they exercisebuildContainerSpec, which is unaffected).cri-toolsconformance test referenced in CreateContainer CRI API should be rejected on stopped sandbox #13599 will now pass with this guard in place.