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

docker_container with strict labels comparison ignores base image labels #314

Closed
RoGryza opened this issue Mar 21, 2022 · 6 comments · Fixed by #370
Closed

docker_container with strict labels comparison ignores base image labels #314

RoGryza opened this issue Mar 21, 2022 · 6 comments · Fixed by #370
Labels
bug Something isn't working docker-plain plain Docker (no swarm, no compose, no stack)

Comments

@RoGryza
Copy link
Contributor

RoGryza commented Mar 21, 2022

SUMMARY

When using docker_container to run a container with non-empty labels, if its base image also defines labels and labels comparison is set to strict the container gets changed.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

docker_container

ANSIBLE VERSION
ansible [core 2.12.3]
  config file = /home/rogryza/code/GitHub/RoGryza/ansible-docker-labels-strict/ansible.cfg
  configured module search path = ['/home/rogryza/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/rogryza/code/GitHub/RoGryza/ansible-docker-labels-strict/.direnv/python-3.10.2/lib/python3.10/site-packages/ansible
  ansible collection location = /home/rogryza/code/GitHub/RoGryza/ansible-docker-labels-strict
  executable location = /home/rogryza/code/GitHub/RoGryza/ansible-docker-labels-strict/.direnv/python-3.10.2/bin/ansible
  python version = 3.10.2 (main, Jan 15 2022, 19:56:27) [GCC 11.1.0]
  jinja version = 3.0.3
  libyaml = True
COLLECTION VERSION
# /home/rogryza/code/GitHub/RoGryza/ansible-docker-labels-strict/ansible_collections
Collection       Version
---------------- -------
community.docker 2.3.0  

# /home/rogryza/code/GitHub/RoGryza/ansible-docker-labels-strict/.direnv/python-3.10.2/lib/python3.10/site-packages/ansible_collections
Collection       Version
---------------- -------
community.docker 2.2.1  
CONFIGURATION
COLLECTIONS_PATHS(/home/rogryza/code/GitHub/RoGryza/ansible-docker-labels-strict/ansible.cfg) = ['/home/rogryza/code/GitHub/RoGryza/ansible-docker-labels-strict']
OS / ENVIRONMENT

Observed in archlinux and Ubuntu 21.04:

$ uname -r
5.16.15-arch1-1
$ docker --version
Docker version 20.10.13, build a224086349
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 20.04.4 LTS
Release:        20.04
Codename:       focal
$ docker --version
Docker version 20.10.7, build 20.10.7-0ubuntu5~20.04.2
STEPS TO REPRODUCE

Create a base image with from a Dockerfile which sets labels through the LABEL instruction.
Run the docker_container task twice using this image and the same parameters, with some labels set and comparisons.labels set to strict.

There's an example repo here.
Dockerfile:

FROM alpine:3.14.4

LABEL foo=bar

CMD sleep infinity

playbook:

- name: Test strict labels
  hosts: localhost
  connection: local
  tasks:
  - name: Create dummy image
    community.docker.docker_image:
      name: dummy
      build:
        path: .
      source: build
  - name: Run dummy image
    community.docker.docker_container:
      name: dummy
      image: dummy
      state: started
      comparisons:
        labels: strict
      labels:
        dummy: "something"
  - name: Run dummy image again - should be no diff
    community.docker.docker_container:
      name: dummy
      image: dummy
      state: started
      comparisons:
        labels: strict
      labels:
        dummy: "something"
  - name: Cleanup container
    community.docker.docker_container:
      name: dummy 
      state: absent
  - name: Cleanup image
    community.docker.docker_image:
      name: dummy 
      state: absent
$ ansible-playbook --diff strict_labels.yaml
EXPECTED RESULTS

The container should not be changed in the second docker_container task:

PLAY [Test strict labels] *************************************************************************************************************************

...

TASK [Run dummy image again - should be no diff] **************************************************************************************************
ok: [localhost]

...
ACTUAL RESULTS
PLAY [Test strict labels] ******************************************************

TASK [Gathering Facts] *********************************************************
ok: [localhost]

TASK [Create dummy image] ******************************************************
changed: [localhost]

TASK [Run dummy image] *********************************************************
--- before
+++ after
@@ -1,4 +1,4 @@
 {
-    "exists": false,
-    "running": false
+    "exists": true,
+    "running": true
 }

changed: [localhost]

TASK [Run dummy image again - should be no diff] *******************************
--- before
+++ after
@@ -1,7 +1,6 @@
 {
     "labels": {
-        "dummy": "something",
-        "foo": "bar"
+        "dummy": "something"
     },
     "running": true
 }

changed: [localhost]

TASK [Cleanup container] *******************************************************
--- before
+++ after
@@ -1,4 +1,4 @@
 {
-    "exists": true,
-    "running": true
+    "exists": false,
+    "running": false
 }

changed: [localhost]

TASK [Cleanup image] ***********************************************************
changed: [localhost]

PLAY RECAP *********************************************************************
localhost                  : ok=6    changed=5    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
@RoGryza
Copy link
Contributor Author

RoGryza commented Mar 21, 2022

I'd be happy to submit a PR if this is actually considered a bug. It's surprising behaviour to me, I know I can use allow_more_present to compare labels but I expected the task to be idempotent

@felixfontein felixfontein added bug Something isn't working docker-plain plain Docker (no swarm, no compose, no stack) labels Mar 22, 2022
@felixfontein
Copy link
Collaborator

Hmm, this is somewhat tricky. I think it is a bug, but I'm not sure what the solution should be. The main problem here is that labels present in the image cannot be removed, only overwritten.

I can think of two behaviors (when strict comparison is used):

  1. Ignore labels present in the image when they should be removed;
  2. Fail when such labels are present and should be removed.

Maybe we need to make this behavior configurable?

@RoGryza
Copy link
Contributor Author

RoGryza commented Apr 1, 2022

Is there an use case for option 2? I think it's reasonable to want to overwrite labels, personally I'd only use the first option - ignore labels present in the image when they should be removed, though we should also check if the value present in the container matches the image as well. What do you think?

@felixfontein
Copy link
Collaborator

Is there an use case for option 2?

If you want to make sure that a label isn't there, or if it is there does not have a specific value, having 2. is very useful. If suddenly the image adds a label you don't want to have with a value you don't want to, you find out rather quickly since the module won't create the container. Then you can consider whether you want to mask the label's value by another one you explicitly supply, or (try to) fix the image, or do something else. Just ignoring that there are extra labels could be fatal depending on the label and how the labels are used.

@RoGryza
Copy link
Contributor Author

RoGryza commented Apr 2, 2022

That makes sense. Since new labels in the image can mess things up I think the default behaviour should be failing when there are labels that can't be removed, if you're OK with adding more configuration we could then add some config like ignore_image_labels?

@felixfontein
Copy link
Collaborator

The default behavior should be ignore, since failing would be a breaking change (that cannot be merged). I would probably call the option something like image_label_mismatch with string choices (instead of boolean values) ignore and fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docker-plain plain Docker (no swarm, no compose, no stack)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants