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 OS grain inconsistencies if lsb-release is installed or not #61626

Merged
merged 6 commits into from
Nov 30, 2022

Conversation

bdrung
Copy link
Contributor

@bdrung bdrung commented Feb 11, 2022

Most Linux distributions ship an os-release file by default. Some do not ship lsb-release information, but they can be installed afterwards. Installing/Removing lsb-release can lead to different OS grain values.

OS grain without lsb-release with lsb-release
AlmaLinux 8 oscodename AlmaLinux 8.5 (Arctic Sphynx) ArcticSphynx
Astra CE os Astra (Orel) AstraLinuxCE
Astra CE os_family Astra (Orel) Debian
Astra CE osfullname Astra Linux (Orel) AstraLinuxCE
Astra CE 2.12.40 osfinger Astra Linux (Orel)-2 AstraLinuxCE-2
Debian osfullname Debian GNU/Linux Debian
Mendel osfullname Mendel GNU/Linux Mendel
Mendel 10 osfinger Mendel GNU/Linux-10 Mendel-10
Mint osfullname Linux Mint Linuxmint
Mint 20.3 osfinger Linuxmint-20 Linux Mint-20
Pop osfullname Pop!_OS Pop
Pop 20.04 osfinger Pop!_OS-20 Pop-20
Rocky osfullname Rocky Linux Rocky
Rocky 8 osfinger Rocky Linux-8 Rocky-8
Rocky 8 oscodename Rocky Linux 8.5 (Green Obsidian) GreenObsidian

The current code that determines the OS grains on Linux is a mess: First lsb-release is queried. If that fails, fall back to read os-release and parse some /etc/*-release files. Then query _linux_distribution and use a mixtures of those for the OS grains. _linux_distribution queries the Python distro library. distro queries the os-release file, lsb-release, and then /etc/*-release.

Rewrite the code that determines the OS grains on Linux. Solely rely on the data provided by the os-release file. To not cause regressions, only switch the distribution that has been tested. All other distributions will use the legacy code (which was moved to _legacy_linux_distribution_data).

The new code derives the os_family grain from the ID_LIKE field from os-release (see #59061 for this feature request). To enable this feature, the new code needs to be used by default (and not just for selected distributions).

This commit introduces a few changes to the OS grains:

  • AlmaLinux and Rocky Linux extract the codename from the VERSION field now instead of using the full PRETTY_NAME.
  • Mendel uses now Mendel GNU/Linux as osfullname and correctly extracts the osrelease from PRETTY_NAME.
  • Pop!_OS changes the osfullname from Pop to Pop!_OS.
  • Astra Linux changes the osfullname from AstraLinuxCE to Astra Linux (Orel) and AstraLinuxSE to Astra Linux (Smolensk) respectively.

Fixes #61618

This merge request contains the commits from the merge request #61597, #61589, and #61619. So I recommend to review and merge those first.

@bdrung
Copy link
Contributor Author

bdrung commented Feb 11, 2022

Added the fix for Astra Linux.

@Ch3LL Ch3LL added the P1 Priority 1 label Sep 19, 2022
@dmurphy18 dmurphy18 requested review from dmurphy18 and removed request for krionbsd September 22, 2022 15:53
Copy link
Contributor

@dmurphy18 dmurphy18 left a comment

Choose a reason for hiding this comment

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

@bdrung Can you resolve the conflicts for the two files and update.
Going through the changes etc., but need the conflicts resolved regardless.

@dmurphy18 dmurphy18 added the Sulfur v3006.0 release code name and version label Sep 22, 2022
@bdrung
Copy link
Contributor Author

bdrung commented Sep 26, 2022

The first pull request in this series #61597 got merged. The next pull request in this series is #61589

@bdrung
Copy link
Contributor Author

bdrung commented Sep 30, 2022

The next pull request in this series #61589 got merged. The next pull request in this series is #61619.

@bdrung
Copy link
Contributor Author

bdrung commented Sep 30, 2022

Pull request #61619 got merged. I rebased this merge request on master now.

Copy link
Contributor

@dmurphy18 dmurphy18 left a comment

Choose a reason for hiding this comment

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

Need to fix the Lint errors before reviewing
Update: And now the failing tests

@bdrung
Copy link
Contributor Author

bdrung commented Sep 30, 2022

Some test cases fail due to #62220 (comment)

@garethgreenaway
Copy link
Contributor

Lint errors are unrelated to this PR.

@garethgreenaway
Copy link
Contributor

This should fix things #62790

@bdrung bdrung force-pushed the fix-os_data-inconsistencies branch from 34e7f06 to 6df7275 Compare October 4, 2022 14:34
Copy link
Contributor

@dmurphy18 dmurphy18 left a comment

Choose a reason for hiding this comment

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

Looks good to merge once all the failing tests are resolved

twangboy
twangboy previously approved these changes Oct 27, 2022
@MKLeb
Copy link
Contributor

MKLeb commented Oct 27, 2022

re-run all

@twangboy
Copy link
Contributor

The failing tests might be related.

repo_content = "deb {opts} [https://repo.saltproject.io/py3/{}/{}/{arch}/latest](https://repo.saltproject.io/py3/%7B%7D/%7B%7D/%7Barch%7D/latest) {} main".format(
                self.fullname,
>               self.grains["lsb_distrib_release"],
                self.grains["oscodename"],
                arch=self.grains["osarch"],
                opts=opts,
            )
E           KeyError: 'lsb_distrib_release'

@garethgreenaway
Copy link
Contributor

@bdrung Just making sure you saw the failing tests on this PR and that they seem to be related to your changes. Thanks!

Copy link
Contributor

@dmurphy18 dmurphy18 left a comment

Choose a reason for hiding this comment

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

@bdrung Are you still interested in this PR ?, the 3 failing Debian tests need fixing.

…8_chars

`freedesktop_os_release` must return a dictionary containing at least
`NAME`, `ID`, and `PRETTY_NAME` or raise an `OSError`. Fix the mock in
`test_linux_proc_files_with_non_utf8_chars` to return a minimal
dictionary instead of an empty mock.

Signed-off-by: Benjamin Drung <[email protected]>
Replace `lsb_distrib_release` by `osrelease` in the tests to avoid
needing to set the lsb_* values.

Signed-off-by: Benjamin Drung <[email protected]>
@bdrung bdrung force-pushed the fix-os_data-inconsistencies branch 2 times, most recently from 74e4181 to 5b22c18 Compare November 20, 2022 00:31
bdrung and others added 3 commits November 20, 2022 01:40
Most Linux distributions ship an os-release file by default. Some do not
ship lsb-release information, but they can be installed afterwards.
Installing/Removing lsb-release can lead to different OS grain values.

| OS               | grain      | without lsb-release              | with lsb-release |
|------------------|------------|----------------------------------|------------------|
| AlmaLinux 8      | oscodename | AlmaLinux 8.5 (Arctic Sphynx)    | ArcticSphynx     |
| Astra CE         | os         | Astra (Orel)                     | AstraLinuxCE     |
| Astra CE         | os_family  | Astra (Orel)                     | Debian           |
| Astra CE         | osfullname | Astra Linux (Orel)               | AstraLinuxCE     |
| Astra CE 2.12.40 | osfinger   | Astra Linux (Orel)-2             | AstraLinuxCE-2   |
| Debian           | osfullname | Debian GNU/Linux                 | Debian           |
| Mendel           | osfullname | Mendel GNU/Linux                 | Mendel           |
| Mendel 10        | osfinger   | Mendel GNU/Linux-10              | Mendel-10        |
| Mint             | osfullname | Linux Mint                       | Linuxmint        |
| Mint 20.3        | osfinger   | Linuxmint-20                     | Linux Mint-20    |
| Pop              | osfullname | Pop!_OS                          | Pop              |
| Pop 20.04        | osfinger   | Pop!_OS-20                       | Pop-20           |
| Rocky            | osfullname | Rocky Linux                      | Rocky            |
| Rocky 8          | osfinger   | Rocky Linux-8                    | Rocky-8          |
| Rocky 8          | oscodename | Rocky Linux 8.5 (Green Obsidian) | GreenObsidian    |

The current code that determines the OS grains on Linux is a mess: First
lsb-release is queried. If that fails, fall back to read os-release and
parse some `/etc/*-release` files. Then query `_linux_distribution` and
use a mixtures of those for the OS grains. `_linux_distribution` queries
the Python `distro` library. `distro` queries the os-release file,
lsb-release, and then `/etc/*-release`.

Rewrite the code that determines the OS grains on Linux. Solely rely on
the data provided by the os-release file. To not cause regressions, only
switch the distribution that has been tested. All other distributions
will use the legacy code (which was moved to
`_legacy_linux_distribution_data`).

The new code derives the `os_family` grain from the `ID_LIKE` field from
os-release (see saltstack#59061 for this
feature request). To enable this feature, the new code needs to be used
by default (and not just for selected distributions).

This commit introduces a few changes to the OS grains:

* AlmaLinux and Rocky Linux extract the codename from the `VERSION` field
  now instead of using the full `PRETTY_NAME`.
* Mendel uses now `Mendel GNU/Linux` as `osfullname` and correctly
  extracts the `osrelease` from `PRETTY_NAME`.
* Pop!_OS changes the `osfullname` from `Pop` to `Pop!_OS`.
* Astra Linux changes the `osfullname` from `AstraLinuxCE` to
  `Astra Linux (Orel)` and `AstraLinuxSE` to `Astra Linux (Smolensk)`
  respectively.

Fixes saltstack#61618
Signed-off-by: Benjamin Drung <[email protected]>
Some distribution derive the OS grains purely from os-release
information and therefore `_linux_distribution` is not called any more.
So remove the test data for unused code paths.

Signed-off-by: Benjamin Drung <[email protected]>
@bdrung
Copy link
Contributor Author

bdrung commented Nov 20, 2022

Fixed failing Debian tests.

@Ch3LL Ch3LL merged commit 6c2b2f9 into saltstack:master Nov 30, 2022
@bdrung bdrung deleted the fix-os_data-inconsistencies branch November 30, 2022 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Priority 1 Sulfur v3006.0 release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Changing OS grains after installing/removing lsb-release
7 participants