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

Podvm builder dockerfile #517

Closed

Conversation

littlejawa
Copy link
Contributor

Creating a new Dockerfile dedicated to building the podvm-builder image in Konflux.
It seems we can't use the existing Dockerfile as it is, because of the subscription
management done in Konflux.
(see PR #516)

This Dockerfile is a clean copy of Dockerfile.podvm-builder, with the subscription
lines removed.
Also updating the source references to kata and cloud-api-adaptor to point to our forks under the Openshift org.

Creating a new Dockerfile dedicated to building the podvm-builder image in Konflux.
It seems we can't use the existing Dockerfile as it is, because of the subscription
management done in Konflux.
(see PR openshift#516)

This Dockerfile is a clean copy of Dockerfile.podvm-builder, with the subscription
lines removed.

Signed-off-by: Julien Ropé <[email protected]>
For the Openshift-dedicated Dockerfile, make the source references point
to the forks under the Openshift org.

Signed-off-by: Julien Ropé <[email protected]>
@openshift-ci openshift-ci bot requested review from gkurz and tbuskey January 30, 2025 09:47
@littlejawa
Copy link
Contributor Author

@spotlesstofu - I'd like your comments on this one, and/or on the #516 one.
The solution I'm going with (Adding a new Dockerfile) is not ideal, as it duplicates code, but I'm not sure how to deal with the subscription properly. Have you faced something like that with Trustee?

At the moment, my vote is to go with the duplicated Dockerfile, and if/when we find a better way, fix it to keep a single one. But any suggestion is welcome :-)

@jensfr jensfr requested a review from snir911 January 30, 2025 11:58
Copy link

openshift-ci bot commented Jan 30, 2025

@littlejawa: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@snir911
Copy link
Contributor

snir911 commented Jan 30, 2025

Thanks @littlejawa , have you tried to comment this line out? if it's still identified by kuflux also as a comment I'd simply remove the line completely and add a comment mentioning "for unsubscribe systems subscribe using subscription manager as the first build step" or something like that

@littlejawa
Copy link
Contributor Author

Yes, commenting it is enough.
I didn't dare removing it, but if it's an option, that would be nice. :-)
Should I reopen the other PR ?

@snir911
Copy link
Contributor

snir911 commented Jan 30, 2025

I'd leave it commented out, it's useful when you build on unsubscribed system

@littlejawa
Copy link
Contributor Author

I have re-opened #516 and commented the lines in the Dockerfile. It builds now.
If I get approvals and merge 516, we can close this one


RUN mkdir -p /scripts

ADD lib.sh libvirt-podvm-image-handler.sh aws-podvm-image-handler.sh azure-podvm-image-handler.sh /scripts/
Copy link
Member

Choose a reason for hiding this comment

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

Probably this will conflict with #519 but I can handle on my side. Just adding a note so we (I) don't forget :)


ARG ORG_ID
ARG ACTIVATION_KEY

Copy link
Member

@beraldoleal beraldoleal Jan 31, 2025

Choose a reason for hiding this comment

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

About the subscription code that was here... I'm not familiar on how konflux handles this (maybe @spotlesstofu).

But, I was wondering if something like this would work:

RUN if [[ -z "${KONFLUX_BUILD}" ]]; then \
    rm -f /etc/rhsm-host && rm -f /etc/pki/entitlement-host; \
    subscription-manager register --org=${ORG_ID} --activationkey=${ACTIVATION_KEY}; \
    fi

Not sure if the konflux already exposes any variable, but if not, maybe we can configure those variables within the tekton folder. This way we could avoid having two dockerfiles. Wdyt?

@littlejawa
Copy link
Contributor Author

Dropping in favor of #516

@littlejawa littlejawa closed this Feb 3, 2025
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.

3 participants