List workloads once to ensure consistency

cmd/thv-operator/controllers/virtualmcpserver_controller.go

@@ -287,20 +287,30 @@ func (r *VirtualMCPServerReconciler) ensureAllResources(
 metav1.ConditionTrue,
 )
 
+// List workloads once and pass to functions that need them
+// This ensures consistency - all functions use the same workload list
+// rather than listing at different times which could yield different results
+workloadDiscoverer := workloads.NewK8SDiscovererWithClient(r.Client, vmcp.Namespace)
+workloadNames, err := workloadDiscoverer.ListWorkloadsInGroup(ctx, vmcp.Spec.GroupRef.Name)
+if err != nil {
+ctxLogger.Error(err, "Failed to list workloads in group")
+return fmt.Errorf("failed to list workloads in group: %w", err)
+}
+
 // Ensure RBAC resources
 if err := r.ensureRBACResources(ctx, vmcp); err != nil {
 ctxLogger.Error(err, "Failed to ensure RBAC resources")
 return err
 }
 
 // Ensure vmcp Config ConfigMap
-if err := r.ensureVmcpConfigConfigMap(ctx, vmcp); err != nil {
+if err := r.ensureVmcpConfigConfigMap(ctx, vmcp, workloadNames); err != nil {
 ctxLogger.Error(err, "Failed to ensure vmcp Config ConfigMap")
 return err
 }
 
 // Ensure Deployment
-if result, err := r.ensureDeployment(ctx, vmcp); err != nil {
+if result, err := r.ensureDeployment(ctx, vmcp, workloadNames); err != nil {
 return err
 } else if result.RequeueAfter > 0 {
 return nil
@@ -422,6 +432,7 @@ func (r *VirtualMCPServerReconciler) getVmcpConfigChecksum(
 func (r *VirtualMCPServerReconciler) ensureDeployment(
 ctx context.Context,
 vmcp *mcpv1alpha1.VirtualMCPServer,
+workloadNames []string,
 ) (ctrl.Result, error) {
 ctxLogger := log.FromContext(ctx)
 
@@ -441,7 +452,7 @@ func (r *VirtualMCPServerReconciler) ensureDeployment(
 err = r.Get(ctx, types.NamespacedName{Name: vmcp.Name, Namespace: vmcp.Namespace}, deployment)
 
 if errors.IsNotFound(err) {
-dep := r.deploymentForVirtualMCPServer(ctx, vmcp, vmcpConfigChecksum)
+dep := r.deploymentForVirtualMCPServer(ctx, vmcp, vmcpConfigChecksum, workloadNames)
 if dep == nil {
 return ctrl.Result{}, fmt.Errorf("failed to create Deployment object")
 }
@@ -470,8 +481,8 @@ func (r *VirtualMCPServerReconciler) ensureDeployment(
 
 // Deployment exists - check if it needs to be updated
 // deploymentNeedsUpdate performs a detailed comparison to avoid unnecessary updates
-if r.deploymentNeedsUpdate(ctx, deployment, vmcp, vmcpConfigChecksum) {
-newDeployment := r.deploymentForVirtualMCPServer(ctx, vmcp, vmcpConfigChecksum)
+if r.deploymentNeedsUpdate(ctx, deployment, vmcp, vmcpConfigChecksum, workloadNames) {
+newDeployment := r.deploymentForVirtualMCPServer(ctx, vmcp, vmcpConfigChecksum, workloadNames)
 if newDeployment == nil {
 return ctrl.Result{}, fmt.Errorf("failed to create updated Deployment object")
 }
@@ -603,6 +614,7 @@ func (r *VirtualMCPServerReconciler) deploymentNeedsUpdate(
 deployment *appsv1.Deployment,
 vmcp *mcpv1alpha1.VirtualMCPServer,
 vmcpConfigChecksum string,
+workloadNames []string,
 ) bool {
 if deployment == nil || vmcp == nil {
 return true
@@ -612,7 +624,7 @@ func (r *VirtualMCPServerReconciler) deploymentNeedsUpdate(
 return true
 }
 
-if r.containerNeedsUpdate(ctx, deployment, vmcp) {
+if r.containerNeedsUpdate(ctx, deployment, vmcp, workloadNames) {
 return true
 }
 
@@ -632,6 +644,7 @@ func (r *VirtualMCPServerReconciler) containerNeedsUpdate(
 ctx context.Context,
 deployment *appsv1.Deployment,
 vmcp *mcpv1alpha1.VirtualMCPServer,
+workloadNames []string,
 ) bool {
 if deployment == nil || vmcp == nil || len(deployment.Spec.Template.Spec.Containers) == 0 {
 return true
@@ -651,7 +664,7 @@ func (r *VirtualMCPServerReconciler) containerNeedsUpdate(
 }
 
 // Check if environment variables have changed
-expectedEnv := r.buildEnvVarsForVmcp(ctx, vmcp)
+expectedEnv := r.buildEnvVarsForVmcp(ctx, vmcp, workloadNames)
 if !reflect.DeepEqual(container.Env, expectedEnv) {
 return true
 }

cmd/thv-operator/controllers/virtualmcpserver_controller_test.go

@@ -325,7 +325,7 @@ func TestVirtualMCPServerEnsureDeployment(t *testing.T) {
 PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
 }
 
-result, err := r.ensureDeployment(context.Background(), vmcp)
+result, err := r.ensureDeployment(context.Background(), vmcp, []string{})
 require.NoError(t, err)
 assert.Equal(t, ctrl.Result{}, result)
 
@@ -1280,7 +1280,7 @@ func TestVirtualMCPServerContainerNeedsUpdate(t *testing.T) {
 Ports: []corev1.ContainerPort{
 {ContainerPort: 4483},
 },
-Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp),
+Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp, []string{}),
 },
 },
 ServiceAccountName: vmcpServiceAccountName(vmcp.Name),
@@ -1304,7 +1304,7 @@ func TestVirtualMCPServerContainerNeedsUpdate(t *testing.T) {
 Ports: []corev1.ContainerPort{
 {ContainerPort: 8080},
 },
-Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp),
+Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp, []string{}),
 },
 },
 ServiceAccountName: vmcpServiceAccountName(vmcp.Name),
@@ -1354,7 +1354,7 @@ func TestVirtualMCPServerContainerNeedsUpdate(t *testing.T) {
 Ports: []corev1.ContainerPort{
 {ContainerPort: 4483},
 },
-Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp),
+Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp, []string{}),
 },
 },
 ServiceAccountName: "wrong-service-account",
@@ -1378,7 +1378,7 @@ func TestVirtualMCPServerContainerNeedsUpdate(t *testing.T) {
 Ports: []corev1.ContainerPort{
 {ContainerPort: 4483},
 },
-Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp),
+Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp, []string{}),
 },
 },
 ServiceAccountName: vmcpServiceAccountName(vmcp.Name),
@@ -1395,7 +1395,7 @@ func TestVirtualMCPServerContainerNeedsUpdate(t *testing.T) {
 t.Run(tt.name, func(t *testing.T) {
 t.Parallel()
 
-needsUpdate := reconciler.containerNeedsUpdate(context.Background(), tt.deployment, tt.vmcp)
+needsUpdate := reconciler.containerNeedsUpdate(context.Background(), tt.deployment, tt.vmcp, []string{})
 assert.Equal(t, tt.expectedUpdate, needsUpdate)
 })
 }
@@ -1675,7 +1675,7 @@ func TestVirtualMCPServerDeploymentNeedsUpdate(t *testing.T) {
 Ports: []corev1.ContainerPort{
 {ContainerPort: 4483},
 },
-Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp),
+Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp, []string{}),
 },
 },
 ServiceAccountName: vmcpServiceAccountName(vmcp.Name),
@@ -1708,7 +1708,7 @@ func TestVirtualMCPServerDeploymentNeedsUpdate(t *testing.T) {
 Ports: []corev1.ContainerPort{
 {ContainerPort: 4483},
 },
-Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp),
+Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp, []string{}),
 },
 },
 ServiceAccountName: vmcpServiceAccountName(vmcp.Name),
@@ -1739,7 +1739,7 @@ func TestVirtualMCPServerDeploymentNeedsUpdate(t *testing.T) {
 Ports: []corev1.ContainerPort{
 {ContainerPort: 4483},
 },
-Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp),
+Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp, []string{}),
 },
 },
 ServiceAccountName: vmcpServiceAccountName(vmcp.Name),
@@ -1770,7 +1770,7 @@ func TestVirtualMCPServerDeploymentNeedsUpdate(t *testing.T) {
 Ports: []corev1.ContainerPort{
 {ContainerPort: 4483},
 },
-Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp),
+Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp, []string{}),
 },
 },
 ServiceAccountName: vmcpServiceAccountName(vmcp.Name),
@@ -1786,7 +1786,7 @@ func TestVirtualMCPServerDeploymentNeedsUpdate(t *testing.T) {
 t.Run(tt.name, func(t *testing.T) {
 t.Parallel()
 
-needsUpdate := reconciler.deploymentNeedsUpdate(context.Background(), tt.deployment, vmcp, vmcpConfigChecksum)
+needsUpdate := reconciler.deploymentNeedsUpdate(context.Background(), tt.deployment, vmcp, vmcpConfigChecksum, []string{})
 assert.Equal(t, tt.expectedUpdate, needsUpdate)
 })
 }
@@ -2114,7 +2114,7 @@ func TestVirtualMCPServerEnsureDeployment_ConfigMapNotFound(t *testing.T) {
 Scheme: scheme,
 }
 
-result, err := reconciler.ensureDeployment(context.Background(), vmcp)
+result, err := reconciler.ensureDeployment(context.Background(), vmcp, []string{})
 
 // Should requeue after 5 seconds when ConfigMap not found
 assert.NoError(t, err)
@@ -2165,7 +2165,7 @@ func TestVirtualMCPServerEnsureDeployment_CreateDeployment(t *testing.T) {
 Scheme: scheme,
 }
 
-result, err := reconciler.ensureDeployment(context.Background(), vmcp)
+result, err := reconciler.ensureDeployment(context.Background(), vmcp, []string{})
 
 assert.NoError(t, err)
 assert.Equal(t, ctrl.Result{}, result)
@@ -2250,7 +2250,7 @@ func TestVirtualMCPServerEnsureDeployment_UpdateDeployment(t *testing.T) {
 Scheme: scheme,
 }
 
-result, err := reconciler.ensureDeployment(context.Background(), vmcp)
+result, err := reconciler.ensureDeployment(context.Background(), vmcp, []string{})
 
 assert.NoError(t, err)
 assert.Equal(t, ctrl.Result{}, result)
@@ -2332,7 +2332,7 @@ func TestVirtualMCPServerEnsureDeployment_NoUpdateNeeded(t *testing.T) {
 Ports: []corev1.ContainerPort{
 {ContainerPort: 4483},
 },
-Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp),
+Env: reconciler.buildEnvVarsForVmcp(context.Background(), vmcp, []string{}),
 },
 },
 ServiceAccountName: vmcpServiceAccountName(vmcp.Name),
@@ -2348,7 +2348,7 @@ func TestVirtualMCPServerEnsureDeployment_NoUpdateNeeded(t *testing.T) {
 
 reconciler.Client = k8sClient
 
-result, err := reconciler.ensureDeployment(context.Background(), vmcp)
+result, err := reconciler.ensureDeployment(context.Background(), vmcp, []string{})
 
 assert.NoError(t, err)
 assert.Equal(t, ctrl.Result{}, result)

cmd/thv-operator/controllers/virtualmcpserver_deployment.go

@@ -19,7 +19,6 @@ import (
 ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil"
 "github.com/stacklok/toolhive/cmd/thv-operator/pkg/runconfig/configmap/checksum"
 "github.com/stacklok/toolhive/pkg/container/kubernetes"
-"github.com/stacklok/toolhive/pkg/vmcp/workloads"
 )
 
 const (
@@ -64,14 +63,15 @@ func (r *VirtualMCPServerReconciler) deploymentForVirtualMCPServer(
 ctx context.Context,
 vmcp *mcpv1alpha1.VirtualMCPServer,
 vmcpConfigChecksum string,
+workloadNames []string,
 ) *appsv1.Deployment {
 ls := labelsForVirtualMCPServer(vmcp.Name)
 replicas := int32(1)
 
 // Build deployment components using helper functions
 args := r.buildContainerArgsForVmcp()
 volumeMounts, volumes := r.buildVolumesForVmcp(vmcp)
-env := r.buildEnvVarsForVmcp(ctx, vmcp)
+env := r.buildEnvVarsForVmcp(ctx, vmcp, workloadNames)
 deploymentLabels, deploymentAnnotations := r.buildDeploymentMetadataForVmcp(ls, vmcp)
 deploymentTemplateLabels, deploymentTemplateAnnotations := r.buildPodTemplateMetadata(ls, vmcp, vmcpConfigChecksum)
 podSecurityContext, containerSecurityContext := r.buildSecurityContextsForVmcp(ctx, vmcp)
@@ -173,6 +173,7 @@ func (*VirtualMCPServerReconciler) buildVolumesForVmcp(
 func (r *VirtualMCPServerReconciler) buildEnvVarsForVmcp(
 ctx context.Context,
 vmcp *mcpv1alpha1.VirtualMCPServer,
+workloadNames []string,
 ) []corev1.EnvVar {
 env := []corev1.EnvVar{}
 
@@ -197,7 +198,7 @@ func (r *VirtualMCPServerReconciler) buildEnvVarsForVmcp(
 env = append(env, r.buildOIDCEnvVars(vmcp)...)
 
 // Mount outgoing auth secrets
-env = append(env, r.buildOutgoingAuthEnvVars(ctx, vmcp)...)
+env = append(env, r.buildOutgoingAuthEnvVars(ctx, vmcp, workloadNames)...)
 
 // Note: Other secrets (Redis passwords, service account credentials) may be added here in the future
 // following the same pattern of mounting from Kubernetes Secrets as environment variables.
@@ -259,6 +260,7 @@ func (*VirtualMCPServerReconciler) buildOIDCEnvVars(vmcp *mcpv1alpha1.VirtualMCP
 func (r *VirtualMCPServerReconciler) buildOutgoingAuthEnvVars(
 ctx context.Context,
 vmcp *mcpv1alpha1.VirtualMCPServer,
+workloadNames []string,
 ) []corev1.EnvVar {
 var env []corev1.EnvVar
 
@@ -268,14 +270,8 @@ func (r *VirtualMCPServerReconciler) buildOutgoingAuthEnvVars(
 
 // Mount secrets from discovered ExternalAuthConfigs (discovered mode)
 if vmcp.Spec.OutgoingAuth.Source == OutgoingAuthSourceDiscovered {
-discoveredSecrets, err := r.discoverExternalAuthConfigSecrets(ctx, vmcp)
-if err != nil {
-ctxLogger := log.FromContext(ctx)
-ctxLogger.V(1).Info("Failed to discover ExternalAuthConfig secrets, continuing without them",
-"error", err)
-} else {
-env = append(env, discoveredSecrets...)
-}
+discoveredSecrets := r.discoverExternalAuthConfigSecrets(ctx, vmcp, workloadNames)
+env = append(env, discoveredSecrets...)
 }
 
 // Mount secrets from inline ExternalAuthConfigRefs
@@ -305,18 +301,12 @@ func (r *VirtualMCPServerReconciler) buildOutgoingAuthEnvVars(
 func (r *VirtualMCPServerReconciler) discoverExternalAuthConfigSecrets(
 ctx context.Context,
 vmcp *mcpv1alpha1.VirtualMCPServer,
-) ([]corev1.EnvVar, error) {
+workloadNames []string,
+) []corev1.EnvVar {
 ctxLogger := log.FromContext(ctx)
 var envVars []corev1.EnvVar
 seenConfigs := make(map[string]bool) // Track which ExternalAuthConfigs we've already processed
 
-// Get workload names from the group
-workloadDiscoverer := workloads.NewK8SDiscovererWithClient(r.Client, vmcp.Namespace)
-workloadNames, err := workloadDiscoverer.ListWorkloadsInGroup(ctx, vmcp.Spec.GroupRef.Name)
-if err != nil {
-return nil, fmt.Errorf("failed to list workloads in group: %w", err)
-}
-
 // Discover ExternalAuthConfigs from MCPServers
 // TODO: Optimize this by doing a List operation with a label selector or field selector
 // to fetch all MCPServers in the namespace at once, then filter by names, rather than
@@ -354,7 +344,7 @@ func (r *VirtualMCPServerReconciler) discoverExternalAuthConfigSecrets(
 }
 }
 
-return envVars, nil
+return envVars
 }
 
 // discoverInlineExternalAuthConfigSecrets discovers ExternalAuthConfigs referenced in inline Backends

cmd/thv-operator/controllers/virtualmcpserver_deployment_test.go

@@ -54,7 +54,7 @@ func TestDeploymentForVirtualMCPServer(t *testing.T) {
 PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
 }
 
-deployment := r.deploymentForVirtualMCPServer(context.Background(), vmcp, "test-checksum")
+deployment := r.deploymentForVirtualMCPServer(context.Background(), vmcp, "test-checksum", []string{})
 
 require.NotNil(t, deployment)
 assert.Equal(t, vmcp.Name, deployment.Name)
@@ -134,7 +134,7 @@ func TestBuildEnvVarsForVmcp(t *testing.T) {
 }
 
 r := &VirtualMCPServerReconciler{}
-env := r.buildEnvVarsForVmcp(context.Background(), vmcp)
+env := r.buildEnvVarsForVmcp(context.Background(), vmcp, []string{})
 
 // Should have VMCP_NAME and VMCP_NAMESPACE
 foundName := false
@@ -344,7 +344,7 @@ func TestDeploymentNeedsUpdate(t *testing.T) {
 }
 
 // Test nil inputs
-assert.True(t, r.deploymentNeedsUpdate(context.Background(), nil, nil, ""))
+assert.True(t, r.deploymentNeedsUpdate(context.Background(), nil, nil, "", []string{}))
 
 vmcp := &mcpv1alpha1.VirtualMCPServer{
 ObjectMeta: metav1.ObjectMeta{
@@ -354,7 +354,7 @@ func TestDeploymentNeedsUpdate(t *testing.T) {
 }
 
 // Test with nil deployment
-assert.True(t, r.deploymentNeedsUpdate(context.Background(), nil, vmcp, "checksum"))
+assert.True(t, r.deploymentNeedsUpdate(context.Background(), nil, vmcp, "checksum", []string{}))
 }
 
 // TestServiceNeedsUpdate tests service update detection