Merge pull request #2094 from SergeyKanzhelev/runpodsandbox-state-contract-a-5a4f9333

RunPodSandbox NRI state contract and blocking behavior

Author: k8s-ci-robot

pkg/validate/nri_linux.go

@@ -17,9 +17,12 @@ limitations under the License.
 package validate
 
 import (
+	"context"
 	"fmt"
+	"sync"
 	"time"
 
+	nri "github.com/containerd/nri/pkg/api"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	internalapi "k8s.io/cri-api/pkg/apis"
@@ -32,14 +35,18 @@ import (
 var _ = framework.KubeDescribe("NRI", func() {
 	f := framework.NewDefaultCRIFramework()
 
-	var rc internalapi.RuntimeService
+	var (
+		rc internalapi.RuntimeService
+		ic internalapi.ImageManagerService
+	)
 
 	BeforeEach(func() {
 		if framework.TestContext.NRISocketPath == "" {
 			Skip("NRI socket not configured (use -nri-socket flag)")
 		}
 
 		rc = f.CRIClient.CRIRuntimeClient
+		ic = f.CRIClient.CRIImageClient
 	})
 
 	Context("pod sandbox lifecycle", Serial, func() {
@@ -149,4 +156,259 @@ var _ = framework.KubeDescribe("NRI", func() {
 			podID = ""
 		})
 	})
+
+	Context("RunPodSandbox contract", Serial, func() {
+		var (
+			testStub  *NRITestStub
+			podID     string
+			podConfig *runtimeapi.PodSandboxConfig
+		)
+
+		AfterEach(func(ctx SpecContext) {
+			// Capture the fallback sandbox ID before cleanup resets events.
+			cleanupID := podID
+			if cleanupID == "" && testStub != nil {
+				cleanupID = testStub.Plugin.LastRunPodSandboxID()
+			}
+
+			// Stop the stub to unblock any hooks that may be holding
+			// a RunPodSandbox call, allowing it to complete.
+			if testStub != nil {
+				testStub.Cleanup()
+			}
+
+			if cleanupID != "" {
+				if err := rc.StopPodSandbox(ctx, cleanupID); err != nil {
+					framework.Logf("AfterEach: StopPodSandbox(%s) failed: %v", cleanupID, err)
+				}
+
+				if err := rc.RemovePodSandbox(ctx, cleanupID); err != nil {
+					framework.Logf("AfterEach: RemovePodSandbox(%s) failed: %v", cleanupID, err)
+				}
+			}
+		})
+
+		It("should not expose sandbox while RunPodSandbox hook is in progress", func(ctx SpecContext) {
+			// This test validates the spec contract: during RunPodSandbox hook execution,
+			// the sandbox MUST NOT be visible via List or Status, and workload containers
+			// MUST NOT start.
+
+			// Channel to control blocking behavior
+			hookBlocking := make(chan struct{})
+			hookReached := make(chan struct{})
+
+			var hookPodID string
+
+			var err error
+
+			testStub, err = StartNRITestStub("cri-test-nri-block-run", "00")
+			Expect(err).NotTo(HaveOccurred(), "failed to start NRI test stub")
+
+			// Configure stub to block on RunPodSandbox
+			testStub.Plugin.OnRunPodSandbox = func(hookCtx context.Context, pod *nri.PodSandbox) error {
+				hookPodID = pod.GetId()
+
+				close(hookReached)
+				// Block until test signals to continue or context is cancelled (cleanup)
+				select {
+				case <-hookBlocking:
+				case <-hookCtx.Done():
+				}
+
+				return nil
+			}
+
+			By("triggering RunPodSandbox in a goroutine")
+
+			podSandboxName := "nri-test-block-run-" + framework.NewUUID()
+			uid := framework.DefaultUIDPrefix + framework.NewUUID()
+			namespace := framework.DefaultNamespacePrefix + framework.NewUUID()
+			podConfig = &runtimeapi.PodSandboxConfig{
+				Metadata: framework.BuildPodSandboxMetadata(podSandboxName, uid, namespace, framework.DefaultAttempt),
+				Linux: &runtimeapi.LinuxPodSandboxConfig{
+					CgroupParent: common.GetCgroupParent(ctx, rc),
+				},
+				Labels: framework.DefaultPodLabels,
+			}
+
+			var (
+				runErr   error
+				runPodID string
+				runWg    sync.WaitGroup
+			)
+
+			runWg.Go(func() {
+				runPodID, runErr = rc.RunPodSandbox(ctx, podConfig, framework.TestContext.RuntimeHandler)
+			})
+
+			By("waiting for RunPodSandbox hook to be reached")
+
+			select {
+			case <-hookReached:
+				// Hook is now blocking
+			case <-time.After(30 * time.Second):
+				close(hookBlocking) // unblock to avoid goroutine leak
+				Fail("Timed out waiting for RunPodSandbox NRI hook to fire")
+			}
+
+			By("verifying sandbox is NOT listed while hook is blocking")
+			// The sandbox should not appear in ListPodSandbox in any state
+			pods, listErr := rc.ListPodSandbox(ctx, nil)
+			Expect(listErr).NotTo(HaveOccurred())
+
+			sandboxFound := false
+
+			for _, pod := range pods {
+				if pod.GetId() == hookPodID {
+					sandboxFound = true
+
+					break
+				}
+			}
+
+			Expect(sandboxFound).To(BeFalse(),
+				"Sandbox %s MUST NOT be listed while RunPodSandbox hook is blocking", hookPodID)
+
+			By("verifying PodSandboxStatus is not accessible while hook is blocking")
+
+			if hookPodID != "" {
+				statusResp, statusErr := rc.PodSandboxStatus(ctx, hookPodID, false)
+				// Ideally the sandbox should not be found at all. Some runtimes may
+				// return a non-Ready status instead of NotFound — both are acceptable.
+				if statusErr == nil && statusResp != nil && statusResp.GetStatus() != nil {
+					Expect(statusResp.GetStatus().GetState()).NotTo(Equal(runtimeapi.PodSandboxState_SANDBOX_READY),
+						"Sandbox MUST NOT report Ready state while RunPodSandbox hook is in progress")
+				}
+			}
+
+			By("releasing the hook and verifying pod becomes Ready")
+			close(hookBlocking)
+			runWg.Wait()
+			Expect(runErr).NotTo(HaveOccurred(), "RunPodSandbox should succeed after hook returns")
+			Expect(runPodID).NotTo(BeEmpty())
+			podID = runPodID
+
+			// After hook completes, sandbox should be Ready
+			statusResp, err := rc.PodSandboxStatus(ctx, podID, false)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(statusResp.GetStatus().GetState()).To(Equal(runtimeapi.PodSandboxState_SANDBOX_READY),
+				"Sandbox should be Ready after RunPodSandbox hook completes")
+		})
+
+		It("should not start workload containers until RunPodSandbox hook completes", func(ctx SpecContext) {
+			// This test validates that workload container creation fails while the
+			// RunPodSandbox hook is still running. The NRI hook callback receives the
+			// sandbox ID, so we use that to attempt CreateContainer before RunPodSandbox
+			// returns. The container creation must fail because the sandbox is not ready.
+			hookBlocking := make(chan struct{})
+			hookReached := make(chan struct{})
+
+			var (
+				err       error
+				hookPodID string
+			)
+
+			testStub, err = StartNRITestStub("cri-test-nri-block-container", "00")
+			Expect(err).NotTo(HaveOccurred(), "failed to start NRI test stub")
+
+			// Configure stub to block RunPodSandbox and capture the sandbox ID
+			testStub.Plugin.OnRunPodSandbox = func(hookCtx context.Context, pod *nri.PodSandbox) error {
+				hookPodID = pod.GetId()
+
+				close(hookReached)
+
+				select {
+				case <-hookBlocking:
+				case <-hookCtx.Done():
+				}
+
+				return nil
+			}
+
+			By("pulling the test image before triggering RunPodSandbox")
+			framework.PullPublicImage(ctx, ic, framework.TestContext.TestImageList.DefaultTestContainerImage, nil)
+
+			By("triggering RunPodSandbox in a goroutine")
+
+			podSandboxName := "nri-test-block-container-" + framework.NewUUID()
+			uid := framework.DefaultUIDPrefix + framework.NewUUID()
+			namespace := framework.DefaultNamespacePrefix + framework.NewUUID()
+			podConfig = &runtimeapi.PodSandboxConfig{
+				Metadata: framework.BuildPodSandboxMetadata(podSandboxName, uid, namespace, framework.DefaultAttempt),
+				Linux: &runtimeapi.LinuxPodSandboxConfig{
+					CgroupParent: common.GetCgroupParent(ctx, rc),
+				},
+				Labels: framework.DefaultPodLabels,
+			}
+
+			var (
+				runErr   error
+				runPodID string
+				runWg    sync.WaitGroup
+			)
+
+			runWg.Go(func() {
+				runPodID, runErr = rc.RunPodSandbox(ctx, podConfig, framework.TestContext.RuntimeHandler)
+			})
+
+			By("waiting for RunPodSandbox hook to be reached")
+
+			select {
+			case <-hookReached:
+				// Hook is now blocking
+			case <-time.After(30 * time.Second):
+				close(hookBlocking)
+				Fail("Timed out waiting for RunPodSandbox NRI hook to fire")
+			}
+
+			By("attempting container creation while RunPodSandbox hook is blocking")
+
+			containerName := "nri-test-while-blocked-" + framework.NewUUID()
+			containerConfig := &runtimeapi.ContainerConfig{
+				Metadata: framework.BuildContainerMetadata(containerName, framework.DefaultAttempt),
+				Image: &runtimeapi.ImageSpec{
+					Image:              framework.TestContext.TestImageList.DefaultTestContainerImage,
+					UserSpecifiedImage: framework.TestContext.TestImageList.DefaultTestContainerImage,
+				},
+				Command: framework.DefaultPauseCommand,
+				Linux:   &runtimeapi.LinuxContainerConfig{},
+			}
+
+			blockedContainerID, createErr := rc.CreateContainer(ctx, hookPodID, containerConfig, podConfig)
+			Expect(createErr).To(HaveOccurred(),
+				"CreateContainer MUST fail while RunPodSandbox hook is in progress "+
+					"(sandbox %s is not ready)", hookPodID)
+			Expect(blockedContainerID).To(BeEmpty(),
+				"No container ID should be returned when creation fails")
+
+			By("releasing the hook and verifying pod becomes Ready")
+			close(hookBlocking)
+			runWg.Wait()
+			Expect(runErr).NotTo(HaveOccurred(), "RunPodSandbox should succeed after hook returns")
+			Expect(runPodID).NotTo(BeEmpty())
+			podID = runPodID
+
+			By("verifying container creation succeeds after hook completes")
+
+			containerName = "nri-test-after-hook-" + framework.NewUUID()
+			containerConfig = &runtimeapi.ContainerConfig{
+				Metadata: framework.BuildContainerMetadata(containerName, framework.DefaultAttempt),
+				Image:    &runtimeapi.ImageSpec{Image: framework.TestContext.TestImageList.DefaultTestContainerImage},
+				Command:  framework.DefaultPauseCommand,
+				Linux:    &runtimeapi.LinuxContainerConfig{},
+			}
+			containerID := framework.CreateContainer(ctx, rc, ic, containerConfig, podID, podConfig)
+			Expect(containerID).NotTo(BeEmpty(),
+				"Container creation should succeed after RunPodSandbox hook completes")
+
+			// Clean up container
+			if err := rc.StopContainer(ctx, containerID, 0); err != nil {
+				framework.Logf("StopContainer(%s) failed: %v", containerID, err)
+			}
+
+			if err := rc.RemoveContainer(ctx, containerID); err != nil {
+				framework.Logf("RemoveContainer(%s) failed: %v", containerID, err)
+			}
+		})
+	})
 })

pkg/validate/nri_util_linux.go

@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"slices"
 	"sync"
 	"time"
 
@@ -176,6 +177,30 @@ func (p *NRITestPlugin) Events() []NRIEvent {
 	return result
 }
 
+// Reset clears all recorded events.
+func (p *NRITestPlugin) Reset() {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	p.events = nil
+}
+
+// LastRunPodSandboxID returns the pod sandbox ID from the most recent RunPodSandbox event,
+// or empty string if none recorded. This is useful for cleanup in AfterEach when the test
+// may have failed before capturing the pod ID from the CRI call.
+func (p *NRITestPlugin) LastRunPodSandboxID() string {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	for i := range slices.Backward(p.events) {
+		if p.events[i].Type == EventRunPodSandbox {
+			return p.events[i].PodSandboxID
+		}
+	}
+
+	return ""
+}
+
 // WaitForEventCount waits until at least count events are recorded, or times out.
 func (p *NRITestPlugin) WaitForEventCount(count int, timeout time.Duration) ([]NRIEvent, error) {
 	deadline := time.Now().Add(timeout)
@@ -374,7 +399,8 @@ func (ts *NRITestStub) Stop() {
 	}
 }
 
-// Cleanup stops the stub.
+// Cleanup stops the stub and resets events.
 func (ts *NRITestStub) Cleanup() {
 	ts.Stop()
+	ts.Plugin.Reset()
 }