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

Update BuildAh to v1.23.1 #959

Conversation

SaschaSchwarze0
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 commented Dec 9, 2021

Changes

I cherry-picked the changes from update buildah image to v1.23.1 #951. In my local tries, the BuildAh build was hanging as well like in the GitHub action runs. No indication for me why. Low resources was my guess, and with more resources it succeeded.

fyi @wanjunlei

Submitter Checklist

  • Includes tests if functionality changed/was added
  • Includes docs if changes are user-facing
  • Set a kind label on this PR
  • Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

Update Buildah to 1.23.1, introduce parameters to setup registry configuration

@SaschaSchwarze0 SaschaSchwarze0 added the kind/dependency-change Categorizes issue or PR as related to changing dependencies label Dec 9, 2021
@openshift-ci openshift-ci bot added the release-note Label for when a PR has specified a release note label Dec 9, 2021
@SaschaSchwarze0 SaschaSchwarze0 added this to the release-v0.8.0 milestone Jan 5, 2022
@SaschaSchwarze0
Copy link
Member Author

Hi @adambkaplan, @gabemontero, @HeavyWombat, may somebody please have a look at this one? Thank you.

@gabemontero
Copy link
Member

/approve

@SaschaSchwarze0 should we update the e2e tests that use samples/buildrun/buildrun_buildah_cr.yaml to exercise the new parameters?

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 20, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gabemontero

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 20, 2022
@SaschaSchwarze0
Copy link
Member Author

@SaschaSchwarze0 should we update the e2e tests that use samples/buildrun/buildrun_buildah_cr.yaml to exercise the new parameters?

Valid question. I am not too familiar with them so far (that part I took from the other PR, I only increased the resources to make it functional again):

  • For registry-search we probably need a Dockerfile with a FROM that consumes from for example our samples but without the ghcr.io prefix. And then we set registry-search to ghcr.io and it should work?
  • For registry-insecure, I am not sure. In our kind cluster we use an instance of registry:2 without HTTPS = it is insecure, but those tests passed all the time.
  • For registry-block we would need a negative test that expects a buildrun to fail when we block the host of a FROM image.

I would put that into a separate issue. I would actually want to tackle this after the array parameters are merged because we can then change the parameters that are introduced here to arrays as that's what they all should be. And then we can also add build-args in that step and only need to touch everything once.

@gabemontero
Copy link
Member

@SaschaSchwarze0 should we update the e2e tests that use samples/buildrun/buildrun_buildah_cr.yaml to exercise the new parameters?

Valid question. I am not too familiar with them so far (that part I took from the other PR, I only increased the resources to make it functional again):

* For `registry-search` we probably need a Dockerfile with a FROM that consumes from for example our samples but without the `ghcr.io` prefix. And then we set registry-search to ghcr.io and it should work?

* For `registry-insecure`, I am not sure. In our kind cluster we use an instance of registry:2 without HTTPS = it is insecure, but those tests passed all the time.

* For `registry-block` we would need a negative test that expects a buildrun to fail when we block the host of a FROM image.

I would put that into a separate issue. I would actually want to tackle this after the array parameters are merged because we can then change the parameters that are introduced here to arrays as that's what they all should be. And then we can also add build-args in that step and only need to touch everything once.

separate issue works for me @SaschaSchwarze0

I'll open it shortly, unless you beat me to it.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2022
@SaschaSchwarze0
Copy link
Member Author

I'll open it shortly, unless you beat me to it.

Thanks @gabemontero. I bet you already. Issue is #988. :-)

@gabemontero
Copy link
Member

you did beat me to it :-)

#988

@openshift-merge-robot openshift-merge-robot merged commit a962084 into shipwright-io:main Jan 20, 2022
@SaschaSchwarze0 SaschaSchwarze0 deleted the sascha-buildah-v1.23.1 branch January 20, 2022 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/dependency-change Categorizes issue or PR as related to changing dependencies lgtm Indicates that a PR is ready to be merged. release-note Label for when a PR has specified a release note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants