chore: various changes + fixes (#50)

- change FeatureFlags embed FeatureFlag than string type
- move generic function getOperatorNamespace into utils
- fix DistributionType Name and Image can only be set one of them
- move get port value in to generic function GetServicePort
- remove networkpolicy ingress which matches everything
- allow ContainerSpec to be empty which falls to use zero-value
- add extra label "app.kubernetes.io/instance" on deployment/pod for service to work if multiple CR gets created
- change from types.NamespacedName{} to client.ObjectKeyFromObject()


Approved-by: leseb

Approved-by: VaishnaviHire

Author: zdtsw

api/v1alpha1/llamastackdistribution_types.go

@@ -243,7 +243,7 @@ spec:
                     type: object
                     x-kubernetes-validations:
                     - message: Only one of name or image can be specified
-                      rule: has(self.name) != has(self.image)
+                      rule: '!(has(self.name) && has(self.image))'
                   podOverrides:
                     description: PodOverrides allows advanced pod-level customization.
                     properties:
@@ -1978,7 +1978,6 @@ spec:
                         x-kubernetes-int-or-string: true
                     type: object
                 required:
-                - containerSpec
                 - distribution
                 type: object
             required:

config/crd/bases/llamastack.io_llamastackdistributions.yaml

@@ -23,7 +23,6 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"strings"
 	"sync"
 	"time"
 
@@ -47,6 +46,10 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+const (
+	operatorConfigData = "llama-stack-operator-config"
+)
+
 // LlamaStackDistributionReconciler reconciles a LlamaStack object.
 type LlamaStackDistributionReconciler struct {
 	client.Client
@@ -74,9 +77,8 @@ func (r *LlamaStackDistributionReconciler) Reconcile(ctx context.Context, req ct
 	if err != nil {
 		return ctrl.Result{}, err
 	}
-
-	// Instance not found - skip reconciliation
 	if instance == nil {
+		r.Log.Info("LlamaStackDistribution resource not found, skipping reconciliation", "namespacedName", req.NamespacedName)
 		return ctrl.Result{}, nil
 	}
 
@@ -182,7 +184,7 @@ func (r *LlamaStackDistributionReconciler) reconcilePVC(ctx context.Context, ins
 	}
 
 	found := &corev1.PersistentVolumeClaim{}
-	err := r.Get(ctx, types.NamespacedName{Name: pvc.Name, Namespace: pvc.Namespace}, found)
+	err := r.Get(ctx, client.ObjectKeyFromObject(pvc), found)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
 			logger.Info("Creating PVC", "pvc", pvc.Name)
@@ -224,11 +226,17 @@ func (r *LlamaStackDistributionReconciler) reconcileDeployment(ctx context.Conte
 		Spec: appsv1.DeploymentSpec{
 			Replicas: &instance.Spec.Replicas,
 			Selector: &metav1.LabelSelector{
-				MatchLabels: map[string]string{llamav1alpha1.DefaultLabelKey: llamav1alpha1.DefaultLabelValue},
+				MatchLabels: map[string]string{
+					llamav1alpha1.DefaultLabelKey: llamav1alpha1.DefaultLabelValue,
+					"app.kubernetes.io/instance":  instance.Name,
+				},
 			},
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{llamav1alpha1.DefaultLabelKey: llamav1alpha1.DefaultLabelValue},
+					Labels: map[string]string{
+						llamav1alpha1.DefaultLabelKey: llamav1alpha1.DefaultLabelValue,
+						"app.kubernetes.io/instance":  instance.Name,
+					},
 				},
 				Spec: podSpec,
 			},
@@ -241,18 +249,18 @@ func (r *LlamaStackDistributionReconciler) reconcileDeployment(ctx context.Conte
 // reconcileService manages the Service if ports are defined.
 func (r *LlamaStackDistributionReconciler) reconcileService(ctx context.Context, instance *llamav1alpha1.LlamaStackDistribution) error {
 	// Use the container's port (defaulted to 8321 if unset)
-	port := instance.Spec.Server.ContainerSpec.Port
-	if port == 0 {
-		port = llamav1alpha1.DefaultServerPort
-	}
+	port := deploy.GetServicePort(instance)
 
 	service := &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      instance.Name + "-service",
+			Name:      deploy.GetServiceName(instance),
 			Namespace: instance.Namespace,
 		},
 		Spec: corev1.ServiceSpec{
-			Selector: map[string]string{llamav1alpha1.DefaultLabelKey: llamav1alpha1.DefaultLabelValue},
+			Selector: map[string]string{
+				llamav1alpha1.DefaultLabelKey: llamav1alpha1.DefaultLabelValue,
+				"app.kubernetes.io/instance":  instance.Name,
+			},
 			Ports: []corev1.ServicePort{{
 				Name: llamav1alpha1.DefaultServicePortName,
 				Port: port,
@@ -269,11 +277,8 @@ func (r *LlamaStackDistributionReconciler) reconcileService(ctx context.Context,
 
 // getServerURL returns the URL for the LlamaStack server.
 func (r *LlamaStackDistributionReconciler) getServerURL(instance *llamav1alpha1.LlamaStackDistribution, path string) *url.URL {
-	serviceName := fmt.Sprintf("%s-service", instance.Name)
-	port := instance.Spec.Server.ContainerSpec.Port
-	if port == 0 {
-		port = llamav1alpha1.DefaultServerPort
-	}
+	serviceName := deploy.GetServiceName(instance)
+	port := deploy.GetServicePort(instance)
 
 	return &url.URL{
 		Scheme: "http",
@@ -342,7 +347,7 @@ func (r *LlamaStackDistributionReconciler) getProviderInfo(ctx context.Context,
 	return response.Data, nil
 }
 
-// updateStatus refreshes the LlamaStack status.
+// updateStatus refreshes the LlamaStackDistribution status.
 func (r *LlamaStackDistributionReconciler) updateStatus(ctx context.Context, instance *llamav1alpha1.LlamaStackDistribution) error {
 	deployment := &appsv1.Deployment{}
 	err := r.Get(ctx, types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace}, deployment)
@@ -441,11 +446,7 @@ func (r *LlamaStackDistributionReconciler) reconcileNetworkPolicy(ctx context.Co
 		return deploy.HandleDisabledNetworkPolicy(ctx, r.Client, networkPolicy, r.Log)
 	}
 
-	// Use the container's port (defaulted to 8321 if unset)
-	port := instance.Spec.Server.ContainerSpec.Port
-	if port == 0 {
-		port = llamav1alpha1.DefaultServerPort
-	}
+	port := deploy.GetServicePort(instance)
 
 	// get operator namespace
 	operatorNamespace, err := deploy.GetOperatorNamespace()
@@ -457,6 +458,7 @@ func (r *LlamaStackDistributionReconciler) reconcileNetworkPolicy(ctx context.Co
 		PodSelector: metav1.LabelSelector{
 			MatchLabels: map[string]string{
 				llamav1alpha1.DefaultLabelKey: llamav1alpha1.DefaultLabelValue,
+				"app.kubernetes.io/instance":  instance.Name,
 			},
 		},
 		PolicyTypes: []networkingv1.PolicyType{
@@ -465,7 +467,7 @@ func (r *LlamaStackDistributionReconciler) reconcileNetworkPolicy(ctx context.Co
 		Ingress: []networkingv1.NetworkPolicyIngressRule{
 			{
 				From: []networkingv1.NetworkPolicyPeer{
-					{
+					{ // to match all pods in all namespaces
 						PodSelector: &metav1.LabelSelector{
 							MatchLabels: map[string]string{
 								"app.kubernetes.io/part-of": llamav1alpha1.DefaultContainerName,
@@ -485,8 +487,8 @@ func (r *LlamaStackDistributionReconciler) reconcileNetworkPolicy(ctx context.Co
 			},
 			{
 				From: []networkingv1.NetworkPolicyPeer{
-					{
-						PodSelector: &metav1.LabelSelector{}, // Empty podSelector to match all pods
+					{ // to match all pods in matched namespace
+						PodSelector: &metav1.LabelSelector{},
 						NamespaceSelector: &metav1.LabelSelector{
 							MatchLabels: map[string]string{
 								"kubernetes.io/metadata.name": operatorNamespace,
@@ -503,27 +505,53 @@ func (r *LlamaStackDistributionReconciler) reconcileNetworkPolicy(ctx context.Co
 					},
 				},
 			},
-			{
-				From: []networkingv1.NetworkPolicyPeer{
-					{
-						PodSelector: &metav1.LabelSelector{}, // Empty podSelector to match all pods
-					},
-				},
-				Ports: []networkingv1.NetworkPolicyPort{
-					{
-						Protocol: (*corev1.Protocol)(ptr.To("TCP")),
-						Port: &intstr.IntOrString{
-							IntVal: port,
-						},
-					},
-				},
-			},
 		},
 	}
 
 	return deploy.ApplyNetworkPolicy(ctx, r.Client, r.Scheme, instance, networkPolicy, r.Log)
 }
 
+// createDefaultConfigMap creates a ConfigMap with default feature flag values.
+func createDefaultConfigMap(configMapName types.NamespacedName) (*corev1.ConfigMap, error) {
+	featureFlags := featureflags.FeatureFlags{
+		EnableNetworkPolicy: featureflags.FeatureFlag{
+			Enabled: featureflags.NetworkPolicyDefaultValue,
+		},
+	}
+
+	featureFlagsYAML, err := yaml.Marshal(featureFlags)
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal default feature flags: %w", err)
+	}
+
+	return &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      configMapName.Name,
+			Namespace: configMapName.Namespace,
+		},
+		Data: map[string]string{
+			featureflags.FeatureFlagsKey: string(featureFlagsYAML),
+		},
+	}, nil
+}
+
+// parseFeatureFlags extracts and parses feature flags from ConfigMap data.
+func parseFeatureFlags(configMapData map[string]string) (bool, error) {
+	enableNetworkPolicy := featureflags.NetworkPolicyDefaultValue
+
+	featureFlagsYAML, exists := configMapData[featureflags.FeatureFlagsKey]
+	if !exists {
+		return enableNetworkPolicy, nil
+	}
+
+	var flags featureflags.FeatureFlags
+	if err := yaml.Unmarshal([]byte(featureFlagsYAML), &flags); err != nil {
+		return false, fmt.Errorf("failed to parse feature flags: %w", err)
+	}
+
+	return flags.EnableNetworkPolicy.Enabled, nil
+}
+
 // NewLlamaStackDistributionReconciler creates a new reconciler with default image mappings.
 func NewLlamaStackDistributionReconciler(ctx context.Context, client client.Client, scheme *runtime.Scheme,
 	clusterInfo *cluster.ClusterInfo) (*LlamaStackDistributionReconciler, error) {
@@ -534,44 +562,36 @@ func NewLlamaStackDistributionReconciler(ctx context.Context, client client.Clie
 		return nil, fmt.Errorf("failed to get operator namespace: %w", err)
 	}
 
-	// Read the ConfigMap
+	// Get the ConfigMap
+	// If the ConfigMap doesn't exist, create it with default feature flags
+	// If the ConfigMap exists, parse the feature flags from the Configmap
 	configMap := &corev1.ConfigMap{}
 	configMapName := types.NamespacedName{
-		Name:      "llama-stack-operator-config",
+		Name:      operatorConfigData,
 		Namespace: operatorNamespace,
 	}
-	err = client.Get(ctx, configMapName, configMap)
-	if err != nil {
-		if k8serrors.IsNotFound(err) {
-			// Create the ConfigMap if it doesn't exist
-			configMap = &corev1.ConfigMap{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      configMapName.Name,
-					Namespace: configMapName.Namespace,
-				},
-				Data: map[string]string{
-					featureflags.FeatureFlagsKey: fmt.Sprintf("%s: %v",
-						featureflags.EnableNetworkPolicyKey, featureflags.NetworkPolicyDefaultValue),
-				},
-			}
-			if err = client.Create(ctx, configMap); err != nil {
-				return nil, fmt.Errorf("failed to create ConfigMap: %w", err)
-			}
-		} else {
-			return nil, fmt.Errorf("failed to read ConfigMap: %w", err)
+
+	if err = client.Get(ctx, configMapName, configMap); err != nil {
+		if !k8serrors.IsNotFound(err) {
+			return nil, fmt.Errorf("failed to get ConfigMap: %w", err)
 		}
-	}
 
-	// Parse the feature flag
-	enableNetworkPolicy := featureflags.NetworkPolicyDefaultValue
-	if featureFlagsYAML, exists := configMap.Data[featureflags.FeatureFlagsKey]; exists {
-		var flags featureflags.FeatureFlags
-		if err := yaml.Unmarshal([]byte(featureFlagsYAML), &flags); err != nil {
-			log.Error(err, "failed to parse feature flags")
-		} else if flags.EnableNetworkPolicy != "" {
-			enableNetworkPolicy = strings.ToLower(flags.EnableNetworkPolicy) == "true"
+		// ConfigMap doesn't exist, create it with defaults
+		configMap, err = createDefaultConfigMap(configMapName)
+		if err != nil {
+			return nil, fmt.Errorf("failed to generate default configMap: %w", err)
+		}
+
+		if err = client.Create(ctx, configMap); err != nil {
+			return nil, fmt.Errorf("failed to create ConfigMap: %w", err)
 		}
 	}
+
+	// Parse feature flags from ConfigMap
+	enableNetworkPolicy, err := parseFeatureFlags(configMap.Data)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse feature flags: %w", err)
+	}
 	return &LlamaStackDistributionReconciler{
 		Client:              client,
 		Scheme:              scheme,

controllers/llamastackdistribution_controller.go

@@ -100,11 +100,10 @@ func configurePodStorage(instance *llamav1alpha1.LlamaStackDistribution, contain
 func (r *LlamaStackDistributionReconciler) validateDistribution(instance *llamav1alpha1.LlamaStackDistribution) error {
 	// If using distribution name, validate it exists in clusterInfo
 	if instance.Spec.Server.Distribution.Name != "" {
-		clusterInfo := r.ClusterInfo
-		if clusterInfo == nil {
+		if r.ClusterInfo == nil {
 			return errors.New("failed to initialize cluster info")
 		}
-		if _, exists := clusterInfo.DistributionImages[instance.Spec.Server.Distribution.Name]; !exists {
+		if _, exists := r.ClusterInfo.DistributionImages[instance.Spec.Server.Distribution.Name]; !exists {
 			return fmt.Errorf("failed to validate distribution: %s. Distribution name not supported", instance.Spec.Server.Distribution.Name)
 		}
 	}