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 "'ansible' ModuleNotFoundError" in Disaster Recovery scripts #503

Merged
merged 2 commits into from
Jun 7, 2022

Conversation

barpavel
Copy link
Member

@barpavel barpavel commented May 26, 2022

When executing any Disaster Recovery script, the following error is received:

[root@storage-ge-13 files]# ./ovirt-dr generate
Traceback (most recent call last):
  File "./ovirt-dr", line 15, in <module>
    import fail_back
  File "/usr/share/ansible/collections/ansible_collections/redhat/rhv/roles/disaster_recovery/files/fail_back.py", line 14, in <module>
    from ansible.module_utils.six.moves import input
ModuleNotFoundError: No module named 'ansible'

The reason for that is the following import statement: from ansible.module_utils.six.moves import input.

There is no need to use "six" library at all since Python2 is deprecated!
All "import"s of "six" module can be deleted now and the builtin Python3 "input()" function will be used instead.

Bug-Url: https://bugzilla.redhat.com/2090264

@barpavel barpavel added the bug Something isn't working label May 26, 2022
@michalskrivanek
Copy link
Member

needs changelog entry, other than that lgtm

@ahadas
Copy link
Member

ahadas commented May 26, 2022

AFAIU there is no need to use "six" library at all since Python2 is deprecated! All "import"s of "six" module can be deleted now and the builtin Python3 "input" function will be used instead.

@barpavel we may have 4.2 (or more probably 4.3) clusters that are based on EL7, where we used Python 2, wouldn't the DR be affected?

@nirs
Copy link
Member

nirs commented May 26, 2022

This part of the commit message is not helpful and best removed:

Executing:
  pip3 install ansible
solves the issue, but is not the right solution.

I compared the installed packages on the VM where the "pip3" command was executed
(and by that fixed the issue) versus a VM where it was not executed (and the
"ModuleNotFoundError" happens) and unfortunately failed to pinpoint what makes the difference.
Also noticed that changing the import to:
  from six.moves import input
also solves the issue, thus "ansible.module_utils" prefix is the one that fails the flow.

I have no idea why the problem didn't happen in 4.4, but probably the investigation
doesn't worth the effort: AFAIU there is no need to use "six" library at all since
Python2 is deprecated!
All "import"s of "six" module can be deleted now and the builtin Python3 "input"
function will be used instead.

The issue seems to be using old python 2 compatibility code that is not needed
in the last 2 years, and likely removed from ansible. Since we can use the builtin
input module there is no need for the compatibility hack.

But what worries me is:

 ModuleNotFoundError: No module named 'ansible'

It can mean that you are using the wrong python version, which is a bigger problem.

@barpavel
Copy link
Member Author

needs changelog entry, other than that lgtm

Will do.

@nirs
Copy link
Member

nirs commented May 26, 2022

The real issue seems to be here:
https://github.com/oVirt/ovirt-ansible-collection/blob/master/roles/disaster_recovery/files/ovirt-dr

If this code should use ansible, it should use python3.8.

@barpavel
Copy link
Member Author

from six.moves import input

AFAIU there is no need to use "six" library at all since Python2 is deprecated! All "import"s of "six" module can be deleted now and the builtin Python3 "input" function will be used instead.

@barpavel we may have 4.2 (or more probably 4.3) clusters that are based on EL7, where we used Python 2, wouldn't the DR be affected?

In that case (if we still use Python2) I can change the import from
from ansible.module_utils.six.moves import input
to
from six.moves import input
as I described in the description.

@barpavel
Copy link
Member Author

This part of the commit message is not helpful and best removed:

Executing:
  pip3 install ansible
solves the issue, but is not the right solution.

I compared the installed packages on the VM where the "pip3" command was executed
(and by that fixed the issue) versus a VM where it was not executed (and the
"ModuleNotFoundError" happens) and unfortunately failed to pinpoint what makes the difference.
Also noticed that changing the import to:
  from six.moves import input
also solves the issue, thus "ansible.module_utils" prefix is the one that fails the flow.

I have no idea why the problem didn't happen in 4.4, but probably the investigation
doesn't worth the effort: AFAIU there is no need to use "six" library at all since
Python2 is deprecated!
All "import"s of "six" module can be deleted now and the builtin Python3 "input"
function will be used instead.

The issue seems to be using old python 2 compatibility code that is not needed in the last 2 years, and likely removed from ansible. Since we can use the builtin input module there is no need for the compatibility hack.

But what worries me is:

 ModuleNotFoundError: No module named 'ansible'

It can mean that you are using the wrong python version, which is a bigger problem.

Yes, strange, I see a lot of usages from ansible.module_utils.XYZ in ovirt-ansible-collection, so maybe indeed the issue is different and need to understand why QE env fails on that....

@barpavel
Copy link
Member Author

barpavel commented May 26, 2022

The real issue seems to be here: https://github.com/oVirt/ovirt-ansible-collection/blob/master/roles/disaster_recovery/files/ovirt-dr

If this code should use ansible, it should use python3.8.

I see all DR files use:
#!/usr/bin/python3
While others (including those having from ansible.module_utils.XYZimports) use:
#!/usr/bin/python.
Will check that soon (in 2-3 hours).

@nirs
Copy link
Member

nirs commented May 26, 2022

The important question is if this script depends on ansible python package, or is is a standalone tool using other command line tools.

In the first case you want to use python3.8 since this is where ansible stuff is installed. In the second case using python3 is fine.

@mwperina
Copy link
Member

The important question is if this script depends on ansible python package, or is is a standalone tool using other command line tools.

In the first case you want to use python3.8 since this is where ansible stuff is installed. In the second case using python3 is fine.

So it's not entirely true. AFAIR iif there is a python script executed from ansible playbook, then it's executed by the python installed on the target host and not the control host (the host where you execute the playbook from).

Of course you can force which python version you want to use using command directive and not relying on shebang, for example:

- name: Run python script
  ansible.builtin.command: /usr/bin/python38 /usr/share/mypackage/my_script.py
  register: mymotd

Just be aware that we are still supporting both EL7 and EL8 hosts (and hopefully soon also EL9 in oVirt), so you should choose only installed python version for each OS version (unless you want to install additional packages of course)

@mnecas Am I right?

@barpavel
Copy link
Member Author

barpavel commented May 26, 2022

I must admit that after all the comments from so many people - and I'm really thankful for your comments - I'm totally confused and have no clue what is the problem and what is the right solution and why it worked in 4.4 and stopped working :(

@ahadas if we should support Python2, then probably need to keep on using six library for compatibility.

@nirs I see that on QE machines #!/usr/bin/python3 is replaced (probably during installation) with /usr/libexec/platform-python which is mapped to Python 3.6 and not 3.8: /usr/libexec/platform-python -> ./platform-python3.6. So it fails to find ansible, but after pip3 install ansible it does work with the same Python 3.6.8. How come?

@mnecas @mwperina maybe Martin Perina already provided the answer, but I didn't understand it, sorry then. I see there are lots of files having from ansible.X.Y... (for example in ovirt-ansible-collection/plugins/modules). Which makes me suspect that having from ansible.module_utils in the code is absolutely legal, so my solution to eliminate the redundant six library (or at least ansible.module_utils prefix) is not the right direction to fix the root cause of ModuleNotFoundError: No module named 'ansible' issue. Do you understand why it fails on QE env and why it started to happen now (worked in 4.4)?

And in regard to what @nirs wrote If this code should use ansible, it should use python3.8: is it possible to work in Python 3.6.8 (QE env) or Python2 (cluster versions 4.2. & 4.3 as @ahadas mentioned) while having from ansible.X.Y... in the code?

@nirs
Copy link
Member

nirs commented May 27, 2022

I don't think we can support python 2 at this point. Users on RHEL 7 need to use the ovirt-dr script distributed for RHEL 7. Users on RHEL 8 can use only python 3.

The ovirt-dr script is not run by ansible - it runs ansible playbooks, so we cannot control
python version using ansible, it must be controlled by the #!/usr/bin/pythonXXX line.

Using the built in input function is the right way. Lets focus on this fix now and leave
the question of which python version to use for later.

@barpavel barpavel force-pushed the ansible_ModuleNotFoundError branch from 0cc9a3e to 23f3392 Compare May 30, 2022 08:22
@barpavel barpavel changed the title Fixing "'ansible' ModuleNotFoundError" in Disaster Recovery scripts Fix "'ansible' ModuleNotFoundError" in Disaster Recovery scripts May 30, 2022
@barpavel barpavel force-pushed the ansible_ModuleNotFoundError branch from 23f3392 to a548fac Compare May 30, 2022 08:44
@barpavel
Copy link
Member Author

This part of the commit message is not helpful and best removed

Removed the redundant description from both the commit and PR messages.

@barpavel barpavel force-pushed the ansible_ModuleNotFoundError branch from a548fac to 5eb74af Compare May 30, 2022 08:51
@barpavel
Copy link
Member Author

needs changelog entry, other than that lgtm

Done. Added the changelog entry according to examples from other PRs.

@barpavel barpavel requested a review from nirs May 30, 2022 08:59
@michalskrivanek
Copy link
Member

/ost

@ahadas
Copy link
Member

ahadas commented May 31, 2022

@ahadas if we should support Python2, then probably need to keep on using six library for compatibility.

I'm not sure if we should keep it or not because I don't remember if we execute things on the hosts as part of the DR - my point was that if we execute things on the hosts then we should take into consideration that some of the hosts have python 2, and then if we drop support for python 2 it means those clusters couldn't take part in the process

@barpavel
Copy link
Member Author

barpavel commented May 31, 2022

@ahadas if we should support Python2, then probably need to keep on using six library for compatibility.

I'm not sure if we should keep it or not because I don't remember if we execute things on the hosts as part of the DR - my point was that if we execute things on the hosts then we should take into consideration that some of the hosts have python 2, and then if we drop support for python 2 it means those clusters couldn't take part in the process

The DR scripts connect to engines (primary & secondary) by using the necessary configuration, i.e.,

dr_sites_primary_url: http://10.35.1.20:8080/ovirt-engine/api
dr_sites_primary_username: admin@internal
dr_sites_primary_ca_file: /etc/pki/ovirt-engine/ca.pem

# Please fill in the following properties for the secondary site:
dr_sites_secondary_url: http://10.35.0.195:8080/ovirt-engine/api
dr_sites_secondary_username: admin@internal
dr_sites_secondary_ca_file: /etc/pki/ovirt-engine/ca.pem

Also providing SD details, cluster information, etc.

But AFAIR we don't run directly on hosts.
The secondary site should already have the host and storage, but should be empty:

The failover is achieved by configuring a Red Hat Virtualization environment in the secondary site, which requires:
- An active Red Hat Virtualization Manager.
- A data center and clusters.
- Networks with the same general connectivity as the primary site.
- Active hosts capable of running critical virtual machines after failover.
.......
The storage domains that store the virtual machine disks in Site A is replicated. Site B has no virtual machines or attached storage domains.

https://access.redhat.com/documentation/en-us/red_hat_virtualization/4.4/html/disaster_recovery_guide/active_passive#active_passive_overview

In short, all commands are sent to engine and engine communicates with the host(s) to create VMs, disks, ...
There is no hosts' configuration that need to be provided, only engine's configuration.

@ahadas
Copy link
Member

ahadas commented May 31, 2022

@barpavel ok, great, thanks for checking

When executing any Disaster Recovery script, the following error is received:
  [root@storage-ge-13 files]# ./ovirt-dr generate
  Traceback (most recent call last):
    File "./ovirt-dr", line 15, in <module>
      import fail_back
    File "/usr/share/ansible/collections/ansible_collections/redhat/rhv/roles/disaster_recovery/files/fail_back.py", line 14, in <module>
      from ansible.module_utils.six.moves import input
  ModuleNotFoundError: No module named 'ansible'

The reason for that is the following import statement:
  from ansible.module_utils.six.moves import input

There is no need to use "six" library at all since Python2 is deprecated!
All "import"s of "six" module can be deleted now and the builtin Python3
"input()" function will be used instead.

Signed-off-by: Pavel Bar <pbar@redhat.com>
Bug-Url: https://bugzilla.redhat.com/2090264
@barpavel barpavel force-pushed the ansible_ModuleNotFoundError branch from 3472d2a to 47b071e Compare June 6, 2022 11:42
@barpavel
Copy link
Member Author

barpavel commented Jun 6, 2022

/ost

@mnecas mnecas merged commit 02813b6 into oVirt:master Jun 7, 2022
mnecas added a commit to mnecas/ovirt-ansible-collection that referenced this pull request Jun 13, 2022
…Virt#503)

When executing any Disaster Recovery script, the following error is received:
  [root@storage-ge-13 files]# ./ovirt-dr generate
  Traceback (most recent call last):
    File "./ovirt-dr", line 15, in <module>
      import fail_back
    File "/usr/share/ansible/collections/ansible_collections/redhat/rhv/roles/disaster_recovery/files/fail_back.py", line 14, in <module>
      from ansible.module_utils.six.moves import input
  ModuleNotFoundError: No module named 'ansible'

The reason for that is the following import statement:
  from ansible.module_utils.six.moves import input

There is no need to use "six" library at all since Python2 is deprecated!
All "import"s of "six" module can be deleted now and the builtin Python3
"input()" function will be used instead.

Signed-off-by: Pavel Bar <pbar@redhat.com>
Bug-Url: https://bugzilla.redhat.com/2090264

Co-authored-by: Martin Nečas <necas.marty@gmail.com>
@barpavel barpavel deleted the ansible_ModuleNotFoundError branch June 16, 2022 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants