Conversation
Romain-Geissler-1A
force-pushed
the
master
branch
from
6edb7d9 to
0fd75bd
Compare
|
Note: this will require #633 to be merged first. |
|
Note: when this will be merged and docker/containerd-packaging#270 will be merged/delivered, I have the local changes to build for CentOS 9 too. |
Romain-Geissler-1A
force-pushed
the
master
branch
from
0fd75bd to
c6d8007
Compare
Romain-Geissler-1A
changed the title
|
Ping |
rpm/SPECS/docker-ce.spec
Outdated
There was a problem hiding this comment.
Should we make this conditional based on rhel (/centos) version? If it's not deprecated in rhel7 and centos7, (or 8) we could make it something like;
| %if ! 0%{?rhel} | |
| %if 0%{?rhel} == 7 || 0%{?rhel} == 8 |
There was a problem hiding this comment.
Using straight (==) comparisons to account for other distros who would like not have the rhel macro (but double check if my suggestion is correct 😅)
There was a problem hiding this comment.
Well, libcgroup is deprecated in RHEL 7 already: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/resource_management_guide/chap-using_libcgroup_tools
The libcgroup package, which was the main tool for cgroup management in previous versions of Red Hat Enterprise Linux, is now deprecated. To avoid conflicts, do not use libcgroup tools for default resource controllers (listed in Available Controllers in Red Hat Enterprise Linux 7) that are now an exclusive domain of systemd. This leaves a limited space for applying libcgroup tools, use it only when you need to manage controllers not currently supported by systemd, such as net_prio.
There was a problem hiding this comment.
The devil can be in the detail
This leaves a limited space for applying libcgroup tools, use it only when you need to manage controllers not currently supported by systemd, such as net_prio.
so I think we should consider keeping it (for now) for the versions that still carry the package
There was a problem hiding this comment.
Ok, then. I used the expression %if 0%{?rhel} <= 8 does it fly for you ?
thaJeztah
changed the title
Romain-Geissler-1A
force-pushed
the
master
branch
from
c6d8007 to
a8e12b8
Compare
|
Note that another prerequisite to building for RHEL/CentOS 9 is considering something like docker/containerd-packaging#270 (which is failing the CI tests, but since external contributors can't see the Jenkins logs, I don't know why it's failing). |
|
Ping |
1 similar comment
|
Ping |
|
Ping |
Romain-Geissler-1A
changed the title
|
@thaJeztah Pull request rebased and updated to adopt your style as in docker/containerd-packaging#283. I have also modified the pull request purpose, now it removes libcgroup dependency for RHEL >= 9 and also builds a package for CentOS 9 at the same time. Most likely the CI will remain red until a containerd package for CentOS 9 is delivered in the official docker repositories. |
Romain-Geissler-1A
force-pushed
the
master
branch
3 times, most recently
from
f504b8f to
fbca96d
Compare
| # In aarch64 (arm64) images, the altarch repo is specified as repository, but | ||
| # failing, so replace the URL. | ||
| RUN if [ -f /etc/yum.repos.d/CentOS-Stream-Sources.repo ]; then sed -i 's/altarch/centos/g' /etc/yum.repos.d/CentOS-Stream-Sources.repo; fi | ||
|
|
There was a problem hiding this comment.
This one was an odd one; I know we added it at some point, then we removed it (because it appeared it didn't have the altarch anymore); #416 (comment)
But later, we had to add it back, because for some reason, our pipeline started failing again on this; see #446
Could it be these can be set dynamically, depending on what mirror is hit?
| # In aarch64 (arm64) images, the altarch repo is specified as repository, but | ||
| # failing, so replace the URL. | ||
| RUN if [ -f /etc/yum.repos.d/CentOS-Stream-Sources.repo ]; then sed -i 's/altarch/centos/g' /etc/yum.repos.d/CentOS-Stream-Sources.repo; fi | ||
|
|
There was a problem hiding this comment.
This one was an odd one; I know we added it at some point, then we removed it (because it appeared it didn't have the altarch anymore); #416 (comment)
But later, we had to add it back, because for some reason, our pipeline started failing again on this; see #446
Could it be these can be set dynamically, depending on what mirror is hit?
There was a problem hiding this comment.
I thought this was some old hack no longer necessary. But if you say it's still needed, I will put it back.
There was a problem hiding this comment.
The honest answer: "I don't know!" 😂 (I also thought it was redundant, and then... all of a sudden, it failed in one of our internal pipelines)
Perhaps we should keep it "just to be safe" (it looks like it doesn't hurt to have it, and would be a no-op if it's not needed).
Signed-off-by: Romain Geissler <romain.geissler@amadeus.com>
|
Overall; changes look good to me; I'm running a build locally, as CI doesn't pick up changes in Jenkinsfiles for users without write-access |
|
Working on a (temporary) fix for failing CI; looks like someone's vanity URL went down; #684 |
|
I kicked CI, but looks like GitHub actions doesn't do a merge before running again (whereas jenkins does). I tested this locally, and it looks to be working as expected |
2 participants
This shall be backported to branch 20.10 too.