-
Notifications
You must be signed in to change notification settings - Fork 3.8k
further reduce uses of libcontainer and update runc v1.0.0-rc93 #4717
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
further reduce uses of libcontainer and update runc v1.0.0-rc93 #4717
Conversation
|
Skipping CI for Draft Pull Request. |
|
Build succeeded.
|
|
Hmm.. something failing; perhaps it doesn't deal well with the fork? |
010c9a4 to
64cca75
Compare
script/setup/install-runc
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to check these scripts; I've been bit by similar approaches in docker/docker (given, those didn't use vendoring)
go get -d will use go modules to get the dependency, and therefore first get go modules from master, before switching to the selected commit, which sometimes may cause unexpected results (see moby/moby#41560)
64cca75 to
050158b
Compare
|
Build succeeded.
|
050158b to
0d4c476
Compare
|
Build succeeded.
|
|
Interesting; are machined on GH Actions not cleaned up between builds? |
0d4c476 to
fb82710
Compare
|
Build succeeded.
|
fb82710 to
9414ab7
Compare
|
Build succeeded.
|
9414ab7 to
0d0452b
Compare
|
Build succeeded.
|
"protocol 'temporarily https' is not supported" 🤔 edit: well, of course, I know 😂 It's attempting to parse |
0d0452b to
3e6ce68
Compare
|
Build succeeded.
|
|
Hmmm... looks like something doesn't cleanup between runs |
|
opencontainers/runc#2679 is now merged |
3e6ce68 to
666ca0f
Compare
|
Build succeeded.
|
666ca0f to
417d802
Compare
|
Build succeeded.
|
417d802 to
57eb737
Compare
|
Build succeeded.
|
|
Is this still draft? |
Looks like this import was not needed for the test; simplified the test by just using the device-path (a counter would work, but for debugging, having the list of paths can be useful). Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: opencontainers/runc@v1.0.0-rc92...v1.0.0-rc93 also removes dependency on libcontainer/configs Signed-off-by: Sebastiaan van Stijn <[email protected]>
4be9cde to
04d061f
Compare
|
After this; dependency on runc vendoring is reduced to; $ tree vendor/github.com/opencontainers/runc/
vendor/github.com/opencontainers/runc/
├── LICENSE
├── NOTICE
└── libcontainer
├── devices
│ ├── device.go
│ ├── device_unix.go
│ ├── device_windows.go
│ └── devices.go
└── user
├── MAINTAINERS
├── lookup.go
├── lookup_unix.go
├── lookup_windows.go
└── user.go
3 directories, 11 files |
|
Build succeeded.
|
estesp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
almost passed :( test timeout as it just didn't finish by 10m inside the Fedora-in-vagrant-on-macOS 🤓 |
|
Does it need a longer timeout? Don't think we can restart only a single job, correct? |
|
/retest |
|
LGTM the failed CI job seems be because of a bad download |
|
ah, yes, the runc maintainers also ran into intermittent 404's for fedora; opencontainers/runc#2787 |
|
opened #4999 to add the same hack |
depends on:
After this, we only vendor:
devices.HostDevices()(used in a single test in pkg/cri/server/container_create_linux_test.go)devices.DeviceFromPath(()(used in a single location; pkg/cri/opts/spec_linux.go)The updated version of runc and gocapability add support for new capabilities
added in kernel 5.9 (syndtr/gocapability@d983527...42c35b4)