Skip to content

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Nov 10, 2020

depends on:

After this, we only vendor:

  • libcontainer/user
    • used in oci/spec_opts.go (perhaps this is something to move to runtime-spec?)
  • libcontainer/devices
    • 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)
  • libcontainer/nsenter ("fake" dependency; pulled in because it contains C code)

The updated version of runc and gocapability add support for new capabilities
added in kernel 5.9 (syndtr/gocapability@d983527...42c35b4)

  • CAP_PERFMON
  • CAP_BPF
  • CAP_CHECKPOINT_RESTORE

@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 10, 2020

Build succeeded.

@thaJeztah
Copy link
Member Author

Hmm.. something failing; perhaps it doesn't deal well with the fork?

fatal: reference is not a tree: c7f874992f5322b2783580edef6ba6ae3e4aaa6e
Error: Process completed with exit code 128.

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch 2 times, most recently from 010c9a4 to 64cca75 Compare November 10, 2020 11:35
Comment on lines 26 to 27
Copy link
Member Author

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)

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from 64cca75 to 050158b Compare November 10, 2020 11:41
@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 10, 2020

Build succeeded.

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from 050158b to 0d4c476 Compare November 10, 2020 19:54
@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 10, 2020

Build succeeded.

@thaJeztah
Copy link
Member Author

Interesting; are machined on GH Actions not cleaned up between builds?

Bringing machine 'default' up with 'virtualbox' provider...
==> default: Checking if box 'fedora/32-cloud-base' version '32.20200422.0' is up to date...
==> default: Running provisioner: install-runc (shell)...
    default: Running: inline script
    default: + /root/go/src/github.com/containerd/containerd/script/setup/install-runc
    default: fatal: destination path '/root/go/src/github.com/opencontainers/runc' already exists and is not an empty directory.
The SSH command responded with a non-zero exit status. Vagrant
assumes that this means the command failed. The output for this command
should be in the log above. Please read the output to determine what
went wrong.
Error: Process completed with exit code 1.

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from 0d4c476 to fb82710 Compare November 18, 2020 22:39
@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 18, 2020

Build succeeded.

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from fb82710 to 9414ab7 Compare November 19, 2020 10:10
@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 19, 2020

Build succeeded.

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from 9414ab7 to 0d0452b Compare November 19, 2020 11:45
@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 19, 2020

Build succeeded.

@thaJeztah
Copy link
Member Author

thaJeztah commented Nov 19, 2020

Interesting; https://github.com/containerd/containerd/pull/4717/checks?check_run_id=1423934789

 /usr/bin/install -c -m 644 libseccomp.pc '/usr/local/lib/pkgconfig'
+ ldconfig
+ rm -rf /tmp/tmp.QeIT0Nsktc
Cloning into '/home/runner/work/containerd/containerd/src/github.com/opencontainers/runc'...
fatal: protocol 'temporarily
https' is not supported
Error: Process completed with exit code 128.

"protocol 'temporarily https' is not supported" 🤔

edit: well, of course, I know 😂

It's attempting to parse vendor.conf, and uses this line to vendor a dependency:

# FIXME temporarily vendoring from my fork (https://github.com/opencontainers/runc/pull/2679)
github.com/opencontainers/runc                      isolate_device https://github.com/thaJeztah/runc.git

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from 0d0452b to 3e6ce68 Compare November 19, 2020 12:12
@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 19, 2020

Build succeeded.

@thaJeztah
Copy link
Member Author

Hmmm... looks like something doesn't cleanup between runs

Bringing machine 'default' up with 'virtualbox' provider...
==> default: Checking if box 'fedora/32-cloud-base' version '32.20200422.0' is up to date...
==> default: Running provisioner: install-runc (shell)...
    default: Running: inline script
    default: + /root/go/src/github.com/containerd/containerd/script/setup/install-runc
    default: fatal: destination path '/root/go/src/github.com/opencontainers/runc' already exists and is not an empty directory.
The SSH command responded with a non-zero exit status. Vagrant
assumes that this means the command failed. The output for this command
should be in the log above. Please read the output to determine what
went wrong.
Error: Process completed with exit code 1.

@AkihiroSuda
Copy link
Member

opencontainers/runc#2679 is now merged

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from 3e6ce68 to 666ca0f Compare December 3, 2020 15:25
@theopenlab-ci
Copy link

theopenlab-ci bot commented Dec 3, 2020

Build succeeded.

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from 666ca0f to 417d802 Compare December 3, 2020 17:29
@theopenlab-ci
Copy link

theopenlab-ci bot commented Dec 3, 2020

Build succeeded.

@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from 417d802 to 57eb737 Compare December 7, 2020 13:43
@theopenlab-ci
Copy link

theopenlab-ci bot commented Dec 7, 2020

Build succeeded.

@AkihiroSuda
Copy link
Member

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]>
@thaJeztah thaJeztah force-pushed the reduce_libcontainer_use branch from 4be9cde to 04d061f Compare February 4, 2021 15:14
@thaJeztah thaJeztah changed the title [draft] further reduce uses of libcontainer further reduce uses of libcontainer and update runc v1.0.0-rc93 Feb 4, 2021
@thaJeztah thaJeztah marked this pull request as ready for review February 4, 2021 15:15
@thaJeztah
Copy link
Member Author

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

@theopenlab-ci
Copy link

theopenlab-ci bot commented Feb 4, 2021

Build succeeded.

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp
Copy link
Member

estesp commented Feb 4, 2021

almost passed :( test timeout as it just didn't finish by 10m inside the Fedora-in-vagrant-on-macOS 🤓

@thaJeztah
Copy link
Member Author

Does it need a longer timeout? Don't think we can restart only a single job, correct?

@thaJeztah
Copy link
Member Author

/retest

@dims
Copy link
Member

dims commented Feb 4, 2021

LGTM

the failed CI job seems be because of a bad download

2021-02-04T17:00:59.5843050Z     default: Downloading: https://vagrantcloud.com/fedora/boxes/33-cloud-base/versions/33.20201019.0/providers/virtualbox.box
2021-02-04T17:00:59.6362490Z 
2021-02-04T17:00:59.7170450Z �[KProgress: 0% (Rate: 0*/s, Estimated time remaining: --:--:--)
2021-02-04T17:00:59.7172020Z �[KDownload redirected to host: download.fedoraproject.org
2021-02-04T17:00:59.7708170Z 
2021-02-04T17:00:59.9694180Z �[KProgress: 100% (Rate: 1766/s, Estimated time remaining: --:--:--)
2021-02-04T17:01:00.0522640Z �[KProgress: 0% (Rate: 0/s, Estimated time remaining: --:--:--)
2021-02-04T17:01:00.0713810Z An error occurred while downloading the remote file. The error
2021-02-04T17:01:00.0714940Z message, if any, is reproduced below. Please fix this error and try
2021-02-04T17:01:00.0715840Z again.
2021-02-04T17:01:00.0716320Z 
2021-02-04T17:01:00.0716840Z The requested URL returned error: 404 Not Found

@thaJeztah
Copy link
Member Author

ah, yes, the runc maintainers also ran into intermittent 404's for fedora; opencontainers/runc#2787

@thaJeztah
Copy link
Member Author

opened #4999 to add the same hack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants