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

empty-dir: Fix bug in the way empty-dirs are handled for overlay #1826

Merged
merged 1 commit into from
Jul 1, 2019

Conversation

amshinde
Copy link
Member

With #1485, we moved the default medium empty-dir creation to the
sandbox rootfs. This worked for devicemapper, but in case of overlay
the "local" directory was being created outside the sandbox rootfs.
As a result we were seeing the behaviour seen in #1818.

Fixes #1818

Signed-off-by: Archana Shinde archana.m.shinde@intel.com

@amshinde
Copy link
Member Author

cc @awprice

@amshinde amshinde requested a review from egernst June 24, 2019 20:21
@@ -1285,7 +1285,7 @@ func (k *kataAgent) handleEphemeralStorage(mounts []specs.Mount) []*grpc.Storage

// handleLocalStorage handles local storage within the VM
// by creating a directory in the VM from the source of the mount point.
func (k *kataAgent) handleLocalStorage(mounts []specs.Mount, sandboxID string) []*grpc.Storage {
func (k *kataAgent) handleLocalStorage(mounts []specs.Mount, sandboxID string, rootfsSuffix string) []*grpc.Storage {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function doesn't have any unit tests so I think, given the subtle nature of this bug, we need one.

Copy link
Member

@egernst egernst left a comment

Choose a reason for hiding this comment

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

LGTM, though agree wrt unit test.

With kata-containers#1485, we moved the default medium empty-dir creation to the
sandbox rootfs. This worked for devicemapper, but in case of overlay
the "local" directory was being created outside the sandbox rootfs.
As a result we were seeing the behaviour seen in kata-containers#1818.

Fixes kata-containers#1818

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
@amshinde
Copy link
Member Author

@jodh-intel @egernst Test added!

@amshinde
Copy link
Member Author

/test

Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

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

Thanks @amshinde.

@jodh-intel
Copy link
Contributor

Ping @kata-containers/runtime !

@gnawux
Copy link
Member

gnawux commented Jun 28, 2019

Looks this PR make the behavior of overlay identical to DeviceMapper, which is good to me.

While having chatted with @bergwolf, he has some other suggestions.

@egernst
Copy link
Member

egernst commented Jul 1, 2019

@bergwolf let's pull in further changes in new PR?

@egernst egernst merged commit 3a45481 into kata-containers:master Jul 1, 2019
@amshinde amshinde deleted the empty-dir-fix-overlay branch October 4, 2019 18:18
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.

K8s emptyDir on virtio-fs provisioned on memory filesystem
5 participants