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

[v4.4.1-crio] Bump c/common to v0.52.0 #22355

Conversation

TomSweeneyRedHat
Copy link
Member

As the title says, bumping the c/common package to v0.52.0

in the v4.4.1-crio branch via a request from the CRI-O team.

Addresses: https://issues.redhat.com/browse/OCPBUGS-32144

[NO NEW TESTS NEEDED]

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 11, 2024
@TomSweeneyRedHat
Copy link
Member Author

@kwilczynski PTAL

This is dragging a ton of stuff in, I hope it's not too much.

@TomSweeneyRedHat TomSweeneyRedHat added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 17, 2024
go.mod Outdated
github.com/containers/conmon v2.0.20+incompatible
github.com/containers/image/v5 v5.24.1
github.com/containers/image/v5 v5.25.0
Copy link
Member

Choose a reason for hiding this comment

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

This aligns with CRI-O release 1.25. Newer releases are already on version 5.27.0 or newer.

go.mod Outdated
github.com/containernetworking/cni v1.1.2
github.com/containernetworking/plugins v1.2.0
github.com/containers/buildah v1.29.3
github.com/containers/common v0.51.3
github.com/containers/common v0.52.0
Copy link
Member

Choose a reason for hiding this comment

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

This aligns with CRI-O release 1.25. Newer releases are already on version 0.53.0 or newer.

@kwilczynski
Copy link
Member

This is dragging a ton of stuff in, I hope it's not too much.

@TomSweeneyRedHat, the two most important dependencies here for us are the c/common and c/image, and updates to both look OK from our point of view.

We are good to go, I believe.

@kwilczynski
Copy link
Member

/approve
/lgtm

Copy link
Contributor

openshift-ci bot commented Apr 30, 2024

@kwilczynski: changing LGTM is restricted to collaborators

In response to this:

/approve
/lgtm

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.

@kwilczynski
Copy link
Member

@TomSweeneyRedHat, even though we look good from our key dependencies point of view, we have some anciliary dependency issues per:

vendor/github.com/openshift/imagebuilder/dockerfile/parser/parser.go:122:12: undefined: system.LCOWSupported
vendor/github.com/openshift/imagebuilder/dockerfile/parser/parser.go:157:12: undefined: system.LCOWSupported

Let me have a look as to how we resolved this in CRI-O. I believe a dependency update would fix this.

@kwilczynski
Copy link
Member

@TomSweeneyRedHat, even though we look good from our key dependencies point of view, we have some anciliary dependency issues per:

vendor/github.com/openshift/imagebuilder/dockerfile/parser/parser.go:122:12: undefined: system.LCOWSupported
vendor/github.com/openshift/imagebuilder/dockerfile/parser/parser.go:157:12: undefined: system.LCOWSupported

Let me have a look as to how we resolved this in CRI-O. I believe a dependency update would fix this.

This missing method issue has been fixed in the following release of the openshift/imagebuilder package:

We should upgrade the dependency to version 1.2.4 (a proper full release) or newer. We also use version 1.2.4 of the said package in CRI-O 1.25, where this failure does not occur.

go.mod Outdated
github.com/opencontainers/runc v1.1.12
github.com/opencontainers/runtime-spec v1.1.0-rc.1
github.com/opencontainers/runtime-tools v0.9.1-0.20221014010322-58c91d646d86
github.com/opencontainers/runtime-tools v0.9.1-0.20221107090550-2e043c6bd626
github.com/opencontainers/selinux v1.11.0
github.com/openshift/imagebuilder v1.2.4-0.20220711175835-4151e43600df
Copy link
Member

Choose a reason for hiding this comment

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

@TomSweeneyRedHat, if possible, this needs a version bump to:

Suggested change
github.com/openshift/imagebuilder v1.2.4-0.20220711175835-4151e43600df
github.com/openshift/imagebuilder v1.2.4

This looks confusing; however, it's how this project tagged their release.

@kwilczynski
Copy link
Member

kwilczynski commented May 16, 2024

@TomSweeneyRedHat, do you think you could see about these comments:

To fix the build issue and potentially also move forward with this branch? I would be much obliged.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 17, 2024
@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

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.

@TomSweeneyRedHat TomSweeneyRedHat force-pushed the dev/tsweeney/common_0.52.0_bump-v4.4.1-crio branch from 74bdc90 to 6071a2b Compare June 7, 2024 20:53
Copy link
Contributor

openshift-ci bot commented Jun 7, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kwilczynski, TomSweeneyRedHat

The full list of commands accepted by this bot can be found 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 removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 7, 2024
TomSweeneyRedHat added a commit to TomSweeneyRedHat/podman that referenced this pull request Jun 8, 2024
As the title says.  This replaces
containers#22355.

I'm not sure how I managed, but the original PR: containers#22355,
I accidentally pushed into the v4.4.1-crio branch as a merge commit.

I've reverted that, and redid the vendor for c/common 0.52.0

This dragged in c/image v5.25.0, and I also needed to bump
image builder as the version of Docker that also got dragged in
dropped the LCOWSupported functionality and c/storage by that
time had taken it over.

So long story short, a lot more changes than I'm entirely comfortable
with.

@kwilczynski PTAL.  Sorry about the very slow response on this, we had
to get some CVE fixes first, and in so doing, discovered the CI in this
branch was pretty badly broken and had to fix that .

Addresses: https://issues.redhat.com/browse/OCPBUGS-32144

Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>
@TomSweeneyRedHat
Copy link
Member Author

Replaced by: #22944

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants