-
Notifications
You must be signed in to change notification settings - Fork 373
virtcontainers: Add support for ephemeral volumes #458
Conversation
PSS Measurement: Memory inside container: |
This PR implements the discussion #443 to avoid using devices at sandbox level. |
The design looks good to me, but need changes from kata-agent before we can merge this. |
@harche It seems that the PR works w/o agent change, right? The missing piece in kata agent is just the ref counting part but tmpfs handling is already there. |
@bergwolf @WeiZhang555 This PR works w/o agent change. |
@WeiZhang555 Since this PR works without any changes in kata-agent, I think once I fix the CI issues around cyclomatic complexities it can be merged. Also, I think I can close #307, as we aren't going to pursue that path anymore. What do you guys think? @WeiZhang555 @bergwolf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reworking this @harche.
This looks good to me, I have only one comment but this could be addressed in a separate PR as part of some refactoring.
cli/create.go
Outdated
// of the same pod the already existing volume is reused. | ||
for idx, mnt := range ociSpec.Mounts { | ||
if IsEphemeralStorage(mnt.Source) { | ||
ociSpec.Mounts[idx].Type = "ephemeral" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to define the type ephemeral
(as other types) inside a separate package that can be imported both from the agent and the runtime. This can be done in a follow up PR, but this would be better than simply hardcode the type at each end of the chain (runtime on one side, and the agent on the other side.)
PSS Measurement: Memory inside container: |
Codecov Report
@@ Coverage Diff @@
## master #458 +/- ##
==========================================
+ Coverage 64.16% 64.25% +0.08%
==========================================
Files 87 87
Lines 8942 8969 +27
==========================================
+ Hits 5738 5763 +25
- Misses 2584 2586 +2
Partials 620 620
Continue to review full report at Codecov.
|
I believe it can work w/o agent change, as @bergwolf described, we need a reference counter in kata-agent to mark when the "ephemeral" volume can be unmounted, without this, the exit of any container could lead to umount of the tmpfs which will break the "ephemeral volume", this could happen when one container in POD failed to start and has to be restarted several times. |
@harche Could you please fix UT coverage and remove the wip tag? Then the PR is ready to get merged. |
PSS Measurement: Memory inside container: |
Build succeeded (third-party-check pipeline).
|
PSS Measurement: Memory inside container: |
Build succeeded (third-party-check pipeline).
|
PSS Measurement: Memory inside container: |
Build succeeded (third-party-check pipeline).
|
Ephemeral volumes should not be passed at 9pfs mounts. They should be created inside the VM. This patch disables ephemeral volumes from getting mounted as 9pfs from the host and instead a corresponding tmpfs is created inside the VM. Fixes : kata-containers#61 Signed-off-by: Harshal Patil <harshal.patil@in.ibm.com>
PSS Measurement: Memory inside container: |
Build succeeded (third-party-check pipeline).
|
The codecov failure I think we can safely merge this but I want to understand why In that case, I would suggest we either remove @egernst @sboeuf @jodh-intel @grahamwhaley @WeiZhang555 @gnawux WDYT? |
Moving |
Ephemeral volumes should not be passed at 9pfs mounts.
They should be created inside the VM.
This patch disables ephemeral volumes from getting
mounted as 9pfs from the host and instead a corresponding
tmpfs is created inside the VM.
Fixes : #61
Signed-off-by: Harshal Patil harshal.patil@in.ibm.com