-
Notifications
You must be signed in to change notification settings - Fork 24
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
Konflux(ROX-22195): Install RHEL RPMs with subscription manager workaround #1573
Conversation
Co-authored-by: red-hat-trusted-app-pipeline <[email protected]>
Co-authored-by: red-hat-trusted-app-pipeline <123456+red-hat-trusted-app-pipeline[bot]@users.noreply.github.com>
Co-authored-by: red-hat-trusted-app-pipeline <123456+red-hat-trusted-app-pipeline[bot]@users.noreply.github.com>
…nto tm/rhtap-onboarding
Co-authored-by: red-hat-trusted-app-pipeline <123456+red-hat-trusted-app-pipeline[bot]@users.noreply.github.com>
Co-authored-by: red-hat-trusted-app-pipeline <[email protected]>
Co-authored-by: red-hat-trusted-app-pipeline <123456+red-hat-trusted-app-pipeline[bot]@users.noreply.github.com>
…nto tm/rhtap-onboarding
Co-authored-by: red-hat-trusted-app-pipeline <123456+red-hat-trusted-app-pipeline[bot]@users.noreply.github.com>
Co-authored-by: red-hat-trusted-app-pipeline <123456+red-hat-trusted-app-pipeline[bot]@users.noreply.github.com>
Co-authored-by: red-hat-trusted-app-pipeline <123456+red-hat-trusted-app-pipeline[bot]@users.noreply.github.com>
wget \ | ||
which \ | ||
unzip \ |
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.
Updated the dependency list to https://gitlab.cee.redhat.com/stackrox/rhacs-midstream/-/blob/rhacs-1.0-rhel-8/distgit/containers/rhacs-collector-slim/Dockerfile.in?ref_type=heads. Previously it was the list of upstream dependencies because of the CentOS base image.
This is the diff between slim and full Dockerfiles:
|
Co-authored-by: Misha Sugakov <[email protected]>
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.
LGTM
I realized the naming suggestion I made was not the best one. Feel free to ship it with our without addressing the last two comments.
@@ -40,7 +40,9 @@ FROM scratch as builder | |||
|
|||
COPY --from=rpm-implanter-builder /mnt / | |||
|
|||
COPY . . | |||
ARG SOURCES_DIR=/staging |
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.
Optional, cosmetics, and up to you.
ARG SOURCES_DIR=/staging | |
ARG SOURCES_DIR=/src |
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 think the chances of a directory in ubi existing called /src
are slightly higher than /staging
, so let's keep it like that.
@@ -40,7 +40,9 @@ FROM scratch as builder | |||
|
|||
COPY --from=rpm-implanter-builder /mnt / | |||
|
|||
COPY . . | |||
ARG SOURCES_DIR=/staging |
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.
Optional, cosmetics, and up to you.
ARG SOURCES_DIR=/staging | |
ARG SOURCES_DIR=/src |
Description
This PR changes the dependency installations in the Konflux images from using CentOS dependencies to using a UBI image with workaround entitlements to install RHEL RPMs.
Also harmonized the Dockerfiles between
-slim
and-full
based on midstream.Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
Passing Konflux pipeline is sufficient.