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

Trigger a new image build when we detect that the Containerfile has changed. #811

Conversation

SkrrtBacharach
Copy link
Contributor

Fixes #646

@sshnaidm
Copy link
Member

@SkrrtBacharach thanks for contribution! Let me review it thoroughly. Can you meanwhile please sign your commit with git commit -s? It's required by containers org.

@SkrrtBacharach
Copy link
Contributor Author

@SkrrtBacharach thanks for contribution! Let me review it thoroughly. Can you meanwhile please sign your commit with git commit -s? It's required by containers org.

@sshnaidm Do I have to sign with my real name/email? Would love to sign under an alias if that's possible

@SkrrtBacharach SkrrtBacharach force-pushed the idempotency-with-containerfile-hashing branch from 448363a to 589f1d4 Compare August 14, 2024 21:59
@sshnaidm
Copy link
Member

@SkrrtBacharach thanks for contribution! Let me review it thoroughly. Can you meanwhile please sign your commit with git commit -s? It's required by containers org.

@sshnaidm Do I have to sign with my real name/email? Would love to sign under an alias if that's possible

No, you don't have of course, it can be any. If you can't, it's fine, I can force the job to pass :)

@SkrrtBacharach SkrrtBacharach force-pushed the idempotency-with-containerfile-hashing branch 4 times, most recently from ccb9c48 to 6716f66 Compare August 20, 2024 19:24
@SkrrtBacharach
Copy link
Contributor Author

@SkrrtBacharach thanks for contribution! Let me review it thoroughly. Can you meanwhile please sign your commit with git commit -s? It's required by containers org.

@sshnaidm Do I have to sign with my real name/email? Would love to sign under an alias if that's possible

No, you don't have of course, it can be any. If you can't, it's fine, I can force the job to pass :)

Done

@SkrrtBacharach SkrrtBacharach force-pushed the idempotency-with-containerfile-hashing branch 3 times, most recently from b4bbc8c to f455b78 Compare September 7, 2024 12:41
@SkrrtBacharach SkrrtBacharach force-pushed the idempotency-with-containerfile-hashing branch from f455b78 to a651c42 Compare September 9, 2024 21:45
@SkrrtBacharach
Copy link
Contributor Author

@sshnaidm please let me know if you have questions/comments/concerns with my PR. Thanks for your help with this.

Copy link
Member

@sshnaidm sshnaidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @SkrrtBacharach for the contribution and sorry for a such long delay. My nits are about Python style only.


args_containerfile_hash = ""

context_has_containerfile = self.path and self._find_containerfile_from_context() != ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
context_has_containerfile = self.path and self._find_containerfile_from_context() != ""
context_has_containerfile = self.path and self._find_containerfile_from_context()

if image:
digest_before = image[0].get('Digest', image[0].get('digest'))
labels = image[0].get('Labels') or {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
labels = image[0].get('Labels') or {}
labels = image[0].get('Labels', {})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consistently get a test failure when I make this change

TASK [podman_image : Pull image from docker.io with short url again] ***********
task path: /home/gw/.ansible/collections/ansible_collections/containers/podman/tests/output/.tmp/integration/podman_image-fzafk80g-ÅÑŚÌβŁÈ/tests/integration/targets/podman_image/tasks/main.yml:34
Using module file /home/gw/.ansible/collections/ansible_collections/containers/podman/plugins/modules/podman_image.py
Pipelining is enabled.
<testhost> ESTABLISH LOCAL CONNECTION FOR USER: gw
<testhost> EXEC /bin/sh -c '/home/gw/venvs/molecule-plugins/bin/python3 && sleep 0'
The full traceback is:
Traceback (most recent call last):
  File "<stdin>", line 121, in <module>
  File "<stdin>", line 113, in _ansiballz_main
  File "<stdin>", line 61, in invoke_module
  File "<frozen runpy>", line 226, in run_module
  File "<frozen runpy>", line 98, in _run_module_code
  File "<frozen runpy>", line 88, in _run_code
  File "/tmp/ansible_containers.podman.podman_image_payload_zbm4x3jp/ansible_containers.podman.podman_image_payload.zip/ansible_collections/containers/podman/plugins/modules/podman_image.py", line 1033, in <module>
  File "/tmp/ansible_containers.podman.podman_image_payload_zbm4x3jp/ansible_containers.podman.podman_image_payload.zip/ansible_collections/containers/podman/plugins/modules/podman_image.py", line 1028, in main
  File "/tmp/ansible_containers.podman.podman_image_payload_zbm4x3jp/ansible_containers.podman.podman_image_payload.zip/ansible_collections/containers/podman/plugins/modules/podman_image.py", line 462, in __init__
  File "/tmp/ansible_containers.podman.podman_image_payload_zbm4x3jp/ansible_containers.podman.podman_image_payload.zip/ansible_collections/containers/podman/plugins/modules/podman_image.py", line 579, in present
TypeError: argument of type 'NoneType' is not iterable
fatal: [testhost]: FAILED! => {
    "changed": false,
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 121, in <module>\n  File \"<stdin>\", line 113, in _ansiballz_main\n  File \"<stdin>\", line 61, in invoke_module\n  File \"<frozen runpy>\", line 226, in run_module\n  File \"<frozen runpy>\", line 98, in _run_module_code\n  File \"<frozen runpy>\", line 88, in _run_code\n  File \"/tmp/ansible_containers.podman.podman_image_payload_zbm4x3jp/ansible_containers.podman.podman_image_payload.zip/ansible_collections/containers/podman/plugins/modules/podman_image.py\", line 1033, in <module>\n  File \"/tmp/ansible_containers.podman.podman_image_payload_zbm4x3jp/ansible_containers.podman.podman_image_payload.zip/ansible_collections/containers/podman/plugins/modules/podman_image.py\", line 1028, in main\n  File \"/tmp/ansible_containers.podman.podman_image_payload_zbm4x3jp/ansible_containers.podman.podman_image_payload.zip/ansible_collections/containers/podman/plugins/modules/podman_image.py\", line 462, in __init__\n  File \"/tmp/ansible_containers.podman.podman_image_payload_zbm4x3jp/ansible_containers.podman.podman_image_payload.zip/ansible_collections/containers/podman/plugins/modules/podman_image.py\", line 579, in present\nTypeError: argument of type 'NoneType' is not iterable\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sshnaidm I couldn't figure out why this was giving me an error.

plugins/modules/podman_image.py Outdated Show resolved Hide resolved
plugins/modules/podman_image.py Outdated Show resolved Hide resolved
plugins/modules/podman_image.py Outdated Show resolved Hide resolved
@SkrrtBacharach SkrrtBacharach force-pushed the idempotency-with-containerfile-hashing branch from b248cbe to ec02a05 Compare September 23, 2024 19:03
@sshnaidm
Copy link
Member

Thanks a lot!

@sshnaidm sshnaidm merged commit 58edc41 into containers:master Sep 23, 2024
10 checks passed
@SkrrtBacharach
Copy link
Contributor Author

SkrrtBacharach commented Sep 23, 2024

Thanks a lot!

@sshnaidm There was one change I didn't make, which was giving me a test failure. It's here: #811 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman_image don't rebuilt the image if I set a new containerfile
2 participants