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

fix(podman_image): skip empty volume items #833

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

bmenant
Copy link
Contributor

@bmenant bmenant commented Aug 29, 2024

Skip empty volume items with podman image build:

- containers.podman.podman_image:
    name: myimage
    path: /tmp/myimage
    build:
      volume:
        - "{{ '/tmp/whatever:/mnt/whatever' if false }}" # results in "" (empty string), which leads to podman error
    state: build

This PR adds a check in podman_image module, just like it is already done over here: https://github.com/containers/ansible-podman-collections/blob/master/plugins/module_utils/podman/podman_container_lib.py#L859-L863

@bmenant bmenant force-pushed the fix/empty-volume-items branch from e318bf0 to f6aad07 Compare August 29, 2024 17:59
@sshnaidm
Copy link
Member

sshnaidm commented Sep 2, 2024

Well, the condition is legit, but putting an empty string there is definitely wrong. If you want to exclude specific volumes it's better to prepare "volumes" variable before.

@bmenant
Copy link
Contributor Author

bmenant commented Sep 3, 2024

Well, I understand it’s wrong, but from an Ansible playbook point of view, would it hurt much if it was a little loose? It sometimes helps to write tasks more naturally IMHO.
At least, it should be consistent with podman_container module:

---
- name: podman volume opts playbook
  hosts: localhost
  connection: local
  gather_facts: false

  vars:
    some_condition: false                                               # container | image (build)
    volopts:                                                            # ----------|--------------
      - "{{ [ 'host_dir:container_dir' ] if some_condition else [] }}"  # pass      | pass
      - "{{ [ 'host_dir:container_dir' ] if some_condition }}"          # pass      | fail
      -                                                                 #           |
        - "{{ 'host_dir:container_dir' if some_condition }}"            # pass      | fail 

  tasks:
    - name: podman_container volume
      loop: "{{ volopts }}"
      containers.podman.podman_container:
        volume: "{{ item }}"
        image: "quay.io/podman/hello"
        name: test-podman-container-volume
        recreate: true
        state: created 
    - name: podman_image volume
      loop: "{{ volopts }}"
      containers.podman.podman_image:
        build:
          container_file: "FROM scratch"
          volume: "{{ item }}"
        force: true
        name: test-podman-image-volume
        path: "{{ playbook_dir }}"
        state: build
PLAY [podman volume opts] ******************************************************************************************************************************************************************************************************

TASK [podman_container volume] *************************************************************************************************************************************************************************************************
changed: [localhost] => (item=[])
changed: [localhost] => (item=)
changed: [localhost] => (item=[''])

TASK [podman_image volume] *****************************************************************************************************************************************************************************************************
changed: [localhost] => (item=[])
failed: [localhost] (item=) => {"ansible_loop_var": "item", "changed": false, "item": "", "msg": "Failed to build image test-podman-image-volume:latest:  Error: creating build executor: incorrect volume format \"\", should be host-dir:ctr-dir[:option]\n"}
failed: [localhost] (item=['']) => {"ansible_loop_var": "item", "changed": false, "item": [""], "msg": "Failed to build image test-podman-image-volume:latest:  Error: creating build executor: incorrect volume format \"\", should be host-dir:ctr-dir[:option]\n"}

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

@sshnaidm
Copy link
Member

sshnaidm commented Sep 3, 2024

In your case it's possible either to select non-empty in volopts:

-   name: podman_container volume
    loop: "{{ volopts | select('length') | list }}"
    containers.podman.podman_container:
      volume: "{{ item }}"
      image: "quay.io/podman/hello"
      name: test-podman-container-volume
      recreate: true
      state: created 

or to add when condition:

-   name: podman_container volume
    loop: "{{ volopts }}"
    when: item | length > 0
    containers.podman.podman_container:
      volume: "{{ item }}"
      image: "quay.io/podman/hello"
      name: test-podman-container-volume
      recreate: true
      state: created

I'll merge it, but there are multiple ways to do it right instead.

@sshnaidm sshnaidm merged commit 4973c86 into containers:master Sep 3, 2024
10 checks passed
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.

2 participants