Description

Write unit tests covering all new operator behaviors introduced by TASK-004, TASK-005, TASK-006, and TASK-007: spec-driven replica setting (nil-passthrough and explicit values), stdio transport hard cap, session storage warning conditions, Redis env var injection via SecretKeyRef, backendReplicas passthrough in RunConfig, and terminationGracePeriodSeconds presence on generated Deployments. The existing mcpserver_replicas_test.go covers externally-driven (HPA) replica preservation; new test cases here cover the distinct spec-driven code paths added by the upstream tasks.

Context

Epic THV-0047 adds horizontal scaling support to the ToolHive operator by extending MCPServer and VirtualMCPServer CRDs with replica counts, session storage configuration, and Redis injection. The upstream tasks (TASK-004 through TASK-007) implement the operator-side changes but delegate formal test coverage to this task. Without these tests, the new code paths — nil-passthrough replicas, stdio cap enforcement, session storage warning conditions, Redis env var injection, and RunConfig population — have no automated verification. This task closes that gap.

Dependencies: #280 (TASK-004 — MCPServer reconciler/deployment builder), #281 (TASK-005 — MCPServer RunConfig Redis/BackendReplicas), #278 (TASK-006 — VirtualMCPServer deployment builder), #277 (TASK-007 — VirtualMCPServer vMCP ConfigMap Redis injection)
Blocks: None (this is the final task in the DAG)

Acceptance Criteria

• mcpserver_replicas_test.go is extended with spec-driven replica test cases: spec.replicas == nil yields Deployment.Spec.Replicas == nil; spec.replicas = 1 yields *Deployment.Spec.Replicas == 1; spec.replicas = 3 with transport sse yields *Deployment.Spec.Replicas == 3
• A test for the stdio transport hard cap verifies that the reconciler produces a Deployment with *Spec.Replicas == 1 and sets a StdioReplicaCapped status condition when spec.transport == "stdio" and spec.replicas = 3
• A test verifies the session storage warning condition (SessionStorageWarning) is set with status True when spec.replicas = 2 and spec.sessionStorage is nil
• A test verifies the session storage warning condition is set with status True when spec.replicas = 2 and spec.sessionStorage.provider == "memory"
• A test verifies the session storage warning condition is set with status False when spec.replicas = 2 and spec.sessionStorage.provider == "redis" (condition cleared)
• Tests for createRunConfigFromMCPServer verify: backendReplicas = nil yields nil RunConfig.BackendReplicas; backendReplicas = 3 yields RunConfig.BackendReplicas pointing to int32(3); sessionStorage.provider == "redis" populates RunConfig.SessionRedis with correct address, db, and keyPrefix; sessionStorage.provider == "memory" yields nil RunConfig.SessionRedis; passwordRef is NOT present anywhere in the serialized RunConfig
• Tests for deploymentForMCPServer (proxyrunner pod) verify: THV_SESSION_REDIS_PASSWORD env var is injected with correct ValueFrom.SecretKeyRef fields when sessionStorage.provider == "redis" and passwordRef is set; no THV_SESSION_REDIS_PASSWORD env var appears when sessionStorage is nil
• Tests for deploymentForVirtualMCPServer verify: THV_SESSION_REDIS_PASSWORD env var is injected correctly when sessionStorage.provider == "redis" and passwordRef is set; no THV_SESSION_REDIS_PASSWORD env var when sessionStorage is nil or provider != "redis" or passwordRef is nil
• Tests for deploymentForMCPServer and deploymentForVirtualMCPServer verify that Spec.Template.Spec.TerminationGracePeriodSeconds is non-nil and set to the expected constant value (e.g., 30 seconds) in all cases
• Tests for deploymentForVirtualMCPServer verify the nil-passthrough replica behavior: spec.replicas == nil yields Spec.Replicas == nil; spec.replicas = 1 yields *Spec.Replicas == 1; spec.replicas = 3 yields *Spec.Replicas == 3
• Tests for ensureVmcpConfigConfigMap (or the converter) verify: Redis address, db, and keyPrefix appear in the marshaled config.yaml when provider == "redis"; the config.yaml omits sessionStorage entirely when provider == "memory" or spec.sessionStorage == nil; no password field appears in config.yaml
• All pre-existing tests in mcpserver_replicas_test.go continue to pass — the new spec-driven tests do not interfere with the HPA-scenario tests
• go test ./cmd/thv-operator/... ./pkg/runner/... ./pkg/vmcp/config/... passes without failures
• Code reviewed and approved

Technical Approach

Recommended Implementation

The new tests span four existing test files (and may introduce one or two new files if the scope warrants it). The primary organization is:

1. Extend mcpserver_replicas_test.go with spec-driven replica cases

The existing TestReplicaBehavior table-driven test covers externally-driven scaling (HPA scenario: the deployment already has N replicas, the reconciler runs). The new tests cover the complementary path: the MCPServer spec itself sets Replicas, and the deployment builder/reconciler must respect it. Since these are fundamentally different code paths (not just different table rows), add them as new top-level test functions rather than extending the existing table:

func TestSpecDrivenReplicasNil(t *testing.T) { ... }
func TestSpecDrivenReplicasExplicit(t *testing.T) { ... }   // table with 1 and 3
func TestSpecDrivenStdioCapAt1(t *testing.T) { ... }
func TestSessionStorageWarningCondition(t *testing.T) { ... }  // table: nil, memory, redis
func TestTerminationGracePeriodMCPServer(t *testing.T) { ... }

The setup pattern mirrors TestDefaultCreationHasOneReplica: create an MCPServer with the desired spec (including new Replicas and SessionStorage fields), create a fake client with fake.NewClientBuilder(), instantiate newTestMCPServerReconciler, call reconciler.Reconcile, fetch the updated Deployment, and assert on Spec.Replicas, Spec.Template.Spec.TerminationGracePeriodSeconds, and the MCPServer's Status.Conditions.

For condition assertions, fetch the updated MCPServer after reconcile and use apimeta.FindStatusCondition to check the condition type, status, and reason.

2. Extend mcpserver_runconfig_test.go with BackendReplicas and Redis cases

Add new test cases to the existing TestCreateRunConfigFromMCPServer table (or add a new table function TestRunConfigScalingFields). The createTestMCPServerWithConfig helper at line 45 may need to be augmented with a new builder helper that accepts BackendReplicas *int32 and SessionStorage *mcpv1alpha1.SessionStorageConfig. Test serialization by calling json.Marshal on RunConfig and asserting the presence/absence of backend_replicas and session_redis keys.

3. Extend virtualmcpserver_deployment_test.go with replica, termination grace, and Redis env var cases

The existing TestDeploymentForVirtualMCPServer at line 37 already asserts *Deployment.Spec.Replicas == 1. After TASK-006, this assertion will need updating (nil-passthrough means nil when spec.replicas == nil). Add additional test cases covering:

• spec.replicas = nil → Spec.Replicas == nil
• spec.replicas = 3 → *Spec.Replicas == 3
• TerminationGracePeriodSeconds is set in all cases
• buildRedisPasswordEnvVar returns the correct corev1.EnvVar when provider == "redis" and passwordRef is set, and returns empty/nil for all other combinations

For the buildRedisPasswordEnvVar helper tests, call the function directly (it is a method on *VirtualMCPServerReconciler) with a minimal VirtualMCPServer struct containing only the relevant SessionStorage field.

4. Extend virtualmcpserver_vmcpconfig_test.go with Redis session storage cases

Add test cases to TestCreateVmcpConfigFromVirtualMCPServer (or a new TestVmcpConfigRedisInjection table) that:

• Set vmcp.Spec.SessionStorage with provider == "redis", address, db, and keyPrefix
• Marshal the resulting config to YAML and assert on the presence of sessionStorage.address, sessionStorage.db, sessionStorage.keyPrefix
• Confirm no password field appears under sessionStorage
• Set provider == "memory" and confirm sessionStorage is absent from the YAML

The existing gopkg.in/yaml.v3 import in virtualmcpserver_vmcpconfig_test.go is already available for YAML assertions.

Patterns & Frameworks

• Table-driven tests with t.Parallel(): All new tests must use t.Parallel() and t.Run(tt.name, func(...) { t.Parallel() ... }) following the existing TestReplicaBehavior pattern. Use github.com/stretchr/testify/assert and github.com/stretchr/testify/require for assertions.
• fake.NewClientBuilder() pattern: Construct a fake Kubernetes client with fake.NewClientBuilder().WithScheme(testScheme).WithObjects(...).WithStatusSubresource(...).Build(). The scheme must include both corev1 and mcpv1alpha1.
• newTestMCPServerReconciler helper: Use this to construct a properly initialized MCPServerReconciler with a mock platform detector and ImageValidationAlwaysAllow.
• apimeta.FindStatusCondition: Use k8s.io/apimachinery/pkg/api/meta.FindStatusCondition(&status.Conditions, conditionType) to retrieve and assert on status conditions after reconcile.
• Direct function call for builder tests: For deploymentForMCPServer, deploymentForVirtualMCPServer, and buildRedisPasswordEnvVar, call the functions directly rather than going through a full reconcile loop when testing only the deployment shape. This reduces setup overhead and makes assertions more targeted.
• JSON marshaling for RunConfig assertions: Use json.Marshal(runConfig) and json.Unmarshal into a map[string]interface{} to assert on the presence or absence of fields without depending on exact struct shapes.

Code Pointers

• cmd/thv-operator/controllers/mcpserver_replicas_test.go — Primary file to extend; contains TestReplicaBehavior, TestConfigUpdatePreservesReplicas, and TestDefaultCreationHasOneReplica as reference patterns. Note: TestDefaultCreationHasOneReplica at line 459 asserts *deployment.Spec.Replicas == 1 for a server without spec.replicas set; after TASK-004, this may need updating to nil (nil-passthrough). Verify with the TASK-004 implementer.
• cmd/thv-operator/controllers/mcpserver_runconfig_test.go — Contains createTestMCPServerWithConfig helper at line 45 and TestCreateRunConfigFromMCPServer table at line 61; extend with Redis and BackendReplicas cases.
• cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go — Contains TestDeploymentForVirtualMCPServer at line 37 with existing replica assertion at line 64 (*deployment.Spec.Replicas == 1); this assertion will need updating after TASK-006 changes the default to nil when spec.replicas is nil.
• cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go — Contains TestCreateVmcpConfigFromVirtualMCPServer table and newTestConverter helper; extend for Redis session storage fields.
• cmd/thv-operator/controllers/mcpserver_test_helpers_test.go — newTestMCPServerReconciler helper at line 30; use this to construct the reconciler in all MCPServer tests.
• cmd/thv-operator/api/v1alpha1/mcpserver_types.go — Source of ConditionStdioReplicaCapped, ConditionSessionStorageWarning, and associated reason constants added by TASK-004; import these rather than using string literals in tests.
• cmd/thv-operator/controllers/virtualmcpserver_deployment.go lines 289–329 — buildOIDCEnvVars — reference for the SecretKeyRef env var pattern that buildRedisPasswordEnvVar (added by TASK-006) follows; read to understand the expected corev1.EnvVar shape to assert against.
• pkg/runner/config.go — RunConfig struct with BackendReplicas and SessionRedis fields added by TASK-005; import runner.SessionRedisConfig for test assertions.
• pkg/vmcp/config/config.go — Config struct with SessionStorage *SessionStorageConfig field added by TASK-007; import vmcpconfig.SessionStorageConfig for converter test assertions.

Component Interfaces

Key types needed for test assertions (all defined by upstream tasks):

// From cmd/thv-operator/api/v1alpha1/mcpserver_types.go (added by TASK-004)
const ConditionStdioReplicaCapped  = "StdioReplicaCapped"
const ConditionSessionStorageWarning = "SessionStorageWarning"
const ConditionReasonStdioReplicaCapped = "StdioTransportCapAt1"
const ConditionReasonSessionStorageMissing = "SessionStorageMissingForReplicas"
const ConditionReasonSessionStorageConfigured = "SessionStorageConfigured"

// From pkg/runner/config.go (added by TASK-005)
type SessionRedisConfig struct {
    Address   string `json:"address,omitempty" yaml:"address,omitempty"`
    DB        int32  `json:"db,omitempty" yaml:"db,omitempty"`
    KeyPrefix string `json:"key_prefix,omitempty" yaml:"key_prefix,omitempty"`
}
// runner.RunConfig also gains:
//   BackendReplicas *int32
//   SessionRedis *SessionRedisConfig

// From pkg/vmcp/config/config.go (added by TASK-007)
type SessionStorageConfig struct {
    Address   string `json:"address,omitempty" yaml:"address,omitempty"`
    DB        int32  `json:"db,omitempty" yaml:"db,omitempty"`
    KeyPrefix string `json:"keyPrefix,omitempty" yaml:"keyPrefix,omitempty"`
}
// vmcpconfig.Config also gains:
//   SessionStorage *SessionStorageConfig

Illustrative test structure for spec-driven replica nil-passthrough (extend mcpserver_replicas_test.go):

func TestSpecDrivenReplicasNil(t *testing.T) {
    t.Parallel()
    name, namespace := "spec-nil-replica", testNamespaceDefault
    mcpServer := &mcpv1alpha1.MCPServer{
        ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
        Spec: mcpv1alpha1.MCPServerSpec{
            Image: "test-image:latest", Transport: "sse", ProxyPort: 8080,
            Replicas: nil, // explicit nil — hands-off mode
        },
    }
    // ... fake client setup, reconcile, fetch deployment
    assert.Nil(t, deployment.Spec.Replicas, "spec.replicas == nil must yield Deployment.Spec.Replicas == nil")
}

Illustrative test for stdio cap with condition assertion:

func TestSpecDrivenStdioCapAt1(t *testing.T) {
    t.Parallel()
    replicas := int32(3)
    mcpServer := &mcpv1alpha1.MCPServer{
        // ...
        Spec: mcpv1alpha1.MCPServerSpec{
            Transport: "stdio", Replicas: &replicas, // ...
        },
    }
    // ... reconcile
    // Assert deployment has 1 replica
    assert.Equal(t, int32(1), *deployment.Spec.Replicas)
    // Assert condition is set
    condition := apimeta.FindStatusCondition(updatedMCPServer.Status.Conditions,
        mcpv1alpha1.ConditionStdioReplicaCapped)
    require.NotNil(t, condition)
    assert.Equal(t, metav1.ConditionTrue, condition.Status)
    assert.Equal(t, mcpv1alpha1.ConditionReasonStdioReplicaCapped, condition.Reason)
}

Illustrative test for Redis env var injection in VirtualMCPServer deployment:

func TestBuildRedisPasswordEnvVar(t *testing.T) {
    t.Parallel()
    tests := []struct {
        name        string
        sessionStorage *mcpv1alpha1.SessionStorageConfig
        wantEnvVars int
        wantEnvName string
        wantSecretName string
        wantSecretKey  string
    }{
        {
            name: "redis with passwordRef injects env var",
            sessionStorage: &mcpv1alpha1.SessionStorageConfig{
                Provider: "redis",
                PasswordRef: &mcpv1alpha1.SecretKeyRef{Name: "redis-secret", Key: "password"},
            },
            wantEnvVars: 1, wantEnvName: "THV_SESSION_REDIS_PASSWORD",
            wantSecretName: "redis-secret", wantSecretKey: "password",
        },
        {name: "nil sessionStorage returns no env vars", sessionStorage: nil, wantEnvVars: 0},
        {name: "memory provider returns no env vars",
            sessionStorage: &mcpv1alpha1.SessionStorageConfig{Provider: "memory"}, wantEnvVars: 0},
        {name: "redis without passwordRef returns no env vars",
            sessionStorage: &mcpv1alpha1.SessionStorageConfig{Provider: "redis"}, wantEnvVars: 0},
    }
    // ...
}

Testing Strategy

Unit Tests

MCPServer reconciler / deployment builder (extend mcpserver_replicas_test.go):

• TestSpecDrivenReplicasNil — spec.replicas == nil produces a Deployment with Spec.Replicas == nil (nil-passthrough)
• TestSpecDrivenReplicas1 — spec.replicas = 1 produces *Spec.Replicas == 1
• TestSpecDrivenReplicas3SseTransport — spec.replicas = 3, transport sse produces *Spec.Replicas == 3
• TestSpecDrivenStdioCapAt1 — spec.transport = "stdio", spec.replicas = 3 produces a Deployment with *Spec.Replicas == 1 and sets StdioReplicaCapped condition with status True and reason StdioTransportCapAt1
• TestSessionStorageWarningConditionNilStorage — spec.replicas = 2, spec.sessionStorage = nil sets SessionStorageWarning condition with status True and reason SessionStorageMissingForReplicas
• TestSessionStorageWarningConditionMemory — spec.replicas = 2, spec.sessionStorage.provider = "memory" sets SessionStorageWarning with status True
• TestSessionStorageWarningConditionRedis — spec.replicas = 2, spec.sessionStorage.provider = "redis" sets SessionStorageWarning with status False and reason SessionStorageConfigured
• TestTerminationGracePeriodMCPServer — deploymentForMCPServer output always has Spec.Template.Spec.TerminationGracePeriodSeconds equal to the expected constant

MCPServer RunConfig (extend mcpserver_runconfig_test.go):

• backendReplicas = nil → RunConfig.BackendReplicas == nil
• backendReplicas = 3 → *RunConfig.BackendReplicas == 3
• sessionStorage.provider = "memory" → RunConfig.SessionRedis == nil
• sessionStorage = nil → RunConfig.SessionRedis == nil
• sessionStorage.provider = "redis", address "redis.svc:6379", db 2, keyPrefix "thv:" → RunConfig.SessionRedis equals {Address: "redis.svc:6379", DB: 2, KeyPrefix: "thv:"}
• Serialized RunConfig JSON omits backend_replicas when nil; includes it when set
• Serialized RunConfig JSON does not contain any password or credential when passwordRef is set in the CRD spec

VirtualMCPServer deployment builder (extend virtualmcpserver_deployment_test.go):

• spec.replicas = nil → Spec.Replicas == nil
• spec.replicas = 1 → *Spec.Replicas == 1
• spec.replicas = 3 → *Spec.Replicas == 3
• TerminationGracePeriodSeconds is set to the expected constant in all replica variants
• buildRedisPasswordEnvVar with provider = "redis" and passwordRef.name = "redis-secret", passwordRef.key = "password" returns one corev1.EnvVar named THV_SESSION_REDIS_PASSWORD with correct ValueFrom.SecretKeyRef
• buildRedisPasswordEnvVar with nil sessionStorage returns no env vars
• buildRedisPasswordEnvVar with provider = "memory" returns no env vars
• buildRedisPasswordEnvVar with provider = "redis" and nil passwordRef returns no env vars

VirtualMCPServer vMCP ConfigMap (extend virtualmcpserver_vmcpconfig_test.go):

• provider = "redis" with all fields set → marshaled config.yaml contains sessionStorage.address, sessionStorage.db, sessionStorage.keyPrefix
• provider = "redis" → marshaled config.yaml does NOT contain any password field
• provider = "memory" → config.yaml omits sessionStorage entirely
• spec.sessionStorage = nil → config.yaml omits sessionStorage entirely

Integration Tests

• Not in scope for this task; these unit tests provide sufficient automated coverage for the new operator behaviors

Edge Cases

• spec.replicas = 0 — Deployment has *Spec.Replicas == 0 (scale-to-zero is valid); verify the stdio cap logic does not interfere with zero-replica requests for stdio transport
• backendReplicas = 0 — RunConfig.BackendReplicas is a pointer to int32(0), not nil; verify json.Marshal includes the field (omitempty only applies to nil pointers, not zero-value pointed-to integers)
• sessionStorage.db = 0 — Verify the zero-value DB field is correctly serialized in both RunConfig and vMCP config YAML (omitempty on int32 will omit zero; if vMCP requires explicit db: 0, the YAML tag may need to be yaml:"db" without omitempty — coordinate with TASK-007 implementer on the chosen behavior)
• Nil-passthrough after initial set: updating MCPServer from spec.replicas = 2 to spec.replicas = nil does not set Deployment.Spec.Replicas to any value (verify deploymentNeedsUpdate preserves this invariant)
• Pre-existing TestDefaultCreationHasOneReplica may need updating if TASK-004 changes the default behavior for spec.replicas = nil from 1 to nil; coordinate with the TASK-004 implementer before marking that test as passing

Out of Scope

• Application-level session storage implementation inside proxyrunner or vMCP — separate epics
• Integration tests with a real Kubernetes API server or live Redis instance
• HPA integration tests
• Redis deployment, provisioning, or lifecycle
• CLI, UI, or any changes outside cmd/thv-operator/ and pkg/runner/ and pkg/vmcp/config/
• New feature implementation — this task writes tests only; all implementation is in TASK-004 through TASK-007

References

• Epic THV-0047: Horizontal Scaling - CRD and Operator Changes — https://github.com/stacklok/stacklok-epics/issues/264
• Upstream TASK-004: MCPServer reconciler and deployment builder updates — https://github.com/stacklok/stacklok-epics/issues/280
• Upstream TASK-005: MCPServer RunConfig Redis and BackendReplicas injection — https://github.com/stacklok/stacklok-epics/issues/281
• Upstream TASK-006: VirtualMCPServer deployment builder updates — https://github.com/stacklok/stacklok-epics/issues/278
• Upstream TASK-007: VirtualMCPServer vMCP ConfigMap Redis injection — https://github.com/stacklok/stacklok-epics/issues/277
• RFC THV-0047: Manual Horizontal Scaling for vMCP and Proxy Runner — RFC: Horizontal Scaling for vMCP and Proxy Runner toolhive-rfcs#47
• Existing HPA-scenario replica tests: cmd/thv-operator/controllers/mcpserver_replicas_test.go
• Existing RunConfig tests: cmd/thv-operator/controllers/mcpserver_runconfig_test.go
• Existing VirtualMCPServer deployment tests: cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go
• Existing vMCP ConfigMap tests: cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go