Add condition when command/shell modules should return OK (not changed)

5 views
Skip to first unread message

Pavel Brezina

unread,
Feb 22, 2023, 6:41:44 AM2/22/23
to Ansible Development
Hi,
this is about https://github.com/ansible/ansible/issues/80025 which was closed without giving a reason.

Having this feature implemented would help shell/command become really idempotent a giving correct result. It should be also quite simple to implement, given how the command module is written.

I would like to know the reason, why is Ansible team not interested in this functionality.

Thanks,
Pavel

Mark Chappell

unread,
Feb 22, 2023, 8:56:21 AM2/22/23
to Pavel Brezina, Ansible Development
Hi Pavel,

I can only speak as a community maintainer.  However, from experience, trying to pack extra logic like that into modules results in the code becoming less reliable.  The "check" parameter you've suggested is effectively duplicating the functionality of "when", while not being as flexible.

My alternative suggestion would be to split the "test" shell task (which you can set to always use `changed_when: False`) from the "update" shell task and use the pre-existing when controls to decide if the "update" shell task even needs to run.

"""
- name: Check record
  shell: record-is-present
  register: check
  changed_when: False

- name: Add record
  shell: add-record
  register: result
  when: check.rc == 0
"""

This also has the advantage that it's also very clear why things happened and you can access the output of both shell commands.

While I do sometimes miss the "check" type logic that's available with Puppet, it can also cause confusion in and of itself, and is often a sign that something would be much better handled as a custom module rather than a shell task.


Mark

--
You received this message because you are subscribed to the Google Groups "Ansible Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ansible-deve...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ansible-devel/2ec7f7c8-b337-45f1-9bfa-919b427e8dcan%40googlegroups.com.


--
Mark Chappell
Senior Principal Systems Engineer, Red Hat GmbH
Tel: +49 89 205 071 284   Mob: +49 172 7 32 16 87
Fax: +49 89 205 071 111
GnuPG: 6FEA E991 09E8 6CA2 0498  8696 D9E0 55E6 C46E A90E

Sitz: Werner von Siemens Ring 12, D-85630 Grasbrunn
Handelsregister: Amtsgericht München, HRB 153243,
Geschäftsführer: Ryan Barnhart, Charles Cachera, Michael O'Neill, Amy Ross

Pavel Březina

unread,
Feb 22, 2023, 12:05:51 PM2/22/23
to Mark Chappell, Ansible Development
On 2/22/23 14:55, Mark Chappell wrote:
> Hi Pavel,
>
> I can only speak as a community maintainer.  However, from experience,
> trying to pack extra logic like that into modules results in the code
> becoming less reliable.  The "check" parameter you've suggested is
> effectively duplicating the functionality of "when", while not being as
> flexible.
>
> My alternative suggestion would be to split the "test" shell task (which
> you can set to always use `changed_when: False`) from the "update" shell
> task and use the pre-existing when controls to decide if the "update"
> shell task even needs to run.
>
> """
> - name: Check record
>   shell: record-is-present
>   register: check
>   changed_when: False
>
> - name: Add record
>   shell: add-record
>   register: result
>   when: check.rc == 0
> """
>
> This also has the advantage that it's also very clear why things
> happened and you can access the output of both shell commands.
Hi Mark, thank you for your comment and opening a discussion.

Your solution is incomplete. If record-is-present returns non zero
value, then it fails and playbook stops. So I dare to say that it is not
that intuitive to write it out of head as it should be. Obviously, we
can add failed_when: False to fix this.

In my opinion, it does not follow Ansible philosophy. It is a workaround
for missing core functionality, not a solution. It would be a solution
if Ansible recognized changed/failed/skipped results. But it has
ok/changed/failed/skipped.

Therefore I do not want to have two tasks that report (ok, skipped) and
add lots of additional lines and logic. I would like to have single task
that can report ok when the host is in desired state and without the
need of implementing that logic myself. The skipped result should really
not reflect that system is in correct state, but rather that the task is
not applicable.

Outputs of the check command can be added to the result, I don't see any
problem in this.

>
> While I do sometimes miss the "check" type logic that's available with
> Puppet, it can also cause confusion in and of itself, and is often a
> sign that something would be much better handled as a custom module
> rather than a shell task.

Well, yes. But if you are going to write custom module for every single
command you need, then you won't get very far. Especially since such
things should also be shared across projects and maintained. That is a
lot of hustle for single line straight forward commands.

> Mark
>
> On Wed, 22 Feb 2023 at 12:41, Pavel Brezina <pbre...@redhat.com
> <mailto:pbre...@redhat.com>> wrote:
>
> Hi,
> this is about https://github.com/ansible/ansible/issues/80025
> <https://github.com/ansible/ansible/issues/80025> which was closed
> without giving a reason.
>
> Having this feature implemented would help shell/command become
> really idempotent a giving correct result. It should be also quite
> simple to implement, given how the command module is written.
>
> I would like to know the reason, why is Ansible team not interested
> in this functionality.
>
> Thanks,
> Pavel
>
> --
> You received this message because you are subscribed to the Google
> Groups "Ansible Development" group.
> To unsubscribe from this group and stop receiving emails from it,
> send an email to ansible-deve...@googlegroups.com
> <mailto:ansible-deve...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/ansible-devel/2ec7f7c8-b337-45f1-9bfa-919b427e8dcan%40googlegroups.com <https://groups.google.com/d/msgid/ansible-devel/2ec7f7c8-b337-45f1-9bfa-919b427e8dcan%40googlegroups.com?utm_medium=email&utm_source=footer>.
>
>
>
> --
> Mark Chappell
> Senior Principal Systems Engineer, Red Hat GmbH
> Tel: +49 89 205 071 284   Mob: +49 172 7 32 16 87
> Fax: +49 89 205 071 111
> GnuPG: 6FEA E991 09E8 6CA2 0498  8696 D9E0 55E6 C46E A90E
>
> Red Hat GmbH <https://www.redhat.com/de/global/dach>,
Reply all
Reply to author
Forward
0 new messages