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

Support user-supplied Docker image archive derivations #92

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tomeon
Copy link
Contributor

@tomeon tomeon commented Jan 29, 2020

First of all, thank you for this fantastic utility! I am so pleased to be able to describe Docker Compose environments using Nix instead of YAML :)

This PR introduces support for user-supplied Docker image archives via the attribute service.<name>.image.drv. The idea is that users can invoke dockerTools.buildImage, dockerTools.buildLayeredImage, or any other means of constructing a derivation that evaluates to a Docker image archive, then hand this off to Arion for use in the Compose environment.

This permits Arion users to exercise fine-grained control over their Docker images, even finer than what is possible with service.<name>.image.rawConfig. Additionally, it makes it easier to manipulate images separately from Arion (e.g., docker loading an archive constructed with a dockerTools function, then docker pushing it to some registry).

I am a Nix neophyte, and anticipate that I've committed any number of misjudgments here. Some questions that spring to mind:

  1. Is service.<name>.image.drv a good name? Maybe there's something more descriptive/straightforward?
  2. Is stronger type-checking warranted? Right now, service.<name>.image.drv accepts either null or any derivation whatever, regardless of whether it evaluates to a Docker image archive when built. This seems fine for an option marked internal, but maybe not one intended for public consumption. Checking for the AFAIK-dockerTools-specific imageName or passthru.imageTag is probably too stringent; maybe there are sentinels of Docker imagehood not specific to dockerTools? OTOH maybe docker load-time errors are acceptable here, as they are basically analogous to the runtime errors that would occur upon mis-specifying service.<name>.service.name?
  3. Is there a better way to handle constructing a default image derivation than build.image = config.image.drv or builtImage? It does not feel DRY to have both build.image and image.drv, but I couldn't find a straightforward way to remove the duplication. In particular, the following construct:
{
  image.drv = mkOption {
    type = nullOr package;
    default = builtImage;
  };
}

led to the error:

$ env -u NIX_PATH nix-build --show-trace ./nix/ci.nix 
error: while evaluating the attribute 'text' of the derivation 'agent-options' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/pkgs/build-support/trivial-builders.nix:7:14:
while evaluating the attribute 'optionsAsciiDoc' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/nixos/lib/make-options-doc/default.nix:132:3:
while evaluating anonymous function at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/attrsets.nix:234:10, called from undefined position:
while evaluating 'singleAsciiDoc' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/nixos/lib/make-options-doc/default.nix:94:26, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/attrsets.nix:234:16:
while evaluating the attribute 'default' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/nixos/lib/make-options-doc/default.nix:37:44:
while evaluating 'substFunction' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/nixos/lib/make-options-doc/default.nix:28:19, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/nixos/lib/make-options-doc/default.nix:37:54:
while evaluating the attribute 'default' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/options.nix:154:44:
while evaluating 'scrubOptionValue' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/options.nix:173:22, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/options.nix:154:54:
while evaluating 'isDerivation' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/attrsets.nix:305:18, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/options.nix:174:8:
while evaluating the attribute 'default' at /var/lib/hercules-ci-agent/work/eval-75444b67e5c0aab9/primary/tomeon-arion-2afdefd/src/nix/modules/service/image.nix:42:7:
while evaluating the module argument `pkgs' in "/var/lib/hercules-ci-agent/work/eval-75444b67e5c0aab9/primary/tomeon-arion-2afdefd/src/nix/modules/service/image.nix":
while evaluating the attribute '_module.args.pkgs' at undefined position:
while evaluating anonymous function at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:75:45, called from undefined position:
while evaluating the attribute 'value' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:336:9:
while evaluating the option `services.<name>._module.args':
while evaluating the attribute 'mergedValue' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:368:5:
while evaluating anonymous function at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:368:32, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:368:19:
while evaluating 'merge' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/types.nix:283:20, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:371:8:
while evaluating 'filterAttrs' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/attrsets.nix:124:23, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/types.nix:284:35:
while evaluating anonymous function at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/attrsets.nix:125:29, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/attrsets.nix:125:18:
while evaluating anonymous function at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/types.nix:284:51, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/attrsets.nix:125:62:
while evaluating the attribute 'pkgs' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/attrsets.nix:344:7:
while evaluating anonymous function at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/types.nix:284:86, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/attrsets.nix:344:15:
while evaluating the attribute 'optionalValue' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:375:5:
while evaluating the attribute 'values' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:362:9:
while evaluating anonymous function at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:358:19, called from /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:358:14:
while evaluating the attribute 'value._type' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:454:73:
while evaluating the attribute 'value.content' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:515:14:
while evaluating the attribute 'pkgs' at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:162:9:
while evaluating the module argument `pkgs' in "/var/lib/hercules-ci-agent/work/eval-75444b67e5c0aab9/primary/tomeon-arion-2afdefd/src/nix/modules/composition/docker-compose.nix":
while evaluating the attribute '_module.args.pkgs' at undefined position:
attribute 'pkgs' missing, at /nix/store/g7is268z0zj1qfdvm4vlvihcsvgfplc4-source/lib/modules.nix:163:28

I've also included a fix for an issue in the nixosTest suite: the work directory containing arion-pkgs.nix and arion-compose.nix wasn't being properly cleaned up, leading to all test cases subsequent to the first (minimal) failing to run (more precisely, all test cases subsequent to the first simply re-ran the first).

Happy to put this changeset in a separate PR if you'd prefer.


Thanks in advance for your consideration!

@roberth
Copy link
Member

roberth commented Jan 29, 2020

Thank you!
The change can be improved a bit.

Is service..image.drv a good name?

I try to avoid abbreviations where possible, but a different name may be needed.
It seems like arion could support any path (copied to store) or string (not copied to store). That's nice to have, but the important bit is service.<name>.image.tarball seems like a more suitable name.

Is stronger type-checking warranted? [...] probably too stringent

I agree with your analysis.

Is there a better way to handle constructing a default image derivation than build.image = config.image.drv or builtImage?

Yes. The module system comes with an overriding feature that you probably already know from lib.mkForce, lib.mkDefault etc. We can use this instead of providing a new option.
So this can be simplified by moving build.image to image.tarball, removing the internal = true argument and wrapping the definition (config section) in lib.mkDefault.

@roberth
Copy link
Member

roberth commented Jan 29, 2020

bors try

bors bot added a commit that referenced this pull request Jan 29, 2020
@bors
Copy link
Contributor

bors bot commented Jan 29, 2020

try

Build failed

@tomeon
Copy link
Contributor Author

tomeon commented Jan 30, 2020

Thank you for the feedback, @roberth! I will make the changes you highlighted, hopefully within the next few days.

In the meantime, I took a look at the build failures, and they appear to be due to an OOM condition that occurs before arion down exits in the full-nixos scenario:

machine: must succeed: cd work && NIX_PATH=nixpkgs='/nix/store/4cwp20vgdmd23300qgnnfhi0mqjn39xh-g7is268z0zj1qfdvm4vlvihcsvgfplc4-source' arion down
machine# [   85.840402] dhcpcd[720]: br-c367538c3181: no IPv6 Routers available
machine# [   89.709152] dhcpcd[720]: veth9c51ffd: no IPv6 Routers available
machine# [  551.305541] dhcpcd-run-hook invoked oom-killer: gfp_mask=0x6200ca(GFP_HIGHUSER_MOVABLE), nodemask=(null), order=0, oom_score_adj=0

I suspect that this is a latent issue exposed by the changes I made to the test suite; will bisect to be sure.

@tomeon tomeon force-pushed the arbitrary-image-drv-support branch 2 times, most recently from ad24d09 to 457b92c Compare October 14, 2023 12:31
@tomeon tomeon changed the title [WIP] Support user-supplied Docker image archive derivations Support user-supplied Docker image archive derivations Oct 14, 2023
@tomeon tomeon force-pushed the arbitrary-image-drv-support branch 2 times, most recently from 9136a62 to 358a2e4 Compare October 14, 2023 15:12
@tomeon
Copy link
Contributor Author

tomeon commented Oct 14, 2023

I will make the changes you highlighted, hopefully within the next few days.

And a mere ~1350 days later, I'm back with some updates (~1350 is a "few days" by geological or cosmological standards, I suppose...).

IIRC the test failure was due to the nixosTest VM running out of memory, and the solution was bumping up virtualisation.memorySize. This was fixed in fd41e1e, so 👍

I've updated the PR as follows:

  1. Renamed image.drv to image.tarball,
  2. Removed build.image, build.imageName, and build.imageTag,
  3. Added image.tag for specifying the tag to assign to the generated image or associate with the image specified in image.tarball,
  4. Added some sanity-checking around image name and tag inference (checking that both the name and tag are set to non-empty strings, basically), and
  5. Added some tests of image name and tag inference logic.

tomeon and others added 4 commits April 28, 2024 10:53
Via the `services.<name>.image.tarball` option.

This permits users to supply any valid image archive, potentially (but
not necessarily) generated by a builder function from
`pkgs.dockerTools`.
and associated subtest.

This tests the functionality of the newly-added
`services.<name>.image.tarball` parameter.
and replace its uses with `image.tarball{,.imageName,.imageTag}`.

This changeset is a hard deprecation of `build.image{,Name,Tag}` and
does not do any option renaming, etc., as the removed options were all
marked as internal.

This changeset also includes code that ensures the newly-relied-upon
options are properly defined, raising user-visible assertion errors if
no sane default image name or tag could be inferred.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants