Merge pull request #12036 from DataDog/eric.mountain/stable-rh-order

fix:cri: Stable order for StatusResponse.RuntimeHandlers

internal/cri/server/status.go

@@ -23,6 +23,7 @@ import (
 	"maps"
 	goruntime "runtime"
 	"slices"
+	"sort"
 
 	"github.com/containerd/containerd/api/services/introspection/v1"
 	"github.com/containerd/log"
@@ -59,9 +60,15 @@ func (c *criService) Status(ctx context.Context, r *runtime.StatusRequest) (*run
 			runtimeCondition,
 			networkCondition,
 		}},
-		RuntimeHandlers: slices.Collect(maps.Values(c.runtimeHandlers)),
-		Features:        c.runtimeFeatures,
+		Features: c.runtimeFeatures,
 	}
+
+	// Ensure stable ordering of runtime handlers in response
+	resp.RuntimeHandlers = slices.Collect(maps.Values(c.runtimeHandlers))
+	sort.SliceStable(resp.RuntimeHandlers, func(i, j int) bool {
+		return resp.RuntimeHandlers[i].Name < resp.RuntimeHandlers[j].Name
+	})
+
 	if r.Verbose {
 		configByt, err := json.Marshal(c.config)
 		if err != nil {

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,86 @@ 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
+		}
+	}
+
+	// Fail if runtime handlers order varies across calls to Status()
+	assert.True(t, sameOrder, "RuntimeHandlers order is unstable across Status() calls")
+}