Allow cloud-init to support creating btrfs partitions #50186

dingxiangfei2009 · 2018-11-10T16:10:01Z

Motivation for this change

cloud-init does not know where is btrfs programmes. It makes btrfs partition creation fails sometimes. This PR includes btrfs-progs into the PATH environment variable.

Request for comment: should we: 1. survey the all possible configurations and add in other missing runtime dependencies of cloud-init; 2. or, we add one option to allow users to supply additional runtime dependencies?

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 nox --run "nox-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)
Fits CONTRIBUTING.md.

Mic92 · 2018-11-10T22:05:15Z

cc @madjar @phile314

phile314

Looks good to me.

Mic92 · 2018-11-12T15:14:24Z

nixos/modules/services/system/cloud-init.nix

@@ -3,7 +3,16 @@
 with lib;

 let cfg = config.services.cloud-init;
-    path = with pkgs; [ cloud-init nettools utillinux e2fsprogs shadow openssh iproute ];
+    path = with pkgs; [
+      btrfs-progs


btrfs-progs adds like 3.5mb. I think we can add an option called btrfs.enable (default: false) and an option ext4.enable (default: true).

@Mic92 Sure. Sounds like a good idea to me.

I will re-write this as a module in the coming commits of this PR.

Makes the optional more self-describing and allows future extensions

Mic92 · 2018-11-13T10:30:55Z

@GrahamcOfBorg test cloud-init

GrahamcOfBorg · 2018-11-13T10:31:18Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: tests.cloud-init

Partial log (click to expand)

Cannot nix-instantiate `tests.cloud-init' because:
error: while evaluating 'recursiveUpdate' at /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/lib/attrsets.nix:415:26, called from /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/lib/attrsets.nix:148:28:
while evaluating 'recursiveUpdateUntil' at /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/lib/attrsets.nix:384:37, called from /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/lib/attrsets.nix:416:5:
while evaluating 'zipAttrsWith' at /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/lib/attrsets.nix:347:21, called from /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/lib/attrsets.nix:394:8:
while evaluating 'zipAttrsWithNames' at /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/lib/attrsets.nix:332:33, called from /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/lib/attrsets.nix:347:27:
while evaluating the attribute 'cloud-init' at /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/nixos/tests/all-tests.nix:42:3:
while evaluating 'handleTest' at /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/nixos/tests/all-tests.nix:17:22, called from /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/nixos/tests/all-tests.nix:42:16:
while evaluating 'discoverTests' at /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/nixos/tests/all-tests.nix:13:19, called from /var/lib/gc-of-borg/nix-test-rs-29/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-29/nixos/tests/all-tests.nix:18:5:
access to path '/nix/store/1w2cd02b5xffky2prjx5afgiqphrhnrd-grahamc-aarch64-community-29' is forbidden in restricted mode

GrahamcOfBorg · 2018-11-13T10:32:09Z

No attempt on x86_64-linux (full log)

The following builds were skipped because they don't evaluate on x86_64-linux: tests.cloud-init

Partial log (click to expand)

Cannot nix-instantiate `tests.cloud-init' because:
error: while evaluating 'recursiveUpdate' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/lib/attrsets.nix:415:26, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/lib/attrsets.nix:148:28:
while evaluating 'recursiveUpdateUntil' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/lib/attrsets.nix:384:37, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/lib/attrsets.nix:416:5:
while evaluating 'zipAttrsWith' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/lib/attrsets.nix:347:21, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/lib/attrsets.nix:394:8:
while evaluating 'zipAttrsWithNames' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/lib/attrsets.nix:332:33, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/lib/attrsets.nix:347:27:
while evaluating the attribute 'cloud-init' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/nixos/tests/all-tests.nix:42:3:
while evaluating 'handleTest' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/nixos/tests/all-tests.nix:17:22, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/nixos/tests/all-tests.nix:42:16:
while evaluating 'discoverTests' at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/nixos/tests/all-tests.nix:13:19, called from /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-0-gustav.ewr1.nix.ci/nixos/tests/all-tests.nix:18:5:
access to path '/nix/store/i33rypm097ms707yz1h43hr8zaqayf23-builder-0-gustav.ewr1.nix.ci' is forbidden in restricted mode

Mic92 · 2018-11-13T11:05:25Z

Looks like a bug in ofborg

grahamc · 2018-11-13T11:19:27Z

OfBorg hasn't changed, must be a change in Nixpkgs.

Mic92 · 2018-11-13T11:24:36Z

@Ekleog hinted it might be: #50233

Ekleog · 2018-11-13T13:50:42Z

Bisecting confirms it is, kind of unsurprisingly given it's where the stack trace appeared, 83b27f6.

What is really weird is the following:

$ nix-build -I . --option restrict-eval true nixos/release.nix -A tests.env --show-trace
... fails with the error message
$ nix-build -I . --option restrict-eval true nixos/tests/env.nix --show-trace
... succeeds

Which would appear to imply that the problem comes from the way release.nix imports the test, ie. with importing from its nixpkgs argument. But hydra also builds the env test as taken from release.nix with restrict-eval, and the change appears not to have hindered the latest evals / rebuilds.

So I don't really understand how & why the problem would arise from https://github.com/NixOS/nixpkgs/blob/master/nixos/release.nix#L21-L27 except on hydra. Especially given the fact that the import is the same than the one at https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/make-test.nix#L3, unless the passed nixpkgs is some kind of trick (I've tried adding the config = {}; locally just in case without more success)

Now, let's see if the problem arises with https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/all-packages.nix#L76-L80 too… it doesn't locally. To add to the confusion.

@GrahamcOfBorg build nixosTests.cloud-init

(cc @roberth given our ongoing discussion on #50301, in case you have an idea about where this could be going wrong)

TL;DR:

This appeared in 83b27f6
https://github.com/NixOS/nixpkgs/blob/master/nixos/release.nix#L21-L27 fails to eval on restrict-eval… except on hydra
https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/all-packages.nix#L76-L80 doesn't fail

GrahamcOfBorg · 2018-11-13T13:57:06Z

Success on x86_64-linux (full log)

Attempted: nixosTests.cloud-init

Partial log (click to expand)

machine: exit status 1
syncing
machine: running command: sync
machine: exit status 0
test script finished in 282.71s
cleaning up
killing machine (pid 600)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/1cdnipy7a708h95knqw29ij4pkv6nfx8-vm-test-run-unnamed

GrahamcOfBorg · 2018-11-13T14:21:58Z

Success on aarch64-linux (full log)

Attempted: nixosTests.cloud-init

Partial log (click to expand)

machine# [   27.658578] cloud-init[822]: |      o So++     |
machine# [   27.668179] cloud-init[822]: |      .o+@*..    |
machine: exit status 0
machine# [   27.673151] cloud-init[822]: |     o o=+.* o   |
test script finished in 28.79s
cleaning up
killing machine (pid 631)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/byhj9prwz402sqgs6hfdbzfraarhqg0q-vm-test-run-unnamed

roberth · 2018-11-13T14:26:30Z

Not a smoking gun, but there's a covert reinvocation of Nixpkgs here, which might be part of the problem. That will be gone after implementing a solution for #50301.

Ekleog · 2018-11-14T02:45:29Z

Correction: hydra builds do fail due to this. Hydra just doesn't report failed evaluations as failures.

@roberth

The current behavior, breaking restrict-eval=true, had been introduced in 209f8b1. See also [1]. This change uses a hack to identify whether we're evaluating in restrict-eval, keeping the optimization when not and reverting to the old behavior when evaluating in restrict-eval. This should fix the issues that appeared at [2] and [3], that followed the change making us actually use this `nixpkgs` argument for tests since 83b27f6 [4]. CC: @roberth [1] NixOS@209f8b1acd4c#commitcomment-28419462 [2] NixOS#50233 (comment) [3] NixOS#50186 [4] NixOS#50233

Ekleog · 2018-11-14T05:37:44Z

@Mic92 @grahamc @roberth The issue with ofborg building tests (and hopefully hydra too) should be fixed with #50342. It appears, if I understand correctly what I've been doing, to have stemmed from an IFD that had stayed unnoticed until now because we never actually used it for the tests: the cleanSource at the top of release.nix

grahamc · 2018-11-14T11:07:58Z

Please have @edolstra review

…

Sent from my iPhone

On Nov 14, 2018, at 00:37, Léo Gaspard ***@***.***> wrote: @Mic92 @grahamc @roberth The issue with ofborg building tests (and hopefully hydra too) should be fixed with #50342. It appears, if I understand correctly what I've been doing, to have stemmed from an IFD that had stayed unnoticed until now because we never actually used it for the tests: the cleanSource at the top of release.nix — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

edolstra · 2018-11-14T11:45:16Z

nixos/modules/services/system/cloud-init.nix

+        type = types.bool;
+        default = false;
+        description = ''
+          Allow the cloud-init service to operate `btrfs` filesystem.


BTW, what does it mean to "operate" a filesystem?

Maybe provision or create?

dingxiangfei2009 changed the title ~~allow cloud-init to support creating btrfs partitions~~ Allow cloud-init to support creating btrfs partitions Nov 10, 2018

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Nov 10, 2018

phile314 approved these changes Nov 11, 2018

View reviewed changes

Mic92 reviewed Nov 12, 2018

View reviewed changes

dingxiangfei2009 force-pushed the cloud-init-btrfs branch from 74ef6f1 to bd40d68 Compare November 13, 2018 04:48

dingxiangfei2009 requested a review from infinisil as a code owner November 13, 2018 04:48

allow cloud-init to support creating btrfs partitions

a965921

dingxiangfei2009 force-pushed the cloud-init-btrfs branch from bd40d68 to a965921 Compare November 13, 2018 05:14

nixos/cloud-init: add enable suffix to ext4/btrfs

a5c7476

Makes the optional more self-describing and allows future extensions

Mic92 merged commit e3ac65f into NixOS:master Nov 13, 2018

Ekleog mentioned this pull request Nov 14, 2018

Fix meta.tests system #50233

Merged

dingxiangfei2009 deleted the cloud-init-btrfs branch November 14, 2018 04:27

Ekleog mentioned this pull request Nov 14, 2018

Fix test evaluation in restrict-eval #50342

Closed

9 tasks

edolstra reviewed Nov 14, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow cloud-init to support creating btrfs partitions #50186

Allow cloud-init to support creating btrfs partitions #50186

dingxiangfei2009 commented Nov 10, 2018 •

edited

Mic92 commented Nov 10, 2018

phile314 left a comment

Mic92 Nov 12, 2018

dingxiangfei2009 Nov 13, 2018

Mic92 commented Nov 13, 2018

GrahamcOfBorg commented Nov 13, 2018

GrahamcOfBorg commented Nov 13, 2018

Mic92 commented Nov 13, 2018

grahamc commented Nov 13, 2018

Mic92 commented Nov 13, 2018

Ekleog commented Nov 13, 2018

GrahamcOfBorg commented Nov 13, 2018

GrahamcOfBorg commented Nov 13, 2018

roberth commented Nov 13, 2018

Ekleog commented Nov 14, 2018

Ekleog commented Nov 14, 2018

grahamc commented Nov 14, 2018 via email

edolstra Nov 14, 2018

Mic92 Nov 14, 2018

Allow cloud-init to support creating btrfs partitions #50186

Allow cloud-init to support creating btrfs partitions #50186

Conversation

dingxiangfei2009 commented Nov 10, 2018 • edited

Motivation for this change

Things done

Mic92 commented Nov 10, 2018

phile314 left a comment

Choose a reason for hiding this comment

Mic92 Nov 12, 2018

Choose a reason for hiding this comment

dingxiangfei2009 Nov 13, 2018

Choose a reason for hiding this comment

Mic92 commented Nov 13, 2018

GrahamcOfBorg commented Nov 13, 2018

GrahamcOfBorg commented Nov 13, 2018

Mic92 commented Nov 13, 2018

grahamc commented Nov 13, 2018

Mic92 commented Nov 13, 2018

Ekleog commented Nov 13, 2018

GrahamcOfBorg commented Nov 13, 2018

GrahamcOfBorg commented Nov 13, 2018

roberth commented Nov 13, 2018

Ekleog commented Nov 14, 2018

Ekleog commented Nov 14, 2018

grahamc commented Nov 14, 2018 via email

edolstra Nov 14, 2018

Choose a reason for hiding this comment

Mic92 Nov 14, 2018

Choose a reason for hiding this comment

dingxiangfei2009 commented Nov 10, 2018 •

edited