Skip to content

🧹 Migrate to buildah action for building multi-arch images #270

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 2 commits into from
Dec 8, 2022
Merged

🧹 Migrate to buildah action for building multi-arch images #270

openshift-merge-robot merged 2 commits into from
Dec 8, 2022

Conversation

@cardil
Copy link
Member

@cardil cardil commented Nov 30, 2022 •
edited
Loading

Changes

  • 🧹 Migrate to buildah action for building multi-arch images

@openshift-ci
Copy link

openshift-ci bot commented Nov 30, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@cardil cardil force-pushed the feature/ko-less-multiarch-builds branch from 5ff402c to ec694fd Compare November 30, 2022 17:36
@cardil
Copy link
Member Author

cardil commented Dec 2, 2022

It fails to build kn-event test images for linux/s390x platform. Builds for linux/ppc64le and linux/amd64 are passing. Compile problems:

e1:

vendor/github.com/json-iterator/go/reflect_struct_encoder.go:123:2: internal compiler error: '(*structFieldEncoder).IsEmbeddedPtrNil': panic during pre-opt deadcode while compiling (*structFieldEncoder).IsEmbeddedPtrNil

e2:

vendor/github.com/pelletier/go-toml/v2/internal/tracker/key.go:22:8: internal compiler error: '(*KeyTracker).UpdateArrayTable': genValue not implemented: v34 = MVC <mem> [val=56,off=0] v21 v30 v10

e3:

vendor/k8s.io/apimachinery/pkg/conversion/converter.go:199:33: internal compiler error: '(*Converter).Convert': panic during decompose builtin while compiling (*Converter).Convert

Build log: https://github.com/cardil/kn-plugin-event/actions/runs/3583459982/jobs/6028914644

Worth adding that ko build passes: https://github.com/openshift-knative/kn-plugin-event/actions/runs/3585939188/jobs/6034534303#step:7:73 One thing that differs is that ko build is running with CGO_ENABLED=0

Another run, with CGO_ENABLED=0, shows fail again. Although the error is different:

/usr/local/go/src/net/http/h2_bundle.go:9229:21: internal compiler error: '(*http2clientConnReadLoop).processWindowUpdate': panic during regalloc while compiling (*http2clientConnReadLoop).processWindowUpdate:

runtime error: index out of range [1899] with length 2591

Build log: https://github.com/cardil/kn-plugin-event/actions/runs/3592174658/jobs/6047577917

I think, I'll try to deploy a s390x RHEL vm and try to reproduce on clean system. Those errors might be related to buildah multi-arch builds....

/hold

@cardil
Copy link
Member Author

cardil commented Dec 7, 2022

The errors described in #270 (comment) has been fixed by openshift-knative/hack#11

@cardil cardil changed the title 🧹 Migrate the multi-arch builds from ko to buildah 🧹 Migrate to buildah action for building multi-arch images Dec 7, 2022
@cardil cardil force-pushed the feature/ko-less-multiarch-builds branch from a68d1cd to d4aeb74 Compare December 7, 2022 14:25
@cardil cardil marked this pull request as ready for review December 7, 2022 14:26
@cardil
Copy link
Member Author

cardil commented Dec 7, 2022

/cc @mgencur

@openshift-ci openshift-ci bot requested a review from mgencur December 7, 2022 14:26
@cardil
Copy link
Member Author

cardil commented Dec 8, 2022

/unhold

/cc @dsimansk

@openshift-ci openshift-ci bot requested a review from dsimansk December 8, 2022 13:38
dsimansk
dsimansk reviewed Dec 8, 2022
View reviewed changes
.github/workflows/images/test/wathola-forwarder/Containerfile
@@ -0,0 +1,16 @@
FROM docker.io/library/golang:1.19 AS builder
Copy link

@dsimansk dsimansk Dec 8, 2022

Choose a reason for hiding this comment

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

Any concerns about hitting docker.io pull limits?

Copy link
Member Author

@cardil cardil Dec 8, 2022

Choose a reason for hiding this comment

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

Yeah. I've looked at the ubi8/go-toolset image before, but it doesn't have the Go 1.19 yet.

The lag of those Go version made us to switch using the OpenShift CI builder image instead, but that image would require authentication...

The auth is also required for ubi8/go-toolset image...

Copy link
Member Author

@cardil cardil Dec 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

We may use quay.io/projectquay/golang, which says it tracks docker.io/library/golang, but it doesn't have s390x variant... 😿

Copy link

@dsimansk dsimansk Dec 8, 2022

Choose a reason for hiding this comment

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

Thanks for the explanation. IMO it's fine for now. We probably don't produce enough builds to hit it.

Copy link
Member Author

@cardil cardil Dec 8, 2022

Choose a reason for hiding this comment

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

/hold

Let's see productization team opinion...

Copy link
Member Author

@cardil cardil Dec 8, 2022

Choose a reason for hiding this comment

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

@dsimansk: Thanks for the explanation. IMO it's fine for now. We probably don't produce enough builds to hit it.

Well, that depends on how the Docker.com counts, right? We will be executing this on Github's infra, so other OSS projects could influence us.

Copy link
Member Author

@cardil cardil Dec 8, 2022

Choose a reason for hiding this comment

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

/unhold

The productization team needs time to answer, if we can use any other multi-arch image for Github actions (preferably without auth).

dsimansk
dsimansk reviewed Dec 8, 2022
View reviewed changes
.github/workflows/multiarch-build.yaml
push:
branches: ['release-*']
branches-ignore:
- 'main'
Copy link

@dsimansk dsimansk Dec 8, 2022

Choose a reason for hiding this comment

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

Should the ignore include ci/* branches?

Copy link

@dsimansk dsimansk Dec 8, 2022

Choose a reason for hiding this comment

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

I'd probably just keep branches: ['release-*'] rather then switch to opposite variant.

Copy link
Member Author

@cardil cardil Dec 8, 2022

Choose a reason for hiding this comment

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

Done in a454a89

Copy link
Member Author

@cardil cardil Dec 8, 2022

Choose a reason for hiding this comment

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

That change is with openshift-knative/hack#12

Both would allow running this workflow on push to some development branch on fork (without pushing to registry thanks to openshift-knative/hack#12)

@dsimansk
Copy link

dsimansk commented Dec 8, 2022

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm label Dec 8, 2022
@openshift-ci
Copy link

openshift-ci bot commented Dec 8, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, dsimansk

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@cardil cardil mentioned this pull request Dec 8, 2022
Merged
@openshift-merge-robot openshift-merge-robot merged commit ec47b1a into openshift-knative:main Dec 8, 2022
@cardil cardil deleted the feature/ko-less-multiarch-builds branch December 8, 2022 15:33
@cardil
Copy link
Member Author

cardil commented Dec 9, 2022

/cherrypick release-1.8

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Dec 9, 2022
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Dec 9, 2022

@cardil: new pull request created: #284

Details

In response to this:

/cherrypick release-1.8

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.

serverless-qe pushed a commit that referenced this pull request Feb 14, 2023
@upodroid
fix broken checksums generation (#270)
bbb9afa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dsimansk dsimansk dsimansk left review comments

@jrangelramos jrangelramos Awaiting requested review from jrangelramos

@lkingland lkingland Awaiting requested review from lkingland

@mgencur mgencur Awaiting requested review from mgencur

Assignees

@dsimansk dsimansk

Labels

approved lgtm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cardil @dsimansk @openshift-cherrypick-robot @openshift-merge-robot