Skip to content

Commit

Permalink
fix(Backend + SDK): Add missing optional field to SecretAsVolume and … (
Browse files Browse the repository at this point in the history 
kubeflow#10550)

* fix(Backend + SDK): Add missing optional field to SecretAsVolume and ConfigMapAsVolume.

Signed-off-by: Revital Sur <eres@il.ibm.com>

* Update after rebase.

Signed-off-by: Revital Sur <eres@il.ibm.com>

* Update after rebase.

Signed-off-by: Revital Sur <eres@il.ibm.com>

* Update after merge.

Signed-off-by: Revital Sur <eres@il.ibm.com>

* Updates after merge with master branch.

Signed-off-by: Revital Sur <eres@il.ibm.com>

---------

Signed-off-by: Revital Sur <eres@il.ibm.com>
  • Loading branch information
@revit13 @VaniHaripriya
revit13 authored and VaniHaripriya committed Sep 23, 2024
1 parent 2a58dd1 commit 954628a
Show file tree
Hide file tree
Showing 12 changed files with 224 additions and 31 deletions.
3 changes: 2 additions & 1 deletion backend/src/v2/driver/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -553,11 +553,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{
Expand Down
89 changes: 87 additions & 2 deletions backend/src/v2/driver/driver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ func Test_extendPodSpecPatch_Secret(t *testing.T) {
{
Name: "secret1",
VolumeSource: k8score.VolumeSource{
Secret: &k8score.SecretVolumeSource{SecretName: "secret1", Optional: &[]bool{false}[0]},
Secret: &k8score.SecretVolumeSource{SecretName: "secret1", Optional: &[]bool{false}[0],},
},
},
},
Expand Down Expand Up @@ -730,7 +730,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]},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion backend/third_party_licenses/apiserver.csv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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
Expand Down
2 changes: 1 addition & 1 deletion backend/third_party_licenses/driver.csv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ github.com/klauspost/compress/flate,https://github.com/klauspost/compress/blob/v
github.com/klauspost/pgzip,https://github.com/klauspost/pgzip/blob/v1.2.6/LICENSE,MIT
github.com/kubeflow/pipelines/api/v2alpha1/go,https://github.com/kubeflow/pipelines/blob/58ce09e07d03/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/lestrrat-go/strftime,https://github.com/lestrrat-go/strftime/blob/v1.0.4/LICENSE,MIT
github.com/mailru/easyjson,https://github.com/mailru/easyjson/blob/v0.7.7/LICENSE,MIT
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 42 additions & 4 deletions kubernetes_platform/python/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,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
Expand Down Expand Up @@ -94,9 +113,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)
```


Expand Down Expand Up @@ -173,7 +211,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
Expand Down
3 changes: 3 additions & 0 deletions kubernetes_platform/python/kfp/kubernetes/config_map.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>`_.
Expand All @@ -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.
Expand All @@ -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)

Expand Down
1 change: 1 addition & 0 deletions kubernetes_platform/python/test/snapshot/data/config_map_as_vol.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,4 @@ platforms:
configMapAsVolume:
- mountPath: /mnt/my_vol
configMapName: my-cm
optional: False
76 changes: 70 additions & 6 deletions kubernetes_platform/python/test/unit/test_config_map.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,66 @@ def my_pipeline():
'exec-comp': {
'configMapAsVolume': [{
'configMapName': 'cm-name',
'mountPath': 'cmpath'
'mountPath': 'cmpath',
'optional': False
}]
}
}
}
}
}
}

def test_use_one_optional_true(self):

@dsl.pipeline
def my_pipeline():
task = comp()
kubernetes.use_config_map_as_volume(
task,
config_map_name='cm-name',
mount_path='cmpath',
optional=True)

assert json_format.MessageToDict(my_pipeline.platform_spec) == {
'platforms': {
'kubernetes': {
'deploymentSpec': {
'executors': {
'exec-comp': {
'configMapAsVolume': [{
'configMapName': 'cm-name',
'mountPath': 'cmpath',
'optional': True
}]
}
}
}
}
}
}

def test_use_one_optional_false(self):

@dsl.pipeline
def my_pipeline():
task = comp()
kubernetes.use_config_map_as_volume(
task,
config_map_name='cm-name',
mount_path='cmpath',
optional=False)

assert json_format.MessageToDict(my_pipeline.platform_spec) == {
'platforms': {
'kubernetes': {
'deploymentSpec': {
'executors': {
'exec-comp': {
'configMapAsVolume': [{
'configMapName': 'cm-name',
'mountPath': 'cmpath',
'optional': False
}]
}
}
Expand Down Expand Up @@ -72,11 +131,13 @@ def my_pipeline():
'configMapAsVolume': [
{
'configMapName': 'cm-name1',
'mountPath': 'cmpath1'
'mountPath': 'cmpath1',
'optional': False
},
{
'configMapName': 'cm-name2',
'mountPath': 'cmpath2'
'mountPath': 'cmpath2',
'optional': False
},
]
}
Expand Down Expand Up @@ -119,7 +180,8 @@ def my_pipeline():
}],
'configMapAsVolume': [{
'configMapName': 'cm-name2',
'mountPath': 'cmpath2'
'mountPath': 'cmpath2',
'optional': False
},]
}
}
Expand Down Expand Up @@ -156,7 +218,8 @@ def my_pipeline():
}],
'configMapAsVolume': [{
'configMapName': 'cm-name',
'mountPath': 'cmpath'
'mountPath': 'cmpath',
'optional': False
}]
}
}
Expand Down Expand Up @@ -289,7 +352,8 @@ def my_pipeline():
}],
'configMapAsVolume': [{
'configMapName': 'cm-name2',
'mountPath': 'cmpath2'
'mountPath': 'cmpath2',
'optional': False
},]
}
}
Expand Down
Loading

0 comments on commit 954628a

Please sign in to comment.