fix(Backend + SDK): Add missing optional field to SecretAsVolume and … (

kubeflow#10550) * fix(Backend + SDK): Add missing optional field to SecretAsVolume and ConfigMapAsVolume. Signed-off-by: Revital Sur <[email protected]> * Update after rebase. Signed-off-by: Revital Sur <[email protected]> * Update after rebase. Signed-off-by: Revital Sur <[email protected]> * Update after merge. Signed-off-by: Revital Sur <[email protected]> * Updates after merge with master branch. Signed-off-by: Revital Sur <[email protected]> --------- Signed-off-by: Revital Sur <[email protected]>
opendatahub-io · Apr 9, 2024 · d834f77 · d834f77
1 parent 6d59438
commit d834f77
Show file tree

Hide file tree

Showing 18 changed files with 557 additions and 40 deletions.
diff --git a/backend/src/v2/driver/driver.go b/backend/src/v2/driver/driver.go
@@ -536,10 +536,11 @@ func extendPodSpecPatch(
 
 	// Get secret mount information
 	for _, secretAsVolume := range kubernetesExecutorConfig.GetSecretAsVolume() {
+		optional := secretAsVolume.Optional != nil && *secretAsVolume.Optional
 		secretVolume := k8score.Volume{
 			Name: secretAsVolume.GetSecretName(),
 			VolumeSource: k8score.VolumeSource{
-				Secret: &k8score.SecretVolumeSource{SecretName: secretAsVolume.GetSecretName()},
+				Secret: &k8score.SecretVolumeSource{SecretName: secretAsVolume.GetSecretName(), Optional: &optional},
 			},
 		}
 		secretVolumeMount := k8score.VolumeMount{
@@ -568,11 +569,12 @@ func extendPodSpecPatch(
 
 	// Get config map mount information
 	for _, configMapAsVolume := range kubernetesExecutorConfig.GetConfigMapAsVolume() {
+		optional := configMapAsVolume.Optional != nil && *configMapAsVolume.Optional
 		configMapVolume := k8score.Volume{
 			Name: configMapAsVolume.GetConfigMapName(),
 			VolumeSource: k8score.VolumeSource{
 				ConfigMap: &k8score.ConfigMapVolumeSource{
-					LocalObjectReference: k8score.LocalObjectReference{Name: configMapAsVolume.GetConfigMapName()}},
+					LocalObjectReference: k8score.LocalObjectReference{Name: configMapAsVolume.GetConfigMapName()}, Optional: &optional},
 			},
 		}
 		configMapVolumeMount := k8score.VolumeMount{

diff --git a/backend/src/v2/driver/driver_test.go b/backend/src/v2/driver/driver_test.go
@@ -530,7 +530,87 @@ func Test_extendPodSpecPatch_Secret(t *testing.T) {
 					{
 						Name: "secret1",
 						VolumeSource: k8score.VolumeSource{
-							Secret: &k8score.SecretVolumeSource{SecretName: "secret1"},
+							Secret: &k8score.SecretVolumeSource{SecretName: "secret1", Optional:  &[]bool{false}[0],},
+						},
+					},
+				},
+			},
+		},
+		{
+			"Valid - secret as volume with optional false",
+			&kubernetesplatform.KubernetesExecutorConfig{
+				SecretAsVolume: []*kubernetesplatform.SecretAsVolume{
+					{
+						SecretName: "secret1",
+						MountPath:  "/data/path",
+						Optional:   &[]bool{false}[0],
+					},
+				},
+			},
+			&k8score.PodSpec{
+				Containers: []k8score.Container{
+					{
+						Name: "main",
+					},
+				},
+			},
+			&k8score.PodSpec{
+				Containers: []k8score.Container{
+					{
+						Name: "main",
+						VolumeMounts: []k8score.VolumeMount{
+							{
+								Name:      "secret1",
+								MountPath: "/data/path",
+							},
+						},
+					},
+				},
+				Volumes: []k8score.Volume{
+					{
+						Name: "secret1",
+						VolumeSource: k8score.VolumeSource{
+							Secret: &k8score.SecretVolumeSource{SecretName: "secret1", Optional: &[]bool{false}[0]},
+						},
+					},
+				},
+			},
+		},
+		{
+			"Valid - secret as volume with optional true",
+			&kubernetesplatform.KubernetesExecutorConfig{
+				SecretAsVolume: []*kubernetesplatform.SecretAsVolume{
+					{
+						SecretName: "secret1",
+						MountPath:  "/data/path",
+						Optional:   &[]bool{true}[0],
+					},
+				},
+			},
+			&k8score.PodSpec{
+				Containers: []k8score.Container{
+					{
+						Name: "main",
+					},
+				},
+			},
+			&k8score.PodSpec{
+				Containers: []k8score.Container{
+					{
+						Name: "main",
+						VolumeMounts: []k8score.VolumeMount{
+							{
+								Name:      "secret1",
+								MountPath: "/data/path",
+							},
+						},
+					},
+				},
+				Volumes: []k8score.Volume{
+					{
+						Name: "secret1",
+						VolumeSource: k8score.VolumeSource{
+							Secret: &k8score.SecretVolumeSource{SecretName: "secret1", Optional: &[]bool{true}[0]},
 						},
 					},
 				},
@@ -647,7 +727,92 @@ func Test_extendPodSpecPatch_ConfigMap(t *testing.T) {
 						Name: "cm1",
 						VolumeSource: k8score.VolumeSource{
 							ConfigMap: &k8score.ConfigMapVolumeSource{
-								LocalObjectReference: k8score.LocalObjectReference{Name: "cm1"}},
+								LocalObjectReference: k8score.LocalObjectReference{Name: "cm1"},
+								Optional:             &[]bool{false}[0],},
+						},
+					},
+				},
+			},
+		},
+		{
+			"Valid - config map as volume with optional false",
+			&kubernetesplatform.KubernetesExecutorConfig{
+				ConfigMapAsVolume: []*kubernetesplatform.ConfigMapAsVolume{
+					{
+						ConfigMapName: "cm1",
+						MountPath:     "/data/path",
+						Optional:      &[]bool{false}[0],
+					},
+				},
+			},
+			&k8score.PodSpec{
+				Containers: []k8score.Container{
+					{
+						Name: "main",
+					},
+				},
+			},
+			&k8score.PodSpec{
+				Containers: []k8score.Container{
+					{
+						Name: "main",
+						VolumeMounts: []k8score.VolumeMount{
+							{
+								Name:      "cm1",
+								MountPath: "/data/path",
+							},
+						},
+					},
+				},
+				Volumes: []k8score.Volume{
+					{
+						Name: "cm1",
+						VolumeSource: k8score.VolumeSource{
+							ConfigMap: &k8score.ConfigMapVolumeSource{
+								LocalObjectReference: k8score.LocalObjectReference{Name: "cm1"},
+								Optional:             &[]bool{false}[0]},
+						},
+					},
+				},
+			},
+		},
+		{
+			"Valid - config map as volume with optional true",
+			&kubernetesplatform.KubernetesExecutorConfig{
+				ConfigMapAsVolume: []*kubernetesplatform.ConfigMapAsVolume{
+					{
+						ConfigMapName: "cm1",
+						MountPath:     "/data/path",
+						Optional:      &[]bool{true}[0],
+					},
+				},
+			},
+			&k8score.PodSpec{
+				Containers: []k8score.Container{
+					{
+						Name: "main",
+					},
+				},
+			},
+			&k8score.PodSpec{
+				Containers: []k8score.Container{
+					{
+						Name: "main",
+						VolumeMounts: []k8score.VolumeMount{
+							{
+								Name:      "cm1",
+								MountPath: "/data/path",
+							},
+						},
+					},
+				},
+				Volumes: []k8score.Volume{
+					{
+						Name: "cm1",
+						VolumeSource: k8score.VolumeSource{
+							ConfigMap: &k8score.ConfigMapVolumeSource{
+								LocalObjectReference: k8score.LocalObjectReference{Name: "cm1"},
+								Optional:             &[]bool{true}[0]},
 						},
 					},
 				},

diff --git a/backend/third_party_licenses/apiserver.csv b/backend/third_party_licenses/apiserver.csv
@@ -62,7 +62,7 @@ github.com/klauspost/cpuid,https://github.com/klauspost/cpuid/blob/v1.3.1/LICENS
 github.com/klauspost/pgzip,https://github.com/klauspost/pgzip/blob/v1.2.5/LICENSE,MIT
 github.com/kubeflow/pipelines/api/v2alpha1/go,https://github.com/kubeflow/pipelines/blob/758c91f76784/api/LICENSE,Apache-2.0
 github.com/kubeflow/pipelines/backend,https://github.com/kubeflow/pipelines/blob/HEAD/LICENSE,Apache-2.0
-github.com/kubeflow/pipelines/kubernetes_platform/go/kubernetesplatform,https://github.com/kubeflow/pipelines/blob/19a24e3e99db/kubernetes_platform/LICENSE,Apache-2.0
+github.com/kubeflow/pipelines/kubernetes_platform/go/kubernetesplatform,https://github.com/kubeflow/pipelines/blob/8b2a099e8c9f/kubernetes_platform/LICENSE,Apache-2.0
 github.com/kubeflow/pipelines/third_party/ml-metadata/go/ml_metadata,https://github.com/kubeflow/pipelines/blob/e1f0c010f800/third_party/ml-metadata/LICENSE,Apache-2.0
 github.com/lann/builder,https://github.com/lann/builder/blob/47ae307949d0/LICENSE,MIT
 github.com/lann/ps,https://github.com/lann/ps/blob/62de8c46ede0/LICENSE,MIT

diff --git a/backend/third_party_licenses/driver.csv b/backend/third_party_licenses/driver.csv
@@ -32,7 +32,7 @@ github.com/josharian/intern,https://github.com/josharian/intern/blob/v1.0.0/lice
 github.com/json-iterator/go,https://github.com/json-iterator/go/blob/v1.1.12/LICENSE,MIT
 github.com/kubeflow/pipelines/api/v2alpha1/go,https://github.com/kubeflow/pipelines/blob/758c91f76784/api/LICENSE,Apache-2.0
 github.com/kubeflow/pipelines/backend,https://github.com/kubeflow/pipelines/blob/HEAD/LICENSE,Apache-2.0
-github.com/kubeflow/pipelines/kubernetes_platform/go/kubernetesplatform,https://github.com/kubeflow/pipelines/blob/e129b0501379/kubernetes_platform/LICENSE,Apache-2.0
+github.com/kubeflow/pipelines/kubernetes_platform/go/kubernetesplatform,https://github.com/kubeflow/pipelines/blob/8b2a099e8c9f/kubernetes_platform/LICENSE,Apache-2.0
 github.com/kubeflow/pipelines/third_party/ml-metadata/go/ml_metadata,https://github.com/kubeflow/pipelines/blob/e1f0c010f800/third_party/ml-metadata/LICENSE,Apache-2.0
 github.com/mailru/easyjson,https://github.com/mailru/easyjson/blob/v0.7.7/LICENSE,MIT
 github.com/modern-go/concurrent,https://github.com/modern-go/concurrent/blob/bacd9c7ef1dd/LICENSE,Apache-2.0

diff --git a/go.mod b/go.mod
@@ -30,7 +30,7 @@ require (
 	github.com/jinzhu/inflection v1.0.0 // indirect
 	github.com/jinzhu/now v1.1.4 // indirect
 	github.com/kubeflow/pipelines/api v0.0.0-20230331215358-758c91f76784
-	github.com/kubeflow/pipelines/kubernetes_platform v0.0.0-20240305195700-19a24e3e99db
+	github.com/kubeflow/pipelines/kubernetes_platform v0.0.0-20240403164522-8b2a099e8c9f
 	github.com/kubeflow/pipelines/third_party/ml-metadata v0.0.0-20230810215105-e1f0c010f800
 	github.com/lestrrat-go/strftime v1.0.4
 	github.com/mattn/go-sqlite3 v1.14.18

diff --git a/go.sum b/go.sum
diff --git a/kubernetes_platform/python/README.md b/kubernetes_platform/python/README.md
@@ -58,6 +58,25 @@ def pipeline():
                                     mount_path='/mnt/my_vol')
 ```
 
+### Secret: As optional source for a mounted volume
+```python
+from kfp import dsl
+from kfp import kubernetes
+
+@dsl.component
+def print_secret():
+    with open('/mnt/my_vol') as f:
+        print(f.read())
+
+@dsl.pipeline
+def pipeline():
+    task = print_secret()
+    kubernetes.use_secret_as_volume(task,
+                                    secret_name='my-secret',
+                                    mount_path='/mnt/my_vol'
+                                    optional=True)
+```
+
 ### ConfigMap: As environment variable
 ```python
 from kfp import dsl
@@ -89,9 +108,28 @@ def print_config_map():
 @dsl.pipeline
 def pipeline():
     task = print_config_map()
-    kubernetes.use_secret_as_volume(task,
-                                    config_map_name='my-cm',
-                                    mount_path='/mnt/my_vol')
+    kubernetes.use_config_map_as_volume(task,
+                                       config_map_name='my-cm',
+                                       mount_path='/mnt/my_vol')
+```
+
+### ConfigMap: As optional source for a mounted volume
+```python
+from kfp import dsl
+from kfp import kubernetes
+
+@dsl.component
+def print_config_map():
+    with open('/mnt/my_vol') as f:
+        print(f.read())
+
+@dsl.pipeline
+def pipeline():
+    task = print_config_map()
+    kubernetes.use_config_map_as_volume(task,
+                                       config_map_name='my-cm',
+                                       mount_path='/mnt/my_vol',
+				       optional=True)
 ```
 
 
@@ -168,7 +206,7 @@ def my_pipeline():
     )
 ```
 
-# Kubernetes Field: Use Kubernetes Field Path as enviornment variable
+### Kubernetes Field: Use Kubernetes Field Path as enviornment variable
 ```python
 from kfp import dsl
 from kfp import kubernetes

diff --git a/kubernetes_platform/python/kfp/kubernetes/config_map.py b/kubernetes_platform/python/kfp/kubernetes/config_map.py
@@ -61,6 +61,7 @@ def use_config_map_as_volume(
     task: PipelineTask,
     config_map_name: str,
     mount_path: str,
+    optional: bool = False,
 ) -> PipelineTask:
     """Use a Kubernetes ConfigMap by mounting its data to the task's container as
     described by the `Kubernetes documentation <https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/#add-configmap-data-to-a-volume>`_.
@@ -69,6 +70,7 @@ def use_config_map_as_volume(
         task: Pipeline task.
         config_map_name: Name of the ConfigMap.
         mount_path: Path to which to mount the ConfigMap data.
+        optional: Optional field specifying whether the ConfigMap must be defined.
 
     Returns:
         Task object with updated ConfigMap configuration.
@@ -79,6 +81,7 @@ def use_config_map_as_volume(
     config_map_as_vol = pb.ConfigMapAsVolume(
         config_map_name=config_map_name,
         mount_path=mount_path,
+        optional=optional,
     )
     msg.config_map_as_volume.append(config_map_as_vol)
 

diff --git a/kubernetes_platform/python/kfp/kubernetes/secret.py b/kubernetes_platform/python/kfp/kubernetes/secret.py
@@ -61,6 +61,7 @@ def use_secret_as_volume(
     task: PipelineTask,
     secret_name: str,
     mount_path: str,
+    optional: bool = False,
 ) -> PipelineTask:
     """Use a Kubernetes Secret by mounting its data to the task's container as
     described by the `Kubernetes documentation <https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets-as-files-from-a-pod>`_.
@@ -69,6 +70,7 @@ def use_secret_as_volume(
         task: Pipeline task.
         secret_name: Name of the Secret.
         mount_path: Path to which to mount the Secret data.
+        optional: Optional field specifying whether the Secret must be defined.
 
     Returns:
         Task object with updated secret configuration.
@@ -79,6 +81,7 @@ def use_secret_as_volume(
     secret_as_vol = pb.SecretAsVolume(
         secret_name=secret_name,
         mount_path=mount_path,
+        optional=optional,
     )
 
     msg.secret_as_volume.append(secret_as_vol)

diff --git a/kubernetes_platform/python/test/snapshot/data/config_map_as_vol.yaml b/kubernetes_platform/python/test/snapshot/data/config_map_as_vol.yaml
@@ -56,3 +56,4 @@ platforms:
           configMapAsVolume:
           - mountPath: /mnt/my_vol
             configMapName: my-cm
+            optional: False
diff --git a/kubernetes_platform/python/test/snapshot/data/secret_as_vol.yaml b/kubernetes_platform/python/test/snapshot/data/secret_as_vol.yaml
@@ -56,3 +56,4 @@ platforms:
           secretAsVolume:
           - mountPath: /mnt/my_vol
             secretName: my-secret
+            optional: False