Make image archive/save idempotent, using image id and repo tags as keys

[iamjpotts]

SUMMARY

The docker_image component supports saving images to a tar file, similar to docker image save command. When a docker_image archive task is run repeatedly, it recreates the file every time.

After this change, the task will compare the Docker image ID of the archive file (if any) vs the image stored in the Docker Engine, and if they are the same, the archive file is left untouched.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

docker_image used with archive_path

ADDITIONAL INFORMATION

Consider a scenario with two servers (localhost and target), where an Ansible playbook synchronizes images to a target server.

1. Playbook run against localhost uses docker_load with archive_path to export an image to a tar
2. Playbook run using ansible.builtin.file synchronizes the directory with the tar file on localhost with a target server directory
3. Playbook run using docker_load imports tar file on target server to target server's docker engine

Currently, this workflow regenerates the tar files over and over again, resending them to the target server each time.

After this change, Step 1 only exports the tar files once, until the source image changes, preventing the network copy from localhost to target on subsequent runs.

Export playbook

- hosts: localhost
  connection: local
  vars:
    docker_images:
      - nginx:1.20

  tasks:

    - name: Docker image export directory 
      ansible.builtin.file:
        path: ~/.docker-image-cache
        state: directory
        mode: '0755'

    - name: Export Docker Images
      community.docker.docker_image:
        name: nginx:1.20
        source: local
        archive_path: ~/.docker-image-cache/nginx-1.20.tar
        state: present

Import playbook

- hosts: target-server
  tasks:

    - name: Sync cache directory
      ansible.builtin.copy:
        src: ~/.docker-image-cache
        dest: ~/

    - name: Import Docker Images
      community.docker.docker_image:
        name: nginx:1.20
        source: load
        load_path: ~/.docker-image-cache/nginx-1.20.tar
        state: present

Output, without this PR, run 1 of 2.

PLAY [localhost] **************************************************************************************************************************************************************************************************************************************************************

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

TASK [Docker image export directory] ******************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Export Docker Images] ***************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

PLAY RECAP ********************************************************************************************************************************************************************************************************************************************************************
localhost                  : ok=3    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

Output, without this PR, run 2 of 2. Shows changed again

PLAY [localhost] **************************************************************************************************************************************************************************************************************************************************************

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

TASK [Docker image export directory] ******************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Export Docker Images] ***************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

PLAY RECAP ********************************************************************************************************************************************************************************************************************************************************************
localhost                  : ok=3    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0 

Output, with this PR, run 1 of 2.

PLAY [localhost] **************************************************************************************************************************************************************************************************************************************************************

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

TASK [Docker image export directory] ******************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Export Docker Images] ***************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

PLAY RECAP ********************************************************************************************************************************************************************************************************************************************************************
localhost                  : ok=3    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

Output, with this PR, run 2 of 2. Shows ok on second run.

PLAY [localhost] **************************************************************************************************************************************************************************************************************************************************************

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

TASK [Docker image export directory] ******************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Export Docker Images] ***************************************************************************************************************************************************************************************************************************************************
ok: [localhost]

PLAY RECAP ********************************************************************************************************************************************************************************************************************************************************************
localhost                  : ok=3    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0 

[iamjpotts]

@felixfontein

I've removed the 'do not merge' label.

I attempted to add two .tar files for use with unit tests but they got flagged by the license checker, since they are not text files and can't be checked for a copyright header.

I worked around that by base64-encoding the files and including them as a string literal.

The files were generated by me using docker image save - is this approach suitable for the licensing issue, or should another be considered?

[felixfontein]

I attempted to add two .tar files for use with unit tests but they got flagged by the license checker, since they are not text files and can't be checked for a copyright header.

I worked around that by base64-encoding the files and including them as a string literal.

The files were generated by me using docker image save - is this approach suitable for the licensing issue, or should another be considered?

I would really avoid adding binary blobs to this repository. How about creating these files during the integration tests (by running docker image save) and then using the freshly generated files?

[iamjpotts]

One of the two tar files is very small (hello world is about 3kb). The other one is larger (about 3 mb) but I can find a smaller substitute, for example creating an image from scratch and adding a text file to it (e.g a copyright notice).

Would two small files be adequate? Is there a way to exclude them from release/distribution packaging?

The unit tests are easier and faster to run, and in principle, I think these tests belong there. They test parsing a file, as opposed to interacting with a service.

Regenerating them each time also introduces variability into the test input; it's nice to have the same exact input across all versions of Python being tested.

[felixfontein]

Would two small files be adequate? Is there a way to exclude them from release/distribution packaging?

I would really avoid adding any kind of binary file, if not absolutely necessary.

The unit tests are easier and faster to run, and in principle, I think these tests belong there. They test parsing a file, as opposed to interacting with a service.

They are faster to run, but they also are a big problem, since you are including copyrighted contents into this repository. Both the hello-world executable and busybox come with copyright and legal conditions attached.

Regenerating them each time also introduces variability into the test input; it's nice to have the same exact input across all versions of Python being tested.

I don't think this will be a problem.

[felixfontein]

Please add a changelog fragment. Thanks.

[iamjpotts]

Changes:

• Added change log fragment
• Replaced committed base-64 encoded tar files with runtime-generated stubs with same structure
• Fixed nested exception handling and added tests for causes
• Replaced action entries with log.debug statements (self.log calls an empty function with pass inside)
• Replaced __exit__ attribute conditionals with close() calls
• Removed old_image_id result
• Moved exception and function to their own new module, module_utils/image_archive.py

[felixfontein]

You have a very strange commit list, I guess you should probably rebase to get rid of all the stray changes which do not belong into this PR.

[iamjpotts]

Changes:

• Fixed docstring syntax of existing and new docstrings
• Added docstring to ImageManager.__init__ to specify type for Docker client wrapper
• Replaced self.log with self.client.module.debug
• Declare alternate image name in same file as test tasks

[iamjpotts]

Made one more docstring change to tickle the build after a transient network related failure.

Ready for review.

[iamjpotts]

I'm now getting build failures which are timeouts waiting for the temporary docker registry to start.

For example

Made a whitespace change to tickle the build and it failed the same way (three times so far).

I don't think this is related to accepting your import change suggestion, but I rebased right away.

Looking at the CI log, I suspect the registry container is failing to stay running (the previous successful task is a start request), but there isn't any log output from the registry container in the CI log to see why.

[felixfontein]

This might be caused by ansible/ansible#78550. Simply ignore that failure for now.

[felixfontein]

Now there's a conflict due to the other PR being merged. After rebasing with the latest main branch all tests should also pass.

[iamjpotts]

Now there's a conflict due to the other PR being merged. After rebasing with the latest main branch all tests should also pass.

I'm updating the docstring style in this PR also, it's not committed yet. I'll drop a message here once the build is green and I've looked over the code one last time.

[iamjpotts]

Build is green.

I did miss one triple-quote to single quote in a test_docker_image.py

If that doesn't concern you, I don't see anything else to change.

[felixfontein]

@iamjpotts thanks a lot for your contribution!