@grahamc would you like to have a look?

@GrahamcOfBorg test docker

cc:@offlinehacker

I've added some || true at the end of the docker {volume,network} rm lines, so that failing to delete a volume or network (presumably because they are in use) does not prevent the docker unit from starting.

I ran into this with gitlab-runner. The helper image it uses to handle shared operations, and specifically the handling of shared cache, creates an anonymous volume which my changes to the docker module will try (and fail) to delete.

I'm unsure about these abstractions being in docker.nix file. Can we split into two files, one only managing docker service, and other managing these additional abstractions over networking and volumes. It's just for sake of having clear boundaries defined, so refactoring and testing is easier.

I would also have a separate systemd unit and not as part of docker postStart, that will separate systemd units in clear way and makes debugging easier.

That seems over-engineered to me, but thanks for the review.

I think it does not change a lot in terms of additonal complexity, but provides a separation between what is a basically systemd unit for docker and all our custom abstraction over it.

The problem is that nixos is already bloated from my point of view, and especially nixpkgs and I'm not against additional abstraction, especially if it provides value to multiple users. Let's just keep it separated, so in the future it can be easier to refactor, does this makes sense?

The other option I was considering is making it easier to run some preStart in docker-containers.nix, which would probably also work. You could just run some docker volume create foo || true in there.

I wrote this PR hoping to improve the nix wrappers around Docker, and I have some patches for the other module down here that I need to clean up for review too.

I also realize that trying to make Docker be actually nix-compliant is a fool's errand, so maybe this entire PR is just misled.

(I know @offlinehacker already gave some feedback, just wanting to officially delegate my opinion to him -- as @mkaito requested my review on IRC.)

This is probably incompatible with imperative creation of volumes and networks.

For compatibility, it's probably best to introduce something similar to users.mutableUsers so imperatively created volumes and networks aren't deleted unless mutability is explicitly disabled. If keeping track of which volumes and networks were created by nix is too much effort, it's possible to disable the volumes and networks options entirely when they're not explicitly enabled.

I think it's bad if whether the docker daemon comes up depends on whether applying these new options fails, but I also think user services may need to depend on these volumes and networks being created, so I agree that it's best if they're handled in a separate systemd unit.

At least, turns out you can just use grep to do a set complement, which is nice.

There's comm in coreutils which was made for set complements.

This is probably incompatible with imperative creation of volumes and networks.

For compatibility, it's probably best to introduce something similar to users.mutableUsers so imperatively created volumes and networks aren't deleted unless mutability is explicitly disabled. If keeping track of which volumes and networks were created by nix is too much effort, it's possible to disable the volumes and networks options entirely when they're not explicitly enabled.

You can't delete used volumes either way. For example, gitlab-runner's docker runner uses an implicit container that handles things like cache sharing and artifact upload. It defines a dynamic volume, which we can not delete because the container is always running when gitlab-runner is. We also can't add it to the set of declarative volumes, because it's not a named volume. This is actually the reason I added || true to all the deletion incantations.

We can create a custom helper image, but that seems like three kinds of overkill to me, and just adds unnecessary maintenance burden.

Trying to make the Docker world deterministic is a bit of a fool's errand.

Perhaps the best idea here is to either call it ensureVolumes and ensureNetwork, or just drop this idea and put some create volume foo in a preStart.

I have some patches to docker-containers.nix, and there's something to be said about putting all this over there.

At least, turns out you can just use grep to do a set complement, which is nice.

There's comm in coreutils which was made for set complements.

Yeah, I saw comm in some of the snippets. Would you say using comm is preferrable here? I didn't find comm -23 <(A) <(B) any more readable or intuitive than some magic grep incantation.

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

1. Search for maintainers and people that previously touched the
   related code and @ mention them in a comment.
2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
   irc.freenode.net.