Skip to content

vendor: drop override for github.com/opencontainers/runc#19817

Closed
giuseppe wants to merge 1 commit intocontainers:mainfrom
giuseppe:drop-runc-override
Closed

vendor: drop override for github.com/opencontainers/runc#19817
giuseppe wants to merge 1 commit intocontainers:mainfrom
giuseppe:drop-runc-override

Conversation

@giuseppe
Copy link
Copy Markdown
Member

@giuseppe giuseppe commented Aug 31, 2023

podman builds fine with the latest runc, so we can drop the override.

Closes: #19795

Does this PR introduce a user-facing change?

None

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Aug 31, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 31, 2023
@giuseppe giuseppe mentioned this pull request Aug 31, 2023
Closed
vrothberg
vrothberg reviewed Aug 31, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@giuseppe giuseppe force-pushed the drop-runc-override branch from ede4dc1 to 6af4701 Compare August 31, 2023 08:57
@giuseppe
Copy link
Copy Markdown
Member Author

giuseppe commented Aug 31, 2023

shurg, we are pulling in a huge new dependency (github.com/cilium/ebpf).

As usual, I forgot to run git add vendor before pushing the first version of the patch

@vrothberg
Copy link
Copy Markdown
Member

vrothberg commented Aug 31, 2023

I think there's still some vendor-dependency fart. The main branch of runc doesn't require cilium anymore. But trying to vendor that will revert many other dependencies.
$ go mod edit -replace github.com/opencontainers/runc=github.com/opencontainers/runc@24ae5c258c41a122f1b96319c087337bac7b308a moves it to the latest commit and doesn't pull in cilium ... but it still replaces.

@giuseppe
Copy link
Copy Markdown
Member Author

giuseppe commented Aug 31, 2023

I still see cilium in the latest revision: https://github.com/opencontainers/runc/blob/main/go.mod#L7

and I get the same bloat locally: it is 59137488 vs 58094336

@kolyshkin do you think it would make sense to have a build tag to drop ebpf?

@vrothberg
Copy link
Copy Markdown
Member

vrothberg commented Sep 1, 2023

I noticed that common is also replacing runc. I guess we need to start there.

@giuseppe giuseppe marked this pull request as draft September 1, 2023 12:46
@openshift-ci 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 Sep 1, 2023
@giuseppe
Copy link
Copy Markdown
Member Author

giuseppe commented Sep 1, 2023

opened a PR for runc to allow disabling eBPF support: opencontainers/runc#4005

let's see if that breaks any test

@giuseppe giuseppe force-pushed the drop-runc-override branch 2 times, most recently from da90610 to 1009bb2 Compare September 1, 2023 21:48
@giuseppe
Copy link
Copy Markdown
Member Author

giuseppe commented Sep 1, 2023 •
edited
Loading

@kolyshkin I am using the latest HEAD version here, still the binary size grew by 836320 bytes

that is why the package is referenced by:

runc/libcontainer/cgroups/fs/devices.go:9:     cgroupdevices "github.com/opencontainers/runc/libcontainer/cgroups/devices"

@giuseppe giuseppe force-pushed the drop-runc-override branch 2 times, most recently from aade16b to bf8fc65 Compare September 1, 2023 23:04
@giuseppe
Copy link
Copy Markdown
Member Author

giuseppe commented Sep 4, 2023

temporary version bump until there is a new runc version: #19854

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 5, 2023
@vrothberg
Copy link
Copy Markdown
Member

vrothberg commented Sep 18, 2023

temporary version bump until there is a new runc version: #19854

@cyphar, do you have another runc version bump scheduled?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 19, 2023

A friendly reminder that this PR had no activity for 30 days.

@giuseppe
vendor: drop runc override
7562606 
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2023
@giuseppe giuseppe mentioned this pull request Nov 2, 2023
Closed
@giuseppe
Copy link
Copy Markdown
Member Author

giuseppe commented Nov 2, 2023

rebased and testing: containers/common#1633

@vrothberg
Copy link
Copy Markdown
Member

vrothberg commented Nov 3, 2023

The system failures look consistent

@giuseppe
Copy link
Copy Markdown
Member Author

giuseppe commented Nov 3, 2023

runc is still bloated, the last version didn't fix the dependency issue with EPBF (reported here: opencontainers/runc#4005), I'll close this PR for now

@giuseppe giuseppe closed this Nov 3, 2023
@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 Feb 2, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@vrothberg vrothberg vrothberg left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none stale-pr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

github.com/opencontainers/runc not being updated

3 participants

@giuseppe @vrothberg @openshift-merge-robot