Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions pkg/container/kubernetes/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"io"
"log/slog"
"os"
"sort"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -357,11 +358,7 @@ func (c *Client) DeployWorkload(ctx context.Context,

attachStdio := options == nil || options.AttachStdio

// Convert environment variables to Kubernetes format
var envVarList []*corev1apply.EnvVarApplyConfiguration
for k, v := range envVars {
envVarList = append(envVarList, corev1apply.EnvVar().WithName(k).WithValue(v))
}
envVarList := buildSortedEnvVarList(envVars)

// Create a pod template spec
podTemplateSpec := ensureObjectMetaApplyConfigurationExists(corev1apply.PodTemplateSpec())
Expand Down Expand Up @@ -448,6 +445,23 @@ func (c *Client) DeployWorkload(ctx context.Context,
return 0, nil
}

// buildSortedEnvVarList converts an env var map to Kubernetes apply configurations
// with deterministically ordered keys. Go map iteration is randomized, and any
// ordering shift changes the pod template hash and triggers an unnecessary
// StatefulSet rollout (#5063).
func buildSortedEnvVarList(envVars map[string]string) []*corev1apply.EnvVarApplyConfiguration {
envKeys := make([]string, 0, len(envVars))
for k := range envVars {
envKeys = append(envKeys, k)
}
sort.Strings(envKeys)
envVarList := make([]*corev1apply.EnvVarApplyConfiguration, 0, len(envKeys))
for _, k := range envKeys {
envVarList = append(envVarList, corev1apply.EnvVar().WithName(k).WithValue(envVars[k]))
}
return envVarList
}

// runConfigGeneration extracts the RunConfig MCPServer generation from options,
// returning 0 when options is nil (backward-compat / non-operator callers).
func runConfigGeneration(options *runtime.DeployWorkloadOptions) int64 {
Expand Down
88 changes: 88 additions & 0 deletions pkg/container/kubernetes/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,94 @@ func TestDeployWorkloadCreatesBackendServices(t *testing.T) {
}
}

// TestDeployWorkloadEnvVarOrderingDeterministic verifies that DeployWorkload
// produces an env list in deterministic (sorted) order regardless of Go map
// iteration randomness. Without this, the StatefulSet pod template hash
// flapped between reconciles, triggering needless pod rollouts (#5063).
func TestDeployWorkloadEnvVarOrderingDeterministic(t *testing.T) {
t.Parallel()

containerName := "test-svc"
envVars := map[string]string{
"ULI_MCP_TOKEN": "tok",
"MCP_PORT": "8080",
"MCP_TRANSPORT": "streamable-http",
"MCP_HOST": "0.0.0.0",
"FASTMCP_PORT": "8080",
}

deploy := func() []corev1.EnvVar {
mockStatefulSet := &appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: containerName, Namespace: "default", UID: "test-uid",
},
Spec: appsv1.StatefulSetSpec{
Replicas: ptr.To(int32(1)),
Template: corev1.PodTemplateSpec{Spec: corev1.PodSpec{Containers: []corev1.Container{}}},
},
Status: appsv1.StatefulSetStatus{ReadyReplicas: 1},
}
clientset := fake.NewClientset(mockStatefulSet)
fakeConfig := &rest.Config{Host: "https://fake-k8s-api.example.com"}
mockDetector := &mockPlatformDetector{platform: PlatformKubernetes}

client := NewClientWithConfigAndPlatformDetector(clientset, fakeConfig, mockDetector)
client.waitForStatefulSetReadyFunc = mockWaitForStatefulSetReady
client.namespaceFunc = func() string { return "default" }

options := runtime.NewDeployWorkloadOptions()
options.PortBindings = map[string][]runtime.PortBinding{"8080/tcp": {{HostPort: "8080"}}}

_, err := client.DeployWorkload(
t.Context(),
"test-image",
containerName,
[]string{"serve"},
envVars,
map[string]string{"app": containerName},
nil,
"streamable-http",
options,
false,
)
require.NoError(t, err)

sts, err := clientset.AppsV1().StatefulSets("default").Get(t.Context(), containerName, metav1.GetOptions{})
require.NoError(t, err)
var mcp *corev1.Container
for i := range sts.Spec.Template.Spec.Containers {
if sts.Spec.Template.Spec.Containers[i].Name == mcpContainerName {
mcp = &sts.Spec.Template.Spec.Containers[i]
break
}
}
require.NotNil(t, mcp, "mcp container should exist on the StatefulSet")
return mcp.Env
}

// Sorted ordering is the contract; assert against it explicitly so the
// reason for the order is documented in the test, not just stable across runs.
expected := []string{"FASTMCP_PORT", "MCP_HOST", "MCP_PORT", "MCP_TRANSPORT", "ULI_MCP_TOKEN"}

first := deploy()
firstNames := make([]string, len(first))
for i, e := range first {
firstNames[i] = e.Name
}
assert.Equal(t, expected, firstNames, "first DeployWorkload should produce sorted env order")

// Run multiple times to catch any non-determinism that would slip through
// a single-call test (Go map iteration order is randomized per call).
for i := 0; i < 10; i++ {
got := deploy()
gotNames := make([]string, len(got))
for j, e := range got {
gotNames[j] = e.Name
}
assert.Equal(t, expected, gotNames, "iteration %d: env order must be deterministic", i)
}
}

// TestAttachToWorkloadExitFunc tests that the exit function is properly configured
// and can be mocked for testing
func TestAttachToWorkloadExitFunc(t *testing.T) {
Expand Down
Loading