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.

Sign up for GitHub

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

nixos-container: use systemd-run instead of nsenter #67332

Merged
merged 1 commit into from Sep 14, 2019
Merged

nixos-container: use systemd-run instead of nsenter #67332

merged 1 commit into from Sep 14, 2019

Conversation

uvNikita
Copy link
Contributor

Motivation for this change

This is the first step for unprivileged nixos containers support.
Fixes #30019. See also #18825, #57083, and #67130.

@Mic92 I see you commented about --pty and --pipe options in #30019. I'm not sure if I should add it here or not.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 @danbst @mmahut @Mic92

@uvNikita
nixos-container: use systemd-run instead of nsenter
7cb100b 
This is the first step for unprivileged nixos containers support.
Fixes NixOS#30019. See also NixOS#18825, NixOS#57083, and NixOS#67130.
@uvNikita uvNikita mentioned this pull request Aug 23, 2019
Draft
10 tasks
@arianvp
Copy link
Member

arianvp commented Sep 5, 2019

Looks good to me

@fpletz fpletz added this to the 19.09 milestone Sep 14, 2019
fpletz
fpletz approved these changes Sep 14, 2019
View reviewed changes
Copy link
Member

@fpletz fpletz left a comment

Choose a reason for hiding this comment

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

Verified that the behaviour is the same and now running all container tests. Will merge after they have passed. Thanks a lot! 👍

@fpletz fpletz merged commit a0a3f5c into NixOS:master Sep 14, 2019
@uvNikita uvNikita deleted the nixos-container/run branch September 16, 2019 08:14
@veprbl
Copy link
Member

veprbl commented Jan 13, 2020

There is an issue reported to be caused by this: #72407 (comment)
Could you, please, check?

@Ma27
Copy link
Member

Ma27 commented Mar 25, 2020

@fpletz @veprbl @arianvp @uvNikita this breaks the container-backend of nixops (and probably more): when deploying a container and running switch-to-configuration via nixos-container run, systemd will be reloaded (which seems to kill the systemd-run call) and causes a hanging command (at least when using nixops). I just confirmed locally that reverting this change fixes the issue.

Please also note that this isn't in the nixos-19.09 tree although it's in the 19.09 milestone.

As this is an actual regression, I'd love to see this resolved before releasing 20.03 (cc @worldofpeace @disassembler). I can currently think of two solutions:

  • Reverting the entire commit (at least for 20.03) and finding a better solution for 20.09
  • Adding some kind of --no-systemd flag to nixos-container run which uses the old nsenter approach.

Any further ideas?

@worldofpeace
Copy link
Contributor

@Ma27 Awesome that your so on top of this stuff. I suggest a revert would be best, we're cutting it very close.

Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Mar 26, 2020
@Ma27
Revert "nixos-container: use systemd-run instead of nsenter"
7f1ba60 
This reverts commit 7cb100b.

This appears to break at least the `container`-backend of `nixops`: when
running `switch-to-configuration` within `nixos-container run`, the
running `systemd`-instance gets reloaded which appears to kill the
`systemd-run` command and causes `nixos-container run` to hang.

The full issue is reported in the original PR[1].

[1] NixOS#67332 (comment)
@Ma27 Ma27 mentioned this pull request Mar 26, 2020
Merged
10 tasks
@Ma27
Copy link
Member

Ma27 commented Mar 26, 2020

Opened #83392 which reverts this on release-20.03. Before merging this, I'd love to see some feedback of the original author and reviewers though :)

@Ma27 Ma27 mentioned this pull request Mar 26, 2020
Open
Ma27 added a commit that referenced this pull request Mar 26, 2020
@Ma27
Revert "nixos-container: use systemd-run instead of nsenter"
ebb6e38 
This reverts commit 7cb100b.

See also #83432.

This appears to break at least the `container`-backend of `nixops`: when
running `switch-to-configuration` within `nixos-container run`, the
running `systemd`-instance gets reloaded which appears to kill the
`systemd-run` command and causes `nixos-container run` to hang.

The full issue is reported in the original PR[1].

[1] #67332 (comment)

(cherry picked from commit 7f1ba60)
@uvNikita uvNikita mentioned this pull request Oct 12, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpletz fpletz fpletz approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
19.09
Development

Successfully merging this pull request may close these issues.

nixos-container should use systemd-run -M <machine>
6 participants
@uvNikita @arianvp @veprbl @Ma27 @worldofpeace @fpletz