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

Ensure exporter progress logs are properly keyed #2953

Merged
merged 1 commit into from
Jul 23, 2022

Conversation

jedevc
Copy link
Member

@jedevc jedevc commented Jul 7, 2022

Fixes an issue with bake's progress output, where multiple exporters of the same type might be invoked, resulting in the logs writing over each other within the same section. Additionally, this will be important in the context of #2760, where the progress logs per output need to be appropriately separated.

Sample bake file:

group "default" {
  targets = ["foo", "bar", "baz"]
}

target "foo" {
  target = "foo"
  tags = [
    "docker.io/x/y:foo"
  ]
  output = ["type=image,push=true,registry.insecure=true"]
}

target "bar" {
  target = "bar"
  tags = [
    "docker.io/x/y:bar"
  ]
  output = ["type=image,push=true,registry.insecure=true"]
}

target "baz" {
  target = "baz"
  tags = [
    "docker.io/x/y:baz"
  ]
  output = ["type=image,push=true,registry.insecure=true"]
}

Before:

...
 => [bar] exporting to image                                                                                                0.2s
 => => exporting layers                                                                                                     0.1s
 => => exporting manifest sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa                           0.0s
 => => exporting manifest sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa                           0.0s
 => => exporting config sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb                             0.0s
 => => exporting config sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb                             0.0s
 => => exporting manifest sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa                           0.0s
 => => pushing layers                                                                                                       0.0s
 => => exporting config sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb                             0.0s
 => => pushing manifest for docker.io/x/y:baz@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa       0.0s
 => => pushing manifest for docker.io/x/y:foo@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa       0.0s
 => => pushing manifest for docker.io/x/y:bar@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa       0.0s

After:

...
 => [bar] exporting to image                                                                                                0.1s
 => => exporting layers                                                                                                     0.0s
 => => exporting manifest sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa                           0.0s
 => => exporting config sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb                             0.0s
 => => pushing layers                                                                                                       0.0s
 => => pushing manifest for docker.io/x/y:bar@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa       0.0s
 => [foo] exporting to image                                                                                                0.1s
 => => exporting layers                                                                                                     0.0s
 => => exporting manifest sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa                           0.0s
 => => exporting config sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb                             0.0s
 => => pushing layers                                                                                                       0.0s
 => => pushing manifest for docker.io/x/y:foo@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa       0.0s
 => [baz] exporting to image                                                                                                0.1s
 => => exporting layers                                                                                                     0.0s
 => => exporting manifest sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa                           0.0s
 => => exporting config sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb                             0.0s
 => => pushing layers                                                                                                       0.0s
 => => pushing manifest for docker.io/x/y:baz@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa       0.0s

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now they are only unique based on image export of cache export. I don't understand how this is related to the bake problem in the initial comment.

This patch ensures that all progress outputs for exporters are properly
keyed uniquely to the session, to ensure that duplicate exporters across
different sessions do not conflict and produce confusing results.

Currently, this behavior isn't relied upon by buildkit, but is used by
buildx bake, where multiple image exporters might be invoked at the same
time.

Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc
Copy link
Member Author

jedevc commented Jul 20, 2022

Woops, have split out the cache inline cache properly as well.

The cache exports need to be properly split out similarly, or we see the same kind of issue for inline cache:

 => [build2] preparing layers for inline cache                                                                               0.0s

vs

 => [build] preparing layers for inline cache                                                                                0.0s
 => [build2] preparing layers for inline cache                                                                               0.0s

and for the other cache types:

 => [build] exporting cache                                                                                                  0.0s
 => => preparing build cache for export                                                                                      0.0s
 => => writing layer sha256:405f018f9d1d0f351c196b841a7c7f226fb8ea448acd6339a9ed8741600275a2                                 0.0s
 => => writing layer sha256:43e589df01ad343faa6df8cead3077e6a243707b2bf6ccdbb4468c403f108bac                                 0.0s
 => => writing config sha256:e6a9dc809c4c1c9ed9e39a1922683daf6cae50efd74a4fa381d6336a27f23ded                                0.0s
 => => writing manifest sha256:cbee18d53f489c98bf8a88d4ff95ed9ebed3c489f80c70cf707aa2195de18057                              0.0s

vs

 => [build] exporting cache                                                                                                  0.0s
 => => preparing build cache for export                                                                                      0.0s
 => => writing layer sha256:405f018f9d1d0f351c196b841a7c7f226fb8ea448acd6339a9ed8741600275a2                                 0.0s
 => => writing layer sha256:43e589df01ad343faa6df8cead3077e6a243707b2bf6ccdbb4468c403f108bac                                 0.0s
 => => writing config sha256:e6a9dc809c4c1c9ed9e39a1922683daf6cae50efd74a4fa381d6336a27f23ded                                0.0s
 => => writing manifest sha256:cbee18d53f489c98bf8a88d4ff95ed9ebed3c489f80c70cf707aa2195de18057                              0.0s
 => [build2] exporting cache                                                                                                 0.0s
 => => preparing build cache for export                                                                                      0.0s
 => => writing layer sha256:405f018f9d1d0f351c196b841a7c7f226fb8ea448acd6339a9ed8741600275a2                                 0.0s
 => => writing layer sha256:43e589df01ad343faa6df8cead3077e6a243707b2bf6ccdbb4468c403f108bac                                 0.0s
 => => writing config sha256:e6a9dc809c4c1c9ed9e39a1922683daf6cae50efd74a4fa381d6336a27f23ded                                0.0s
 => => writing manifest sha256:cbee18d53f489c98bf8a88d4ff95ed9ebed3c489f80c70cf707aa2195de18057                              0.0s

Essentially, we need to force the logs here to be unique per session (previously they were derived exclusively from the exporter's .Name(), e.g. for containerimage this is "exporting to image"). Since bake then spins up a session per target, then all the logs will get separated out properly.

@tonistiigi tonistiigi merged commit 4c33f23 into moby:master Jul 23, 2022
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.

None yet

2 participants