Skip to content

Commit

Permalink
Fix incorrect aggregation of volume mount and dynamic volumes created…
Browse files Browse the repository at this point in the history
… via Hera (#875)

**Pull Request Checklist**
- [x] Fixes #<!--issue number goes here-->
- [x] Tests added
- [x] Documentation/examples added
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**
Hera supports building dynamic volumes via the `Volume` object. That
creates PVCs + PVs dynamically. Users have the liberty to mount these
volumes wherever they want. However, if users have a volume mount
_already_, then the dynamic volumes are not combined with mounts
correctly. That happens because of the incorrect conditions inside of
`build_volume_mounts`. This PR fixes that

Signed-off-by: Flaviu Vadan <flaviuvadan@gmail.com>
  • Loading branch information
flaviuvadan authored Nov 23, 2023
1 parent 936d1b3 commit c2ef94b
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 8 deletions.
69 changes: 69 additions & 0 deletions docs/examples/workflows/volume_mount.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Volume Mount



This example showcases how to create a volume at a workflow level and use it in a container via a mount.


=== "Hera"

```python linenums="1"
from hera.workflows import Container, SecretVolume, Steps, Volume, Workflow
from hera.workflows.models import VolumeMount

with Workflow(
generate_name="test-",
volumes=[SecretVolume(name="service-account-credential", secret_name="service-account-credential")],
entrypoint="test",
) as w:
v_tmp_pod = Volume(
name="tmp-pod",
size="100Mi",
mount_path="/tmp/pod",
)
init_container_example = Container(
name="git-sync",
volume_mounts=[VolumeMount(name="service-account-credential", mount_path="/secrets")],
volumes=[v_tmp_pod],
)
with Steps(name="test") as s:
init_container_example()
```

=== "YAML"

```yaml linenums="1"
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: test-
spec:
entrypoint: test
templates:
- container:
image: python:3.8
volumeMounts:
- mountPath: /secrets
name: service-account-credential
- mountPath: /tmp/pod
name: tmp-pod
name: git-sync
- name: test
steps:
- - name: git-sync
template: git-sync
volumeClaimTemplates:
- metadata:
name: tmp-pod
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 100Mi
volumes:
- name: service-account-credential
secret:
secretName: service-account-credential
```

32 changes: 32 additions & 0 deletions examples/workflows/volume-mount.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: test-
spec:
entrypoint: test
templates:
- container:
image: python:3.8
volumeMounts:
- mountPath: /secrets
name: service-account-credential
- mountPath: /tmp/pod
name: tmp-pod
name: git-sync
- name: test
steps:
- - name: git-sync
template: git-sync
volumeClaimTemplates:
- metadata:
name: tmp-pod
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 100Mi
volumes:
- name: service-account-credential
secret:
secretName: service-account-credential
21 changes: 21 additions & 0 deletions examples/workflows/volume_mount.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
"""This example showcases how to create a volume at a workflow level and use it in a container via a mount."""
from hera.workflows import Container, SecretVolume, Steps, Volume, Workflow
from hera.workflows.models import VolumeMount

with Workflow(
generate_name="test-",
volumes=[SecretVolume(name="service-account-credential", secret_name="service-account-credential")],
entrypoint="test",
) as w:
v_tmp_pod = Volume(
name="tmp-pod",
size="100Mi",
mount_path="/tmp/pod",
)
init_container_example = Container(
name="git-sync",
volume_mounts=[VolumeMount(name="service-account-credential", mount_path="/secrets")],
volumes=[v_tmp_pod],
)
with Steps(name="test") as s:
init_container_example()
16 changes: 8 additions & 8 deletions src/hera/workflows/_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,25 +539,25 @@ def _build_volume_mounts(self) -> Optional[List[VolumeMount]]:
if self.volume_mounts is None and self.volumes is None:
return None

if isinstance(self.volumes, list):
volumes = self.volumes
elif not isinstance(self.volumes, list) and self.volumes is not None:
volumes = [self.volumes]
else:
if self.volumes is None:
volumes = []
else:
volumes = self.volumes if isinstance(self.volumes, list) else [self.volumes]

result = (
None
if self.volumes is None
if not volumes
else [v._build_volume_mount() if issubclass(v.__class__, _BaseVolume) else v for v in volumes]
)

if result is None and self.volume_mounts is None:
return None
elif result is None and self.volume_mounts is not None:
return self.volume_mounts
elif result is not None and self.volume_mounts is None:
return cast(List[VolumeMount], result)

mounts = cast(List[VolumeMount], self.volume_mounts) or [] + cast(List[VolumeMount], result) or []
return mounts or None
return cast(List[VolumeMount], self.volume_mounts) + cast(List[VolumeMount], result)


class ArgumentsMixin(BaseMixin):
Expand Down

0 comments on commit c2ef94b

Please sign in to comment.