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

Jump to bottom

gvisor: init at 2019-11-14 #73097

Merged
merged 3 commits into from Dec 5, 2019
Merged

gvisor: init at 2019-11-14 #73097

merged 3 commits into from Dec 5, 2019

Conversation

andrew-d
Copy link
Contributor

@andrew-d andrew-d commented Nov 9, 2019

Motivation for this change

This is a revamp of #50218 after enough upstream changes that it's possible to build inside a Nix sandbox. This was requested in #39889, but there were some problems with Bazel at the time. I've managed to get this working with buildBazelPackage. At the end of the whole process, gvisor is runnable:

$ /nix/store/7k1c1jikms1pjimk8561x18xpj51dm5l-gvisor-2019-11-08/bin/runsc --help
Usage: runsc <flags> <subcommand> <subcommand args>

Subcommands:
	checkpoint       checkpoint current state of container (experimental)
	create           create a secure container
	delete           delete resources held by a container
	do               Simplistic way to execute a command inside the sandbox. It's to be used for testing only.
	events           display container events such as OOM notifications, cpu, memory, and IO usage statistics

(I also added the containerd shim as well, since it didn't feel worth another PR)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc original reviewers @nlewo, @Profpatsch, and commenters @dtzWill, @benpye and @ghuntley

Closes #50218
Closes #39889

@ofborg ofborg bot requested a review from kalbasit November 9, 2019 08:13
@Profpatsch
Copy link
Member

Do you have sandboxing enabled in your local nix? The build fails, because it tries to download some repository dependencies. Most of the nix world has sandboxing enabled, which restricts network access to fixed-output derivations.

@andrew-d
Copy link
Contributor Author

@Profpatsch - I do, yes. I think the problem is that the buildBazelPackage tools remove rules_cc, which is now an external dependency and then can't be downloaded. There's also some issues with rules_go and x/net/nettest that I'm trying to debug as well.

@andrew-d andrew-d mentioned this pull request Nov 24, 2019
Merged
10 tasks
@andrew-d andrew-d force-pushed the andrew/gvisor-redux branch 2 times, most recently from e79b2a1 to 593b61d Compare November 25, 2019 00:32
@andrew-d
Copy link
Contributor Author

@Profpatsch - Okay, between #74013 and this PR, it's building successfully and reproducibly 🎉

@flokli
Copy link
Contributor

flokli commented Nov 26, 2019

Can you add a simple nixos vm test starting a container, so we can verify it works?

@andrew-d
Copy link
Contributor Author

@flokli - Okay, added a test that exercises both the gvisor do subcommand and gvisor when it's being used as a Docker runtime. I hadn't previously tested the gvisor do subcommand, so I also added a patch to ensure that the path to ip/ipconfig/sysctl is absolute.

@andrew-d
Copy link
Contributor Author

@Profpatsch / @flokli - Okay, rebased now that #74103 is merged. This should be ready to merge now, and tests pass locally.

@andrew-d andrew-d mentioned this pull request Nov 28, 2019
Merged
7 tasks
@andrew-d andrew-d force-pushed the andrew/gvisor-redux branch 2 times, most recently from a852a36 to eddf8c2 Compare November 28, 2019 23:58
flokli
flokli reviewed Dec 2, 2019
View reviewed changes
pkgs/applications/virtualization/gvisor/default.nix Outdated
# available in the $PATH; patch these now.
patches = [ ./gvisor-do-paths.patch ];
postPatch = ''
substituteInPlace runsc/cmd/do.go \
Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of maintainability:

Could we just do a wrapProgram $out/bin/runsc --prefix PATH : ${stdenv.lib.makeBinPath [ iproute iptables procps ]} in installPhase, instead of patching the source code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flokli - Done! And confirmed that it works in the NixOS test as well.

@andrew-d andrew-d changed the title gvisor: init at 2019-11-08 gvisor: init at 2019-11-14 Dec 4, 2019
@andrew-d
Copy link
Contributor Author

andrew-d commented Dec 4, 2019

I also just bumped to an actual tagged release of gvisor, since they tagged release-20191114.0 after I'd opened this PR.

@flokli flokli merged commit 3140fa8 into NixOS:master Dec 5, 2019
@flokli
Copy link
Contributor

flokli commented Dec 5, 2019

Thanks!

dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Dec 5, 2019
@flokli @dtzWill
Merge pull request NixOS#73097 from andrew-d/andrew/gvisor-redux
4091c44 
gvisor: init at 2019-11-14
(cherry picked from commit 3140fa8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flokli flokli flokli left review comments

@Profpatsch Profpatsch Profpatsch left review comments

@kalbasit kalbasit Awaiting requested review from kalbasit

Assignees
No one assigned
Labels
6.topic: nixos 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

package request/idea: gvisor
3 participants
@andrew-d @Profpatsch @flokli