Migrate the Docker context tags version from `<stage>.tag` to `tags.<stage>`. #14376

kaos · 2022-02-05T22:24:07Z

Currently, the context gets values on the form <stage>.tag = <tag>, where stage is either baseimage, stageN or <stage name>.

Given the dynamic nature of this key, it is not well suited to use as a root key for the context, as it may conflict with other values. e.g. if you have a stage named "build_args", it would collide with the context value for build args.

This PR deprecates the current layout in 2.10, in favour of putting them under a tags key instead tags.<stage> = <tag>:

{
  "tags": {
    "baseimage": "latest",
    "stage0": "latest",
    "custom-stage-name": "1.2",
    ...
  },
  "build_args": {...},
  ...
}

…stage>`. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust]

kaos · 2022-02-05T22:26:59Z

Tried the deprecation message on the Docker testproject, and it looks like:

diff --git a/testprojects/src/python/docker/BUILD b/testprojects/src/python/docker/BUILD
index a6e4ffef6..58c01faf4 100644
--- a/testprojects/src/python/docker/BUILD
+++ b/testprojects/src/python/docker/BUILD
@@ -9,7 +9,7 @@ docker_image(
 
 docker_image(
     name="test-example-synth",
-    image_tags=["1.2.5"],
+    image_tags=["1.2.5-{baseimage.tag}", "{tags.baseimage}"],
     instructions=[
         "FROM python:3.7",
         'ENTRYPOINT ["/hello"]',

$ ./pants package testprojects/src/python/docker:test-example-synth
23:19:54.91 [INFO] Completed: Building docker image test-example-synth:1.2.5-3.7 +1 additional tag.
23:19:54.91 [WARN] DEPRECATED: Docker interpolation context type 'DockerfileImageTagsDeprecation' will be removed in version 2.11.0.dev0.

The `<stage>.tag` version values are deprecated in favour of `tags.<stage>`.

See https://github.com/pantsbuild/pants/issues/14023 for more details.
23:19:54.92 [INFO] Built docker images: 
  * test-example-synth:1.2.5-3.7
  * test-example-synth:3.7

Not sure really what to put in as entity on the warn_or_error call there that would make sense here...

Eric-Arellano

Looks reasonable

Eric-Arellano · 2022-02-07T18:56:23Z

src/python/pants/backend/docker/util_rules/docker_build_context.py

+class DockerfileImageTagsDeprecation(DeprecatedDockerInterpolationValue):
+    _removal_version = "2.11.0.dev0"
+    _hint = (
+        "The `<stage>.tag` version values are deprecated in favour of `tags.<stage>`.\n\n"


Would be good to Save A Click and give an example:

For example, rather than using {baseimage.tag} in your Docker template, use {tags.baseimage}.

But then still link to this PR like you do if people still aren't understanding.

Eric-Arellano · 2022-02-07T18:57:18Z

Not sure really what to put in as entity on the warn_or_error call there that would make sense here...

The PR title: "using <stage>.tag rather than tags.<stage> for Docker interpolation

Eric-Arellano · 2022-02-07T23:02:36Z

Rather than using the Class and getattr to deprecate this, I wonder if we can inline using warn_or_error() into docker_build_context.py line 143? I think that could allow us to have a better warning that points to the docker_image target causing issues; otherwise, you'll have to track that down manually.

Also, I'm not sure if this deprecation is always executed? It looks like it would be. We need to make sure the deprecation is silent if you did nothing wrong.

kaos · 2022-02-08T08:08:15Z

Not sure really what to put in as entity on the warn_or_error call there that would make sense here...

The PR title: "using .tag rather than tags. for Docker interpolation

That is the hint, so is in there on the second line of the deprecation message.

Rather than using the Class and getattr to deprecate this, I wonder if we can inline using warn_or_error() into docker_build_context.py line 143?

At that point, we don't know if the deprecated name will be used or not, hence why I placed it in getattr, so it will only trigger the deprecation message when actually referenced.

Also, I'm not sure if this deprecation is always executed? It looks like it would be. We need to make sure the deprecation is silent if you did nothing wrong.

See above.

[...] a better warning that points to the docker_image target causing issues

That's a good idea, I'll take a stab at that.

kaos · 2022-02-08T09:22:54Z

$ ./pants package testprojects/src/python/docker:test-example-synth
10:21:19.84 [WARN] DEPRECATED: Docker interpolation context type 'DockerfileImageTagsDeprecation_baseimage' will be removed in version 2.11.0.dev0.

Interpolation in testprojects/src/python/docker:test-example-synth uses the deprecated placeholder 'baseimage.tag', instead use 'tags.baseimage'.

The `<stage>.tag` version values are deprecated in favour of `tags.<stage>`.
See https://github.com/pantsbuild/pants/issues/14023 for more details.
10:21:21.16 [INFO] Completed: Building docker image test-example-synth:1.2.5 +1 additional tag.

for this build target:

docker_image(
    name="test-example-synth",
    image_tags=["1.2.5", "1.{baseimage.tag}"],
    instructions=[
        "FROM python:3.7",
        'ENTRYPOINT ["/hello"]',
        'CMD ["synthetic!"]',
        "COPY testprojects.src.python.hello.main/main.pex /hello",
    ],
)

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Eric-Arellano · 2022-02-08T15:31:35Z

Awesome! To be crystal clear, if you remove the bad reference and rerun with --no-pantsd, no deprecation message? If so, lgtm. Thanks for this change :)

kaos · 2022-02-08T17:01:13Z

if you remove the bad reference and rerun with --no-pantsd, no deprecation message?

Check. Works as expected both with and without the --no-pantsd flag.

$ ./pants package testprojects/src/python/docker:test-example-synth
17:58:32.04 [WARN] DEPRECATED: Docker interpolation context type 'DockerfileImageTagsDeprecation_baseimage' will be removed in version 2.11.0.dev0.

Interpolation in testprojects/src/python/docker:test-example-synth uses the deprecated placeholder 'baseimage.tag', instead use 'tags.baseimage'.

The `<stage>.tag` version values are deprecated in favour of `tags.<stage>`.
See https://github.com/pantsbuild/pants/issues/14023 for more details.
17:58:33.27 [INFO] Completed: Building docker image test-example-synth:1.2.5 +1 additional tag.
17:58:33.27 [INFO] Built docker images: 
  * test-example-synth:1.2.5
  * test-example-synth:1.3.7

# Edited to the `image_tag` to new syntax.

$ ./pants --no-pantsd package testprojects/src/python/docker:test-example-synth
17:59:05.99 [INFO] Completed: Building docker image test-example-synth:1.2.5 +1 additional tag.
17:59:06.00 [INFO] Built docker images: 
  * test-example-synth:1.2.5
  * test-example-synth:1.3.7

Eric-Arellano

Thank you!

…stage>`. (pantsbuild#14376) Fixes pantsbuild#14023 Currently, the context gets values on the form `<stage>.tag = <tag>`, where stage is either `baseimage`, `stageN` or `<stage name>`. Given the dynamic nature of this key, it is not well suited to use as a root key for the context, as it may conflict with other values. e.g. if you have a stage named "build_args", it would collide with the context value for build args. This PR deprecates the current layout in 2.10, in favour of putting them under a `tags` key instead `tags.<stage> = <tag>`: ```python { "tags": { "baseimage": "latest", "stage0": "latest", "custom-stage-name": "1.2", ... }, "build_args": {...}, ... } ``` # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

…stage>`. (#14376) (#14403)

Migrate the Docker context tags version from <stage>.tag to `tags.<…

40b1d87

…stage>`. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust]

kaos requested a review from Eric-Arellano February 5, 2022 22:24

Eric-Arellano approved these changes Feb 7, 2022

View reviewed changes

kaos added 2 commits February 8, 2022 11:33

propagate source of deprecated use.

380e632

# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]

Merge branch 'main' into docker_version_tags_interpolation_context

5eb8f65

Eric-Arellano approved these changes Feb 8, 2022

View reviewed changes

kaos merged commit bf63c74 into pantsbuild:main Feb 8, 2022

kaos deleted the docker_version_tags_interpolation_context branch February 8, 2022 19:31

kaos added a commit that referenced this pull request Feb 8, 2022

Migrate the Docker context tags version from <stage>.tag to `tags.<…

e824d5e

…stage>`. (#14376) (#14403)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate the Docker context tags version from `<stage>.tag` to `tags.<stage>`. #14376

Migrate the Docker context tags version from `<stage>.tag` to `tags.<stage>`. #14376

kaos commented Feb 5, 2022

kaos commented Feb 5, 2022

Eric-Arellano left a comment

Eric-Arellano Feb 7, 2022

Eric-Arellano commented Feb 7, 2022

Eric-Arellano commented Feb 7, 2022

kaos commented Feb 8, 2022

kaos commented Feb 8, 2022

Eric-Arellano commented Feb 8, 2022

kaos commented Feb 8, 2022

Eric-Arellano left a comment

Migrate the Docker context tags version from <stage>.tag to tags.<stage>. #14376

Migrate the Docker context tags version from <stage>.tag to tags.<stage>. #14376

Conversation

kaos commented Feb 5, 2022

kaos commented Feb 5, 2022

Eric-Arellano left a comment

Choose a reason for hiding this comment

Eric-Arellano Feb 7, 2022

Choose a reason for hiding this comment

Eric-Arellano commented Feb 7, 2022

Eric-Arellano commented Feb 7, 2022

kaos commented Feb 8, 2022

kaos commented Feb 8, 2022

Eric-Arellano commented Feb 8, 2022

kaos commented Feb 8, 2022

Eric-Arellano left a comment

Choose a reason for hiding this comment

Migrate the Docker context tags version from `<stage>.tag` to `tags.<stage>`. #14376

Migrate the Docker context tags version from `<stage>.tag` to `tags.<stage>`. #14376