Skip to content

libcontainer: isolate libcontainer/devices #2679

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

Merged
merged 2 commits into from
Dec 2, 2020

Conversation

thaJeztah
Copy link
Member

Move the Device-related types to libcontainer/devices, so that the package can be used in isolation. Aliases have been created in libcontainer/configs for backward compatibility.

@crosbymichael
Copy link
Member

+1, I'm good with the refactoring

@thaJeztah
Copy link
Member Author

oh! forgot to move it out of draft; wanted to be sure it didn't explode; moving to "ready for review"

@thaJeztah thaJeztah marked this pull request as ready for review November 10, 2020 16:19
AkihiroSuda
AkihiroSuda previously approved these changes Nov 10, 2020
@thaJeztah
Copy link
Member Author

@kolyshkin LGTY?

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM overall. I think we may drop "Deivce" prefix from a few IDs, e.g.

configs.DeviceType -> devices.Type
config.DevicePermissions -> devices.Perms
configs.DeviceRule -> devices.Rule

and so on (not all of them though)

@thaJeztah
Copy link
Member Author

@kolyshkin @AkihiroSuda @crosbymichael updated; PTAL

Changed:

  • devices.DevicePermissions -> devices.Permissions
  • devices.DeviceType -> devices.Type
  • devices.DeviceRule -> devices.Rule

AkihiroSuda
AkihiroSuda previously approved these changes Nov 19, 2020
@thaJeztah
Copy link
Member Author

Hm.. some issue with the SUSE repositories?

Step 12/23 : RUN echo 'deb http://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/Debian_Unstable/ /' > /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list     && wget -nv https://download.opensuse.org/repositories/devel:kubic:libcontainers:stable/Debian_Unstable/Release.key -O- | sudo apt-key add -     && apt-get update     && apt-get install -y --no-install-recommends skopeo     && rm -rf /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list     && apt-get clean     && rm -rf /var/cache/apt /var/lib/apt/lists/*;
 ---> Running in 7b59184181e1
Warning: apt-key output should not be parsed (stdout is not a terminal)
https://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/Debian_Unstable/Release.key:
2020-11-19 11:08:33 ERROR 404: Not Found.
gpg: no valid OpenPGP data found.
The command '/bin/sh -c echo 'deb http://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/Debian_Unstable/ /' > /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list     && wget -nv https://download.opensuse.org/repositories/devel:kubic:libcontainers:stable/Debian_Unstable/Release.key -O- | sudo apt-key add -     && apt-get update     && apt-get install -y --no-install-recommends skopeo     && rm -rf /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list     && apt-get clean     && rm -rf /var/cache/apt /var/lib/apt/lists/*;' returned a non-zero code: 2
Makefile:62: recipe for target 'runcimage' failed
make: *** [runcimage] Error 2
The command "make clean ci cross" exited with 2.
cache.2
store build cache

@thaJeztah
Copy link
Member Author

let me try if close/open kicks CI here

@thaJeztah thaJeztah closed this Nov 19, 2020
@thaJeztah thaJeztah reopened this Nov 19, 2020
@thaJeztah
Copy link
Member Author

Failing again 😞

The command '/bin/sh -c echo 'deb http://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/Debian_Unstable/ /' > /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list     && wget -nv https://download.opensuse.org/repositories/devel:kubic:libcontainers:stable/Debian_Unstable/Release.key -O- | sudo apt-key add -     && apt-get update     && apt-get install -y --no-install-recommends skopeo     && rm -rf /etc/apt/sources.list.d/devel:kubic:libcontainers:stable.list     && apt-get clean     && rm -rf /var/cache/apt /var/lib/apt/lists/*;' returned a non-zero code: 2
Makefile:62: recipe for target 'runcimage' failed
make: *** [runcimage] Error 2
The command "make clean ci cross" exited with 2.
cache.2
store build cache

@cyphar any ideas? is this an issue with the opensuse package repos?

@thaJeztah
Copy link
Member Author

Rebased, as CI should now be fixed by #2686

@AkihiroSuda @kolyshkin @crosbymichael ptal

AkihiroSuda
AkihiroSuda previously approved these changes Nov 23, 2020
@AkihiroSuda AkihiroSuda added this to the 1.0.0-rc93 milestone Nov 23, 2020
Move the Device-related types to libcontainer/devices, so that
the package can be used in isolation. Aliases have been created
in libcontainer/configs for backward compatibility.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Member Author

Rebased again, as CI seemed to be stuck in "preparing" state

@kolyshkin
Copy link
Contributor

travis timed out:

ok 12 checkpoint and restore
ok 13 checkpoint and restore (cgroupns)
ok 14 checkpoint --pre-dump and restore

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

I have seen this before (in cri-o/cri-o), apparently they gradually tighten the resources and thus we need to increase the timeout :(

@kolyshkin
Copy link
Contributor

For now I've just restarted the job

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@kolyshkin
Copy link
Contributor

@AkihiroSuda @mrunalp PTAL

@AkihiroSuda AkihiroSuda merged commit 3517877 into opencontainers:master Dec 2, 2020
@thaJeztah thaJeztah deleted the isolate_device branch December 2, 2020 09:32
shoenig added a commit to hashicorp/nomad that referenced this pull request Jan 14, 2022
This PR upgrades
 - docker dependency to the latest tagged release (v20.10.12)
 - runc dependency to the latest tagged release (v1.0.3)

Docker does not abide by [semver](moby/moby#39302), so it is marked +incompatible,
and transitive dependencies are upgrade manually.

Runc made two minor breaking changes since v1.0.0-rc93

 * cgroup manager .Set changed to accept Resources instead of Cgroup
   opencontainers/runc@3f65946

 * config.Device moved to devices.Device
   opencontainers/runc#2679
shoenig added a commit to hashicorp/nomad that referenced this pull request Jan 15, 2022
This PR upgrades
 - docker dependency to the latest tagged release (v20.10.12)
 - runc dependency to the latest tagged release (v1.0.3)

Docker does not abide by [semver](moby/moby#39302), so it is marked +incompatible,
and transitive dependencies are upgrade manually.

Runc made three relevant breaking changes

 * cgroup manager .Set changed to accept Resources instead of Cgroup
   opencontainers/runc@3f65946

 * config.Device moved to devices.Device
   opencontainers/runc#2679

 * mountinfo.Mounted now returns an error if the specified path does not exist
   https://github.com/moby/sys/blob/mountinfo/v0.5.0/mountinfo/mountinfo.go#L16
shoenig added a commit to hashicorp/nomad that referenced this pull request Jan 18, 2022
This PR upgrades
 - docker dependency to the latest tagged release (v20.10.12)
 - runc dependency to the latest tagged release (v1.0.3)

Docker does not abide by [semver](moby/moby#39302), so it is marked +incompatible,
and transitive dependencies are upgrade manually.

Runc made three relevant breaking changes

 * cgroup manager .Set changed to accept Resources instead of Cgroup
   opencontainers/runc@3f65946

 * config.Device moved to devices.Device
   opencontainers/runc#2679

 * mountinfo.Mounted now returns an error if the specified path does not exist
   https://github.com/moby/sys/blob/mountinfo/v0.5.0/mountinfo/mountinfo.go#L16
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.

4 participants