containers / ansible-podman-collections

Repository for Ansible content that can include playbooks, roles, modules, and plugins for use with the Podman tool
GNU General Public License v3.0
260 stars 141 forks source link

`podman_image` don't rebuilt the image if I set a new containerfile #646

Open CyberFox001 opened 11 months ago

CyberFox001 commented 11 months ago

/kind bug

Description

If I:

The image is not built again.

But the requested image is different because the Containerfile is different. So the image should be rebuild in this case.

Steps to reproduce the issue:

  1. Create a playbook and use podman_image with state to build and build.file to a Containerfile

  2. Run the playbook

  3. Modify the playbook to set podman_image.build.file to another Containerfile

  4. Run the playbook again

Describe the results you received:

The image is not build again.

Describe the results you expected:

The image should be build again.

Version of the containers.podman collection:

1.10.2

Output of ansible --version:

ansible [core 2.15.3]

Output of podman version:

4.3.1

Package info (e.g. output of rpm -q podman or apt list podman):

podman/stable,now 4.3.1+ds1-8+b1 amd64  [installé]
sshnaidm commented 11 months ago

I think it's impossible to know if it's a different Containerfile or same Containerfile with different content, or just same. So it's better to force building if any Containerfile is set. You can use force option. I can set it as a default if Containerfile is set, but need to think if it doesn't break others.

CyberFox001 commented 11 months ago

Yes. I have inspected an image created by podman_image and I don't see any info that can be used to know if we use 2 different Containerfile.

Maybe we can add a new parameter to podman_image, example podman_image.build.record_containerfile_hash, that will save the hash of the used Containerfile to the built image. Maybe as a label.

So, next time we run podman_image, it can compare the hash saved in the image with the one from the given Containerfile. If it's different, the image is rebuilt.

sshnaidm commented 11 months ago

That's an interesting idea! I think we can use "annotations" for that? It's exactly for image metadata info. I think we can add to annotations every Containerfile/Dockerfile hash and if there is an existing hash, then compare it. If no existing hash - then go by the standard procedure. Would you like to create a patch?

CyberFox001 commented 11 months ago

I would like to. :smiley:

But for now I don't have enough time because of school.

sshnaidm commented 2 months ago

@CyberFox001 are you still interested to create a patch or can I take it?

CyberFox001 commented 2 months ago

@sshnaidm Sadly, I'm working on my diploma work right now. And I will be very busy until end of September.

You are free to take it.

SkrrtBacharach commented 1 month ago

So I ran into this issue a while ago, and I've started working on a fix. I have a question though.

Since annotations only work with OCI format images, would it not be better to use a label instead? I've tested with a label, and it seems to work with both formats of image, which seems better.

Here's my branch, it's not finished and is a bit rough still: https://github.com/SkrrtBacharach/ansible-podman-collections/tree/idempotency-with-containerfile-hashing

SkrrtBacharach commented 1 month ago

My PR is code-complete and the tests pass: https://github.com/containers/ansible-podman-collections/pull/811/files

CyberFox001 commented 1 week ago

So I ran into this issue a while ago, and I've started working on a fix. I have a question though.

Since annotations only work with OCI format images, would it not be better to use a label instead? I've tested with a label, and it seems to work with both formats of image, which seems better.

Here's my branch, it's not finished and is a bit rough still: https://github.com/SkrrtBacharach/ansible-podman-collections/tree/idempotency-with-containerfile-hashing

I would say that if label work for the 2 images formats, then it a good solution. But I'm not the main dev.