Mixins install order isn't fixed which results in cache miss and redundant images #2469

tamirkamara · 2022-11-25T13:18:01Z

Describe the bug

When multiple mixins are specified in the definition they are installed in "random" order which doesn't make good use of caching and results in multiple images

To Reproduce

Run ...
Use this porter.yaml from here
Run porter build a few times

Expected behavior

Expect to see just the first build run every layer and the rest to be cached. But you will probably see a few like the below.

Porter Command and Output

Notice that stage-0 5/17 isn't the same in both builds, hence can't be cached.

Build N

 => [internal] load build definition from Dockerfile                                                                                                                     0.0s
 => => transferring dockerfile: 2.12kB                                                                                                                                   0.0s
 => [internal] load .dockerignore                                                                                                                                        0.0s
 => => transferring context: 35B                                                                                                                                         0.0s
 => resolve image config for docker.io/docker/dockerfile-upstream:1.4.0                                                                                                  0.6s
 => CACHED docker-image://docker.io/docker/dockerfile-upstream:1.4.0@sha256:178c4e4a93795b9365dbf6cf10da8fcf517fcb4a17f1943a775c0f548e9fc2ff                             0.0s
 => [internal] load build definition from Dockerfile                                                                                                                     0.0s
 => [internal] load .dockerignore                                                                                                                                        0.0s
 => [internal] load metadata for docker.io/library/debian:buster-slim                                                                                                    0.0s
 => [internal] load build context                                                                                                                                        1.0s
 => => transferring context: 173.47MB                                                                                                                                    0.9s
 => [stage-0  1/17] FROM docker.io/library/debian:buster-slim                                                                                                            0.0s
 => CACHED [stage-0  2/17] RUN useradd nonroot -m -u 65532 -g 0 -o                                                                                                       0.0s
 => CACHED [stage-0  3/17] RUN rm -f /etc/apt/apt.conf.d/docker-clean; echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache        0.0s
 => [stage-0  4/17] RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt     apt-get update &&     apt-get install -y jq="1.5+dfsg-2+b1"  3.0s
 => [stage-0  5/17] RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt  apt-get update && apt-get install -y apt-transport-https lsb-  15.7s
 => [stage-0  6/17] RUN curl -sL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor > /etc/apt/trusted.gpg.d/microsoft.asc.gpg                            0.9s
 => [stage-0  7/17] RUN echo "deb [arch=amd64] https://packages.microsoft.com/repos/azure-cli/ $(lsb_release -cs) main" > /etc/apt/sources.list.d/azure-cli.list         1.1s
 => [stage-0  8/17] RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt  apt-get update && apt-get install -y --no-install-recommends   22.3s
 => [stage-0  9/17] RUN apt-get update && apt-get install -y wget unzip &&  apt-get clean -y && rm -rf /var/lib/apt/lists/* &&  wget https://releases.hashicorp.com/ter  7.6s
 => [stage-0 10/17] COPY terraform/ /cnab/app/terraform/                                                                                                                 0.1s
 => [stage-0 11/17] RUN cd /cnab/app/terraform &&  terraform init -backend=false &&  rm -fr .terraform/providers &&  terraform providers mirror /usr/local/share/terra  27.3s
 => [stage-0 12/17] COPY --link . /cnab/app                                                                                                                              0.3s
 => [stage-0 13/17] RUN rm /cnab/app/porter.yaml                                                                                                                         0.6s
 => [stage-0 14/17] RUN rm -fr /cnab/app/.cnab                                                                                                                           0.6s
 => [stage-0 15/17] COPY --link .cnab /cnab                                                                                                                              0.2s
 => [stage-0 16/17] RUN chgrp -R 0 /cnab && chmod -R g=u /cnab                                                                                                           1.1s
 => [stage-0 17/17] WORKDIR /cnab/app                                                                                                                                    0.1s
 => exporting to image                                                                                                                                                   6.9s
 => => exporting layers                                                                                                                                                  6.9s
 => => writing image sha256:cc4b2d7987b6531fca5bc1b6abf0ca45e27a19527d0b9383da8112e79e7c6c14                                                                             0.0s
 => => naming to docker.io/azuretre/tre-workspace-base:porter-785f646f84ea65af983a9191951a9a24

Build N+1

  => [internal] load build definition from Dockerfile                                                                                                                     0.0s
 => => transferring dockerfile: 2.12kB                                                                                                                                   0.0s
 => [internal] load .dockerignore                                                                                                                                        0.0s
 => => transferring context: 35B                                                                                                                                         0.0s
 => resolve image config for docker.io/docker/dockerfile-upstream:1.4.0                                                                                                  0.6s
 => CACHED docker-image://docker.io/docker/dockerfile-upstream:1.4.0@sha256:178c4e4a93795b9365dbf6cf10da8fcf517fcb4a17f1943a775c0f548e9fc2ff                             0.0s
 => [internal] load .dockerignore                                                                                                                                        0.0s
 => [internal] load build definition from Dockerfile                                                                                                                     0.0s
 => [internal] load metadata for docker.io/library/debian:buster-slim                                                                                                    0.0s
 => [internal] load build context                                                                                                                                        1.0s
 => => transferring context: 173.47MB                                                                                                                                    1.0s
 => [stage-0  1/17] FROM docker.io/library/debian:buster-slim                                                                                                            0.0s
 => CACHED [stage-0  2/17] RUN useradd nonroot -m -u 65532 -g 0 -o                                                                                                       0.0s
 => CACHED [stage-0  3/17] RUN rm -f /etc/apt/apt.conf.d/docker-clean; echo 'Binary::apt::APT::Keep-Downloaded-Packages "true";' > /etc/apt/apt.conf.d/keep-cache        0.0s
 => CACHED [stage-0  4/17] RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt     apt-get update &&     apt-get install -y jq="1.5+dfs  0.0s
 => [stage-0  5/17] RUN apt-get update && apt-get install -y wget unzip &&  apt-get clean -y && rm -rf /var/lib/apt/lists/* &&  wget https://releases.hashicorp.com/ter  9.3s
 => [stage-0  6/17] COPY terraform/ /cnab/app/terraform/                                                                                                                 0.1s
 => [stage-0  7/17] RUN cd /cnab/app/terraform &&  terraform init -backend=false &&  rm -fr .terraform/providers &&  terraform providers mirror /usr/local/share/terra  31.3s
 => [stage-0  8/17] RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt  apt-get update && apt-get install -y apt-transport-https lsb-r  9.7s
 => [stage-0  9/17] RUN curl -sL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor > /etc/apt/trusted.gpg.d/microsoft.asc.gpg                            1.0s
 => [stage-0 10/17] RUN echo "deb [arch=amd64] https://packages.microsoft.com/repos/azure-cli/ $(lsb_release -cs) main" > /etc/apt/sources.list.d/azure-cli.list         0.5s
 => [stage-0 11/17] RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt  apt-get update && apt-get install -y --no-install-recommends   14.1s
 => [stage-0 12/17] COPY --link . /cnab/app                                                                                                                              0.3s
 => [stage-0 13/17] RUN rm /cnab/app/porter.yaml                                                                                                                         0.5s
 => [stage-0 14/17] RUN rm -fr /cnab/app/.cnab                                                                                                                           0.5s
 => [stage-0 15/17] COPY --link .cnab /cnab                                                                                                                              0.2s
 => [stage-0 16/17] RUN chgrp -R 0 /cnab && chmod -R g=u /cnab                                                                                                           0.9s
 => [stage-0 17/17] WORKDIR /cnab/app                                                                                                                                    0.1s
 => exporting to image                                                                                                                                                   8.5s
 => => exporting layers                                                                                                                                                  8.4s
 => => writing image sha256:26b768ddab37d803ceb0313a195f1361690870362551f5d2b92958d99e040f7e                                                                             0.0s
 => => naming to docker.io/azuretre/tre-workspace-base:porter-785f646f84ea65af983a9191951a9a24

Version

Copy the output of porter version below

[porter v1.0.2 (0656f1f)]

The text was updated successfully, but these errors were encountered:

carolynvs · 2022-11-28T16:51:27Z

We should install mixins in the order they are specified in the porter.yaml, so this is a bug that needs to be fixed. Thanks for raising this issue!

When we are querying mixins, such as when we are asking each mixin for Dockerfile lines to include in the bundle image, we call them in go routines. Depending on how quickly they return, the order is not guaranteed and may change. I have updated our mixin query function to sort the results of querying the mixins in a consistent order, the order that they are defined in the bundle. Fixes getporter#2469 During build in particular, this ensures that we optimize layer caching as much as possible by always outputing the same contents of the Dockerfile when building a bundle. Previously, it could include the mixins in varying order which prevented reuse of layers from previous builds. Signed-off-by: Carolyn Van Slyck <[email protected]>

When we are querying mixins, such as when we are asking each mixin for Dockerfile lines to include in the bundle image, we call them in go routines. Depending on how quickly they return, the order is not guaranteed and may change. I have updated our mixin query function to sort the results of querying the mixins in a consistent order, the order that they are defined in the bundle. The Execute function now returns a bit more information about the results of each mixin command, and it's up to the caller to decide how to handle errors when not all mixins are required to have a successful exit code. Lint allows mixins to not implement that command, so in that case we check if the command isn't implemented and skip over it. Fixes getporter#2469 During build in particular, this ensures that we optimize layer caching as much as possible by always outputing the same contents of the Dockerfile when building a bundle. Previously, it could include the mixins in varying order which prevented reuse of layers from previous builds. Signed-off-by: Carolyn Van Slyck <[email protected]>

* Sort mixin output in order declared When we are querying mixins, such as when we are asking each mixin for Dockerfile lines to include in the bundle image, we call them in go routines. Depending on how quickly they return, the order is not guaranteed and may change. I have updated our mixin query function to sort the results of querying the mixins in a consistent order, the order that they are defined in the bundle. The Execute function now returns a bit more information about the results of each mixin command, and it's up to the caller to decide how to handle errors when not all mixins are required to have a successful exit code. Lint allows mixins to not implement that command, so in that case we check if the command isn't implemented and skip over it. Fixes #2469 During build in particular, this ensures that we optimize layer caching as much as possible by always outputing the same contents of the Dockerfile when building a bundle. Previously, it could include the mixins in varying order which prevented reuse of layers from previous builds. Signed-off-by: Carolyn Van Slyck <[email protected]> * Remove unused variable Signed-off-by: Carolyn Van Slyck <[email protected]> * Fix race condition in unit test Signed-off-by: Carolyn Van Slyck <[email protected]> * Make vet happy Signed-off-by: Carolyn Van Slyck <[email protected]> * Fix capturing the error when lint isn't supported The "unsupported command" text is printed to stderr. I have updated our mixin/plugin runner to capture the command output and include it in the returned error when the command fails. That way we can check if a mixin command failed because the command isn't implemented. Signed-off-by: Carolyn Van Slyck <[email protected]> * Incorporate review feedback Signed-off-by: Carolyn Van Slyck <[email protected]> Signed-off-by: Carolyn Van Slyck <[email protected]>

carolynvs · 2022-12-05T15:41:05Z

@tamirkamara A fix is now available in v1.0.3

tamirkamara added the bug Oops, sorry! label Nov 25, 2022

carolynvs added this to Upcoming Releases Nov 28, 2022

carolynvs mentioned this issue Nov 28, 2022

Sort mixin output in order declared #2472

Merged

4 tasks

carolynvs self-assigned this Nov 28, 2022

carolynvs closed this as completed in #2472 Nov 30, 2022

carolynvs moved this to Done in Upcoming Releases Nov 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mixins install order isn't fixed which results in cache miss and redundant images #2469

Mixins install order isn't fixed which results in cache miss and redundant images #2469

tamirkamara commented Nov 25, 2022

carolynvs commented Nov 28, 2022

carolynvs commented Dec 5, 2022

Mixins install order isn't fixed which results in cache miss and redundant images #2469

Mixins install order isn't fixed which results in cache miss and redundant images #2469

Comments

tamirkamara commented Nov 25, 2022

Describe the bug

To Reproduce

Expected behavior

Porter Command and Output

Version

carolynvs commented Nov 28, 2022

carolynvs commented Dec 5, 2022