freifunkh / ansible

Here we store all Ansible roles and configs used for Freifunk Hannover.
MIT License
7 stars 3 forks source link

Yanic role fails #171

Closed 1977er closed 3 years ago

1977er commented 3 years ago

Maybe related to check_mode.

ansible-playbook playbooks/harvester.yml -t yanic -CD

throws

TASK [ffh.yanic : Update yanic to "aff906d734cdff279c7c7cfd17ccc54f70c6c45f"] ****************************************************************************************************
fatal: [harvester]: FAILED! => {}

MSG:

The conditional check 'fresh_install.changed or commit_search.rc==128 or patch_sync.changed' failed. The error was: error while evaluating conditional (fresh_install.changed or commit_search.rc==128 or patch_sync.changed): 'dict object' has no attribute 'rc'

The error appears to be in '/home/okrueger/freifunk/ansible/roles/ffh.yanic/tasks/main.yml': line 70, column 3, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

- name: Update yanic to "{{ yanic_commit }}"
  ^ here
We could be wrong, but this one looks like it might be an issue with
missing quotes. Always quote template expression brackets when they
start a value. For instance:

    with_items:
      - {{ foo }}

Should be written as:

    with_items:
      - "{{ foo }}"
AiyionPrime commented 3 years ago

Well, the command module always provides the entry rc. 1 And that should always be accessible from other tasks. 2

I'm not sure what you expect from our tasks; dry run can not really execute the command, as it would change the system and there's no rollback mechanism in ansible; therefore it doesn't. Which means, ansible has no clue how the rc would look like.

I'm no big fan of dry run; but I suppose one could circumvent this error with default values, which would make dry-run behave differently from a real run, where .rc exists. There's no bug, this cannot be fixed, but only circumvented.

The git module does not support commit search, which would work as a fix and is a ludicrous amount of effort to get rid of this.

Good luck deciding on this...

1: https://docs.ansible.com/ansible/latest/collections/ansible/builtin/command_module.html#return-rc 2: https://www.decodingdevops.com/ansible-when-condition-examples-with-rc/

AiyionPrime commented 3 years ago

In my opinion this should only be used sparsely; I just skimmed through the manpage: There's no option for git-cat-file to 'change' or 'edit' anything on disk.

Since ansible there's an option for this in ansible, called check_mode, which allows to force the mode of a task regardless of specified options. Therefore the task could contain check_mode: no to always execute the task, which is only safe as git-cat-file does not write to the system, except to git reflog.

https://docs.ansible.com/ansible/2.3/playbooks_checkmode.html#enabling-or-disabling-check-mode-for-tasks

I'll test this in a minute...

1977er commented 3 years ago

Yep, sorry for letting you research that checkmode feature on your own. Thats exactly what I meant and what we already use elsewhere.

My expectation is an uninterrupted (checkmode) run. I'm aware that command/shell calls cannot be faked.

AiyionPrime commented 3 years ago

In that case I'll merge #172.