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

Add rule warning about use of run_once with strategy: free #1026

Closed
cboylan opened this issue Sep 2, 2020 · 3 comments · Fixed by #2626
Closed

Add rule warning about use of run_once with strategy: free #1026

cboylan opened this issue Sep 2, 2020 · 3 comments · Fixed by #2626
Assignees
Labels
feature good first issue Bugs ideal for the first time, good for newcomers contributors
Milestone

Comments

@cboylan
Copy link

cboylan commented Sep 2, 2020

Summary

It would be useful for ansible-lint to have a rule that warns against the use of run_once with strategy: free. Ansible ignores run_once with the free strategy which means your tasks are run many times, once for each valid inventory host. Ansible itself does warn against this as well but doesn't fail directly. Being able to catch this early would be great.

Issue Type
  • Feature Idea
@cboylan cboylan added priority/medium new Triage required labels Sep 2, 2020
@ssbarnea ssbarnea self-assigned this Sep 2, 2020
@ssbarnea ssbarnea added this to the 4.4.0 milestone Sep 2, 2020
@ssbarnea
Copy link
Member

ssbarnea commented Sep 2, 2020

ansible 2.10 does produce runtime warning about use of run_once when used with strategy: free but it does not produce any warnings when doing syntax check or when running without free strategy.

This behavior makes the run_once a potential production bomb, as users may change strategy when deploying in production, in some cases when they use tower/awx this can also be changed by the operator.

I am inclined to consider run_once an unpredictable option to use and always warn about its presences, regardless the strategy (especially as it cannot reliably be inferred during linting). I would suggest manually whitelisting each usage, unless someone comes with a better recommendation.

@ansiblejunky
Copy link
Contributor

ansiblejunky commented Sep 2, 2020

Just for info, providing what ansible 2.9.x shows as far as the warning message:

playbook:

---
- hosts: all
  strategy: free
  gather_facts: false

  tasks:

  - debug:
      msg: "Test"
    run_once: True

output from ansible-playbook -i localhost, test.yml:

PLAY [all] ***********************************************************************************************************************
[WARNING]: Using run_once with the free strategy is not currently supported. This task will still be executed for every host in
the inventory list.

TASK [debug] *********************************************************************************************************************
ok: [localhost] => {
    "msg": "Test"
}

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

Perhaps the rule should make the following recommendation to the user... this is in the following docs where it states:

When used together with “serial”, tasks marked as “run_once” will be run on one host in each serial batch. If it’s crucial that the task is run only once regardless of “serial” mode, use when: inventory_hostname == ansible_play_hosts_all[0] construct.

I would be ok with a rule that gives a warning and leaving it up to the individual to make it an error or leave it as warning. However this does not feel like a deprecation issue, but rather just not supported in the specific situation with strategy=free.

Btw, this is another rule where it would be beneficial for lint rules to know a bit more about the context (in this case if the task is within a Play where strategy=free). However, that's a separate enhancement.

@ssbarnea ssbarnea added feature and removed new Triage required labels Dec 31, 2020
@webknjaz webknjaz added the new Triage required label Sep 7, 2021
@bluikko
Copy link
Contributor

bluikko commented Jan 2, 2022

use when: inventory_hostname == ansible_play_hosts_all[0] construct

If the rule proposed in the OP will be added, I for one would not want see the above construct as a remedy recommendation. It may work in some cases but in many cases (especially with the free strategy, where some similar construct is the only option) it is not equivalent at all and is a bad advise.

@ssbarnea ssbarnea removed the new Triage required label Jan 10, 2022
@ssbarnea ssbarnea removed their assignment Jan 10, 2022
@ssbarnea ssbarnea added the good first issue Bugs ideal for the first time, good for newcomers contributors label Jan 10, 2022
@audgirka audgirka self-assigned this Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature good first issue Bugs ideal for the first time, good for newcomers contributors
Projects
None yet
6 participants