Test showing RuntimeHandlers in Status() are unordered

EricMountain · k8s-infra-cherrypick-robot · commit a2fd70639e6a · 2025-07-02T00:52:55.000Z
Signed-off-by: Eric Mountain &lt;eric.mountain@datadoghq.com&gt;
diff --git a/internal/cri/server/status_test.go b/internal/cri/server/status_test.go
@@ -17,9 +17,15 @@
 package server
 
 import (
+	"context"
+	"reflect"
 	"testing"
+	"unsafe"
 
 	"github.com/containerd/containerd/api/services/introspection/v1"
+	containerd "github.com/containerd/containerd/v2/client"
+	coreintrospection "github.com/containerd/containerd/v2/core/introspection"
+	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 )
@@ -48,3 +54,87 @@ func TestRuntimeConditionContainerdHasNoDeprecationWarnings(t *testing.T) {
 		Status: true,
 	}, cond)
 }
+
+// fakeIntrospectionService is a minimal stub that implements the
+// coreintrospection.Service. We need this because criService.Status()
+// invokes the client.IntrospectionService() method.
+type fakeIntrospectionService struct{}
+
+var _ coreintrospection.Service = fakeIntrospectionService{}
+
+func (fakeIntrospectionService) Plugins(ctx context.Context, _ ...string) (*introspection.PluginsResponse, error) {
+	return &introspection.PluginsResponse{}, nil
+}
+
+func (fakeIntrospectionService) Server(ctx context.Context) (*introspection.ServerResponse, error) {
+	return &introspection.ServerResponse{}, nil
+}
+
+func (fakeIntrospectionService) PluginInfo(ctx context.Context, _ string, _ string, _ any) (*introspection.PluginInfoResponse, error) {
+	return &introspection.PluginInfoResponse{}, nil
+}
+
+// newFakeContainerdClient returns a *containerd.Client with a stub
+// IntrospectionService injected via reflection. This avoids needing a real
+// gRPC connection while satisfying criService.Status().
+func newFakeContainerdClient() *containerd.Client {
+	c := &containerd.Client{}
+	sv := reflect.ValueOf(c).Elem().FieldByName("services")
+	if !sv.IsValid() {
+		return c
+	}
+	f := sv.FieldByName("introspectionService")
+	if !f.IsValid() {
+		return c
+	}
+	// Make the unexported/private field introspectionService settable
+	f = reflect.NewAt(f.Type(), unsafe.Pointer(f.UnsafeAddr())).Elem()
+	f.Set(reflect.ValueOf(fakeIntrospectionService{}).Convert(f.Type()))
+	return c
+}
+
+// newStatusTestCRIService creates a minimal CRI service for testing
+func newStatusTestCRIService() *criService {
+	return &criService{
+		client:          newFakeContainerdClient(),
+		runtimeHandlers: make(map[string]*runtime.RuntimeHandler),
+	}
+}
+
+// TestStatusRuntimeHandlersOrdering checks that the runtime handlers
+// returned by Status() are in the same order every time
+func TestStatusRuntimeHandlersOrdering(t *testing.T) {
+	c := newStatusTestCRIService()
+
+	// Forge many runtime handlers to lower risk of accidental stable
+	// ordering on consecutive Status() calls
+	const numHandlers = 100
+	handlers := make(map[string]*runtime.RuntimeHandler, numHandlers)
+	for range numHandlers {
+		h := &runtime.RuntimeHandler{Name: "random-" + uuid.New().String()}
+		handlers[h.Name] = h
+	}
+	c.runtimeHandlers = handlers
+
+	// Call Status() twice
+	resp1, err := c.Status(context.Background(), &runtime.StatusRequest{})
+	assert.NoError(t, err)
+	assert.Len(t, resp1.RuntimeHandlers, len(handlers), "Unexpected number of runtime handlers")
+
+	resp2, err := c.Status(context.Background(), &runtime.StatusRequest{})
+	assert.NoError(t, err)
+	assert.Len(t, resp2.RuntimeHandlers, len(handlers), "Unexpected number of runtime handlers")
+
+	// Check runtime handlers are in the same order
+	sameOrder := true
+	for i := 0; i < len(resp1.RuntimeHandlers); i++ {
+		if resp1.RuntimeHandlers[i].Name != resp2.RuntimeHandlers[i].Name {
+			sameOrder = false
+			break
+		}
+	}
+
+	// The test will fail if the order is the same, showing that the ordering is stable
+	// In the current implementation, order is not stable
+	assert.False(t, sameOrder, "RuntimeHandlers order is stable across Status() calls - unexpected")
+}
