Skip to content

Workaound for ansible module executed twice issue of pytest-ansible#9530

Merged
wangxin merged 1 commit intosonic-net:masterfrom
wangxin:fix-dup-run
Aug 18, 2023
Merged

Workaound for ansible module executed twice issue of pytest-ansible#9530
wangxin merged 1 commit intosonic-net:masterfrom
wangxin:fix-dup-run

Conversation

@wangxin
Copy link
Copy Markdown
Collaborator

@wangxin wangxin commented Aug 17, 2023

Description of PR

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012
  • 202205

Approach

What is the motivation for this PR?

This is to workaround pytest-ansible plugin issue:
Even no extra-inventory is specified, extra_inventory_manager will still be initialized.
ansible/pytest-ansible#135

As of today, pytest-ansible supports host manager and module dispatcher v29, v212, v213. While initializing the pytest-ansible plugin, it tries to collect default ansible configurations and command line options. These options are used for creating host manager. When no extra_inventory is specified, the options passed to host manager will include extra_inventory=None. However, the host manager will still try to create extra_inventory_manager with None as the inventory source. This will cause the module dispatcher to run the module on hosts matching host pattern in both inventory and extra inventory. This would cause an ansible module executed twice on the same host. In case we wish to use the shell module to run command like "rm <some_file>" to delete a file. The second run would fail because the file has been deleted in the first run. For more details, please refer to the Github issue mentioned above.

This change is only required for python3. Currently master and 202305 branch are using python3. For older branches, they are still using python2 and an older version of pytest-ansible which does not have the bug.

How did you do it?

The fix is to overwrite the pytest-ansible plugin's initialize method and remove the extra_inventory option if it is None. This will prevent the host manager from creating extra_inventory_manager with None as the inventory source.

How did you verify/test it?

Tried run a few tests with pytest-ansible 3.2.1 and 4.0.0.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

This is to workaround pytest-ansible plugin issue:
    Even no extra-inventory is specified, extra_inventory_manager will still be initialized.
    ansible/pytest-ansible#135

As of today, pytest-ansible supports host manager and module dispatcher v29, v212, v213.
While initializing the pytest-ansible plugin, it tries to collect default ansible configurations and
command line options. These options are used for creating host manager. When no extra_inventory is specified,
the options passed to host manager will include extra_inventory=None. However, the host manager will still
try to create extra_inventory_manager with None as the inventory source. This will cause the module dispatcher
to run the module on hosts matching host pattern in both inventory and extra inventory. This would cause an
ansible module executed twice on the same host. In case we wish to use the shell module to run command like
"rm <some_file>" to delete a file. The second run would fail because the file has been deleted in the first run.
For more details, please refer to the Github issue mentioned above.

The fix is to overwrite the pytest-ansible plugin's initialize method and remove the extra_inventory option
if it is None. This will prevent the host manager from creating extra_inventory_manager with None as the
inventory source.

Signed-off-by: Xin Wang <[email protected]>
@wangxin wangxin requested review from ganglyu and wsycqyz August 17, 2023 07:26
@ganglyu
Copy link
Copy Markdown
Contributor

ganglyu commented Aug 17, 2023

Can we add a test for this issue?

Copy link
Copy Markdown
Contributor

@wsycqyz wsycqyz left a comment

Choose a reason for hiding this comment

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

LGTM

@wangxin
Copy link
Copy Markdown
Collaborator Author

wangxin commented Aug 18, 2023

Can we add a test for this issue?

This is an issue of third party package. I don't think it is within our own scope.

@wangxin wangxin merged commit d480a19 into sonic-net:master Aug 18, 2023
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Aug 18, 2023
…onic-net#9530)

What is the motivation for this PR?
This is to workaround pytest-ansible plugin issue:
Even no extra-inventory is specified, extra_inventory_manager will still be initialized.
ansible/pytest-ansible#135

As of today, pytest-ansible supports host manager and module dispatcher v29, v212, v213. While initializing the pytest-ansible plugin, it tries to collect default ansible configurations and command line options. These options are used for creating host manager. When no extra_inventory is specified, the options passed to host manager will include extra_inventory=None. However, the host manager will still try to create extra_inventory_manager with None as the inventory source. This will cause the module dispatcher to run the module on hosts matching host pattern in both inventory and extra inventory. This would cause an ansible module executed twice on the same host. In case we wish to use the shell module to run command like "rm <some_file>" to delete a file. The second run would fail because the file has been deleted in the first run. For more details, please refer to the Github issue mentioned above.

This change is only required for python3. Currently master and 202305 branch are using python3. For older branches, they are still using python2 and an older version of pytest-ansible which does not have the bug.

How did you do it?
The fix is to overwrite the pytest-ansible plugin's initialize method and remove the extra_inventory option if it is None. This will prevent the host manager from creating extra_inventory_manager with None as the inventory source.

How did you verify/test it?
Tried run a few tests with pytest-ansible 3.2.1 and 4.0.0.

Signed-off-by: Xin Wang <[email protected]>
@mssonicbld
Copy link
Copy Markdown
Collaborator

Cherry-pick PR to 202305: #9544

mssonicbld pushed a commit that referenced this pull request Aug 18, 2023
…9530)

What is the motivation for this PR?
This is to workaround pytest-ansible plugin issue:
Even no extra-inventory is specified, extra_inventory_manager will still be initialized.
ansible/pytest-ansible#135

As of today, pytest-ansible supports host manager and module dispatcher v29, v212, v213. While initializing the pytest-ansible plugin, it tries to collect default ansible configurations and command line options. These options are used for creating host manager. When no extra_inventory is specified, the options passed to host manager will include extra_inventory=None. However, the host manager will still try to create extra_inventory_manager with None as the inventory source. This will cause the module dispatcher to run the module on hosts matching host pattern in both inventory and extra inventory. This would cause an ansible module executed twice on the same host. In case we wish to use the shell module to run command like "rm <some_file>" to delete a file. The second run would fail because the file has been deleted in the first run. For more details, please refer to the Github issue mentioned above.

This change is only required for python3. Currently master and 202305 branch are using python3. For older branches, they are still using python2 and an older version of pytest-ansible which does not have the bug.

How did you do it?
The fix is to overwrite the pytest-ansible plugin's initialize method and remove the extra_inventory option if it is None. This will prevent the host manager from creating extra_inventory_manager with None as the inventory source.

How did you verify/test it?
Tried run a few tests with pytest-ansible 3.2.1 and 4.0.0.

Signed-off-by: Xin Wang <[email protected]>
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024
…onic-net#9530)

What is the motivation for this PR?
This is to workaround pytest-ansible plugin issue:
Even no extra-inventory is specified, extra_inventory_manager will still be initialized.
ansible/pytest-ansible#135

As of today, pytest-ansible supports host manager and module dispatcher v29, v212, v213. While initializing the pytest-ansible plugin, it tries to collect default ansible configurations and command line options. These options are used for creating host manager. When no extra_inventory is specified, the options passed to host manager will include extra_inventory=None. However, the host manager will still try to create extra_inventory_manager with None as the inventory source. This will cause the module dispatcher to run the module on hosts matching host pattern in both inventory and extra inventory. This would cause an ansible module executed twice on the same host. In case we wish to use the shell module to run command like "rm <some_file>" to delete a file. The second run would fail because the file has been deleted in the first run. For more details, please refer to the Github issue mentioned above.

This change is only required for python3. Currently master and 202305 branch are using python3. For older branches, they are still using python2 and an older version of pytest-ansible which does not have the bug.

How did you do it?
The fix is to overwrite the pytest-ansible plugin's initialize method and remove the extra_inventory option if it is None. This will prevent the host manager from creating extra_inventory_manager with None as the inventory source.

How did you verify/test it?
Tried run a few tests with pytest-ansible 3.2.1 and 4.0.0.

Signed-off-by: Xin Wang <[email protected]>
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
Why I did it
Fix the following issues for Seastone platform:

- system-health issue: show system-health detail will not complete sonic-net#9530, Celestica Seastone DX010-C32: show system-health detail fails with 'Chassis' object has no attribute 'initizalize_system_led' sonic-net#11322
- show platform firmware updates issue: Celestica Seastone DX010-C32: show platform firmware updates sonic-net#11317
- other platform optimization

How I did it
Modify and optimize the platform implememtation.

How to verify it
Manual run the test commands described in these issues.
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
Why I did it
Fix the following issues for Seastone platform:

- system-health issue: show system-health detail will not complete sonic-net#9530, Celestica Seastone DX010-C32: show system-health detail fails with 'Chassis' object has no attribute 'initizalize_system_led' sonic-net#11322
- show platform firmware updates issue: Celestica Seastone DX010-C32: show platform firmware updates sonic-net#11317
- other platform optimization

How I did it
Modify and optimize the platform implememtation.

How to verify it
Manual run the test commands described in these issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants