odo dev on podman: Add support for devfile volume #6328
Conversation
✅ Deploy Preview for odo-docusaurus-preview canceled.
|
openshift-ci
bot
added
the
kind/feature
pkg/dev/podmandev/pod.go
Outdated
| Name: devfileVolume.Name, | ||
| VolumeSource: corev1.VolumeSource{ | ||
| PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ | ||
| ClaimName: getVolumeName(componentName, appName, devfileVolume.Name), |
There was a problem hiding this comment.
For cluster, we use a different naming strategy devfileVolume.Name+componentName+appName for volume components of Devfile. Perhaps we should use the same naming strategy for podman as well?
There was a problem hiding this comment.
You are right, I think I misunderstood the naming strategy on cluster. I have made chages, to change name for devfile volumes and also for the "shared" volume. WDYT?
feloy
force-pushed
the
feature-6151/volumes-on-podman
branch
from
63daaed
to
2c0af4d
Compare
Co-authored-by: Parthvi Vala <pvala@redhat.com>
feloy
force-pushed
the
feature-6151/volumes-on-podman
branch
from
f4a1e1d
to
bfd25e2
Compare
openshift-ci
bot
added
the
lgtm
There was a problem hiding this comment.
Wondering if we shouldn't generate EmptyDir volumes when the Ephemeral preference is true, to be consistent with the cluster mode?
I just saw that Podman could support EmptyDir volumes by creating anonymous volumes (containers/podman#15473).
(But if you prefer, we can handle this in a separate issue/PR as well).
Effectively, podman can handle emptyDir and PVC, and they are instantiated practically the same way: both are podman volumes, and emptyDir ones will be removed when pod is removed, as opposed to the PVC will be removed by odo at the same time the pod is removed. I didn't want to add complexity for this small difference. But we can handle this in a separate PR is necessary |
|
Kudos, SonarCloud Quality Gate passed!
|
There was a problem hiding this comment.
Wondering if we shouldn't generate
EmptyDirvolumes when theEphemeralpreference istrue, to be consistent with the cluster mode? I just saw that Podman could supportEmptyDirvolumes by creating anonymous volumes (containers/podman#15473). (But if you prefer, we can handle this in a separate issue/PR as well).Effectively, podman can handle emptyDir and PVC, and they are instantiated practically the same way: both are podman volumes, and emptyDir ones will be removed when pod is removed, as opposed to the PVC will be removed by odo at the same time the pod is removed.
I didn't want to add complexity for this small difference. But we can handle this in a separate PR is necessary
Okay, no problem. I've added a reminder note about this in the parent Epic.
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rm3l The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/override ci/prow/v4.11-integration-e2e |
|
@feloy: Overrode contexts on behalf of feloy: ci/prow/v4.11-integration-e2e In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
What type of PR is this:
/kind feature
What does this PR do / why we need it:
Which issue(s) this PR fixes:
Fixes #6330
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer: