feat: Support passing labels and annotations from ServingRuntimePodSpec to runtime Pod

.github/workflows/run-fvt.yml

@@ -18,7 +18,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions/setup-go@v2
         with:
-          go-version: '1.17.3'
+          go-version: '1.18.7'
       - name: Setup Minikube
         run: |
           wget --no-verbose https://github.com/kubernetes/minikube/releases/download/v1.25.1/minikube-linux-amd64

.pre-commit-config.yaml

@@ -13,7 +13,7 @@
 # limitations under the License.
 repos:
   - repo: https://github.com/golangci/golangci-lint
-    rev: v1.43.0
+    rev: v1.48.0
     hooks:
       - id: golangci-lint
   - repo: https://github.com/pre-commit/mirrors-prettier

Dockerfile.develop

@@ -17,7 +17,7 @@
 ###############################################################################
 FROM registry.access.redhat.com/ubi8/ubi-minimal:8.4
 
-ARG GOLANG_VERSION=1.17.9
+ARG GOLANG_VERSION=1.18.7
 ARG OPENSHIFT_VERSION=4.9
 ARG KUSTOMIZE_VERSION=4.5.2
 ARG KUBEBUILDER_VERSION=v3.3.0

config/crd/bases/serving.kserve.io_clusterservingruntimes.yaml

@@ -1,4 +1,4 @@
-# Copied from https://github.com/kserve/kserve/blob/ff7014b0c1a79672978d5b0a23af6c5ae1158b3b/config/crd/serving.kserve.io_clusterservingruntimes.yaml
+# Copied from https://github.com/kserve/kserve/blob/532989a2ccef63ff912008359540b0937596f55c/config/crd/serving.kserve.io_clusterservingruntimes.yaml
 #
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
@@ -395,6 +395,10 @@ spec:
                           type: array
                       type: object
                   type: object
+                annotations:
+                  additionalProperties:
+                    type: string
+                  type: object
                 builtInAdapter:
                   properties:
                     env:
@@ -1061,6 +1065,10 @@ spec:
                   type: string
                 httpDataEndpoint:
                   type: string
+                labels:
+                  additionalProperties:
+                    type: string
+                  type: object
                 multiModel:
                   type: boolean
                 nodeSelector:

config/crd/bases/serving.kserve.io_servingruntimes.yaml

@@ -1,4 +1,4 @@
-# Copied from https://github.com/kserve/kserve/blob/ff7014b0c1a79672978d5b0a23af6c5ae1158b3b/config/crd/serving.kserve.io_servingruntimes.yaml
+# Copied from https://github.com/kserve/kserve/blob/532989a2ccef63ff912008359540b0937596f55c/config/crd/serving.kserve.io_servingruntimes.yaml
 #
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
@@ -395,6 +395,10 @@ spec:
                           type: array
                       type: object
                   type: object
+                annotations:
+                  additionalProperties:
+                    type: string
+                  type: object
                 builtInAdapter:
                   properties:
                     env:
@@ -1061,6 +1065,10 @@ spec:
                   type: string
                 httpDataEndpoint:
                   type: string
+                labels:
+                  additionalProperties:
+                    type: string
+                  type: object
                 multiModel:
                   type: boolean
                 nodeSelector:

controllers/modelmesh/runtime.go

@@ -31,7 +31,7 @@ const (
 	ModelDirScale float64 = 1.5
 )
 
-//Sets the model mesh grace period to match the deployment grace period
+// Sets the model mesh grace period to match the deployment grace period
 func (m *Deployment) syncGracePeriod(deployment *appsv1.Deployment) error {
 	if deployment.Spec.Template.Spec.TerminationGracePeriodSeconds != nil {
 		gracePeriodS := deployment.Spec.Template.Spec.TerminationGracePeriodSeconds
@@ -98,13 +98,13 @@ func calculateModelDirSize(rts *kserveapi.ServingRuntimeSpec) *resource.Quantity
 	return resource.NewQuantity(int64(float64(memorySize.Value())*ModelDirScale), resource.BinarySI)
 }
 
-//Adds the provided runtime to the deployment
+// Adds the provided runtime to the deployment
 func (m *Deployment) addRuntimeToDeployment(deployment *appsv1.Deployment) error {
 	rts := m.SRSpec
 
 	// first prepare the common variables needed for both adapter and other containers
 	lifecycle := &corev1.Lifecycle{
-		PreStop: &corev1.Handler{
+		PreStop: &corev1.LifecycleHandler{
 			HTTPGet: &corev1.HTTPGetAction{
 				Path: "/prestop",
 				Port: intstr.FromInt(8090),
@@ -360,32 +360,15 @@ func (m *Deployment) addPassThroughPodFieldsToDeployment(deployment *appsv1.Depl
 }
 
 func (m *Deployment) configureRuntimePodSpecAnnotations(deployment *appsv1.Deployment) error {
-
-	if deployment.Spec.Template.Annotations == nil {
-		deployment.Spec.Template.Annotations = m.AnnotationsMap
-		return nil
-	}
-
-	// apply user configmap annotations
-	for key, value := range m.AnnotationsMap {
-		// set labels for pods created by deployment
-		deployment.Spec.Template.Annotations[key] = value
-	}
-
+	// merging the annotations
+	// priority: ServingRuntimePodSpec > AnnotationsMap > whatever in the deployment template
+	deployment.Spec.Template.Annotations = Union(deployment.Spec.Template.Annotations, m.AnnotationsMap, m.SRSpec.ServingRuntimePodSpec.Annotations)
 	return nil
 }
 
 func (m *Deployment) configureRuntimePodSpecLabels(deployment *appsv1.Deployment) error {
-
-	if deployment.Spec.Template.Labels == nil {
-		deployment.Spec.Template.Labels = m.LabelsMap
-		return nil
-	}
-
-	for key, value := range m.LabelsMap {
-		// set labels for pods created by deployment
-		deployment.Spec.Template.Labels[key] = value
-	}
-
+	// merging the labels
+	// priority: ServingRuntimePodSpec > LabelsMap > whatever in the deployment template
+	deployment.Spec.Template.Labels = Union(deployment.Spec.Template.Labels, m.LabelsMap, m.SRSpec.ServingRuntimePodSpec.Labels)
 	return nil
 }

controllers/modelmesh/runtime_test.go

@@ -375,6 +375,52 @@ func TestConfigureRuntimeAnnotations(t *testing.T) {
 		assert.Nil(t, err)
 		assert.Empty(t, deploy.Spec.Template.Annotations)
 	})
+
+	t.Run("success-set-annotations-from-servingruntime-spec", func(t *testing.T) {
+		deploy := &appsv1.Deployment{}
+		sr := &kserveapi.ServingRuntime{
+			Spec: kserveapi.ServingRuntimeSpec{
+				ServingRuntimePodSpec: kserveapi.ServingRuntimePodSpec{
+					Annotations: map[string]string{
+						"foo":            "bar",
+						"network-policy": "allow-egress",
+					},
+				},
+			},
+		}
+
+		m := Deployment{Owner: sr, AnnotationsMap: map[string]string{}, SRSpec: &sr.Spec}
+
+		err := m.configureRuntimePodSpecAnnotations(deploy)
+		assert.Nil(t, err)
+		assert.Equal(t, deploy.Spec.Template.Annotations["foo"], "bar")
+		assert.Equal(t, deploy.Spec.Template.Annotations["network-policy"], "allow-egress")
+	})
+
+	t.Run("success-overwrite-annotations-from-servingruntime-spec", func(t *testing.T) {
+		deploy := &appsv1.Deployment{}
+		// annotations from user config
+		annotationsData := map[string]string{
+			"foo":            "bar",
+			"network-policy": "allow-egress",
+		}
+		sr := &kserveapi.ServingRuntime{
+			Spec: kserveapi.ServingRuntimeSpec{
+				ServingRuntimePodSpec: kserveapi.ServingRuntimePodSpec{
+					Annotations: map[string]string{
+						"network-policy": "overwritten-by-servingruntime",
+					},
+				},
+			},
+		}
+
+		m := Deployment{Owner: sr, AnnotationsMap: annotationsData, SRSpec: &sr.Spec}
+
+		err := m.configureRuntimePodSpecAnnotations(deploy)
+		assert.Nil(t, err)
+		assert.Equal(t, deploy.Spec.Template.Annotations["foo"], "bar")
+		assert.Equal(t, deploy.Spec.Template.Annotations["network-policy"], "overwritten-by-servingruntime")
+	})
 }
 
 func TestConfigureRuntimeLabels(t *testing.T) {
@@ -407,4 +453,54 @@ func TestConfigureRuntimeLabels(t *testing.T) {
 		assert.Nil(t, err)
 		assert.Empty(t, deploy.Spec.Template.Labels)
 	})
+
+	t.Run("success-set-labels-from-servingruntime-spec", func(t *testing.T) {
+		deploy := &appsv1.Deployment{}
+		sr := &kserveapi.ServingRuntime{
+			Spec: kserveapi.ServingRuntimeSpec{
+				ServingRuntimePodSpec: kserveapi.ServingRuntimePodSpec{
+					Labels: map[string]string{
+						"foo":            "bar",
+						"network-policy": "allow-egress",
+						"cp4s-internet":  "allow",
+					},
+				},
+			},
+		}
+
+		m := Deployment{Owner: sr, LabelsMap: map[string]string{}, SRSpec: &sr.Spec}
+
+		err := m.configureRuntimePodSpecLabels(deploy)
+		assert.Nil(t, err)
+		assert.Equal(t, deploy.Spec.Template.Labels["foo"], "bar")
+		assert.Equal(t, deploy.Spec.Template.Labels["network-policy"], "allow-egress")
+		assert.Equal(t, deploy.Spec.Template.Labels["cp4s-internet"], "allow")
+	})
+
+	t.Run("success-overwrite-labels-from-servingruntime-spec", func(t *testing.T) {
+		deploy := &appsv1.Deployment{}
+		// labels from user config
+		labelData := map[string]string{
+			"foo":            "bar",
+			"network-policy": "allow-egress",
+			"cp4s-internet":  "allow",
+		}
+		sr := &kserveapi.ServingRuntime{
+			Spec: kserveapi.ServingRuntimeSpec{
+				ServingRuntimePodSpec: kserveapi.ServingRuntimePodSpec{
+					Labels: map[string]string{
+						"network-policy": "overwritten-by-servingruntime",
+					},
+				},
+			},
+		}
+
+		m := Deployment{Owner: sr, LabelsMap: labelData, SRSpec: &sr.Spec}
+
+		err := m.configureRuntimePodSpecLabels(deploy)
+		assert.Nil(t, err)
+		assert.Equal(t, deploy.Spec.Template.Labels["foo"], "bar")
+		assert.Equal(t, deploy.Spec.Template.Labels["network-policy"], "overwritten-by-servingruntime")
+		assert.Equal(t, deploy.Spec.Template.Labels["cp4s-internet"], "allow")
+	})
 }

controllers/modelmesh/util.go

@@ -20,12 +20,17 @@ import (
 	"strings"
 
 	kserveapi "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
+	kserveutils "github.com/kserve/kserve/pkg/utils"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/yaml"
 )
 
+var (
+	Union = kserveutils.Union
+)
+
 // Find a container by name in the given deployment, returns (-1, nil) if not found
 func findContainer(name string, deployment *appsv1.Deployment) (index int, container *corev1.Container) {
 	for i := range deployment.Spec.Template.Spec.Containers {