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

layers argument of makeContainerImage is incorrectly named #1221

Closed
Ten0 opened this issue Dec 15, 2023 · 5 comments
Closed

layers argument of makeContainerImage is incorrectly named #1221

Ten0 opened this issue Dec 15, 2023 · 5 comments
Labels
difficulty::low help wanted Will be solved only if someone contributes it

Comments

@Ten0
Copy link

Ten0 commented Dec 15, 2023

}: let
sharedAttrs = {
inherit config;
contents = layers;
created = "1970-01-01T00:00:01Z";
name = "container-image";
tag = "latest";
};
in
if layered
then
__nixpkgs__.dockerTools.buildLayeredImage
(sharedAttrs
// {
inherit extraCommands;
inherit maxLayers;
})

contents is a set of derivations that have to somehow be put in the final image, but it is not specified that every such content should be a layer.
Instead, constrained by maxLayers, layers will most likely be generated from the set of contents in such a way that one layer may contain several derivations, following this algorithm, but also a single content may be split in several layers, at most one per derivation.

=> It appears that the layers argument is misleading. It should instead probably propagate the naming suggested by nixpkgs, that is: contents.

The current naming is confusing: one has to wonder how the maxLayers argument interacts with the layers argument.

@dsalaza4
Copy link
Contributor

Hi @Ten0,

You are right about this.

Aside from the naming convention issue, I would also like to take a look at how dockerTools.buildLayeredImage is actually storing derivations within the container layers, as last time I checked the behavior was:

  1. If currentLayer < maxLayers then store 1 derivation in currentLayer
  2. if currentLayer == maxLayers then store all remaining derivations in last layer

I wonder if this approach is optimal as the last layer ends up with most derivations.

@Ten0
Copy link
Author

Ten0 commented Dec 19, 2023

I don't know that this has changed, it's probably still the same (documentation doesn't specify any more complex behavior at least).

At NixCon 2022 there was somebody talking about an alternative to the standard image builder (that is specifically designed to not re-build layers that were already pushed to the remote repository) and whose closing words were about improving the layering algorithm : nix2container.
talk: https://www.youtube.com/watch?v=ELeMGbFjtTo
blogpost: https://lewo.abesis.fr/posts/nix-build-container-image/

Maybe they have implemented that now? (In any case using that to push images seems essential in a large monorepo if one wants to not re-build the tar stream of all already-pushed layers at every deployment so I'm guessing that would be a great addition to this project)

I guess the optimal algorithm would take into account all images that need to be built to try and do the best factoring of all the images.

@dsalaza4
Copy link
Contributor

dsalaza4 commented Dec 19, 2023

One of the things makes tries to get rid of is having to deal with many different containers (building and deploying them).

We prefer using the makes container itself, point to the repository we want to use, and do both provisioning and execution in runtime.

Instead of building a container for /project1/service1/,

we just do:

image: ghcr.io/fluidattacks/makes/amd64:latest
script: m github:my-username/my-project@main /project1/service1

@Ten0
Copy link
Author

Ten0 commented Dec 19, 2023

Aah I see, that's a very interesting approach!
Now the main.nix makes so much more sense to me!

@dsalaza4 dsalaza4 added help wanted Will be solved only if someone contributes it difficulty::low labels Feb 27, 2024
@dsalaza4
Copy link
Contributor

As mentioned in #502, we will no longer be supporting specific-purpose builtins. Instead of creating a wrapper around dockerTools.buildLayeredImage, we will invite developers to use that function instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty::low help wanted Will be solved only if someone contributes it
Projects
None yet
Development

No branches or pull requests

2 participants