USHIFT-978 Building RPMs in container #1515

pmtk · 2023-03-16T15:43:22Z

No description provided.

ggiguash · 2023-03-16T15:57:51Z

hack/Containerfile.rpm-builder

+    dnf install \
+        --setopt=tsflags=nodocs \
+        --setopt=install_weak_deps=False \
+        --repo=rhel-8-for-x86_64-baseos-rpms \


We're supporting RHEL 9 now in the main branch. Can we use RHEL9 containers / dependencies?

This is because we use registry.ci.openshift.org/openshift/release:rhel-8-release-golang-1.19-openshift-4.13.
I see that rhel-9-release-golang-1.19-openshift-4.13 exists but it fails due to some certificates for rhel-9-for-x86_64-baseos-rpms repo.
Also, we're still using rhel-8-release-golang-1.19-openshift-4.13 in the CI jobs

ggiguash · 2023-03-16T16:01:50Z

Makefile

+	podman build -v /etc/pki/entitlement/:/etc/pki/entitlement -t rpm-builder - < ./hack/Containerfile.rpm-builder
+.PHONY: rpm-builder
+
+rpm-in-container:


Let's simplify the process by having one top-level target?

We already have 'make rpm' & 'make srpm' - can we add 'make rpm-podman'?

Good idea, I'll just jam both building the image (if doesn't exist) and and this target into one

ggiguash · 2023-03-16T16:09:58Z

A general question: why would we want to build in a container if we have a devenv? Are you thinking to use this for building 4.12 branch on a RHEL9 machine for example?

pmtk · 2023-03-16T17:36:10Z

A general question: why would we want to build in a container if we have a devenv? Are you thinking to use this for building 4.12 branch on a RHEL9 machine for example?

We'll be able to use the same image CI uses, i.e. rhel8 (or 9) + go1.19 (which isn't available in the repos yet) and drop the workaround with installing two versions of Go, therefore fixing the microshift.spec to require right Go version (now we "require" 1.18, but it'll fail if 1.19 is not used).
That go1.1x workaround happened twice already so someday it'll happen again, so this would be one less thing to keep in mind.

I feel like QE's pipeline would benefit from being able to build in a container. From what I've seen recently, they only run configure-vm.sh to get that go1.19 side-installed, everything else is in the kickstart.
If we could get make iso containerized QE could drop dev-vm altogether most likely speeding up the procedure.
Same thing locally on laptop, build ISO without needing build vm (most likely with limited resources compared to the laptop).

ggiguash · 2023-03-17T06:13:34Z

Should we also add "go" invocation from the container and get rid of the "go" dependency dillema in the devenv all together? I mean, we could remove "go" installation from configure-vm.sh and have container builds executed by default for compilation and RPM creation.

pmtk · 2023-03-17T06:35:08Z

Should we also add "go" invocation from the container and get rid of the "go" dependency dillema in the devenv all together? I mean, we could remove "go" installation from configure-vm.sh and have container builds executed by default for compilation and RPM creation.

Maybe, we need to think about this.
I'm partial to leaving it as is because I build bins on laptop and copy on VM - I have Go on my laptop either way.
This would also be appreciated by devs with MacOS as Docker on Mac is actually docker on linux vm on Mac

dhellmann · 2023-03-17T17:57:29Z

Should we also add "go" invocation from the container and get rid of the "go" dependency dillema in the devenv all together? I mean, we could remove "go" installation from configure-vm.sh and have container builds executed by default for compilation and RPM creation.

Maybe, we need to think about this. I'm partial to leaving it as is because I build bins on laptop and copy on VM - I have Go on my laptop either way. This would also be appreciated by devs with MacOS as Docker on Mac is actually docker on linux vm on Mac

I may have a different workflow from others on the team. I set up a VM with all of the dependencies and do all of my work in that VM, so I install my editor, the go compiler, gopls, git, etc. I have heard others say they build elsewhere and copy files into the VM, or at least edit elsewhere.

dhellmann · 2023-03-17T17:52:54Z

Makefile

@@ -214,6 +214,20 @@ image-build-iso: rpm
 iso: image-build-configure image-build-iso
 .PHONY: iso

+rpm-podman:
+	@if [ -z "$(shell podman images -q microshift-rpm-builder)" ]; then \


This test is neat. I usually just let podman build run again and it takes a second to realize there is no change, but this looks quicker.

+1 on letting podman build figure out if there's a need to (re)build. It comes with a bonus of automatic rebuild when something changes in the Containerfile.

ggiguash · 2023-03-18T06:58:28Z

@pmtk , let's pull the Containerfile out of the hack directory.
We have a Dockerfile under packaging/images/openshift-ci directory. Since we're using a CI image, should we put the new file somewhere nearby?

ggiguash · 2023-03-18T07:03:32Z

Makefile

+	@if [ -z "$(shell podman images -q microshift-rpm-builder)" ]; then \
+		podman build \
+			-v /etc/pki/entitlement/:/etc/pki/entitlement \
+			-t microshift-rpm-builder - < ./hack/Containerfile.rpm-builder ; \


Do we want to include a version in the image name, i.e. -t microshift-4.13-rpm-builder-rhel8-go1.19? I mean, we should be able to easily cross-compile from different branches on the same machine now, but images may be different.

ggiguash · 2023-03-21T06:32:48Z

Makefile

+  gawk 'BEGIN { FS=":" } \
+	/FROM/ { \
+		match($$0, /rhel-([0-9]+).*golang-([0-9]\.[0-9]{2})-openshift-([0-9]+\.[0-9]+)/,m);\
+		printf "%s-rhel%s-go%s",m[3],m[1],m[2]; }' packaging/images/Containerfile.rpm-builder)


Since we're building the container from make, can we simplify this by defining a variable in the Makefile and passiing it to the Containerfile?

Makefile: GOLANG_CONTAINER=release:rhel-8-release-golang-1.19-openshift-4.13 podman build --env GOLANG_CONTAINER=$GOLANG_CONTAINER ... Containerfile: ENV GOLANG_CONTAINER FROM registry.ci.openshift.org/openshift/$GOLANG_CONTAINER

Following your suggestion, does microshift-builder:rhel-8-release-golang-1.19-openshift-4.13 sound good as an image name:tag?

I think so, because the CI image name/version is the main identifier for it.
Unless, of course, there's a limitation in docker not allowing to use that name as-is.

Changed, PTAL

ggiguash · 2023-03-21T12:03:01Z

/lgtm

openshift-ci · 2023-03-21T12:03:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ggiguash, pmtk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ggiguash,pmtk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2023-03-21T14:05:59Z

@pmtk: 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/test-infra repository. I understand the commands that are listed here.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 16, 2023

openshift-ci bot requested review from mangelajo and oglok March 16, 2023 15:47

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 16, 2023

ggiguash reviewed Mar 16, 2023

View reviewed changes

pmtk force-pushed the 859-rpm-in-container branch from 8ccf333 to 6091274 Compare March 17, 2023 17:39

pmtk force-pushed the 859-rpm-in-container branch from 6091274 to 4788820 Compare March 17, 2023 18:17

pmtk changed the title ~~WIP Building RPMs in container~~ USHIFT-978 Building RPMs in container Mar 17, 2023

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 17, 2023

dhellmann reviewed Mar 17, 2023

View reviewed changes

ggiguash reviewed Mar 18, 2023

View reviewed changes

pmtk force-pushed the 859-rpm-in-container branch from 4788820 to 206a83f Compare March 20, 2023 14:36

ggiguash reviewed Mar 21, 2023

View reviewed changes

pmtk force-pushed the 859-rpm-in-container branch from 206a83f to 1ec4536 Compare March 21, 2023 09:30

building RPMs in container

91ac0ea

pmtk force-pushed the 859-rpm-in-container branch from 1ec4536 to 91ac0ea Compare March 21, 2023 11:45

openshift-ci bot assigned ggiguash Mar 21, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 21, 2023

openshift-merge-robot merged commit 54fc6f7 into openshift:main Mar 21, 2023

pmtk deleted the 859-rpm-in-container branch March 29, 2023 13:26

microshift-rebase-script bot mentioned this pull request Jan 17, 2025

NO-ISSUE: rebase-main-4.19.0-0.nightly-2025-01-16-064700_amd64-2025-01-16_arm64-2025-01-17 #4424

Merged

USHIFT-978 Building RPMs in container #1515

USHIFT-978 Building RPMs in container #1515

Uh oh!

Conversation

pmtk commented Mar 16, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiguash commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pmtk commented Mar 16, 2023

Uh oh!

ggiguash commented Mar 17, 2023

Uh oh!

pmtk commented Mar 17, 2023

Uh oh!

dhellmann commented Mar 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiguash Mar 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiguash commented Mar 18, 2023

Uh oh!

ggiguash Mar 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiguash Mar 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiguash commented Mar 21, 2023

Uh oh!

openshift-ci bot commented Mar 21, 2023

Uh oh!

openshift-ci bot commented Mar 21, 2023

Uh oh!

Uh oh!

ggiguash commented Mar 16, 2023 •

edited

Loading

ggiguash Mar 18, 2023 •

edited

Loading

ggiguash Mar 18, 2023 •

edited

Loading

ggiguash Mar 21, 2023 •

edited

Loading