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

RHELAI-473: Adding rhel ai version #665

Merged
merged 1 commit into from
Jul 14, 2024

Conversation

tsorya
Copy link
Contributor

@tsorya tsorya commented Jul 10, 2024

Adding rhel ai version
Set github hash by defautl as image version.
Add RHEL AI as variant into /etc/os-release in order to use it in insights

NAME="Red Hat Enterprise Linux"
VERSION="9.20240630.0.4 (Plow)"
ID="rhel"
ID_LIKE="fedora"
VERSION_ID="9.4"
PLATFORM_ID="platform:el9"
PRETTY_NAME="Red Hat Enterprise Linux 9.20240630.0.4 (Plow)"
ANSI_COLOR="0;31"
LOGO="fedora-logo-icon"
CPE_NAME="cpe:/o:redhat:enterprise_linux:9::baseos"
HOME_URL="https://www.redhat.com/"
DOCUMENTATION_URL="https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Red Hat Enterprise Linux 9"
REDHAT_BUGZILLA_PRODUCT_VERSION=9.4
REDHAT_SUPPORT_PRODUCT="Red Hat Enterprise Linux"
REDHAT_SUPPORT_PRODUCT_VERSION="9.4"
OSTREE_VERSION='9.20240630.0'
VARIANT_ID=rhel_ai1
VARIANT="RHEL AI"
BUILD_ID='v1.1.3'

@Gregory-Pereira
Copy link
Collaborator

Gregory-Pereira commented Jul 11, 2024

This strikes me as a change we should add downstream as a new containerfile or a patch rather than here in the upstream, but not entirely sure. cc @fabiendupont @lmilbaum

@Gregory-Pereira
Copy link
Collaborator

This strikes me as a change we should add downstream as a new containerfile or a patch rather than here in the upstream, but not entirely sure. cc @fabiendupont @lmilbaum

Just found your comment in slack, the above is resolved by this but going to post it here for posterity:

We should set real version in downstream build, just wanted to have some default

@fabiendupont
Copy link
Contributor

The OS can be something else than RHEL, so not sure that RHEL in the variable name is relevant.
/cc @romfreiman @n1hility

@tsorya
Copy link
Contributor Author

tsorya commented Jul 11, 2024

@fabiendupont do we have some more generic name for it?
I mean i can change to make it ${OS_ID}_AI_VERSION or just something hardcoded that will match better, just don't know what.

@tsorya
Copy link
Contributor Author

tsorya commented Jul 11, 2024

@Gregory-Pereira i think setting git hash in upstream or provide the way for dev to setup their own versioning can be helpful too and in downstream we will set release versions

@tsorya tsorya force-pushed the igal/rhel_ai_version branch 2 times, most recently from b96ea42 to 2a61260 Compare July 11, 2024 10:00
@rhatdan
Copy link
Member

rhatdan commented Jul 11, 2024

I agree this should be done downstream, not upstream.

@tsorya tsorya force-pushed the igal/rhel_ai_version branch 2 times, most recently from 1f521d0 to f2515e9 Compare July 11, 2024 11:29
@tsorya
Copy link
Contributor Author

tsorya commented Jul 11, 2024

I agree this should be done downstream, not upstream.

Currently downstream uses upstream containerfile. I added verification that it is rhel image in order to set this field only for rhel

@tsorya tsorya force-pushed the igal/rhel_ai_version branch from f2515e9 to 46c7c8e Compare July 11, 2024 11:31
@tsorya
Copy link
Contributor Author

tsorya commented Jul 11, 2024

/hold till we will decide which exact var we should change in /etc/os-release

@tsorya
Copy link
Contributor Author

tsorya commented Jul 11, 2024

/hold

Set github hash by defautl as image version.
Add RHEL_AI_VERSION into /etc/os-release in order to use it in
insights

Signed-off-by: Igal Tsoiref <[email protected]>
@tsorya
Copy link
Contributor Author

tsorya commented Jul 12, 2024

/unhold
Seems like we have decision

@tsorya tsorya force-pushed the igal/rhel_ai_version branch from 46c7c8e to e7bd5e3 Compare July 14, 2024 08:36
@@ -146,6 +148,9 @@ RUN mv /etc/selinux /etc/selinux.tmp \
# Install rhc connect for insights telemetry gathering
&& . /etc/os-release && if [ "${ID}" == "rhel" ]; then \
dnf install -y rhc rhc-worker-playbook; \
sed -i -e "/^VARIANT=/ {s/^VARIANT=.*/VARIANT=\"RHEL AI\"/; t}" -e "\$aVARIANT=\"RHEL AI\"\"" /etc/os-release;
Copy link
Contributor

Choose a reason for hiding this comment

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

amd, intel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do in next PR, in anycase GA is nvidia.

@tsorya tsorya changed the title Adding rhel ai version RHELAI-473: Adding rhel ai version Jul 14, 2024
@lmilbaum
Copy link
Collaborator

Joining late to the party. What does this variable used for? If we agree that it is needed, should it be populated via a builds arg file?

Copy link
Collaborator

@lmilbaum lmilbaum left a comment

Choose a reason for hiding this comment

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

Approving this PR to unblock @tsorya from implementing the logic on the insights side. A greater effort is required which will probably change some of the bits contributed by this PR. Filing a Jira issue for the greater effort.

@lmilbaum lmilbaum merged commit 3168496 into containers:main Jul 14, 2024
1 check passed
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.

6 participants