fix(#3448) - vMCP operator deterministically orders servers

pkg/vmcp/aggregator/aggregator.go

@@ -21,6 +21,7 @@ import (
 type BackendDiscoverer interface {
 	// Discover finds all backend workloads in the specified group.
 	// Returns only healthy/running backends.
+	// Results are always sorted alphabetically by backend name to ensure deterministic ordering.
 	// The groupRef format is platform-specific (group name for CLI, MCPGroup name for K8s).
 	Discover(ctx context.Context, groupRef string) ([]vmcp.Backend, error)
 }

pkg/vmcp/aggregator/discoverer.go

@@ -14,6 +14,7 @@ package aggregator
 import (
 	"context"
 	"fmt"
+	"sort"
 
 	rt "github.com/stacklok/toolhive/pkg/container/runtime"
 	"github.com/stacklok/toolhive/pkg/groups"
@@ -120,13 +121,26 @@ func NewBackendDiscovererWithManager(
 //
 // In static mode (when staticBackends are configured), this returns pre-configured backends
 // without any K8s API access. In dynamic mode, it discovers backends at runtime.
-func (d *backendDiscoverer) Discover(ctx context.Context, groupRef string) ([]vmcp.Backend, error) {
+//
+// Results are always sorted alphabetically by backend name to ensure deterministic ordering.
+// This prevents non-deterministic ConfigMap content that would cause unnecessary
+// deployment rollouts (pod cycling). See: https://github.com/stacklok/toolhive/issues/3448
+func (d *backendDiscoverer) Discover(ctx context.Context, groupRef string) (backends []vmcp.Backend, err error) {
+	// Sort backends by name before returning to ensure deterministic ordering
+	defer func() {
+		if len(backends) > 1 {
+			sort.Slice(backends, func(i, j int) bool {
+				return backends[i].Name < backends[j].Name
+			})
+		}
+	}()
+
 	logger.Infof("Discovering backends in group %s", groupRef)
 
 	// Static mode: Use pre-configured backends if available
 	if len(d.staticBackends) > 0 {
 		logger.Infof("Using %d pre-configured static backends (no K8s API access)", len(d.staticBackends))
-		return d.discoverFromStaticConfig()
+		return d.discoverFromStaticConfig(), nil
 	}
 
 	// If staticBackends was explicitly set (even if empty), but groupsManager is nil,
@@ -163,7 +177,6 @@ func (d *backendDiscoverer) Discover(ctx context.Context, groupRef string) ([]vm
 	logger.Debugf("Found %d workloads in group %s, discovering backends", len(typedWorkloads), groupRef)
 
 	// Query each workload and convert to backend
-	var backends []vmcp.Backend
 	for _, workload := range typedWorkloads {
 		backend, err := d.workloadsManager.GetWorkloadAsVMCPBackend(ctx, workload)
 		if err != nil {
@@ -247,7 +260,7 @@ func (d *backendDiscoverer) applyAuthConfigToBackend(backend *vmcp.Backend, back
 
 // discoverFromStaticConfig converts pre-configured static backends into vmcp.Backend objects
 // for use in static mode where no K8s API access is available.
-func (d *backendDiscoverer) discoverFromStaticConfig() ([]vmcp.Backend, error) {
+func (d *backendDiscoverer) discoverFromStaticConfig() []vmcp.Backend {
 	backends := make([]vmcp.Backend, 0, len(d.staticBackends))
 
 	for _, staticBackend := range d.staticBackends {
@@ -279,5 +292,5 @@ func (d *backendDiscoverer) discoverFromStaticConfig() ([]vmcp.Backend, error) {
 			staticBackend.Name, staticBackend.URL, staticBackend.Transport)
 	}
 
-	return backends, nil
+	return backends
 }

pkg/vmcp/aggregator/discoverer_test.go

@@ -552,15 +552,15 @@ func TestBackendDiscoverer_Discover(t *testing.T) {
 		require.NoError(t, err)
 		require.Len(t, backends, 2)
 
-		// Verify MCPServer backend
-		assert.Equal(t, "server1", backends[0].ID)
-		assert.Equal(t, "streamable-http", backends[0].TransportType)
-		assert.Equal(t, "github", backends[0].Metadata["tool_type"])
+		// Backends are sorted alphabetically by name
+		// proxy1 comes before server1 alphabetically
+		assert.Equal(t, "proxy1", backends[0].ID)
+		assert.Equal(t, "sse", backends[0].TransportType)
+		assert.Equal(t, "mcp", backends[0].Metadata["tool_type"])
 
-		// Verify MCPRemoteProxy backend
-		assert.Equal(t, "proxy1", backends[1].ID)
-		assert.Equal(t, "sse", backends[1].TransportType)
-		assert.Equal(t, "mcp", backends[1].Metadata["tool_type"])
+		assert.Equal(t, "server1", backends[1].ID)
+		assert.Equal(t, "streamable-http", backends[1].TransportType)
+		assert.Equal(t, "github", backends[1].Metadata["tool_type"])
 	})
 
 	t.Run("applies authentication to MCPRemoteProxy backends", func(t *testing.T) {
@@ -1308,3 +1308,137 @@ func TestStaticBackendDiscoverer_MetadataGroupOverride(t *testing.T) {
 		})
 	}
 }
+
+// TestBackendDiscoverer_Discover_DeterministicOrdering tests that Discover returns backends
+// in a deterministic order (sorted alphabetically by name) regardless of input order.
+// This prevents non-deterministic ConfigMap content that would cause unnecessary
+// deployment rollouts (pod cycling). See: https://github.com/stacklok/toolhive/issues/3448
+func TestBackendDiscoverer_Discover_DeterministicOrdering(t *testing.T) {
+	t.Parallel()
+
+	// Test with multiple different input orders to ensure output is always sorted
+	testCases := []struct {
+		name           string
+		staticBackends []config.StaticBackendConfig
+	}{
+		{
+			name: "reverse alphabetical order (zebra, middle, alpha)",
+			staticBackends: []config.StaticBackendConfig{
+				{Name: "zebra-backend", URL: "http://zebra:8080", Transport: "sse"},
+				{Name: "middle-backend", URL: "http://middle:8080", Transport: "streamable-http"},
+				{Name: "alpha-backend", URL: "http://alpha:8080", Transport: "sse"},
+			},
+		},
+		{
+			name: "alphabetical order (alpha, middle, zebra)",
+			staticBackends: []config.StaticBackendConfig{
+				{Name: "alpha-backend", URL: "http://alpha:8080", Transport: "sse"},
+				{Name: "middle-backend", URL: "http://middle:8080", Transport: "streamable-http"},
+				{Name: "zebra-backend", URL: "http://zebra:8080", Transport: "sse"},
+			},
+		},
+		{
+			name: "random order (middle, zebra, alpha)",
+			staticBackends: []config.StaticBackendConfig{
+				{Name: "middle-backend", URL: "http://middle:8080", Transport: "streamable-http"},
+				{Name: "zebra-backend", URL: "http://zebra:8080", Transport: "sse"},
+				{Name: "alpha-backend", URL: "http://alpha:8080", Transport: "sse"},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			ctx := context.Background()
+
+			discoverer := NewUnifiedBackendDiscovererWithStaticBackends(
+				tc.staticBackends,
+				nil, // No auth config needed for this test
+				"test-group",
+			)
+
+			backends, err := discoverer.Discover(ctx, "test-group")
+			require.NoError(t, err)
+
+			// Output should ALWAYS be alphabetically sorted regardless of input order
+			require.Len(t, backends, 3, "should include all valid backends")
+			assert.Equal(t, "alpha-backend", backends[0].Name,
+				"first backend should be alpha-backend (alphabetically first)")
+			assert.Equal(t, "middle-backend", backends[1].Name,
+				"second backend should be middle-backend (alphabetically second)")
+			assert.Equal(t, "zebra-backend", backends[2].Name,
+				"third backend should be zebra-backend (alphabetically third)")
+		})
+	}
+}
+
+// TestBackendDiscoverer_Discover_DeterministicOrdering_DynamicMode tests that Discover
+// returns backends in deterministic order when using dynamic mode (K8s API discovery).
+func TestBackendDiscoverer_Discover_DeterministicOrdering_DynamicMode(t *testing.T) {
+	t.Parallel()
+
+	ctrl := gomock.NewController(t)
+	t.Cleanup(ctrl.Finish)
+
+	mockWorkloadDiscoverer := discoverermocks.NewMockDiscoverer(ctrl)
+	mockGroups := mocks.NewMockManager(ctrl)
+
+	// Create backends in non-alphabetical order to test sorting
+	backend1 := &vmcp.Backend{
+		ID:            "zebra-backend",
+		Name:          "zebra-backend",
+		BaseURL:       "http://zebra:8080/mcp",
+		TransportType: "sse",
+		HealthStatus:  vmcp.BackendHealthy,
+	}
+	backend2 := &vmcp.Backend{
+		ID:            "alpha-backend",
+		Name:          "alpha-backend",
+		BaseURL:       "http://alpha:8080/mcp",
+		TransportType: "streamable-http",
+		HealthStatus:  vmcp.BackendHealthy,
+	}
+	backend3 := &vmcp.Backend{
+		ID:            "middle-backend",
+		Name:          "middle-backend",
+		BaseURL:       "http://middle:8080/mcp",
+		TransportType: "sse",
+		HealthStatus:  vmcp.BackendHealthy,
+	}
+
+	mockGroups.EXPECT().Exists(gomock.Any(), testGroupName).Return(true, nil)
+	// Return workloads in non-alphabetical order (zebra, alpha, middle)
+	mockWorkloadDiscoverer.EXPECT().ListWorkloadsInGroup(gomock.Any(), testGroupName).
+		Return([]workloads.TypedWorkload{
+			{Name: "zebra-backend", Type: workloads.WorkloadTypeMCPServer},
+			{Name: "alpha-backend", Type: workloads.WorkloadTypeMCPServer},
+			{Name: "middle-backend", Type: workloads.WorkloadTypeMCPServer},
+		}, nil)
+	mockWorkloadDiscoverer.EXPECT().GetWorkloadAsVMCPBackend(
+		gomock.Any(),
+		workloads.TypedWorkload{Name: "zebra-backend", Type: workloads.WorkloadTypeMCPServer},
+	).Return(backend1, nil)
+	mockWorkloadDiscoverer.EXPECT().GetWorkloadAsVMCPBackend(
+		gomock.Any(),
+		workloads.TypedWorkload{Name: "alpha-backend", Type: workloads.WorkloadTypeMCPServer},
+	).Return(backend2, nil)
+	mockWorkloadDiscoverer.EXPECT().GetWorkloadAsVMCPBackend(
+		gomock.Any(),
+		workloads.TypedWorkload{Name: "middle-backend", Type: workloads.WorkloadTypeMCPServer},
+	).Return(backend3, nil)
+
+	discoverer := NewUnifiedBackendDiscoverer(mockWorkloadDiscoverer, mockGroups, nil)
+	backends, err := discoverer.Discover(context.Background(), testGroupName)
+
+	require.NoError(t, err)
+	require.Len(t, backends, 3)
+
+	// Backends should be sorted alphabetically by name
+	assert.Equal(t, "alpha-backend", backends[0].Name,
+		"first backend should be alpha-backend (alphabetically first)")
+	assert.Equal(t, "middle-backend", backends[1].Name,
+		"second backend should be middle-backend (alphabetically second)")
+	assert.Equal(t, "zebra-backend", backends[2].Name,
+		"third backend should be zebra-backend (alphabetically third)")
+}