-
Notifications
You must be signed in to change notification settings - Fork 789
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
base: master
Are you sure you want to change the base?
Handle CentOS forks #228
Conversation
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 [email protected]
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. /cc @chris-crone |
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.: |
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 |
thanks for the feedback and indeed, I went ahead as I saw there was already some fork handling. Might be more in-line with the concept of checking the |
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.
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. |
hi @jochenw , indeed, I changed the detection to be more "Red Hat standard" oriented. Please note also the valid concerns from the Docker team about forks. |
@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"? |
No, I didn't change any follow-up process that the scripts runs. Just the detection of forks and change the OS ID to |
Okay, I have created #230. |
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. |
Any news regarding this? It's hurting our deployments! No reason why this shouldn't be merged. |
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. |
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. |
Any plans on also including Oracle Linux Server?
Manual installation of docker on this release works perfect with the CentOS distribution. |
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 theget_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 [email protected]