adds dedicated podtemplatespec builder (#2754)

* adds podtemplatespec builder

Signed-off-by: Chris Burns <[email protected]>

* moves more code

Signed-off-by: Chris Burns <[email protected]>

* podtemplatetest

Signed-off-by: Chris Burns <[email protected]>

---------

Signed-off-by: Chris Burns <[email protected]>

Author: ChrisJBurns

cmd/thv-operator/pkg/registryapi/deployment.go

@@ -9,9 +9,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -79,11 +77,7 @@ func (m *manager) ensureDeployment(
 	ctxLogger := log.FromContext(ctx).WithValues("mcpregistry", mcpRegistry.Name)
 
 	// Build the desired deployment configuration
-	deployment, err := m.buildRegistryAPIDeployment(mcpRegistry, configManager)
-	if err != nil {
-		ctxLogger.Error(err, "Failed to build deployment configuration")
-		return nil, fmt.Errorf("failed to build deployment configuration: %w", err)
-	}
+	deployment := m.buildRegistryAPIDeployment(mcpRegistry, configManager)
 	deploymentName := deployment.Name
 
 	// Set owner reference for automatic garbage collection
@@ -94,7 +88,7 @@ func (m *manager) ensureDeployment(
 
 	// Check if deployment already exists
 	existing := &appsv1.Deployment{}
-	err = m.client.Get(ctx, client.ObjectKey{
+	err := m.client.Get(ctx, client.ObjectKey{
 		Name:      deploymentName,
 		Namespace: mcpRegistry.Namespace,
 	}, existing)
@@ -124,16 +118,31 @@ func (m *manager) ensureDeployment(
 // buildRegistryAPIDeployment creates and configures a Deployment object for the registry API.
 // This function handles all deployment configuration including labels, container specs, probes,
 // and storage manager integration. It returns a fully configured deployment ready for Kubernetes API operations.
-func (m *manager) buildRegistryAPIDeployment(
+func (*manager) buildRegistryAPIDeployment(
 	mcpRegistry *mcpv1alpha1.MCPRegistry,
 	configManager config.ConfigManager,
-) (*appsv1.Deployment, error) {
+) *appsv1.Deployment {
 	// Generate deployment name using the established pattern
 	deploymentName := mcpRegistry.GetAPIResourceName()
 
 	// Define labels using common function
 	labels := labelsForRegistryAPI(mcpRegistry, deploymentName)
 
+	// Build the PodTemplateSpec using the functional options pattern
+	// This includes all mount configurations via the builder pattern
+	builder := NewPodTemplateSpecBuilder()
+	podTemplateSpec := builder.Apply(
+		WithLabels(labels),
+		WithAnnotations(map[string]string{
+			"toolhive.stacklok.dev/config-hash": "hash-dummy-value",
+		}),
+		WithServiceAccountName(DefaultServiceAccountName),
+		WithContainer(BuildRegistryAPIContainer(getRegistryAPIImage())),
+		WithRegistryServerConfigMount(registryAPIContainerName, configManager.GetRegistryServerConfigMapName()),
+		WithRegistrySourceMounts(registryAPIContainerName, mcpRegistry.Spec.Registries),
+		WithRegistryStorageMount(registryAPIContainerName),
+	).Build()
+
 	// Create basic deployment specification with named container
 	deployment := &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
@@ -149,82 +158,11 @@ func (m *manager) buildRegistryAPIDeployment(
 					"app.kubernetes.io/component": "registry-api",
 				},
 			},
-			Template: corev1.PodTemplateSpec{
-				ObjectMeta: metav1.ObjectMeta{
-					Labels: labels,
-					Annotations: map[string]string{
-						"toolhive.stacklok.dev/config-hash": "hash-dummy-value",
-					},
-				},
-				Spec: corev1.PodSpec{
-					ServiceAccountName: DefaultServiceAccountName,
-					Containers: []corev1.Container{
-						{
-							Name:  registryAPIContainerName,
-							Image: getRegistryAPIImage(),
-							Args: []string{
-								ServeCommand,
-							},
-							Ports: []corev1.ContainerPort{
-								{
-									ContainerPort: RegistryAPIPort,
-									Name:          RegistryAPIPortName,
-									Protocol:      corev1.ProtocolTCP,
-								},
-							},
-							// Add resource limits and requests for production readiness
-							Resources: corev1.ResourceRequirements{
-								Requests: corev1.ResourceList{
-									corev1.ResourceCPU:    resource.MustParse(DefaultCPURequest),
-									corev1.ResourceMemory: resource.MustParse(DefaultMemoryRequest),
-								},
-								Limits: corev1.ResourceList{
-									corev1.ResourceCPU:    resource.MustParse(DefaultCPULimit),
-									corev1.ResourceMemory: resource.MustParse(DefaultMemoryLimit),
-								},
-							},
-							// Add liveness and readiness probes
-							LivenessProbe: &corev1.Probe{
-								ProbeHandler: corev1.ProbeHandler{
-									HTTPGet: &corev1.HTTPGetAction{
-										Path: HealthCheckPath,
-										Port: intstr.FromInt32(RegistryAPIPort),
-									},
-								},
-								InitialDelaySeconds: LivenessInitialDelay,
-								PeriodSeconds:       LivenessPeriod,
-							},
-							ReadinessProbe: &corev1.Probe{
-								ProbeHandler: corev1.ProbeHandler{
-									HTTPGet: &corev1.HTTPGetAction{
-										Path: ReadinessCheckPath,
-										Port: intstr.FromInt32(RegistryAPIPort),
-									},
-								},
-								InitialDelaySeconds: ReadinessInitialDelay,
-								PeriodSeconds:       ReadinessPeriod,
-							},
-						},
-					},
-				},
-			},
+			Template: podTemplateSpec,
 		},
 	}
 
-	// Configure storage-specific aspects using the new inverted dependency approach
-	if err := m.configureRegistryServerConfigMounts(deployment, registryAPIContainerName, configManager); err != nil {
-		return nil, fmt.Errorf("failed to configure registry server config mounts: %w", err)
-	}
-
-	if err := m.configureRegistrySourceMounts(deployment, mcpRegistry, registryAPIContainerName); err != nil {
-		return nil, fmt.Errorf("failed to configure registry source mounts: %w", err)
-	}
-
-	if err := m.configureRegistryStorageMounts(deployment, registryAPIContainerName); err != nil {
-		return nil, fmt.Errorf("failed to configure registry storage mounts: %w", err)
-	}
-
-	return deployment, nil
+	return deployment
 }
 
 // getRegistryAPIImage returns the registry API container image to use
@@ -240,33 +178,3 @@ func getRegistryAPIImageWithEnvGetter(envGetter func(string) string) string {
 	}
 	return "ghcr.io/stacklok/thv-registry-api:latest"
 }
-
-// findContainerByName finds a container by name in a slice of containers
-func findContainerByName(containers []corev1.Container, name string) *corev1.Container {
-	for i := range containers {
-		if containers[i].Name == name {
-			return &containers[i]
-		}
-	}
-	return nil
-}
-
-// hasVolume checks if a volume with the given name exists in the volumes slice
-func hasVolume(volumes []corev1.Volume, name string) bool {
-	for _, volume := range volumes {
-		if volume.Name == name {
-			return true
-		}
-	}
-	return false
-}
-
-// hasVolumeMount checks if a volume mount with the given name exists in the volume mounts slice
-func hasVolumeMount(volumeMounts []corev1.VolumeMount, name string) bool {
-	for _, mount := range volumeMounts {
-		if mount.Name == name {
-			return true
-		}
-	}
-	return false
-}

cmd/thv-operator/pkg/registryapi/deployment_test.go

@@ -134,8 +134,7 @@ func TestManagerBuildRegistryAPIDeployment(t *testing.T) {
 			manager := &manager{}
 
 			configManager := config.NewConfigManagerForTesting(tt.mcpRegistry)
-			deployment, err := manager.buildRegistryAPIDeployment(tt.mcpRegistry, configManager)
-			require.NoError(t, err)
+			deployment := manager.buildRegistryAPIDeployment(tt.mcpRegistry, configManager)
 			tt.validateResult(t, deployment)
 		})
 	}

cmd/thv-operator/pkg/registryapi/manager.go

@@ -3,10 +3,8 @@ package registryapi
 import (
 	"context"
 	"fmt"
-	"path/filepath"
 
 	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -127,146 +125,6 @@ func (m *manager) IsAPIReady(ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRe
 	return m.CheckAPIReadiness(ctx, deployment)
 }
 
-func (*manager) configureRegistryServerConfigMounts(
-	deployment *appsv1.Deployment,
-	containerName string,
-	configManager config.ConfigManager,
-) error {
-
-	// Find the container by name
-	container := findContainerByName(deployment.Spec.Template.Spec.Containers, containerName)
-	if container == nil {
-		return fmt.Errorf("container '%s' not found in deployment", containerName)
-	}
-
-	// Replace container args completely with the correct set of arguments
-	// This ensures idempotent behavior across multiple reconciliations
-	container.Args = []string{
-		ServeCommand,
-		fmt.Sprintf("--config=%s", filepath.Join(config.RegistryServerConfigFilePath, config.RegistryServerConfigFileName)),
-	}
-
-	// Add ConfigMap volume to deployment if not already present
-	configVolumeName := RegistryServerConfigVolumeName
-	if !hasVolume(deployment.Spec.Template.Spec.Volumes, configVolumeName) {
-		configVolume := corev1.Volume{
-			Name: configVolumeName,
-			VolumeSource: corev1.VolumeSource{
-				ConfigMap: &corev1.ConfigMapVolumeSource{
-					LocalObjectReference: corev1.LocalObjectReference{
-						Name: configManager.GetRegistryServerConfigMapName(),
-					},
-				},
-			},
-		}
-		deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, configVolume)
-	}
-
-	// Add volume mount to the container if not already present
-	if !hasVolumeMount(container.VolumeMounts, configVolumeName) {
-		volumeMount := corev1.VolumeMount{
-			Name:      configVolumeName,
-			MountPath: config.RegistryServerConfigFilePath, // Mount to directory, not the file path
-			ReadOnly:  true,                                // ConfigMaps are always read-only anyway
-		}
-		container.VolumeMounts = append(container.VolumeMounts, volumeMount)
-	}
-
-	return nil
-}
-
-func (*manager) configureRegistryStorageMounts(
-	deployment *appsv1.Deployment,
-	containerName string,
-) error {
-	// Find the container by name
-	container := findContainerByName(deployment.Spec.Template.Spec.Containers, containerName)
-	if container == nil {
-		return fmt.Errorf("container '%s' not found in deployment", containerName)
-	}
-
-	// Add emptyDir volume for storage data if not already present
-	storageDataVolumeName := "storage-data"
-	if !hasVolume(deployment.Spec.Template.Spec.Volumes, storageDataVolumeName) {
-		storageDataVolume := corev1.Volume{
-			Name: storageDataVolumeName,
-			VolumeSource: corev1.VolumeSource{
-				EmptyDir: &corev1.EmptyDirVolumeSource{},
-			},
-		}
-		deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, storageDataVolume)
-	}
-
-	// Add emptyDir mount to the container if not already present
-	if !hasVolumeMount(container.VolumeMounts, storageDataVolumeName) {
-		storageDataVolumeMount := corev1.VolumeMount{
-			Name:      storageDataVolumeName,
-			MountPath: "/data", // You can modify this path as needed
-			ReadOnly:  false,
-		}
-		container.VolumeMounts = append(container.VolumeMounts, storageDataVolumeMount)
-	}
-	return nil
-}
-
-func (*manager) configureRegistrySourceMounts(
-	deployment *appsv1.Deployment,
-	mcpRegistry *mcpv1alpha1.MCPRegistry,
-	containerName string,
-) error {
-
-	// Find the container by name
-	container := findContainerByName(deployment.Spec.Template.Spec.Containers, containerName)
-	if container == nil {
-		return fmt.Errorf("container '%s' not found in deployment", containerName)
-	}
-
-	// Iterate through all registry configurations to handle multiple ConfigMap sources
-	for _, registry := range mcpRegistry.Spec.Registries {
-		if registry.ConfigMapRef != nil {
-			// Create unique volume name for each ConfigMap source
-			volumeName := fmt.Sprintf("registry-data-source-%s", registry.Name)
-
-			// Add volume if it doesn't exist
-			if !hasVolume(deployment.Spec.Template.Spec.Volumes, volumeName) {
-				volume := corev1.Volume{
-					Name: volumeName,
-					VolumeSource: corev1.VolumeSource{
-						ConfigMap: &corev1.ConfigMapVolumeSource{
-							LocalObjectReference: corev1.LocalObjectReference{
-								Name: registry.ConfigMapRef.Name,
-							},
-							// Mount only the specified key as registry.json
-							Items: []corev1.KeyToPath{
-								{
-									Key:  registry.ConfigMapRef.Key,
-									Path: "registry.json",
-								},
-							},
-						},
-					},
-				}
-				deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, volume)
-			}
-
-			// Add volume mount if it doesn't exist
-			if !hasVolumeMount(container.VolumeMounts, volumeName) {
-				// Mount path follows the pattern /config/registry/{registryName}/
-				// This matches what buildFilePath expects in config.go
-				mountPath := filepath.Join(config.RegistryJSONFilePath, registry.Name)
-				volumeMount := corev1.VolumeMount{
-					Name:      volumeName,
-					MountPath: mountPath,
-					ReadOnly:  true, // ConfigMaps are always read-only
-				}
-				container.VolumeMounts = append(container.VolumeMounts, volumeMount)
-			}
-		}
-	}
-
-	return nil
-}
-
 // getConfigMapName generates the ConfigMap name for registry storage
 // This mirrors the logic in ConfigMapStorageManager to maintain consistency
 func getConfigMapName(mcpRegistry *mcpv1alpha1.MCPRegistry) string {