diff --git a/internal/cri/server/status.go b/internal/cri/server/status.go index a888724458..4e9722dfae 100644 --- a/internal/cri/server/status.go +++ b/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 { diff --git a/internal/cri/server/status_test.go b/internal/cri/server/status_test.go index efd380b9b1..6ab9ffc77f 100644 --- 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,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") +}