Fix OS grain inconsistencies if lsb-release is installed or not #61626

bdrung · 2022-02-11T01:13:53Z

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 · 2022-02-11T16:47:23Z

Added the fix for Astra Linux.

dmurphy18

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

bdrung · 2022-09-26T22:53:09Z

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

bdrung · 2022-09-30T19:13:58Z

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

bdrung · 2022-09-30T21:31:29Z

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

dmurphy18

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

bdrung · 2022-09-30T22:00:34Z

Some test cases fail due to #62220 (comment)

garethgreenaway · 2022-10-01T00:18:52Z

Lint errors are unrelated to this PR.

garethgreenaway · 2022-10-01T00:58:03Z

This should fix things #62790

dmurphy18

Looks good to merge once all the failing tests are resolved

MKLeb · 2022-10-27T20:32:36Z

re-run all

twangboy · 2022-10-28T20:52:03Z

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 · 2022-11-04T01:58:02Z

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

dmurphy18

@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 <bdrung@ubuntu.com>

Replace `lsb_distrib_release` by `osrelease` in the tests to avoid needing to set the lsb_* values. Signed-off-by: Benjamin Drung <bdrung@ubuntu.com>

Signed-off-by: Benjamin Drung <bdrung@ubuntu.com>

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 <benjamin.drung@ionos.com>

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 <benjamin.drung@ionos.com>

bdrung · 2022-11-20T14:12:56Z

Fixed failing Debian tests.

bdrung requested a review from a team as a code owner February 11, 2022 01:13

bdrung requested review from krionbsd and removed request for a team February 11, 2022 01:13

bdrung force-pushed the fix-os_data-inconsistencies branch 2 times, most recently from e64d00c to 1be251f Compare February 11, 2022 16:46

This was referenced Feb 14, 2022

[BUG] test_pkgrepo not run on Debian #61641

Open

Fix osmajorrelease on Ubuntu and Pop!_OS #61661

Closed

Ch3LL added the P1 Priority 1 label Sep 19, 2022

dmurphy18 requested review from dmurphy18 and removed request for krionbsd September 22, 2022 15:53

dmurphy18 requested changes Sep 22, 2022

View reviewed changes

dmurphy18 added the Sulfur v3006.0 release code name and version label Sep 22, 2022

bdrung mentioned this pull request Sep 22, 2022

grains: Split os_data into smaller functions #61597

Merged

bdrung force-pushed the fix-os_data-inconsistencies branch from 1be251f to da338c2 Compare September 30, 2022 21:30

bdrung requested a review from dmurphy18 September 30, 2022 21:32

dmurphy18 requested changes Sep 30, 2022

View reviewed changes

bdrung force-pushed the fix-os_data-inconsistencies branch from da338c2 to f9e657f Compare September 30, 2022 22:26

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

dmurphy18 reviewed Oct 18, 2022

View reviewed changes

dmurphy18 approved these changes Oct 18, 2022

View reviewed changes

twangboy previously approved these changes Oct 27, 2022

View reviewed changes

dmurphy18 requested changes Nov 14, 2022

View reviewed changes

bdrung added 2 commits November 19, 2022 16:14

test: Use osrelease instead of lsb_distrib_release

93aaa06

Replace `lsb_distrib_release` by `osrelease` in the tests to avoid needing to set the lsb_* values. Signed-off-by: Benjamin Drung <bdrung@ubuntu.com>

bdrung dismissed twangboy’s stale review via 74e4181 November 19, 2022 15:19

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

Replace lsb_distrib_codename by oscodename

2be9efc

Signed-off-by: Benjamin Drung <bdrung@ubuntu.com>

grains: Remove test data for unused code paths

43996e0

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 <benjamin.drung@ionos.com>

bdrung force-pushed the fix-os_data-inconsistencies branch from 5b22c18 to 43996e0 Compare November 20, 2022 00:53

Merge branch 'master' into fix-os_data-inconsistencies

b0b30de

whytewolf approved these changes Nov 29, 2022

View reviewed changes

twangboy approved these changes Nov 29, 2022

View reviewed changes

Ch3LL merged commit 6c2b2f9 into saltstack:master Nov 30, 2022

bdrung deleted the fix-os_data-inconsistencies branch November 30, 2022 20:17

dmurphy18 mentioned this pull request Nov 17, 2023

[BUG] grain 'lsb_distrib_id' expected by salt/modules/network.py but not populated on Debian #65573

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix OS grain inconsistencies if lsb-release is installed or not #61626

Fix OS grain inconsistencies if lsb-release is installed or not #61626

bdrung commented Feb 11, 2022 •

edited

Loading

bdrung commented Feb 11, 2022

dmurphy18 left a comment

bdrung commented Sep 26, 2022

bdrung commented Sep 30, 2022

bdrung commented Sep 30, 2022

dmurphy18 left a comment •

edited

Loading

bdrung commented Sep 30, 2022

garethgreenaway commented Oct 1, 2022

garethgreenaway commented Oct 1, 2022

dmurphy18 left a comment

MKLeb commented Oct 27, 2022

twangboy commented Oct 28, 2022

garethgreenaway commented Nov 4, 2022

dmurphy18 left a comment

bdrung commented Nov 20, 2022

Fix OS grain inconsistencies if lsb-release is installed or not #61626

Fix OS grain inconsistencies if lsb-release is installed or not #61626

Conversation

bdrung commented Feb 11, 2022 • edited Loading

bdrung commented Feb 11, 2022

dmurphy18 left a comment

Choose a reason for hiding this comment

bdrung commented Sep 26, 2022

bdrung commented Sep 30, 2022

bdrung commented Sep 30, 2022

dmurphy18 left a comment • edited Loading

Choose a reason for hiding this comment

bdrung commented Sep 30, 2022

garethgreenaway commented Oct 1, 2022

garethgreenaway commented Oct 1, 2022

dmurphy18 left a comment

Choose a reason for hiding this comment

MKLeb commented Oct 27, 2022

twangboy commented Oct 28, 2022

garethgreenaway commented Nov 4, 2022

dmurphy18 left a comment

Choose a reason for hiding this comment

bdrung commented Nov 20, 2022

bdrung commented Feb 11, 2022 •

edited

Loading

dmurphy18 left a comment •

edited

Loading