nixos: make virtualisation.docker.storageDriver optional

[bjornfor]

Commit 9bfe92e ("docker: Minor improvements, fix failing test") added
this option and made it mandatory. But docker itself has a default
value, so I don't think NixOS should to force users to make up their
mind about what storage driver to use, when docker can choose one
itself.

[bjornfor]

Fixes #9801

[bjornfor]

CC @offlinehacker @ragnard

[bjornfor]

Commit 9bfe92e is correct; tests do fail without an explicit --storage-driver= option. Hm...

[bjornfor]

I'm no docker expert, but it might be the default backend that is broken, not that --storage-driver= needs to be set.

From nix-build nixos/tests/docker.nix with

diff --git a/nixos/tests/docker.nix b/nixos/tests/docker.nix
index 034dcb0..4c2a852 100644
--- a/nixos/tests/docker.nix
+++ b/nixos/tests/docker.nix
@@ -11,7 +11,7 @@ import ./make-test.nix ({ pkgs, ...} : {
       { config, pkgs, ... }:
         {
           virtualisation.docker.enable = true;
-          virtualisation.docker.storageDriver = "overlay";
+          #virtualisation.docker.storageDriver = "overlay";
         };
     };

docker: running command: systemctl --no-pager show 'docker.service'
docker: exit status 0
docker: must succeed: tar cv --files-from /dev/null | docker import - scratchimg
docker# [    7.686232] systemd[1]: Stopping Name Service Cache Daemon...
docker# [    7.690306] systemd[1]: Starting Name Service Cache Daemon...
docker# [    8.010224] systemd[1]: Started Name Service Cache Daemon.
docker# [    8.044679] systemd[1]: Reached target Services Requiring IP Connectivity.
docker# [    8.052126] dhcpcd[608]: eth0: removing route to 10.0.2.0/24
docker# [   11.846146] loop: Write error at byte offset 478666752, length 4096.
docker# [   11.846648] Buffer I/O error on dev dm-1, logical block 7342190, lost async page write
docker# [   11.847800] loop: Write error at byte offset 478670848, length 4096.
docker# [   11.848748] Buffer I/O error on dev dm-1, logical block 7342191, lost async page write
docker# [   11.850199] loop: Write error at byte offset 478679040, length 4096.
docker# [   11.851189] Buffer I/O error on dev dm-1, logical block 7342193, lost async page write
docker# [   11.852429] loop: Write error at byte offset 478683136, length 4096.
docker# [   11.852958] Buffer I/O error on dev dm-1, logical block 7342194, lost async page write
docker# [   11.853629] loop: Write error at byte offset 478687232, length 4096.
docker# [   11.854634] Buffer I/O error on dev dm-1, logical block 7342195, lost async page write
docker# [   11.855791] loop: Write error at byte offset 478691328, length 4096.
docker# [   11.856701] Buffer I/O error on dev dm-1, logical block 7342196, lost async page write
docker# [   11.857843] loop: Write error at byte offset 478695424, length 4096.
docker# [   11.860577] Buffer I/O error on dev dm-1, logical block 7342197, lost async page write
docker# [   11.862737] loop: Write error at byte offset 478699520, length 4096.
docker# [   11.863951] Buffer I/O error on dev dm-1, logical block 7342198, lost async page write
docker# [   11.865247] loop: Write error at byte offset 478703616, length 4096.
docker# [   11.866483] Buffer I/O error on dev dm-1, logical block 7342199, lost async page write
docker# [   11.867771] loop: Write error at byte offset 478707712, length 4096.
docker# [   11.868779] Buffer I/O error on dev dm-1, logical block 7342200, lost async page write
docker# [   12.610704] device-mapper: thin: 254:0: metadata operation 'dm_pool_commit_metadata' failed: error = -28
docker# [   12.611441] device-mapper: thin: 254:0: aborting current metadata transaction
docker# [   12.615571] device-mapper: thin: 254:0: switching pool to read-only mode
docker# [   12.619174] device-mapper: btree spine: node_check failed: blocknr 0 != wanted 222
docker# [   12.620287] device-mapper: block manager: btree_node validator check failed for block 222
docker# [   12.621995] device-mapper: thin: process_bio_read_only: dm_thin_find_block() failed: error = -15
docker# [   12.623325] device-mapper: btree spine: node_check failed: blocknr 0 != wanted 222
docker# [   12.624380] device-mapper: block manager: btree_node validator check failed for block 222
docker# [   12.625645] device-mapper: thin: process_bio_read_only: dm_thin_find_block() failed: error = -15
docker# [   12.626976] device-mapper: btree spine: node_check failed: blocknr 0 != wanted 222
docker# [   12.628033] device-mapper: block manager: btree_node validator check failed for block 222
docker# [   14.211369] ------------[ cut here ]------------
docker# [   14.212219] WARNING: CPU: 0 PID: 650 at /tmp/nix-build-linux-3.18.21.drv-0/linux-3.18.21/fs/block_dev.c:67 bdev_inode_switch_bdi+0x61/0x90()
docker# [   14.213954] Modules linked in: af_packet dm_thin_pool dm_persistent_data dm_bio_prison dm_bufio crc32c_generic libcrc32c cfg80211 rfkill nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack ip6t_rpfilter ip6table_raw ipt_rpfilter iptable_raw xt_pkttype nf_log_ipv6 nf_log_ipv4 nf_log_common xt_LOG xt_tcpudp ip6table_filter ip6_tables iptable_filter ip_tables x_tables bochs_drm ttm drm_kms_helper ppdev drm ata_generic pata_acpi syscopyarea joydev sysfillrect sysimgblt mousedev intel_agp ide_pci_generic evdev psmouse piix intel_gtt mac_hid i2c_piix4 agpgart ide_core serio_raw i2c_core parport_pc processor parport floppy 8250_fintek button thermal_sys snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd soundcore nf_conntrack_ftp nf_conntrack loop cpufreq_ondemand ipv6 autofs4 9p fscache ext4 crc16 jbd2 mbcache hid_generic usbhid hid sr_mod cdrom 9pnet_virtio virtio_blk 9pnet virtio_net atkbd libps2 uhci_hcd rtc_cmos ata_piix ehci_hcd i8042 serio libata scsi_mod virtio_pci usbcore usb_common unix dm_mod virtio_rng rng_core virtio_console virtio_balloon virtio_ring virtio
docker# [   14.231240] CPU: 0 PID: 650 Comm: mkfs.ext4 Not tainted 3.18.21 #1-NixOS
docker# [   14.232301] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
docker# [   14.233964]  0000000000000009 ffff88000f827d88 ffffffff814b0f51 ffff88001780ecb8
docker# [   14.235489]  0000000000000000 ffff88000f827dc8 ffffffff810671e1 0000000000000000
docker# [   14.236699]  ffff880016416bb8 ffff880016416b30 ffffffff81866d00 000000000006009f
docker# [   14.237857] Call Trace:
docker# [   14.240749]  [<ffffffff814b0f51>] dump_stack+0x46/0x58
docker# [   14.241643]  [<ffffffff810671e1>] warn_slowpath_common+0x81/0xa0
docker# [   14.242545]  [<ffffffff810672ba>] warn_slowpath_null+0x1a/0x20
docker# [   14.243436]  [<ffffffff811e6691>] bdev_inode_switch_bdi+0x61/0x90
docker# [   14.244447]  [<ffffffff811e72a8>] __blkdev_put+0x78/0x1c0
docker# [   14.245338]  [<ffffffff811e7cf0>] blkdev_put+0x50/0x140
docker# [   14.246372]  [<ffffffff811e7e95>] blkdev_close+0x25/0x30
docker# [   14.247216]  [<ffffffff811b03fc>] __fput+0xdc/0x1f0
docker# [   14.247964]  [<ffffffff811b055e>] ____fput+0xe/0x10
docker# [   14.248727]  [<ffffffff8108233f>] task_work_run+0x9f/0xe0
docker# [   14.249598]  [<ffffffff81012c51>] do_notify_resume+0x61/0x90
docker# [   14.250497]  [<ffffffff814b6de7>] int_signal+0x12/0x17
docker# [   14.251318] ---[ end trace 3b4bf8dc043bca5e ]---

Looks like devicemapper (dev dm-1) has some issues...

[bjornfor]

Yes, same error with

diff --git a/nixos/tests/docker.nix b/nixos/tests/docker.nix
index 034dcb0..4c2a852 100644
--- a/nixos/tests/docker.nix
+++ b/nixos/tests/docker.nix
@@ -11,7 +11,7 @@ import ./make-test.nix ({ pkgs, ...} : {
       { config, pkgs, ... }:
         {
           virtualisation.docker.enable = true;
-          virtualisation.docker.storageDriver = "overlay";
+          virtualisation.docker.storageDriver = "devicemapper";
         };
     };

[jgeerds]

Would be great to "fix" this also for 15.09

[bjornfor]

"devicemapper" seems to work fine on my machine. Is it just NixOS VM tests that crashes?

I see a couple of ways forward:

1. remove the virtualisation.docker.storageDriver option, but add --storage-driver= to extraOptions (just to fix VM tests)
2. keep virtualisation.docker.storageDriver, set default to "overlay" (which fixes the VM test) and remove the custom VM test setting.

Thoughts?

[jgeerds]

devicemapper also works on my machine. Is this the default storage driver? We should use the default one

[lucabrunox]

Honestly I think that storagedriver being mandatory is a good thing. What if docker decides to change default value?

Rather we can pick a default value in the nixos option, and I don't even think that's a good idea.

[bjornfor]

@lethalman: That last sentence confused me. What do you suggest?

[bjornfor]

@geerds: Yes, I think "devicemapper" is the default. But it breaks in VM tests. Ideally, we'd find the real cause for the VM test breakage and fix that, instead of changing the defaults.

[bjornfor]

@lethalman: If docker changes its default backend (and NixOS does not have a default one), we'd have to rebuild all containers, apparently. Is that a big deal? (I don't think so.)

But if changing backend is a big deal, we (NixOS) should definitely not do that. Meaning, we use "devicemapper" or null as default for storageDriver. Or remove that option completely and let users pass --storage-driver=x in extraOptions. (I think the last option is what I'm leaning towards at the moment.)

[lucabrunox]

@bjornfor it is a problem, if you have a database or something like that on a container filesystem.

Or we just require the user the specify the storage driver, as is now, perhaps with an assertion which is more user-friendly.

[ragnard]

I agree with @lethalman that forcing you to specify a storage driver is a good thing. Mainly because it is an important decision that will depend on other factors, like what file system you are using.

[bjornfor]

Ah, right. Forgot about data :-)

I tried adding an assert, but it failed: bjornfor@ff74e35

Maybe you know how to fix it?

[bjornfor]

@ragnard, @lethalman: Do you have strong feelings about mandatory storageDriver? I think that since docker doesn't have mandatory --storage-driver=x option, neither should NixOS.

And my attempt at adding an assert failed (see bjornfor@ff74e35). So the current situation is bad and this PR will improve it, I'd like to merge.

[bjornfor]

Don't forget that, without the assert, users get an ugly traceback when they switch on services.docker.enable = true.

[lucabrunox]

My opinion is that it must be mandatory, just like when you create ordinary filesystems. I wonder what happens if docker changes default value...

[bjornfor]

@lethalman: Ok. Can you please have a look at how to fix that assert then? I made an attempt at bjornfor@ff74e35, but it doesn't work.

[lucabrunox]

@bjornfor I've tried your attempt and it indeed doesn't work... cc @nbp :)

[bjornfor]

@lethalman: How about we make the default "devicemapper", which is the current docker default? Then it is explicit and won't change even if docker changes its default.

[lucabrunox]

@bjornfor yeah let's go for it

[nbp]

@bjornfor @lethalman I will investigate after lunch.

If I understand correctly the problem you are trying to work-around is the fact that mandatory options do not output any detailed information, right?

You don't need a nullOr type to add this assertion, the module system can let you query if an option is defined or not, by adding the options attribute and by looking at options.virtualisation.docker.storageDriver.isDefined attribute but I understand that this could be something that we can automate.

[lucabrunox]

@nbp the problem is that assertion will not be triggered. Anyway we'll probably go for a default value at nix level. But would be good to know why the assertion doesn't work as expected...

[bjornfor]

I made a new PR (because the meaning of the PR is changed): #10226

[nbp]

@lethalman what does nixos-option virtualisation.docker.storageDriver returns then?

[nbp]

I cannot reproduce this issue locally, unless the asserting mechanism is no longer working, the option is well evaluated:

$ NIX_PATH=nixos=$(pwd) NIXOS_CONFIG=$(pwd)/test.nix nixos-option assertions
Value:
[ { assertion = false; message = "Option services.docker.storageDriver is not defined (required because\nservices.docker.enable is true).\n"; } … ]

with the following test file:

{
  virtualisation.docker.enable = true;
}

Did you forgot to set the NIX_PATH to use the modified version?

[nbp]

@lethalman @bjornfor Do you have a minimal config which reproduce the lack of error message reports? The assertions are executed when producing the toplevel of the system, which should happen in all cases. Maybe you are using a different target for building which does not involve evaluating these assertions.

[lucabrunox]

Uhm maybe, I was sure I tested of course with that change. Will try again...