-
Notifications
You must be signed in to change notification settings - Fork 54
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
Move remap logic from install.sh to omnitruck #259
Conversation
Tested on Amazon Linux 2 and 2018.03. I'll fire up some older variants to make sure this works further back. |
Our current detection of Amazon Linux 2 is broken. We relied on parsing out /etc/system-release contents and expecting a pretty specific text string format. Amazon changed this between the RC and the final release and broke installation on Amazon Linux 2. Total bummer. This PR removes the logic to remap Amazon to EL 6 or 7 and instead detects Amazon via /etc/os-release and looks up the exact version from that file. This means instead of having install.sh request an download file for EL 6 it will request amazon 2018.03. This lets us handle the remapping directly in omnitruck and it also gives us the ability to actually see from the server side when users are requesting installs on Amazon Linux, which would be pretty handy to know. Furthermore moving the logic out of install.sh and into omnitruck makes it really easy for us to push out a fix in the future. Some users cache install.sh locally and even if we fix the code on our site they're still using the old logic. If install.sh doesn't do as much heavy lifting and instead relies on omnitruck for that we can easily push out logic updates to omnitruck in the event that something like Amazon Linux 3 comes out that needs to make to Redhat 8. Note: This will require a PR to omnitruck to handle the mapping correctly. Signed-off-by: Tim Smith <[email protected]>
@@ -55,19 +55,6 @@ elif test -f "/etc/redhat-release"; then | |||
platform="el" | |||
fi | |||
|
|||
elif test -f "/etc/system-release"; then | |||
platform=`sed 's/^\(.\+\) release.\+/\1/' /etc/system-release | tr '[A-Z]' '[a-z]'` | |||
platform_version=`sed 's/^.\+ release \([.0-9]\+\).*/\1/' /etc/system-release | tr '[A-Z]' '[a-z]'` |
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.
^ i don't think you can delete this?
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.
Amazon has /etc/os-release which is WAY nicer since you can just source it and check the env variables.
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.
oh joy. Amazon Linux 2013/2014 don't have the os-release file.
2013/2014 don't have os-release yet. Since amazon is a rolling release in the 201X series if anyone did a yum update they'd get the file, but just in case let's make sure we can't still parse the old file. Signed-off-by: Tim Smith <[email protected]>
Updated and tested on ancient amazon linux @lamont-granquist |
@@ -55,19 +55,14 @@ elif test -f "/etc/redhat-release"; then | |||
platform="el" | |||
fi | |||
|
|||
# detect amazon linux 2013/2014 which lack /etc/os-release |
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.
/etc/system-release is some fedora-ism, though, not just amazon linux.
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.
yeah i think /etc/redhat-release -> /etc/system-release in fedora at some point, but then systemd's /etc/os-release came along. amazon uses /etc/system-release because they're a "fedoraish" distro which has not yet gone to /etc/os-release, but that can't necessarily be relied upon to uniquely detect amazon.
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.
(or it looks like they do have /etc/os-release... anyway point stands that none of these files are unique to amazon..)
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.
2015+ they switched to os-release. We already had the os-release logic so this way we're just going to continue the same old logic we had, but if we get back amzn from os-release we'll convert it to amazon. That will be a nice future proof way to get the version number without brittle regexing of the release name. For 2013/2014 we'll fall back to that crummy old regex.
Our current detection of Amazon Linux 2 is broken. We relied on parsing out /etc/system-release contents and expecting a pretty specific text string format. Amazon changed this between the RC and the final release and broke installation on Amazon Linux 2. Total bummer.
This PR removes the logic to remap Amazon to EL 6 or 7 and instead detects Amazon via /etc/os-release and looks up the exact version from that file. This means instead of having install.sh request an download file for EL 6 it will request amazon 2018.03. This lets us handle the remapping directly in omnitruck and it also gives us the ability to actually see from the server side when users are requesting installs on Amazon Linux, which would be pretty handy to know.
Furthermore moving the logic out of install.sh and into omnitruck makes it really easy for us to push out a fix in the future. Some users cache install.sh locally and even if we fix the code on our site they're still using the old logic. If install.sh doesn't do as much heavy lifting and instead relies on omnitruck for that we can easily push out logic updates to omnitruck in the event that something like Amazon Linux 3 comes out that needs to make to Redhat 8.
Note: This will require a PR to omnitruck to handle the mapping correctly.
Signed-off-by: Tim Smith [email protected]