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

featurens: Ensure RHEL is correctly identified #508

Merged
merged 1 commit into from
Mar 16, 2018
Merged

Conversation

joerayme
Copy link
Contributor

When trying to identify various RedHat releases, RHEL was not being
picked up as a centos release because the Oracle Linux regex was too
permissive: it would match any release name with ' Linux
Server release' in the name. By being more restrictive with the Oracle
regex, RHEL is now properly identified.

I don't know why the Oracle regex used such a permissive matcher for the
name but it still passes all the tests by replacing it with the word
'Oracle'.

Fixes #436

When trying to identify various RedHat releases, RHEL was not being
picked up as a centos release because the Oracle Linux regex was too
permissive: it would match any release name with '<something> Linux
Server release' in the name. By being more restrictive with the Oracle
regex, RHEL is now properly identified.

I don't know why the Oracle regex used such a permissive matcher for the
name but it still passes all the tests by replacing it with the word
'Oracle'.

Fixes quay#436
@jzelinskie
Copy link
Contributor

@Djelibeybi does this match all possible Oracle Linux versions or are there some that are outliers?

@Djelibeybi
Copy link
Contributor

Djelibeybi commented Jan 17, 2018

There are no outliers:

$ docker run --rm -ti oraclelinux:5 cat /etc/oracle-release
Oracle Linux Server release 5.11

$ docker run --rm -ti oraclelinux:6 cat /etc/oracle-release
Oracle Linux Server release 6.9

$ docker run --rm -ti oraclelinux:7 cat /etc/oracle-release
Oracle Linux Server release 7.4

@Djelibeybi
Copy link
Contributor

Also, I'm not sure if other distributions are providing an /etc/os-release file, but we do for our current releases, i.e. Oracle Linux 6:

$ docker run --rm -ti oraclelinux:6 cat /etc/os-release
NAME="Oracle Linux Server"
VERSION="6.9"
ID="ol"
VERSION_ID="6.9"
PRETTY_NAME="Oracle Linux Server 6.9"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:oracle:linux:6:9:server"
HOME_URL="https://linux.oracle.com/"
BUG_REPORT_URL="https://bugzilla.oracle.com/"

ORACLE_BUGZILLA_PRODUCT="Oracle Linux 6"
ORACLE_BUGZILLA_PRODUCT_VERSION=6.9
ORACLE_SUPPORT_PRODUCT="Oracle Linux"
ORACLE_SUPPORT_PRODUCT_VERSION=6.9

And Oracle Linux 7:

$ docker run --rm -ti oraclelinux:7 cat /etc/os-release
NAME="Oracle Linux Server"
VERSION="7.4"
ID="ol"
VERSION_ID="7.4"
PRETTY_NAME="Oracle Linux Server 7.4"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:oracle:linux:7:4:server"
HOME_URL="https://linux.oracle.com/"
BUG_REPORT_URL="https://bugzilla.oracle.com/"

ORACLE_BUGZILLA_PRODUCT="Oracle Linux 7"
ORACLE_BUGZILLA_PRODUCT_VERSION=7.4
ORACLE_SUPPORT_PRODUCT="Oracle Linux"
ORACLE_SUPPORT_PRODUCT_VERSION=7.4

Using /etc/os-release may be something to consider because I believe the non-RHEL and derivative distributions also provide this file.

@joerayme
Copy link
Contributor Author

joerayme commented Mar 6, 2018

Is there anything stopping this from being merged? Would you prefer to look at the /etc/os-release file?

Copy link
Contributor

@bison bison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

Sorry this took a while.

@bison bison merged commit 8a2ed86 into quay:master Mar 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants