install_vm: Add default osinfo for RHEL distro

[ggbecker]

Description:

• install_vm: Add default osinfo for RHEL distro.
• rhel8-unknown should work for RHEL8+ installations.

[github-actions]

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment

Oracle Linux 8 Environment

[comps]

LGTM, although I'm not sure if you want such a long --help string for an option.

[ggbecker]

LGTM, although I'm not sure if you want such a long --help string for an option.

That was also something I didn't like, but considering I struggled with this option since from the beginning, I'd rather have extra information than too little

[Mab879]

Can I have a bit more context on this pull request? A least for me the only time I need to provide --os-info is for RHEL 10. Is this what this PR is trying to solve?

[comps]

You can also do what some of our other projects do with long help texts - put extra details at the end.

In Python argparse, that's done as ie.

epilog = textwrap.dedent(r"""
    Lorem ipsum dolor sit amet, consectetur adipiscing elit. Pellentesque ege
    efficitur turpis. Nam malesuada nec dolor eget auctor. Donec accumsan enim
    id sem tempus, a sodales ligula sollicitudin. Quisque at tortor at risus congue
    commodo.
""")

parser = argparse.ArgumentParser(
    epilog=epilog,
    formatter_class=argparse.RawTextHelpFormatter,
    usage=f'{sys.argv[0]} [options]',
)

parser.add_argument( ... )

The epilog will appear as a block of text (even with multiple paragraphs, space indentation, etc. if used with RawTextHelpFormatter) at the end of the --help output.

I'm not sure if it's worth doing it here, just mentioning it as an option.

[comps]

Can I have a bit more context on this pull request? A least for me the only time I need to provide --os-info is for RHEL 10. Is this what this PR is trying to solve?

AFAIK Yes, but it's been implemented in a fairly generic way to be applicable to all RHELs, as to not single out RHEL-10.

Either way, virt-install typically detects this via osinfo from the installation URL, so if you're okay with RHEL-10 needing this manually for now until people migrate to newer virt-install versions (newer libosinfo versions), then this PR is not necessary.

But a forced rhel8-unknown (we internally use rhel7-unknown even for Fedora/RHEL-9) is basically, if not literally, the same as the autodetected feature set, so - in practice - it tends to be more future-proof.

Either way works, I guess.

[Mab879]

Can I have a bit more context on this pull request? A least for me the only time I need to provide --os-info is for RHEL 10. Is this what this PR is trying to solve?

AFAIK Yes, but it's been implemented in a fairly generic way to be applicable to all RHELs, as to not single out RHEL-10.

Either way, virt-install typically detects this via osinfo from the installation URL, so if you're okay with RHEL-10 needing this manually for now until people migrate to newer virt-install versions (newer libosinfo versions), then this PR is not necessary.

But a forced rhel8-unknown (we internally use rhel7-unknown even for Fedora/RHEL-9) is basically, if not literally, the same as the autodetected feature set, so - in practice - it tends to be more future-proof.

Either way works, I guess.

Agreed, both ways work. I guess I'm fine with this, I would prefer to use autodetect vs this static assign, but this does make installing new versions of RHEL easier.

[ggbecker]

With @comps suggestion, the help output looks like:

  --console             Connect to a serial console of the VM (to monitor installation progress).
  --disk-unsafe         Set cache unsafe.
  --osinfo OSINFO       Specify OSInfo for virt-install command

--osinfo details: 'rhel8-unknown' is used by default when '--distro' has any of
the RHEL available selected). Run 'virt-install --osinfo list'
to get a list of available options

[ggbecker]

Another option here would be to just include hints on which osinfo to use for unreleased RHEL major versions or to split the KNOWN_DISTROS into two and the second would contain unreleased distros, so we can set the osinfo by default for those unreleased distros. This could be useful for other distros as well.

[Mab879]

Another option here would be to just include hints on which osinfo to use for unreleased RHEL major versions or to split the KNOWN_DISTROS into two and the second would contain unreleased distros, so we can set the osinfo by default for those unreleased distros. This could be useful for other distros as well.

That might work, and I agree it would be flexible for other distros.

[ggbecker]

I have expanded the script with the concept of unreleased distro which might make it clearer. The problem is that there is a time expiration for that code to become obsolete. I'm going to setup a reminder for that to be updated.

[codeclimate]

Code Climate has analyzed commit cfbdea1 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	1
Style	3

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 61.9% (0.0% change).

View more on Code Climate.