Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions backend/src/v2/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,10 +522,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{
Expand Down Expand Up @@ -554,11 +555,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
169 changes: 167 additions & 2 deletions backend/src/v2/driver/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]},
},
},
},
Expand Down Expand Up @@ -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]},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion backend/third_party_licenses/apiserver.csv
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ github.com/klauspost/cpuid/v2,https://github.com/klauspost/cpuid/blob/v2.0.9/LIC
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/9253c7ad7a46/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
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,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/19a24e3e99db/kubernetes_platform/LICENSE,Apache-2.0
github.com/kubeflow/pipelines/kubernetes_platform/go/kubernetesplatform,https://github.com/kubeflow/pipelines/blob/9253c7ad7a46/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
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ require (
github.com/jinzhu/inflection v1.0.0 // indirect
github.com/jinzhu/now v1.1.5 // 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-20240313091817-9253c7ad7a46
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.19
Expand Down
4 changes: 2 additions & 2 deletions go.sum

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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
```


Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions kubernetes_platform/python/kfp/kubernetes/config_map.py
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
3 changes: 3 additions & 0 deletions kubernetes_platform/python/kfp/kubernetes/secret.py
Original file line number Diff line number Diff line change
Expand Up @@ -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>`_.
Expand All @@ -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.
Expand All @@ -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)
Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,4 @@ platforms:
secretAsVolume:
- mountPath: /mnt/my_vol
secretName: my-secret
optional: False
Loading