Fix distro fact handling for Flatcar by johananl · Pull Request #77635 · ansible/ansible

johananl · 2022-04-25T17:34:09Z

SUMMARY

The existence of the file /etc/flatcar/update.conf depends on bootstrap configuration typically provided by the user. For that reason this file is unsuitable for determining distro facts for Flatcar Container Linux. This PR switches to /etc/os-release as the file used for extracting distro facts for Flatcar.

Fixes #77537.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

distribution.py

NOTE: I'm having hard time figuring out how to test this change. There is test/units/module_utils/facts/system/distribution/fixtures/flatcar_2492.0.0.json but I couldn't get reliable results by executing the test which uses this fixture. Any help regarding how to test this change would be appreciated!

pothos · 2022-04-26T02:13:11Z

lib/ansible/module_utils/facts/system/distribution.py

+        if not data:
+            return False, flatcar_facts
+
+        version = re.search("VERSION=(.*)", data)


Maybe we could even rely more on distro and reuse _distro.version() that already provides the VERSION_ID field, or?

I like the idea, however distro inside parse_distribution_file_Flatcar() is a string, not an instance of LinuxDistribution. The version() method on LinuxDistribution is actually already used inside get_distribution_version() which gets invoked from _guess_distribution() which in turns runs before we try processing the dist file. The dist file parsing functions are overriding any facts guessed previously, and AFAICT are supposed to be the authoritative source for distro-specific facts.

Therefore, I think it's safer to explicitly set the relevant facts inside parse_distribution_file_Flatcar() and rely on guessed facts only as a fallback.

johananl · 2022-04-26T13:44:42Z

NOTE: I'm having hard time figuring out how to test this change. There is test/units/module_utils/facts/system/distribution/fixtures/flatcar_2492.0.0.json but I couldn't get reliable results by executing the test which uses this fixture. Any help regarding how to test this change would be appreciated!

I've updated the existing test fixture. The test seems to pass, however I'm not totally sure about the distro key here and whether it is correct.

sivel · 2022-04-26T13:50:29Z

The test seems to pass, however I'm not totally sure about the distro key here and whether it is correct.

That fixture can be generated by running hacking/tests/gen_distribution_version_testcase.py

The existence of the file /etc/flatcar/update.conf depends on bootstrap configuration typically provided by the user. For that reason this file is unsuitable for determining distro facts for Flatcar Container Linux. The distribution_release fact is meaningless in the case of Flatcar since Flatcar doesn't have named releases. The distribution_version fact, however, IS meaningful and should contain a number such as "3139.2.0". - Use /etc/os-release instead of /etc/flatcar/update.conf. - Drop the distribution_release fact. - Set the distribution_version fact. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>

johananl · 2022-04-26T16:27:52Z

The test seems to pass, however I'm not totally sure about the distro key here and whether it is correct.

That fixture can be generated by running hacking/tests/gen_distribution_version_testcase.py

Thanks @sivel. The generated output has result.distribution and result.os_family set to Flatcar Container Linux by Kinvolk though, which isn't what we want (see #77537) 😕
Generating the fixture and then setting these two keys to Flatcar manually seems to work well though.

Following is the full generated fixture, for the record:

{
    "name": "Flatcar Container Linux by Kinvolk 3139.2.0",
    "distro": {
        "codename": "",
        "id": "flatcar",
        "name": "Flatcar Container Linux by Kinvolk",
        "version": "3139.2.0",
        "version_best": "3139.2.0",
        "lsb_release_info": {},
        "os_release_info": {
            "name": "Flatcar Container Linux by Kinvolk",
            "id": "flatcar",
            "id_like": "coreos",
            "version": "3139.2.0",
            "version_id": "3139.2.0",
            "build_id": "2022-04-05-1803",
            "pretty_name": "Flatcar Container Linux by Kinvolk 3139.2.0 (Oklo)",
            "ansi_color": "38;5;75",
            "home_url": "https://flatcar-linux.org/",
            "bug_report_url": "https://issues.flatcar-linux.org",
            "flatcar_board": "amd64-usr",
            "cpe_name": "cpe:2.3:o:flatcar-linux:flatcar_linux:3139.2.0:*:*:*:*:*:*:*"
        }
    },
    "input": {
        "/etc/os-release": "NAME=\"Flatcar Container Linux by Kinvolk\"\nID=flatcar\nID_LIKE=coreos\nVERSION=3139.2.0\nVERSION_ID=3139.2.0\nBUILD_ID=2022-04-05-1803\nPRETTY_NAME=\"Flatcar Container Linux by Kinvolk 3139.2.0 (Oklo)\"\nANSI_COLOR=\"38;5;75\"\nHOME_URL=\"https://flatcar-linux.org/\"\nBUG_REPORT_URL=\"https://issues.flatcar-linux.org\"\nFLATCAR_BOARD=\"amd64-usr\"\nCPE_NAME=\"cpe:2.3:o:flatcar-linux:flatcar_linux:3139.2.0:*:*:*:*:*:*:*\"\n",
        "/etc/lsb-release": "DISTRIB_ID=\"Flatcar Container Linux by Kinvolk\"\nDISTRIB_RELEASE=3139.2.0\nDISTRIB_CODENAME=\"Oklo\"\nDISTRIB_DESCRIPTION=\"Flatcar Container Linux by Kinvolk 3139.2.0 (Oklo)\"\n",
        "/usr/lib/os-release": "NAME=\"Flatcar Container Linux by Kinvolk\"\nID=flatcar\nID_LIKE=coreos\nVERSION=3139.2.0\nVERSION_ID=3139.2.0\nBUILD_ID=2022-04-05-1803\nPRETTY_NAME=\"Flatcar Container Linux by Kinvolk 3139.2.0 (Oklo)\"\nANSI_COLOR=\"38;5;75\"\nHOME_URL=\"https://flatcar-linux.org/\"\nBUG_REPORT_URL=\"https://issues.flatcar-linux.org\"\nFLATCAR_BOARD=\"amd64-usr\"\nCPE_NAME=\"cpe:2.3:o:flatcar-linux:flatcar_linux:3139.2.0:*:*:*:*:*:*:*\"\n"
    },
    "platform.dist": [
        "flatcar",
        "3139.2.0",
        ""
    ],
    "result": {
        "distribution": "Flatcar Container Linux by Kinvolk",
        "distribution_version": "3139.2.0",
        "distribution_release": "NA",
        "distribution_major_version": "3139",
        "os_family": "Flatcar Container Linux by Kinvolk"
    },
    "platform.release": "5.15.32-flatcar"
}

- Generate the fixture using gen_distribution_version_testcase.py. - Override result.distribution and result.os_family manually as the generator script gives wrong values. - Use a recent Flatcar version. Signed-off-by: Johanan Liebermann <jliebermann@microsoft.com>

johananl · 2022-04-27T12:02:52Z

I've updated the fixture to be the output above, with the following manual overrides:

Set result.distribution to Flatcar.
Set result.os_family to Flatcar.

I hope that makes sense.

s-hertel · 2022-09-07T15:58:21Z

/azp run

azure-pipelines · 2022-09-07T15:58:35Z

Azure Pipelines successfully started running 1 pipeline(s).

webknjaz · 2022-09-07T16:35:23Z

/azp run

azure-pipelines · 2022-09-07T16:35:33Z

Azure Pipelines successfully started running 1 pipeline(s).

pothos reviewed Apr 26, 2022

View reviewed changes

johananl force-pushed the fix-flatcar-distro branch 4 times, most recently from f64753e to f874b57 Compare April 26, 2022 13:42

bcoca removed the needs_triage Needs a first human triage before being processed. label Apr 26, 2022

johananl force-pushed the fix-flatcar-distro branch from f874b57 to 91b3bfc Compare April 26, 2022 16:17

johananl force-pushed the fix-flatcar-distro branch from 91b3bfc to d461326 Compare April 27, 2022 12:02

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 5, 2022

ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 7, 2022

bcoca merged commit fbd8286 into ansible:devel Sep 7, 2022

johananl deleted the fix-flatcar-distro branch September 8, 2022 09:01

johananl mentioned this pull request Sep 8, 2022

Fix Flatcar detection in node role kubernetes-sigs/image-builder#970

Merged

ansible locked and limited conversation to collaborators Sep 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix distro fact handling for Flatcar#77635

Fix distro fact handling for Flatcar#77635
bcoca merged 2 commits intoansible:develfrom
johananl:fix-flatcar-distro

johananl commented Apr 25, 2022

Uh oh!

pothos Apr 26, 2022

Uh oh!

johananl Apr 26, 2022

Uh oh!

johananl commented Apr 26, 2022

Uh oh!

sivel commented Apr 26, 2022

Uh oh!

johananl commented Apr 26, 2022 •

edited

Loading

Uh oh!

johananl commented Apr 27, 2022

Uh oh!

s-hertel commented Sep 7, 2022

Uh oh!

azure-pipelines bot commented Sep 7, 2022

Uh oh!

webknjaz commented Sep 7, 2022

Uh oh!

azure-pipelines bot commented Sep 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

johananl commented Apr 25, 2022

SUMMARY

ISSUE TYPE

COMPONENT NAME

Uh oh!

pothos Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

johananl Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

johananl commented Apr 26, 2022

Uh oh!

sivel commented Apr 26, 2022

Uh oh!

johananl commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johananl commented Apr 27, 2022

Uh oh!

s-hertel commented Sep 7, 2022

Uh oh!

azure-pipelines bot commented Sep 7, 2022

Uh oh!

webknjaz commented Sep 7, 2022

Uh oh!

azure-pipelines bot commented Sep 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

johananl commented Apr 26, 2022 •

edited

Loading