openwisp / ansible-openwisp2

Ansible role that installs and upgrades OpenWISP.
https://openwisp.io/docs/dev/ansible/
BSD 3-Clause "New" or "Revised" License
474 stars 137 forks source link

[Ubuntu 20.04], [ansible 2.13.2]: Issue with apt fact processing leads to python apt packages not being processed #393

Closed broadstack-au closed 2 years ago

broadstack-au commented 2 years ago

"Out of the box" install on Ubuntu 20.04, the pip install of uwsgi failed with...

      *** uWSGI compiling embedded plugins ***
      [x86_64-linux-gnu-gcc -pthread] plugins/python/python_plugin.o
      In file included from plugins/python/python_plugin.c:1:
      plugins/python/uwsgi_python.h:2:10: fatal error: Python.h: No such file or directory
          2 | #include <Python.h>
            |          ^~~~~~~~~~
      compilation terminated.
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for uwsgi
  Running setup.py clean for uwsgi
Failed to build uwsgi

Python package installation is being skipped:

Install python3 packages...
Tuesday 30 August 2022  16:23:28 +1000 (0:00:00.065)       0:00:20.855 ********
  openwisp2.local skipped: {
    "changed": false,
    "skip_reason": "Conditional result was False"
}

I assume it's because the fact check

  when: openwisp2_should_install_python_37 == "False\n"

disagrees with

Check openwisp2 should install python 3.7...
Tuesday 30 August 2022  16:23:28 +1000 (0:00:00.610)       0:00:20.436 ********
  openwisp2.local ok: {
    "ansible_facts": {
        "openwisp2_should_install_python_37": false
    },
    "changed": false
}

Change introduced here https://github.com/openwisp/ansible-openwisp2/commit/92da57657a88cdb08ab67222da75802ca287a486#diff-bd68d57a0208eff15ccf26bd0a9e052d8f76cc2b5305f2a544456a8a5f135e55R201

Am unsure if "False\n" was purely a typo, otherwise I'd just submit a PR

nemesifier commented 2 years ago

@broadstack-au can you check with the debug module what is the value of "openwisp2_should_install_python_37"?

broadstack-au commented 2 years ago

Yup, it's a straight bool false with my env. (Output is in the original comment)

broadstack-au commented 2 years ago

It looks like the positive case was changed from "True\n" to true here: https://github.com/openwisp/ansible-openwisp2/commit/db7f16a66b9862255ba70a9a9d46a6fe144dc487#diff-bd68d57a0208eff15ccf26bd0a9e052d8f76cc2b5305f2a544456a8a5f135e55L189-L195

(I don't have access to enough environments to be confident that it should always be a straight bool test)

nemesifier commented 2 years ago

Thanks for your analysis.

Yup, it's a straight bool false with my env. (Output is in the original comment)

From the code I see that if openwisp2_should_install_python_37 it means this instruction returns false:

openwisp2_installed_python.stdout.split(' ')[1] is version_compare('3.7', 'le')

What is the output you get from /usr/bin/python3 --version?

Another question, is this block executing for you?

- name: Install python3 packages
  apt:
    name:
      - python3-pip
      - python3-dev
      - python3-virtualenv
  retries: 5
  delay: 10
  register: result
  until: result is success
  when: openwisp2_should_install_python_37 == "False\n"
broadstack-au commented 2 years ago

Python version is Python 3.8.10 (Version: 3.8.10-0ubuntu1~20.04.5) "Check installed python version..." agrees:

Check installed python version...
Thursday 01 September 2022  20:08:54 +1000 (0:00:02.034)       0:00:46.607 ****
  openwisp2.local ok: {
    "changed": false,
    "cmd": [
        "python3",
        "--version"
    ],
    "delta": "0:00:00.005000",
    "end": "2022-09-01 10:02:11.562374",
    "invocation": {
        "module_args": {
            "_raw_params": "python3 --version",
            "_uses_shell": false,
            "argv": null,
            "chdir": null,
            "creates": null,
            "executable": null,
            "removes": null,
            "stdin": null,
            "stdin_add_newline": true,
            "strip_empty_ends": true,
            "warn": false
        }
    },
    "msg": "",
    "rc": 0,
    "start": "2022-09-01 10:02:11.557374",
    "stderr": "",
    "stderr_lines": [],
    "stdout": "Python 3.8.10",
    "stdout_lines": [
        "Python 3.8.10"
    ]
}

All the python blocks get skipped:

Check openwisp2 should install python 3.7...
Thursday 01 September 2022  20:08:56 +1000 (0:00:02.151)       0:00:48.759 ****
  openwisp.local ok: {
    "ansible_facts": {
        "openwisp2_should_install_python_37": false
    },
    "changed": false
}
...
...
Install software-properties-common...
Thursday 01 September 2022  20:08:56 +1000 (0:00:00.100)       0:00:48.860 ****
  openwisp.local skipped: {
    "changed": false,
    "skip_reason": "Conditional result was False"
}
Add deadsnakes PPA...
Thursday 01 September 2022  20:08:56 +1000 (0:00:00.070)       0:00:48.930 ****
  openwisp.local skipped: {
    "changed": false,
    "skip_reason": "Conditional result was False"
}
Install Python 3.7...
Thursday 01 September 2022  20:08:57 +1000 (0:00:00.081)       0:00:49.012 ****
  openwisp.local skipped: {
    "changed": false,
    "skip_reason": "Conditional result was False"
}
Set python 3.7...
Thursday 01 September 2022  20:08:57 +1000 (0:00:00.072)       0:00:49.085 ****
  openwisp.local skipped: {
    "changed": false,
    "skip_reason": "Conditional result was False"
}

The bit that puzzles me is how it ever worked - given what I'm seeing - unless it's an ansible / jinja filter implementation change.

Either way - Assuming you have access to a host with python <= 3.7x, I've put in a PR for validation.

With the assumption that the tests will sniff out whether I'm right on this one, I've submitted a PR The task does exactly what I expect it to with that PR.

broadstack-au commented 2 years ago

Sorry - missed the output from the specific task requested. Result was in original message

Install python3 packages...
Tuesday 30 August 2022  16:23:28 +1000 (0:00:00.065)       0:00:20.855 ********
  openwisp2.local skipped: {
    "changed": false,
    "skip_reason": "Conditional result was False"
}
nemesifier commented 2 years ago

@broadstack-au so if you change that block to:

- name: Install python3 packages
  apt:
    name:
      - python3-pip
      - python3-dev
      - python3-virtualenv
  retries: 5
  delay: 10
  register: result
  until: result is success
  when: not openwisp2_should_install_python_37

Does it work as expected?

PS: just noticed your open PR, thanks! See here: https://github.com/openwisp/ansible-openwisp2/pull/394#pullrequestreview-1093170035 .

broadstack-au commented 2 years ago

Yeah, that works too - was trying to keep it equivalent with the positive test. Happy to amend both if you prefer.

broadstack-au commented 2 years ago

Ahh - I see - you'd like to get rid of some ansible-lint errors? I'm happy to do that chore for you as a follow-up.

nemesifier commented 2 years ago

No worries, so after applying https://github.com/openwisp/ansible-openwisp2/pull/394 to your local copy of the ansible role, are you able to install successfully?

broadstack-au commented 2 years ago

I am - yes