Fact results inaccurate (distro module issue)?

handcraftedcomputers commented 1 year ago

Describe the bug

This could be a me issue (I am a neophyte to pyinfra). I have several "older Linux distributions" under my care. When running pyinfra inventory.py fact server.LinuxDistribution, the facts returned for these servers match the host I'm running pyinfra from, not the target. Although the files appear to be returned successfully, the distro module is unable to interpret correctly (see the following output):

$ pyinfra -vvv inventory.py fact server.LinuxDistribution

--> Loading config...

--> Loading inventory...

--> Connecting to hosts...
    [server1] Connected
    [server2] Connected

--> Gathering facts...
[server2] >>> sh -c 'cd /etc/ && for file in $(ls -pdL *-release | grep -v /); do echo "/etc/${file}"; cat "/etc/${file}"; echo ---; done'
[server1] >>> sh -c 'cd /etc/ && for file in $(ls -pdL *-release | grep -v /); do echo "/etc/${file}"; cat "/etc/${file}"; echo ---; done'
[server1] /etc/lsb-release
[server1] DISTRIB_ID=Ubuntu
[server1] DISTRIB_RELEASE=9.10
[server1] DISTRIB_CODENAME=karmic
[server1] DISTRIB_DESCRIPTION="Ubuntu 9.10"
[server1] ---
    [server1] Loaded fact server.LinuxDistribution
[server2] /etc/centos-release
[server2] CentOS release 6.10 (Final)
[server2] ---
[server2] /etc/redhat-release
[server2] CentOS release 6.10 (Final)
[server2] ---
[server2] /etc/system-release
[server2] CentOS release 6.10 (Final)
[server2] ---
    [server2] Loaded fact server.LinuxDistribution

--> Fact data for: server.LinuxDistribution
{
    "server1": {
        "name": "Debian",
        "major": 11,
        "minor": null,
        "release_meta": {
            "PRETTY_NAME": "Debian GNU/Linux 11 (bullseye)",
            "NAME": "Debian GNU/Linux",
            "VERSION_ID": "11",
            "VERSION": "11 (bullseye)",
            "VERSION_CODENAME": "bullseye",
            "ID": "debian",
            "HOME_URL": "https://www.debian.org/",
            "SUPPORT_URL": "https://www.debian.org/support",
            "BUG_REPORT_URL": "https://bugs.debian.org/",
            "CODENAME": "bullseye"
        }
    },
    "server2": {
        "name": "Debian",
        "major": 11,
        "minor": null,
        "release_meta": {
            "PRETTY_NAME": "Debian GNU/Linux 11 (bullseye)",
            "NAME": "Debian GNU/Linux",
            "VERSION_ID": "11",
            "VERSION": "11 (bullseye)",
            "VERSION_CODENAME": "bullseye",
            "ID": "debian",
            "HOME_URL": "https://www.debian.org/",
            "SUPPORT_URL": "https://www.debian.org/support",
            "BUG_REPORT_URL": "https://bugs.debian.org/",
            "CODENAME": "bullseye"
        }
    }
}

The result should definitely not be Debian 11 for a Centos 6 and Ubuntu 9.04 (I did say they were old) servers. While I'm not expecting facts to necessarily identify these older servers, defaulting to the underlying host's OS is unexpected behaviour.

To Reproduce

$ pyinfra inventory.py fact server.LinuxDistribution

(where inventory.py points to a couple of archaic distributions).

Host is an up to date Debian 11 laptop.

Expected behavior

Either undefined (if distro module was unable to find an appropriate match), or the appropriate response.

Meta

Include output of pyinfra --support.

--> Support information:

If you are having issues with pyinfra or wish to make feature requests, please
check out the GitHub issues at https://github.com/Fizzadar/pyinfra/issues .
When adding an issue, be sure to include the following:

System: Linux
  Platform: Linux-5.10.0-20-amd64-x86_64-with-glibc2.31
  Release: 5.10.0-20-amd64
  Machine: x86_64
pyinfra: v2.6
Executable: /home/dt/Work/pyinfra/venv/bin/pyinfra
Python: 3.9.2 (CPython, GCC 10.2.1 20210110)

How was pyinfra installed (source/pip)? Via pip in venv.
Consider including output with -vv and --debug. As noted above
Additional info Version of distro is v1.8.0. Output captured while debugging from distro's return result (note: showing os_release_file pointing inappropriately to the localhost's /usr/lib/os-release):

LinuxDistribution(os_release_file='/usr/lib/os-release', distro_release_file='/tmp/tmpis7_en3u/etc/centos-release', include_lsb=False, include_uname=False, include_oslevel=True, root_dir=None, _os_release_info={'pretty_name': 'Debian GNU/Linux 11 (bullseye)', 'name': 'Debian GNU/Linux', 'version_id': '11', 'version': '11 (bullseye)', 'version_codename': 'bullseye', 'id': 'debian', 'home_url': 'https://www.debian.org/', 'support_url': 'https://www.debian.org/support', 'bug_report_url': 'https://bugs.debian.org/', 'codename': 'bullseye', 'release_codename': 'bullseye'}, _lsb_release_info={}, _distro_release_info={'name': 'CentOS', 'version_id': '6.10', 'codename': 'Final', 'id': 'centos'}, _uname_info={}, _oslevel_info='')

Additional additional info I am enjoying pyinfra much more than Ansible (irrespective of this issue :) )

handcraftedcomputers commented 1 year ago

So, poking at this further (as such is my wont), it appears that patching distro._UNIXCONFDIR as pyinfra currently does breaks the result of distro.LinuxDistribution() since at least distro v1.6.0 when the root_dir named parameter was introduced. With the attached test script, you can see the difference in the return result (using the files returned by Centos 6 fact retrieval) when using the patched distro._UNIXCONFDIR variable versus using the named root_dir parameter. The fix is relatively simple, but I'm not sure what (if anything) this would break. At a bare minimum, distro would need to be pinned >= 1.6.0 for the root_dir named parameter to work, but given that the distro package was moved subordinate to distro in v1.7.0 (i.e. from distro import distro) this would need to be done anyway.

new_rootdir_distro:
{'id': 'centos',
 'major': 6,
 'minor': 10,
 'name': 'CentOS',
 'release_meta': {}}

old_rootdir_distro:
{'id': 'debian',
 'major': 11,
 'minor': None,
 'name': 'Debian '
         'GNU/Linux',
 'release_meta': {'BUG_REPORT_URL': 'https://bugs.debian.org/',
                  'CODENAME': 'bullseye',
                  'HOME_URL': 'https://www.debian.org/',
                  'ID': 'debian',
                  'NAME': 'Debian '
                          'GNU/Linux',
                  'PRETTY_NAME': 'Debian '
                                 'GNU/Linux '
                                 '11 '
                                 '(bullseye)',
                  'SUPPORT_URL': 'https://www.debian.org/support',
                  'VERSION': '11 '
                             '(bullseye)',
                  'VERSION_CODENAME': 'bullseye',
                  'VERSION_ID': '11'}}

distro_test.py.txt

Fizzadar commented 1 year ago

Thank you for digging into this @handcraftedcomputers - indeed it looks like there's another variable that needs patching to cover this, but I think it's a good time to bump the minimum version up to 1.6 and remove the patching hack entirely.

Fizzadar commented 1 year ago

Made this change: https://github.com/Fizzadar/pyinfra/commit/06d61c9e9fb6980228afe02cec44a3f8cd815a1a.

Fizzadar commented 1 year ago

Fix was released in 2.6.2!

pyinfra-dev / pyinfra