Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return early in fetcher if no uris are specified. #335

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Return early in fetcher if no uris are specified. #335

wants to merge 1 commit into from

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants