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

feat(manager docker) switch to be based on ubi9-minimal #4244

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

yaronkaikov
Copy link
Collaborator

To achieve OpenShift certification for the Scylla-Operator, our Scylla Manager container images must be based on Red Hat's UBI images.

Fixes: #4242

To achieve OpenShift certification for the Scylla-Operator, our Scylla Manager container images need to be based on Red Hat's UBI images.

Fixes: #4242
@yaronkaikov
Copy link
Collaborator Author

@karol-kokoszka can we merge this?

@karol-kokoszka
Copy link
Collaborator

Yes, we can.

@karol-kokoszka karol-kokoszka merged commit 59372b6 into master Feb 12, 2025
51 checks passed
@karol-kokoszka karol-kokoszka deleted the switch-docker-ubi branch February 12, 2025 08:24
@Michal-Leszczynski
Copy link
Collaborator

@yaronkaikov does this PR fix #4242? I'm asking in the context of this comment.

@mflendrich
Copy link

This PR isn't enough to close #4242 - the preflight check won't pass, at least the labels are missing.

@yaronkaikov
Copy link
Collaborator Author

@yaronkaikov does this PR fix #4242? I'm asking in the context of this comment.

probably not, but we can send a follow-up patches to make it right. Just not sure when

@mflendrich
Copy link

follow-up sounds good 👍. Good to see manager on UBI, however the entire list (at least the labels) is a blocking requirement for openshift certification as well. So we'll need to get it scheduled somehow.

@yaronkaikov could you please reopen #4242 so that we don't lose track of the tail end?
I don't have reopen rights on the repo. Thanks 🙏

@yaronkaikov
Copy link
Collaborator Author

I re-opened it

@@ -1,15 +1,13 @@
FROM ubuntu
ARG ARCH=amd64
FROM docker.io/redhat/ubi9-minimal:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

Should have used quay.io really.

It's unclear why we use latest, but we can review this decision later.

Copy link
Contributor

@mykaul mykaul Feb 13, 2025

Choose a reason for hiding this comment

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

In fact, we should use registry.access.redhat.com/ubi9/ubi-minimal:latest

apt-get clean && \
RUN microdnf update && \
microdnf -y upgrade && \
microdnf install -y ca-certificates && \
rm -rf /var/lib/apt/lists/*
Copy link
Contributor

@mykaul mykaul Feb 13, 2025

Choose a reason for hiding this comment

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

Is this the way to clean microdnf residue?

I thought it is
microdnf clean all

rm -rf /var/lib/apt/lists/*

COPY release/scylla-manager-agent*$ARCH.deb /
RUN dpkg -i scylla-manager-agent*$ARCH.deb && rm /scylla-manager-agent*.deb
COPY release/scylla-manager-agent*$ARCH.rpm /
Copy link
Contributor

Choose a reason for hiding this comment

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

This creates another layer, which I assume we don't squash later?

@mykaul
Copy link
Contributor

mykaul commented Feb 13, 2025

@yaronkaikov apologies for not reviewing this, but I have left some comments. I believe we need a follow-up to this work - shall I open another issue?

@yaronkaikov
Copy link
Collaborator Author

@yaronkaikov apologies for not reviewing this, but I have left some comments. I believe we need a follow-up to this work - shall I open another issue?

I have re-opened the original issue since we are missing some labels for the certification, please add relevant info there, or open a new one.

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.

Move to UBI based container images for Scylla Manager containers
5 participants