Bug 1070597 - [RFE] Avoid assuming unknown distros are based on RHEL 6
Summary: [RFE] Avoid assuming unknown distros are based on RHEL 6
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Beaker
Classification: Retired
Component: general
Version: 0.15
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: 0.18
Assignee: Dan Callaghan
QA Contact: tools-bugs
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-02-27 08:13 UTC by Nick Coghlan
Modified: 2018-02-06 00:41 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2014-09-04 05:41:15 UTC
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1070575 0 unspecified CLOSED hardcoded distro name and version in beaker-import (Red Hat Enterprise Linux and CentOS) 2021-02-22 00:41:40 UTC

Internal Links: 1070575

Description Nick Coghlan 2014-02-27 08:13:50 UTC
Currently, the default kickstart templates include some conditional logic based on OSMajor for Fedora and RedHatEnterpriseLinux.

This causes problems when attempting to use the default templates with a *derived* distro like CentOS - the reports OSMajor as CentOS<version>, which means some of the checks in the default templates aren't handled correctly.

To allow derived distros to be handled gracefully, it should be possible to nominate another distro as a "base" distro. The templates would then be updated to check the base distro, rather than the derived distro that is being installed (if no base distro is defined, then it would default to the distro).

Rather than having overly complicated logic directly in the templates, some appropriate Jinja2 filters should be defined to encapsulate the relevant distro and base distro checking logic.

Associated requirement: to handle cases where (for example) CentOS 7 trees have been imported, but no RHEL 7 trees, it will need to be possible to create a Distro entry directly, rather than having to import a Distro Tree into a lab to get it created implicitly.

Open question: should the kickstart generation logic include the appropriate per-distro snippet overrides for the base distro after those for the derived distro?


For additional background, see the thread at: https://lists.fedorahosted.org/pipermail/beaker-devel/2014-February/000969.html

Comment 2 Raymond Mancy 2014-03-04 01:19:15 UTC
(In reply to Nick Coghlan from comment #0)
> Currently, the default kickstart templates include some conditional logic
> based on OSMajor for Fedora and RedHatEnterpriseLinux.
> 
> This causes problems when attempting to use the default templates with a
> *derived* distro like CentOS - the reports OSMajor as CentOS<version>, which
> means some of the checks in the default templates aren't handled correctly.
> 
> To allow derived distros to be handled gracefully, it should be possible to
> nominate another distro as a "base" distro. The templates would then be
> updated to check the base distro, rather than the derived distro that is
> being installed (if no base distro is defined, then it would default to the
> distro).
> 
> Rather than having overly complicated logic directly in the templates, some
> appropriate Jinja2 filters should be defined to encapsulate the relevant
> distro and base distro checking logic.
> 
> Associated requirement: to handle cases where (for example) CentOS 7 trees
> have been imported, but no RHEL 7 trees, it will need to be possible to
> create a Distro entry directly, rather than having to import a Distro Tree
> into a lab to get it created implicitly.
> 

So this bit here invalidates the possibility of having an OSMajor->OSMajor relationship in the DB. It would just have to be a varchar entry. It's also interesting that the template logic expects a certain OSMajor name, and our CentOS osmajor expects to use a certain OSMajor name as its base, but that OSMajor doesn't actually exist anywhere.

An OSMajor entry represents an installed distro. But that's not what we want here, we just want a genus type. On option may be to have a genus table which would just be the genus name (like 'RedHatEnterpriseLinux7'), and every OSMajor would be of a particular genus. That way we don't have rows with varchar columns describing OSMajors that don't exist, nor 100% of the rows with a NULL entry for that column in most Beaker installs.



> Open question: should the kickstart generation logic include the appropriate
> per-distro snippet overrides for the base distro after those for the derived
> distro?
> 
> 
> For additional background, see the thread at:
> https://lists.fedorahosted.org/pipermail/beaker-devel/2014-February/000969.
> html

Comment 3 Dan Callaghan 2014-03-04 02:30:43 UTC
We would want the "base distro" to also take effect for system exclusions and install options. In that case it probably has to be a relationship OSVersion -> OSVersion instead, since the system exclusions and install options can currently be specified by OSMajor or by OSVersion.

Alternatively, if we want to just focus on the problem of kickstart templates/snippets: most of the current conditionals could be converted into runtime feature checks (e.g. instead of conditionalizing wget vs. curl, we would just look for whichever is available). That just leaves the actual kickstart syntax itself, where we need to avoid using newer features on distros that lack them. To handle that it might be enough for a distro/OSVersion/OSMajor to declare what Anaconda version it uses (or better said, which Anaconda syntax level it supports). Pykickstart does keep some notion of syntax levels so that might be worth exploring.

Comment 4 Raymond Mancy 2014-03-04 03:21:13 UTC
(In reply to Raymond Mancy from comment #2)
> (In reply to Nick Coghlan from comment #0)
> > Currently, the default kickstart templates include some conditional logic
> > based on OSMajor for Fedora and RedHatEnterpriseLinux.
> > 
> > This causes problems when attempting to use the default templates with a
> > *derived* distro like CentOS - the reports OSMajor as CentOS<version>, which
> > means some of the checks in the default templates aren't handled correctly.
> > 
> > To allow derived distros to be handled gracefully, it should be possible to
> > nominate another distro as a "base" distro. The templates would then be
> > updated to check the base distro, rather than the derived distro that is
> > being installed (if no base distro is defined, then it would default to the
> > distro).
> > 
> > Rather than having overly complicated logic directly in the templates, some
> > appropriate Jinja2 filters should be defined to encapsulate the relevant
> > distro and base distro checking logic.
> > 
> > Associated requirement: to handle cases where (for example) CentOS 7 trees
> > have been imported, but no RHEL 7 trees, it will need to be possible to
> > create a Distro entry directly, rather than having to import a Distro Tree
> > into a lab to get it created implicitly.
> > 
> 
> So this bit here invalidates the possibility of having an OSMajor->OSMajor
> relationship in the DB. It would just have to be a varchar entry. It's also
> interesting that the template logic expects a certain OSMajor name, and our
> CentOS osmajor expects to use a certain OSMajor name as its base, but that
> OSMajor doesn't actually exist anywhere.
> 
> An OSMajor entry represents an installed distro. But that's not what we want
> here, we just want a genus type. On option may be to have a genus table
> which would just be the genus name (like 'RedHatEnterpriseLinux7'), and
> every OSMajor would be of a particular genus. That way we don't have rows
> with varchar columns describing OSMajors that don't exist, nor 100% of the
> rows with a NULL entry for that column in most Beaker installs.
> 

This comment assumed that OSMajor would exist only if we have imported a distro of that type, which is the current behaviour. If we were to populate this table in advance though, it effectively becomes what genus would have been. I think this is what Nick meant by 'create a Distro entry directly'. I think we could just pre-populate the OSMajor table unconditionally and do away with any immediate need for a UI as part of this change.


> 
> 
> > Open question: should the kickstart generation logic include the appropriate
> > per-distro snippet overrides for the base distro after those for the derived
> > distro?
> > 
> > 
> > For additional background, see the thread at:
> > https://lists.fedorahosted.org/pipermail/beaker-devel/2014-February/000969.
> > html

Comment 5 Nick Coghlan 2014-03-04 03:25:51 UTC
If we go with the base distro idea, I think linking OSVersion -> OSVersion would be a bit too complicated. After all, two different distro versions can already both inherit from a common OSMajor without either inheriting from the other, so I don't see why an OSMajor inheriting from a base OSMajor needs to imply that an OSVersion needs to inherit from a base OSVersion.

That is, today there's no special relationship between RHEL 6.3 and 6.4, aside from the fact that they share RedHatEnterpriseLinux6 as their OSMajor. If we add the ability to have a CentOS6 OSMajor that uses RedHatEnterpriseLinux6 as a base, I don't see a compelling reason for CentOS 6.3 and 6.4 to *automatically* share settings with the corresponding RHEL version - there's no requirement that the versions of a derivative correspond in any way with versions of the original, even though that happens to be the case for RHEL and CentOS.

I think it's worth pursuing at least that much rather than focusing solely on the kickstarts, since there *are* OSMajor level settings that could be usefully applied to derivatives as well (the need to set "serial" on RHEL7 and derivatives comes to mind).

Comment 6 Dan Callaghan 2014-07-03 01:13:59 UTC
(In reply to Dan Callaghan from comment #3)
> Alternatively, if we want to just focus on the problem of kickstart
> templates/snippets: most of the current conditionals could be converted into
> runtime feature checks (e.g. instead of conditionalizing wget vs. curl, we
> would just look for whichever is available). That just leaves the actual
> kickstart syntax itself, where we need to avoid using newer features on
> distros that lack them. To handle that it might be enough for a
> distro/OSVersion/OSMajor to declare what Anaconda version it uses (or better
> said, which Anaconda syntax level it supports). Pykickstart does keep some
> notion of syntax levels so that might be worth exploring.

I experimented a bit more with this idea yesterday. Converting all the distro conditionals in %pre/%post to use runtime checks (e.g. for curl vs wget) is fairly easy, I have a patch for that.

The problematic part is the kickstart syntax, or more precisely the presence or absence of Anaconda features which Beaker needs to account for.

Some of these we can handle easily enough by moving the variations into a ksmeta variable which is then set per OS major (and Beaker could ship with some default install options pre-populated for well-known distros like RedHatEnterpriseLinux5, CentOS5, etc). For example, default mode of 'cmdline' (RHEL6) vs. empty string (RHEL5), or 'key --skip' (RHEL6) vs. 'key 49af89414d147589' (RHEL5).

But there are other variations which are trickier than that. I was hoping we could deal with those with a variable anaconda_version and then changing the conditionals to

    {% if anaconda_version >= 14 %}...{%endif}

and so forth. But Anaconda's versioning is no way near that simple. Here are two examples showing where that idea falls over:

* The unsupported_hardware command (a recent mis-feature where you are required to add this command to your kickstart if running on hardware that Anaconda thinks is unsupported, otherwise it refuses to install). This command was added in anaconda 13.21.181 (RHEL6.something onwards) and 20.5 (Fedora-something onwards), in earlier versions it is a syntax error.

* The <type> argument to the device command was required up until anaconda 11.3.0.45 but became a syntax error from that version onwards. (Realistically, that means it is required up to RHEL5 and a syntax error in RHEL6 onwards.)

So at this point, the base distro hack is looking like a much simpler solution...

Comment 7 Dan Callaghan 2014-07-17 07:28:02 UTC
Okay, I spent a bit more time experimenting with this and I think I have come up with a workable solution that doesn't require the idea of "base distros" and the attendant problems.

We start by converting everything possible to do runtime feature checking (anything in a bash scriptlet):

http://gerrit.beaker-project.org/3203
http://gerrit.beaker-project.org/3204

The remaining template conditionals are abstracted to ksmeta variables populated by Beaker (we were already doing this with 'end' and 'systemd' already):

http://gerrit.beaker-project.org/3205
http://gerrit.beaker-project.org/3206
http://gerrit.beaker-project.org/3207

We introduce a "default" kickstart template which is used as a last resort. It assumes the latest syntax/features (so it's essentially the RHEL7/Fedora template) but with variables to disable things if needed (so RHEL6 can use the same template):

http://gerrit.beaker-project.org/3208
http://gerrit.beaker-project.org/3209

We could extend this further (merging all the templates back to RHEL3 into the "default" template, with suitable conditionals) but there is little point. We won't be seeing any new custom distros based on RHEL3-5. The RHEL3-5 templates can just remain as they are.

So the key points about this approach are:

* Maintains complete backwards compatibility with all existing snippets, templates, and variables. All the names and inheritance orders are kept unchanged. We don't have to worry with edge cases from the base distro approach like "if RedHatStorage2 is based on RedHatEnterpriseLinux6, and I have a custom RedHatEnterpriseLinux6 template but the standard RedHatStorage2 template, which one will Beaker use?"

* Kickstart output is mostly identical to what you get now. There are a few small differences which should have no effect (changed order of some commands, removed "key --skip" command that was already ignored by Anconda).

* Unrecognised distros are effectively treated as the latest Fedora/RHEL. You can add them to Beaker and they will work with no extra effort, thanks to the "default" template. (Actually there are some other problematic places like harness repos, but ignore those for now.) The conditionals are all flipped around so that we start assuming everything is new and supported, and then conditionally disable things on older distros.

* Instead of having conditionals strewn throughout our templates, we have a single place where we hardcode all the various rules like "RHEL7 has systemd" etc. That place is OSMajor.default_install_options.

* Admins have complete control over the conditional behaviour for each OS major, by setting ksmeta variables on the OS major in Beaker's database. This works equally well for distros Beaker knows about (you can tell it RedHatEnterpriseLinux7 really doesn't use systemd) or for completely unrecognised distros. And the variables are even documented :-)

Comment 8 Nick Coghlan 2014-07-21 03:59:31 UTC
I like it, but I think this is still worth writing up as a design proposal for broader discussion. The main issue is that changing the way we generate our kickstarts is something with broad implications for automated testing on existing instances.

I think it's a reasonable idea, and we definitely need to do it fairly soon, but we also need to give users a chance to poke holes in the concept before going ahead with it.

Comment 9 Nick Coghlan 2014-08-06 02:39:13 UTC
Design proposal is up at https://beaker-project.org/dev/proposals/custom-distros.html

We'll get this into Beaker 0.18.

Comment 12 Dan Callaghan 2014-08-20 06:50:38 UTC
I just noticed that we have both has_systemd (new variable) and systemd (old variable for compatibility). Beaker sets both, but only the former is documented. That means if you just set !has_systemd you will still have systemd=True for compatibility. so the snippets will still assume systemd is present which is not the desired outcome.

We will need to either just get rid of the systemd variable entirely (it was never documented and was not actually settable prior to 0.17.2) or else do a more complicated dance with populating and reading it.

Comment 13 Amit Saha 2014-08-20 06:55:45 UTC
(In reply to Dan Callaghan from comment #12)
> I just noticed that we have both has_systemd (new variable) and systemd (old
> variable for compatibility). Beaker sets both, but only the former is
> documented. That means if you just set !has_systemd you will still have
> systemd=True for compatibility. so the snippets will still assume systemd is
> present which is not the desired outcome.
> 
> We will need to either just get rid of the systemd variable entirely (it was
> never documented and was not actually settable prior to 0.17.2) or else do a
> more complicated dance with populating and reading it.

I see no point in having both variables. Let's just remove "systemd" and have it in the release note pointing out so, just in case somebody was using it.

Comment 14 Dan Callaghan 2014-08-20 07:28:43 UTC
http://gerrit.beaker-project.org/3261

Comment 17 Dan Callaghan 2014-09-04 05:41:15 UTC
Beaker 0.18.0 has been released.


Note You need to log in before you can comment on or make changes to this bug.