Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

agent: pass correct mount type to agent for ephemeral volumes #1439

Conversation

YongjiXie
Copy link
Contributor

The "ephemeral" is just used to indicate ephemeral volumes in
runtime. We should not pass it to agent. Instead, "bind" should be
the correct mount type to be passed.

Fixes: #1438

Signed-off-by: Xie Yongji xieyongji@baidu.com

The "ephemeral" is just used to indicate ephemeral volumes in
runtime. We should not pass it to agent. Instead, "bind" should be
the correct mount type to be passed.

Fixes: kata-containers#1438

Signed-off-by: Xie Yongji <xieyongji@baidu.com>
@YongjiXie YongjiXie closed this Mar 28, 2019
@YongjiXie YongjiXie reopened this Mar 28, 2019
Copy link

@sboeuf sboeuf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @amshinde PTAL

@@ -1119,6 +1119,8 @@ func (k *kataAgent) handleEphemeralStorage(mounts []specs.Mount) []*grpc.Storage
if mnt.Type == kataEphemeralDevType {
// Set the mount source path to a path that resides inside the VM
mounts[idx].Source = filepath.Join(ephemeralPath, filepath.Base(mnt.Source))
// Set the mount type to "bind"
mounts[idx].Type = "bind"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the original code works only because k8s would set the mount type to bind as well to tell runc to bindmount the host tmpfs mountpoint to container's ephemeral volume. But it does feel safer to explicitly set it. Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bergwolf yes, that is the case today, which is why it works. k8s already sets this to "bind".
@YongjiXie This change is not absolutely required, I guess its good to have it set explicitly for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bergwolf @amshinde Actually what I did in this patch is changing the mount type from “ephemeral” to "bind". Without this patch, we will pass something like: Mounts:<destination:"/mnt" source:"/run/kata-containers/sandbox/ephemeral/cache" type:"ephemeral" options:"rbind" options:"rprivate" options:"rw" > to kata agent.

And I guess the original code works only because we have "rbind" option. And libcontainer will still do mounting although it doesn't know the mount type in kata agent.

Thanks,
Yongji

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YongjiXie I see. The problem is that instead of exposing a proper sandbox level storage structure, we modified oci spec to indicate an ephemeral mount. Thanks for the explanation!

@bergwolf
Copy link
Member

bergwolf commented Apr 1, 2019

/retest

1 similar comment
@bergwolf
Copy link
Member

bergwolf commented Apr 2, 2019

/retest

@jcvenegas
Copy link
Member

@GabyCT does the suse job errors are fixed now ?
@amshinde I'd like your ack here to merge this PR

@GabyCT
Copy link
Contributor

GabyCT commented Apr 10, 2019

@jcvenegas no, the opensuse issue is still there kata-containers/ci#136

@amshinde
Copy link
Member

@jcvenegas @GabyCT Are the opensuse and nemy CIs supposed to pass?
If not, please merge.

@GabyCT
Copy link
Contributor

GabyCT commented Apr 10, 2019

@amshinde opensuse is not but nemu yes

@amshinde
Copy link
Member

Ok. I am getting a 404 on the nemu one.Restarting all
/test

@bergwolf bergwolf merged commit 168665b into kata-containers:master Apr 16, 2019
@YongjiXie YongjiXie deleted the pass-correct-mount-type-for-ephemeral-volumes branch April 16, 2019 07:42
@egernst egernst mentioned this pull request Apr 16, 2019
This was referenced May 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass correct mount type to agent for ephemeral volumes
6 participants