Skip to content

Conversation

@carlonelong
Copy link
Contributor

Change-Id: I9e78b0a9a58e6d84885eae768a5d76e13288c7af

Change-Id: I9e78b0a9a58e6d84885eae768a5d76e13288c7af
@lava
Copy link
Contributor

lava commented May 23, 2019

Hey @carlonelong ,

thanks for contributing!

Your patch looks reasonable to me, however when I ran it through our CI it seemed to break a few unit tests, in particular:

Test Name Duration Age
mesos-ec2-ubuntu-16.04-SSL.Mesos.DockerContainerizerTest.ROOT_DOCKER_UNPRIVILEGED_USER_NonRootSandbox 6 sec 1
mesos-ec2-ubuntu-16.04-SSL.Mesos.MesosContainerizerExecuteTest.ROOT_UNPRIVILEGED_USER_SandboxFileOwnership 0.1 sec 1
mesos-ec2-ubuntu-16.04-SSL.Mesos.MesosContainerizerDestroyTest.DestroyWhileFetching 15 sec 1
mesos-ec2-ubuntu-16.04-SSL.Mesos.ProvisionerDockerTest.ROOT_INTER

(and the same for debian9, centos6/7, ubuntu 14.04, etc.)

I did not have the time to investigate what exactly is causing these tests to fail, but I think this will need to be investigated before we can commit the patch.

Best regards,
Benno

@carlonelong
Copy link
Contributor Author

carlonelong commented May 28, 2019

@lava
Hi. After reading fetcher.cpp more carefully, I find that the reason these tests failed is that we'll call "chown" in FetcherProcess::run.
We may move this part ahead to solve this. But it's not elegant enough.

I also checked MESOS-5218/6843/6217. And I agreed with what Jie Yu said:"The fetcher code is a mess. We should try to clean it up if possible IMO."

I think, FIFO and named pipes used in containerd(between a shim and its container) might be good examples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants