feat: add pull_image setting to `docker_registry_image` datasource

[saitho]

trafficstars

Resolves #172

Going by my suggestion I tried skipping the local image check. I didn't know if it's possible to check if the sha256_digest referenced by pull_triggers changed...

So I ended up adding a new setting "pull_image" to the docker_registry_image datasource which is false per default. If enabled, it will pull the image if the sha256_digest was modified during the read.

I can't assess whether this change affects other resources as well. Also I wasn't sure how to test that properly. Please tell me what you think about this change. :+1:

[suzuki-shunsuke]

https://github.com/kreuzwerker/terraform-provider-docker/pull/178/checks?check_run_id=2384975622#step:4:12

Error: GPG private key required

https://github.com/kreuzwerker/terraform-provider-docker/blob/2845519dce58ff9f0564e93b2a2909e37873b28b/.github/workflows/compile.yaml#L25-L30

These secrets can't be available to builds of forks.

https://github.community/t/make-secrets-available-to-builds-of-forks/16166/33

[suzuki-shunsuke]

Thank you for your contribution! 👍

[suzuki-shunsuke]

This branch is out-of-date with the base branch. Please update the branch.

[saitho]

These secrets can't be available to builds of forks.

The job execution can be restricted to run only on the main repository:

if: github.repository == 'kreuzwerker/terraform-provider-docker'

Should I add that? Or is there a way to build binaries with goreleaser without the GPG secret for forked releases?

[mavogel]

@saitho sorry for this, you're one of our first forks. Please rebase on the master. I fixed the issue in https://github.com/kreuzwerker/terraform-provider-docker/commit/ca2663562579a7094e1990c7cbeb48cb59ab1c85 because for the compile action we don't need the signing

[saitho]

@saitho sorry for this, you're one of our first forks. Please rebase on the master. I fixed the issue in ca26635 because for the compile action we don't need the signing

Alright, no worries :) I rebased, pipeline looks good now.

[mavogel]

@suzuki-shunsuke please re-review :)

[suzuki-shunsuke]

I think we can use schema#ResourceData.HasChange.

Oh, I found a problem. ForceNew of pull_triggers is true, so we can't check if pull_triggers is changed. When pull_triggers is changed, the docker_image resource would be recreated. If docker_image's keep_locally is false, the old image is removed from local and the image digest is updated, but if keep_locally is true the old image isn't removed so the image digest isn't updated.

The workaround is to make keep_locally false.

To solve this problem, I guess we have to change ForceNew of pull_triggers to false (BREAKING CHANGE).

[suzuki-shunsuke]

I changed ForceNew of pull_triggers to false in #184 . Then the local image digest was updated properly, but the other problem was found. When terraform plan is run, even if a docker_image resource is updated, the dependent docker_container resource isn't updated. I guess the cause is Computed of docker_image.latest is true.

[github-actions[bot]]

This pull request is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days. If you don't want this pull request to be closed, please set the label pinned.

[mavogel]

@saitho and @suzuki-shunsuke are you still on this?

[saitho]

I'm still interested in getting this solved. However I'm not too much into Terraform to solve the need-to-apply-twice issue in the PR. If I can still do anything, let me know. Maybe I'll experiment with it later.

[suzuki-shunsuke]

saitho and suzuki-shunsuke are you still on this?

No. I have no idea to solve the problem.

• https://github.com/kreuzwerker/terraform-provider-docker/pull/178#issuecomment-826062175
• https://github.com/kreuzwerker/terraform-provider-docker/pull/178#issuecomment-826069637

Do you have any idea?

[stephenliberty]

$.02:

We got bit by the related bug (#172). After mulling this over for a bit, I think you might be trying to solve a use case that was created by accident, rather than the one that users really want. Maybe. This is a long and slightly opinionated rant, I'm sorry.

Right now it seems that the bug is as follows (for sake of clarity of conversation):

1. Create docker_registry_image resource, let's say nginx:latest
2. Create docker_image resource, pull trigger set to registry_image, keep_locally set to true
3. Upstream registry image changes, but local image does not change

I don't think the registry image is the issue here - I think it's performing just fine and shouldn't be touched. I also think that the local image is working fine too! We're just telling the provider to drop "latest" onto the machine, and not to delete the image if the resource goes away, right? When the upstream sha changes and it's recreated, the provider finds that the latest tag already exists, and instead of forcibly doing anything with that image, it simply says "cool, image tag is here, requested state matches". If we wanted something more specific, wouldn't it be better to.. well.. be specific?

Again, for sake of conversation, the situation is easily reproducible via the following:

docker pull nginx:1.15.3-alpine
docker tag nginx:1.15.3-alpine nginx:latest

terraform {
  required_providers {
    docker = {
      source = "kreuzwerker/docker"
      version = "2.15.0"
    }
  }
}

data "docker_registry_image" "nginx" {
  name = "nginx:latest"
}

resource "docker_image" "nginx" {
  keep_locally = true
  name          = data.docker_registry_image.nginx.name
  pull_triggers = [data.docker_registry_image.nginx.sha256_digest]
}
output "nginx_sha" {
  value = docker_image.nginx.repo_digest
}

terraform apply
<snip>
docker_image.nginx: Creation complete after 0s [id=sha256:994032453556b56420d66d53b7d8db1a74e1193165e2a070e50f533d849d9833nginx:latest]
<snip>
Outputs:

nginx_sha = "nginx@sha256:829a63ad2b1389e393e5decf5df25860347d09643c335d1dc3d91d25326d3067"

<different machine>
docker pull nginx:1.15.3-alpine
1.15.3-alpine: Pulling from library/nginx
c67f3896b22c: Pull complete
89df19b7dc5c: Pull complete
c34d3c110d67: Pull complete
9ac8b8076c62: Pull complete
Digest: sha256:829a63ad2b1389e393e5decf5df25860347d09643c335d1dc3d91d25326d3067
Status: Downloaded newer image for nginx:1.15.3-alpine
docker.io/library/nginx:1.15.3-alpine

Even though nginx 1.15 is not even close to latest, the state that we've requested does indeed exist - there is a local docker_image with the tag nginx:latest, so everything immediately succeeds - the output resource is the sha256 of 1.15. It doesn't matter if the actual latest sha updates, because the state we're expecting still exists.

If we really wanted the always-latest-image, using the following would likely net a better result:

//same as above, with this change:
resource "docker_image" "nginx" {
  keep_locally = true
  name          = "nginx@${data.docker_registry_image.nginx.sha256_digest}"
  pull_triggers = [data.docker_registry_image.nginx.sha256_digest]
}
output "nginx_sha" {
  value = docker_image.nginx.repo_digest
}

terraform apply
<snip>
docker_image.nginx: Modifications complete after 10s [id=sha256:e9ce56a96f8e0e9f75051f258a595d1257bd6bb91913b79455ea77e67e686c5cnginx@sha256:d1ce0f99f6a8acc9162c29497014716c44d126f1d41deee40a2c13e3d9d9b02a]
<snip>
Outputs:

nginx_sha = "sha256:d1ce0f99f6a8acc9162c29497014716c44d126f1d41deee40a2c13e3d9d9b02a"

<different machine>
docker pull nginx:latest
latest: Pulling from library/nginx
7d63c13d9b9b: Already exists
5cb019b641b5: Pull complete
d477de77abf8: Pull complete
c60e7d4c1c30: Pull complete
365a49996569: Pull complete
039c6e901970: Pull complete
Digest: sha256:d1ce0f99f6a8acc9162c29497014716c44d126f1d41deee40a2c13e3d9d9b02a
Status: Downloaded newer image for nginx:latest
docker.io/library/nginx:latest

So, in this case, I'm getting exactly what I expected - as of the time of run, I got the latest version of nginx downloaded. Which is likely the actual use case.

My opinion is that instead of trying to fix this behavior, you give two new, backwards compatible items and update the docs:

1. add another output that gives an easily digestible link to the image. In cases where an image is passed in by variable, trying to parse that out would be a pain - being able to say, for instance, data.docker_registry_image.nginx.pull_uri would be incredibly helpful.
2. add a tag resource that would allow tag creation, so if there is a use case for actually making sure that there is a (for example) nginx:latest on a machine and that it's kept up to date even with keep_locally set to true, you could do that. Maybe something like..

resource "docker_tag" "nginx_latest" {
   tag = "nginx:latest"
   from = docker_image.nginx.repo_digest
}