-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
In CentOS 8.x Docker install Step: remove podman when existing #8016
Conversation
Welcome @panpan0000! |
Hi @panpan0000. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
actually, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @oomichi
state: absent | ||
when: | ||
- ansible_os_family == 'RedHat' | ||
- (docker_versioned_pkg[docker_version | string] is search('docker-ce')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request message says both containerd.io and docker-ce can be conflict against podman.
Why doesn't here check containerd?
Isn't it enough to just check like
- container_manager in ['docker', 'containerd']
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
containerd.io
is part/dependency of docker-ce
defined in roles/container-engine/docker/vars/*.yml
So condition check by docker-ce
will be enough.
moreover, this pre-upgrade.yml
belongs to roles/container-engine/docker
, this folder is for docker
.
When user wants to install containerd
only without docker, whether it will conflict with podman ? I've no chance to test it so far, and this problem belongs to roles/container-engine/containerd
sub folder.
And the original pre-upgrade.yml
separate Debian & RedHat into 2 Ensure old versions of Docker are not installed
task. I think only one package with state absent
is enough. But I don't want to involve multiple change into one PR and left it as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your explanation, I see your point.
/ok-to-test
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: floryut, panpan0000 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Thanks @panpan0000 |
What type of PR is this?
What this PR does / why we need it:
To Fix : #7289
in CentOS 8.x like 8.4, when the OS installation was configured as "generic server" or so on,
podman
will be installed by default.But podman will conflict with
containerd.io-1.4.9-3.1.el8
and docker-ce.So my fix will erase the existing podman in container-engine step.
Which issue(s) this PR fixes:
Fixes #7289
The confliction between podman and containerd.io/docker is as below:
Special notes for your reviewer:
Does this PR introduce a user-facing change?: