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

Handle CentOS forks #228

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Handle CentOS forks #228

wants to merge 2 commits into from

Conversation

nunix
Copy link

@nunix nunix commented Apr 29, 2021

Since CentOS 8 going into a rolling release mode with CentOS Stream, new forks have appeared for keeping the "stable release".

The first one to be available is Almalinux and the current script does not support it.

This PR addresses this gap, and as I don't yet the name/IDs of the future forks, I decided to go with a case statement, as it will be easier to manage in the long run.

By adding the case statement in the get_distribution() function, this avoids any impact on the creation of the distribution URL used in the script.

Finally, this fixes the issue in the Moby repo: moby/moby#42337

Signed-off-by: nunix corsair@hey.com

Since CentOS 8 going into a rolling release mode with CentOS Stream, new forks have appeared for keeping the "stable release".

The first one to be available is Almalinux and the current script does not support it.

This PR addresses this gap, and as I don't yet the name/IDs of the future forks, I decided to go with a `case` statement, as it will be easier to manage in the long run.

By adding the `case` statement in the `get_distribution()` function, this avoids any impact on the creation of the distribution URL used in the script.

Signed-off-by: nunix corsair@hey.com
@thaJeztah
Copy link
Member

I'm a bit cautious on adding support for forks; we did have code for Debian derivatives in the past, and it was hell; things would often break (either due to "matching" not picking the right variant, or because the forks we just slightly different), adding forks to the script gives the suggestion that they're supported, but we can't give any guarantees.
(also see https://docs.docker.com/engine/install/#other-linux-distributions)

/cc @chris-crone

@chris-crone
Copy link
Member

Yes, I am a bit worried about maintaining this over time as well. Could we have a manual override or something to make it explicit that you're opting into installing packages on an unsupported distro?

e.g.: curl https://get.docker.com | DISTRO=centos sh

@thaJeztah
Copy link
Member

Manual override could possibly work.

Quick solution would be to change the error message, and add a link to that "other distros" section, so that users can follow the manual instructions for the distro that's the closest match

@nunix
Copy link
Author

nunix commented Apr 29, 2021

thanks for the feedback and indeed, I went ahead as I saw there was already some fork handling.
Another possible way, would be to check the ID_LIKE as we will have "debian" for Ubuntu for example, and "rhel centos fedora" in the case of Almalinux.

Might be more in-line with the concept of checking the /etc/os-release file?

Instead of checking for the new CentOS forks IDs, the check is made on the file "redhat-release".

This should include any future fork without the need to update this script.

I kept the check in the "get_distribution()" function, but let me know if it would make sense to have a  "check_forked_redhat()" function instead, in order to stay aligned with the current script logic.
@jochenw
Copy link

jochenw commented May 16, 2021

I understand, that this PR assumes the presence of /etc/redhat-release on CentOS forks? Confirming, that this should work on AlmaLinux, thus supersedes moby/moby#42337.

@nunix
Copy link
Author

nunix commented May 16, 2021

hi @jochenw , indeed, I changed the detection to be more "Red Hat standard" oriented.
I tested it with AlmaLinux and lately with Rocky Linux too and both went fine. But please feel free to test it on your own.

Please note also the valid concerns from the Docker team about forks.

@jochenw
Copy link

jochenw commented May 18, 2021

@nunix This is, most likely, not a good place to ask. On the other hand, as we are already discussing CentOS, and compatibles:

Did you address the necessity to do a "dnf -y remove runc"?

@nunix
Copy link
Author

nunix commented May 18, 2021

No, I didn't change any follow-up process that the scripts runs. Just the detection of forks and change the OS ID to centos.
If the script was doing it already for CentOS, then it will continue doing it for the forks.

@jochenw
Copy link

jochenw commented May 18, 2021

Okay, I have created #230.

@mattvw
Copy link

mattvw commented Oct 28, 2021

Any updates on this as we approach the end of 2021 and thus the EOL of normal CentOS 8?

I know this might not be the right place, but on the same subject... any idea if we will still have Docker CE RPM repos for CentOS 8 or some CentOS-replacement (Rocky Linux is at the front IMO) after CentOS 8 goes EOL? What kind of packaging support is expected after the end of this year? CentOS Stream may not work as a real replacement since it's not a CentOS 8 clone...

Thanks.

@markd69
Copy link

markd69 commented Jan 15, 2022

Any news regarding this? It's hurting our deployments!

No reason why this shouldn't be merged.

@austin-millan
Copy link

Any way to get this this script working with AlmaLinux, a RHEL fork?

$ cat /etc/redhat-release 
AlmaLinux release 9.0 (Emerald Puma)
$ curl -fsSL https://raw.githubusercontent.com/docker/docker-install/d4716364808e26093a8fde88ebc94cc99b5cef3f/install.sh -o install-docker.sh
$ sudo sh ./install-docker.sh 
# Executing docker install script, commit: 

ERROR: Unsupported distribution 'forked distribution, switching to centos id
centos

@markd69
Copy link

markd69 commented Jul 19, 2022

Any way to get this this script working with AlmaLinux, a RHEL fork?

$ cat /etc/redhat-release 
AlmaLinux release 9.0 (Emerald Puma)
$ curl -fsSL https://raw.githubusercontent.com/docker/docker-install/d4716364808e26093a8fde88ebc94cc99b5cef3f/install.sh -o install-docker.sh
$ sudo sh ./install-docker.sh 
# Executing docker install script, commit: 

ERROR: Unsupported distribution 'forked distribution, switching to centos id
centos

Check the commits tab on this pull request and make the changes to your own version of the script. Docker doesn’t appear to want to merge it, or add centos 9 support.

@AkihiroSuda
Copy link
Contributor

An alternative proposal:

The support term of CentOS Stream is much shorter than Rocky and Alma, so it is probably inevitable to create RPMs for Rocky and Alma without depending on CentOS Stream.

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.

None yet

8 participants