feat(controller): migrate service reconciliation to kustomize manifest (#105)

Migrate service creation from programmatic to manifest-based Kustomizer pipeline, with added service comparison utilities to detect unexpected changes and enforce field mutability boundaries between operator-managed and cluster-managed state.


Approved-by: rhdedgar

Approved-by: VaishnaviHire

Author: mfleader

controllers/llamastackdistribution_controller.go

@@ -225,28 +225,21 @@ func (r *LlamaStackDistributionReconciler) reconcileResources(ctx context.Contex
 		}
 	}
 
-	// Reconcile the NetworkPolicy
-	if err := r.reconcileNetworkPolicy(ctx, instance); err != nil {
-		return fmt.Errorf("failed to reconcile NetworkPolicy: %w", err)
-	}
-
 	// Reconcile manifest-based resources
 	if err := r.reconcileManifestResources(ctx, instance); err != nil {
 		return err
 	}
 
+	// Reconcile the NetworkPolicy
+	if err := r.reconcileNetworkPolicy(ctx, instance); err != nil {
+		return fmt.Errorf("failed to reconcile NetworkPolicy: %w", err)
+	}
+
 	// Reconcile the Deployment
 	if err := r.reconcileDeployment(ctx, instance); err != nil {
 		return fmt.Errorf("failed to reconcile Deployment: %w", err)
 	}
 
-	// Reconcile the Service if ports are defined, else use default port
-	if instance.HasPorts() {
-		if err := r.reconcileService(ctx, instance); err != nil {
-			return fmt.Errorf("failed to reconcile service: %w", err)
-		}
-	}
-
 	return nil
 }
 
@@ -656,36 +649,6 @@ func (r *LlamaStackDistributionReconciler) reconcileDeployment(ctx context.Conte
 	return deploy.ApplyDeployment(ctx, r.Client, r.Scheme, instance, deployment, logger)
 }
 
-// reconcileService manages the Service if ports are defined.
-func (r *LlamaStackDistributionReconciler) reconcileService(ctx context.Context, instance *llamav1alpha1.LlamaStackDistribution) error {
-	logger := log.FromContext(ctx)
-	// Use the container's port (defaulted to 8321 if unset)
-	port := deploy.GetServicePort(instance)
-
-	service := &corev1.Service{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      deploy.GetServiceName(instance),
-			Namespace: instance.Namespace,
-		},
-		Spec: corev1.ServiceSpec{
-			Selector: map[string]string{
-				llamav1alpha1.DefaultLabelKey: llamav1alpha1.DefaultLabelValue,
-				"app.kubernetes.io/instance":  instance.Name,
-			},
-			Ports: []corev1.ServicePort{{
-				Name: llamav1alpha1.DefaultServicePortName,
-				Port: port,
-				TargetPort: intstr.IntOrString{
-					IntVal: port,
-				},
-			}},
-			Type: corev1.ServiceTypeClusterIP,
-		},
-	}
-
-	return deploy.ApplyService(ctx, r.Client, r.Scheme, instance, service, logger)
-}
-
 // getServerURL returns the URL for the LlamaStack server.
 func (r *LlamaStackDistributionReconciler) getServerURL(instance *llamav1alpha1.LlamaStackDistribution, path string) *url.URL {
 	serviceName := deploy.GetServiceName(instance)

controllers/manifests/base/kustomization.yaml

@@ -5,9 +5,10 @@ resources:
 - pvc.yaml
 - serviceaccount.yaml
 - scc-binding.yaml
+- service.yaml
 
 labels:
-- includeSelectors: true
+- includeSelectors: false
   pairs:
     app.kubernetes.io/managed-by: llama-stack-operator
     app.kubernetes.io/part-of: llama-stack

controllers/manifests/base/service.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: service
+spec:
+  type: ClusterIP
+  selector: {}
+  ports:
+  - name: http
+    protocol: TCP

pkg/deploy/deploy.go

@@ -8,7 +8,6 @@ import (
 	"github.com/go-logr/logr"
 	llamav1alpha1 "github.com/llamastack/llama-stack-k8s-operator/api/v1alpha1"
 	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -42,28 +41,3 @@ func ApplyDeployment(ctx context.Context, cli client.Client, scheme *runtime.Sch
 	}
 	return nil
 }
-
-// ApplyService creates or updates the Service.
-func ApplyService(ctx context.Context, cli client.Client, scheme *runtime.Scheme,
-	instance *llamav1alpha1.LlamaStackDistribution, service *corev1.Service, logger logr.Logger) error {
-	if err := ctrl.SetControllerReference(instance, service, scheme); err != nil {
-		return fmt.Errorf("failed to set controller reference: %w", err)
-	}
-
-	found := &corev1.Service{}
-	err := cli.Get(ctx, client.ObjectKeyFromObject(service), found)
-	if err != nil && errors.IsNotFound(err) {
-		logger.Info("Creating Service", "service", service.Name)
-		return cli.Create(ctx, service)
-	} else if err != nil {
-		return fmt.Errorf("failed to fetch Service: %w", err)
-	}
-
-	if !reflect.DeepEqual(found.Spec.Selector, service.Spec.Selector) || !reflect.DeepEqual(found.Spec.Ports, service.Spec.Ports) {
-		found.Spec.Selector = service.Spec.Selector
-		found.Spec.Ports = service.Spec.Ports
-		logger.Info("Updating Service", "service", service.Name)
-		return cli.Update(ctx, found)
-	}
-	return nil
-}

pkg/deploy/kustomizer.go

@@ -8,6 +8,7 @@ import (
 	"slices"
 
 	llamav1alpha1 "github.com/llamastack/llama-stack-k8s-operator/api/v1alpha1"
+	"github.com/llamastack/llama-stack-k8s-operator/pkg/compare"
 	"github.com/llamastack/llama-stack-k8s-operator/pkg/deploy/plugins"
 	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -173,6 +174,10 @@ func patchResource(ctx context.Context, cli client.Client, desired, existing *un
 			"name", existing.GetName(),
 			"namespace", existing.GetNamespace())
 		return nil
+	} else if existing.GetKind() == "Service" {
+		if err := compare.CheckAndLogServiceChanges(ctx, cli, desired); err != nil {
+			return fmt.Errorf("failed to validate resource mutations while patching: %w", err)
+		}
 	}
 
 	data, err := json.Marshal(desired)
@@ -227,6 +232,34 @@ func applyPlugins(resMap *resmap.ResMap, ownerInstance *llamav1alpha1.LlamaStack
 				TargetKind:        "ClusterRoleBinding",
 				CreateIfNotExists: true,
 			},
+			{
+				SourceValue:       getServicePort(ownerInstance),
+				DefaultValue:      llamav1alpha1.DefaultServerPort,
+				TargetField:       "/spec/ports/0/port",
+				TargetKind:        "Service",
+				CreateIfNotExists: true,
+			},
+			{
+				SourceValue:       getServicePort(ownerInstance),
+				DefaultValue:      llamav1alpha1.DefaultServerPort,
+				TargetField:       "/spec/ports/0/targetPort",
+				TargetKind:        "Service",
+				CreateIfNotExists: true,
+			},
+			{
+				SourceValue:       nil,
+				DefaultValue:      llamav1alpha1.DefaultLabelValue,
+				TargetField:       "/spec/selector/" + llamav1alpha1.DefaultLabelKey,
+				TargetKind:        "Service",
+				CreateIfNotExists: true,
+			},
+			{
+				SourceValue:       nil,
+				DefaultValue:      ownerInstance.GetName(),
+				TargetField:       "/spec/selector/app.kubernetes.io~1instance",
+				TargetKind:        "Service",
+				CreateIfNotExists: true,
+			},
 		},
 	})
 	if err := fieldTransformerPlugin.Transform(*resMap); err != nil {
@@ -245,6 +278,15 @@ func getStorageSize(instance *llamav1alpha1.LlamaStackDistribution) string {
 	return ""
 }
 
+// getServicePort returns the service port or nil if not specified.
+func getServicePort(instance *llamav1alpha1.LlamaStackDistribution) any {
+	if instance.Spec.Server.ContainerSpec.Port != 0 {
+		return instance.Spec.Server.ContainerSpec.Port
+	}
+	// Returning nil signals the field transformer to use the default value.
+	return nil
+}
+
 func FilterExcludeKinds(resMap *resmap.ResMap, kindsToExclude []string) (*resmap.ResMap, error) {
 	filteredResMap := resmap.New()
 	for _, res := range (*resMap).Resources() {

pkg/deploy/kustomizer_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	llamav1alpha1 "github.com/llamastack/llama-stack-k8s-operator/api/v1alpha1"
+	"github.com/llamastack/llama-stack-k8s-operator/pkg/deploy/plugins"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
@@ -530,3 +531,53 @@ func TestFilterExcludeKinds(t *testing.T) {
 		require.Equal(t, "Deployment", (*filtered).Resources()[0].GetKind())
 	})
 }
+
+func TestSetDefaultPort(t *testing.T) {
+	// arrange
+	// instance with no custom port and service with empty port values
+	instance := &llamav1alpha1.LlamaStackDistribution{
+		Spec: llamav1alpha1.LlamaStackDistributionSpec{
+			Server: llamav1alpha1.ServerSpec{
+				ContainerSpec: llamav1alpha1.ContainerSpec{
+					Port: 0, // no port configured
+				},
+			},
+		},
+	}
+
+	service := newTestResource(t, "v1", "Service", "test-service", "test-ns", map[string]any{
+		"ports": []any{
+			map[string]any{"port": nil}, // empty port to trigger default
+		},
+	})
+
+	fieldMutator := plugins.CreateFieldMutator(plugins.FieldMutatorConfig{
+		Mappings: []plugins.FieldMapping{
+			{
+				SourceValue:       getServicePort(instance), // tests getServicePort() integration with kustomizer
+				DefaultValue:      llamav1alpha1.DefaultServerPort,
+				TargetField:       "/spec/ports/0/port",
+				TargetKind:        "Service",
+				CreateIfNotExists: true,
+			},
+		},
+	})
+
+	resMap := resmap.New()
+	require.NoError(t, resMap.Append(service))
+
+	// act
+	// apply field transformation
+	require.NoError(t, fieldMutator.Transform(resMap))
+
+	// assert
+	// port should be set to default value
+	transformedService := resMap.Resources()[0]
+	serviceMap, err := transformedService.Map()
+	require.NoError(t, err)
+	ports, ok := serviceMap["spec"].(map[string]any)["ports"].([]any)
+	require.True(t, ok)
+	actualPort, ok := ports[0].(map[string]any)["port"]
+	require.True(t, ok)
+	require.Equal(t, int(llamav1alpha1.DefaultServerPort), actualPort)
+}