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

fix CI #12343

Merged
merged 1 commit into from
Nov 18, 2021
Merged

fix CI #12343

merged 1 commit into from
Nov 18, 2021

Conversation

vrothberg
Copy link
Member

@vrothberg vrothberg commented Nov 18, 2021

Our fedora-minimal image on Quay bases on fedora-minimal:latest which
starting with F35 removed a number of binaries that our CI depends on.
Fix that by pulling fedora-minimal:34 from the Fedora registry
directly.

Once the build bot on Quay has been disabled, we move the image over
there to make sure that it will not change over time.

Signed-off-by: Valentin Rothberg [email protected]

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 18, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vrothberg

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 Nov 18, 2021
@vrothberg
Copy link
Member Author

@edsantiago @cevich PTAL

@edsantiago
Copy link
Member

Failure looks real:

Error: error looking up supplemental groups for container f58c950ef2786dc33244b949da6b038a269db0aa03b20e35955cd1186e9e62d5: Unable to find group kvm

@@ -1515,8 +1515,8 @@ USER mail`, BB)
Expect(session.ErrorToString()).To(ContainSubstring("file descriptor 3 is not available"))
})

It("podman run --privileged and --group-add", func() {
groupName := "kvm"
FIt("podman run --privileged and --group-add", func() {
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a debug leak.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for catching this!

@mheon
Copy link
Member

mheon commented Nov 18, 2021

LGTM for reference

Our fedora-minimal image on Quay bases on fedora-minimal:latest which
starting with F35 removed a number of binaries that our CI depends on.
Fix that by pulling `fedora-minimal:34` from the Fedora registry
directly.

Once the build bot on Quay has been disabled, we move the image over
there to make sure that it will not change over time.

Signed-off-by: Valentin Rothberg <[email protected]>
@cevich
Copy link
Member

cevich commented Nov 18, 2021

Once the build bot on Quay has been disabled, we move the image over
there to make sure that it will not change over time.

Q: We own the Dockerfile that makes this image, why not simply add a RUN microdnf install shadow-utils to it?

@vrothberg
Copy link
Member Author

Q: We own the Dockerfile that makes this image, why not simply add a RUN microdnf install shadow-utils to it?

I find it wasteful to rebuild an image (that never changes) after each PR.

@edsantiago
Copy link
Member

I have super-strong opinions about test images: they should not change. Ever.

For discussion (later): I recommend using my system-test convention. Not foo:latest, but foo:YYYYMMDD.

@cevich
Copy link
Member

cevich commented Nov 18, 2021

I have super-strong opinions about test images: they should not change. Ever.

I agree with this also. I was just saying, to get things working we can make the image however we want for now. The auto-build trigger has been disabled.

@cevich
Copy link
Member

cevich commented Nov 18, 2021

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2021
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2021
@edsantiago
Copy link
Member

Flake is the usual registry.redhat.io one. Restarted.

@vrothberg
Copy link
Member Author

Flake is the usual registry.redhat.io one. Restarted.

Thank you!

@cevich
Copy link
Member

cevich commented Nov 18, 2021

IRC Discussion summary:

  • I disabled the auto-build trigger for quay.io/libpod/fedora-minimal
  • I manually build and pushed a F34 version of quay.io/libpod/fedora-minimal:latest and quay.io/libpod/fedora-minimal:34
  • A future PR will partially revert some of the changes here, to make use of quay.io/libpod/fedora-minimal:34 going forward.
  • Further followup PRs are needed to update CI on podman branches, to also use the tagged quay image.

@vrothberg
Copy link
Member Author

Thank you, @cevich !

@edsantiago
Copy link
Member

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2021
@vrothberg
Copy link
Member Author

🕺 thanks everybody!

@openshift-merge-robot openshift-merge-robot merged commit 9b96494 into containers:main Nov 18, 2021
@vrothberg vrothberg deleted the fix-ci branch November 18, 2021 15:50
cevich added a commit to cevich/buildah that referenced this pull request Nov 19, 2021
Around 18th of Nov. 2021, a new/incompatible
`quay.io/libpod/fedora-minimal:latest` was pushed by the quay
auto-builder servicing the podman repo.  This caused a number of
problems across several branches.  To address this, the auto-builder was
disabled and a fixed F34 based image was tagged and pushed as both
`latest` and `34`.  However, all repositories which use this test image
need to be individually updated to reference it.

See containers/podman#12343

Signed-off-by: Chris Evich <[email protected]>
edsantiago pushed a commit to edsantiago/libpod that referenced this pull request Nov 22, 2021
[Backport containers#12343, which fixes the change in fedora-minimal image]

Our fedora-minimal image on Quay bases on fedora-minimal:latest which
starting with F35 removed a number of binaries that our CI depends on.
Fix that by pulling `fedora-minimal:34` from the Fedora registry
directly.

Once the build bot on Quay has been disabled, we move the image over
there to make sure that it will not change over time.

Signed-off-by: Valentin Rothberg <[email protected]>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants