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

Remove 'docker-' prefix for containerd and runc binaries #37907

Merged
merged 3 commits into from
Sep 26, 2018

Conversation

tiborvass
Copy link
Contributor

This allows to run the daemon in environments that have upstream containerd installed.

Signed-off-by: Tibor Vass tibor@docker.com

@tiborvass tiborvass changed the title Remove 'docker-' prefix for containerd and runc binaries [WIP] Remove 'docker-' prefix for containerd and runc binaries Sep 24, 2018
@tiborvass tiborvass force-pushed the remove-docker-prefix-containerd branch from d077cbb to a417216 Compare September 24, 2018 21:03
@codecov
Copy link

codecov bot commented Sep 24, 2018

Codecov Report

Merging #37907 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #37907      +/-   ##
==========================================
- Coverage    36.1%   36.09%   -0.02%     
==========================================
  Files         610      610              
  Lines       45115    45121       +6     
==========================================
- Hits        16291    16288       -3     
- Misses      26584    26593       +9     
  Partials     2240     2240

@tiborvass tiborvass changed the title [WIP] Remove 'docker-' prefix for containerd and runc binaries Remove 'docker-' prefix for containerd and runc binaries Sep 24, 2018
@tiborvass tiborvass force-pushed the remove-docker-prefix-containerd branch from a417216 to d62e211 Compare September 24, 2018 21:39
This allows to run the daemon in environments that have upstream containerd installed.

Signed-off-by: Tibor Vass <tibor@docker.com>
Copy link
Contributor

@seemethere seemethere left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 👼

Tibor Vass added 2 commits September 25, 2018 16:58
Signed-off-by: Tibor Vass <tibor@docker.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
Copy link
Member

@yongtang yongtang left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda
Copy link
Member

I'm wondering this might cause packaging issues.
e.g. apt/dnf-installing both docker-ce and runc is no longer possible?

@thaJeztah
Copy link
Member

ping @seemethere ^^

@seemethere
Copy link
Contributor

docker-ce doesn't install runc anymore our containerd package does. But yeah our containerd package would essentially overwrite the runc binary installed with the runc distro provided package. Original intention was to have a docker distributed runc package as well but that probably won't happen until late Q4 for us.

@cyphar
Copy link
Contributor

cyphar commented Nov 29, 2018

This is a very bad idea -- Docker has always been stuck on an older version of runc and now we have to do workarounds in order to be able to split the two in the distribution (because, for instance, cri-o uses a newer version).

There is no workaround for this change either! --default-runtime doesn't help because it saves the runtime in the RuntimeConfig (which then causes container to not be able to start on a daemon reboot where the runtime is no longer defined -- we found this out the hard way).

Speaking on the openSUSE side of things, we will carry a revert of this patch.

But yeah our containerd package would essentially overwrite the runc binary installed with the runc distro provided package.

This is not a good practice when packaging (and distributions won't accept such packages), not to mention that attempting to install such packages on most distributions will result in warnings due to file conflicts between packages.

@lifubang
Copy link
Contributor

Yes, it is. Another issue: When I use my own default containerd to start docker, once I kill containerd, then restart dockerd, dockerd will start his own contanerd, then the old containerd-shim is not stopped, and another containerd-shim is started, there are two containerd-shim for one container.

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

Successfully merging this pull request may close these issues.

None yet

9 participants