Bug 719536

Please make sure you hide them not only from the distro list, but from reserve workflow page, and other, excluded families for a machine, and other places there might be a reference to a distro or an empty distro family.

Possible dupe of bug 707614

Simon, only the reserve workflow part is dupe of 707614. I'd include 707614
into this one, as it's with a bit wider requirements.

*** Bug 707614 has been marked as a duplicate of this bug. ***

This will be addressed in the upcoming Beaker 0.9 release:

* Reserve workflow will not offer distros which are expired from all labs
* Distro and distro tree search pages will not list distros which are expired from all labs

We have not yet done anything about the excluded families checkboxes on the system page (as per comment 2), and that sounds like a good idea, so I will leave this bug open for now.

Updating description to reflect the fact that the Excluded Families tab is the only problematic one left.

Added to internal dev backlog, since Andrew pointed out how unreadable our Excluded Families tab currently is due to the cruft that has built up over the years. It isn't just old distros - it's ones where the metadata was buggy (e.g. "0" as the OSMajor), so Beaker imported nonsense.

The current UI (huge pile of checkboxes) is not really sustainable, given that the number of OS majors in Beaker is continuing to grow rapidly -- even if we were to exclude the ones that are truly cruft.

The exclusions should just be presented as a list of (arch, OS major) and (arch, OS major, OS minor) tuples, which is how they are actually represented in the db. Adding a new exclusion should just mean selecting the arch, typing the OS major (with typeahead assitance, or perhaps a select box) and optionally typing the OS minor, then clicking Add.

This tab is currently one of the last remaining TG widgets so this will be an opportunity to port it to Backbone and add an API as well.

Just to recap the problems we want to address here:

1) In the UI, the list can be insanely long. Each architecture the system supports has it's own list of os majors.

2) Adjusting the excluded families list for a bunch of system is currently not possible or cumbersome at best. A workaround exists by using CSV export/imports (see Bug 1568751 for example).

3) For older hardware in the pool, there is no way to exclude this hardware from any future osmajor which gets added. In other words, there is no way to turn the excluded list into an include list. No UI/API end point currently exists to tell Beaker, that older hardware is only supported up to os major X  (see Bug 743077 for that).

4) It is not possible to exclude a system from an entire family (e.g. CentOS, Fedora) (see Bug 1410495)

I'm currently not certain if all of these points can be addressed without adjusting the database (#3 looks that way).

(In reply to Roman Joost from comment #12)
> I'm currently not certain if all of these points can be addressed without
> adjusting the database (#3 looks that way).

Yes certainly #3 and #4 need changes to the way we represent the exclusions. But I guess since this RFE involves a complete rewrite of the UI, that is the best time to do those too.

From a UI point of view I came to the same ideas what Dan has thought of in comment 10. That should cover 1) as well as 2) since the API will be exposed via our CLI, so batch updates should be possible. I'm not sure how well the UI will scale when it comes to poin 3). I envisioned a little switch between inclusion/exclusion, but it has consequences. So in order to avoid confusion, the UI should probably make it clear, that can either exclude or include, not both. Whether that is a separate tab in order to make this distinction clear or not, I don't know yet.

I think at first, lets change the current excluded families with a new API endpoint and changes to bkr client. Then check how we can address the inclusions.

We had a quick chat about the way to go. Altering the excluded and included families will have to become two separate API end points. The UI can then tie this together with a small toggle.

The excluded families end point then can be:

POST /systems/<fqdn>/excluded-distro-trees/

and the data model would be:

* for a whole major version: {'arch': XXX, 'osmajor': XXX}
* for a specific osminor {'arch': XXX, 'osmajor': XXX, 'osminor': '1'}

Instead of "included" call it "exclusive". This matches the precedent we have with tasks (excluded arches / exclusive arches, excluded osmajors / exclusive osmajors) which itself matches the RPM model (ExcludeArch / ExclusiveArch).

"Exclusive OS Majors" is clearer than "Included" (included in what?)

I've spend a bit of time on getting back into the JS side of things and to try out some ideas I have with the UI. Terminology as well as templates are all over the shop currently, but this will change.

I've started implementing the API end points for excluded-distro-trees: GET and POST (including happy path tests). Currently the data is based purely on their names, e.g.:

  {'arch': 'i386', 'osmajor': 'CentOS6'}

both for getting and setting. It seems I can uniquely identify OSVersion, OSMajor and Arch by their names, but I'm suspicious if this will not create problems down the road (e.g. in the UI). Suppose if there is need, I can always add the id's of the database objects, but since I do not have to PATCH end points or manipulate the objects directly I think I should be fine. Will see ...

I've got the listing working and working on the removal of items. If I have it wired up right, I think I should be able to get away with a sync() of the collection. I'm currently missing some UI tests, so I need to write those as well (forgot the TDD here...).

One other thing which popped into my mind today is the initial state for the UI if we want to provide a way to edit exclusive and excluding families. If we have two separate API end points, I would have to query both of them to establish what has been set on the system: exclusive or excluding?

(In reply to Roman Joost from comment #18)
> One other thing which popped into my mind today is the initial state for the
> UI if we want to provide a way to edit exclusive and excluding families. If
> we have two separate API end points, I would have to query both of them to
> establish what has been set on the system: exclusive or excluding?

The initial state (including whether it is an exclusive list or an exclusion list) should be populated in the page by the server when it renders the template. It shouldn't need to send an AJAX request to the API to fetch the current state.

If both are empty then really it is an exclusion list with no exclusions.

> The initial state (including whether it is an exclusive list or an exclusion
> list) should be populated in the page by the server when it renders the
> template. It shouldn't need to send an AJAX request to the API to fetch the
> current state.
> 
> If both are empty then really it is an exclusion list with no exclusions.

Hm... I do have that advantage in the UI to populate the initial state with server side rendering, but I won't have that with the Beaker client, unless I've missed something. How would you be able to figure out the initial state there?

I had first thought of a data structure like this:

GET /systems/<fqdn>/distro-trees/

{ type: 'exclusive',
  data: [{<list like above>}] }

That would work for both client and UI. Granted, the naming needs work ...

Well in the CLI you are not presenting a list of checkboxes... there is no initial state. The CLI should be modelled based on the operations offered in the API. So the operations are adding or removing from the exclusion list -- not replacing the entire thing.

I see two choices for the CLI... either jam more stuff into system-modify using more switches. Then the arch and OS version have to all be encoded together which is super awkward. Something like:

bkr system-modify --add-excluded-distro=osmajor=RedHatEnterpriseLinux7,arch=x86_64

bkr system-modify --add-excluded-distro=osversion=RedHatEnterpriseLinux7.5,arch=x86_64

bkr system-modify --remove-excluded-distro=osmajor=RedHatEnterpriseLinux7,arch=x86_64

bkr system-modify --add-exclusive-distro=osmajor...

But it's probably cleaner to introduce separate subcommands.

bkr system-exclusion-add --osmajor RedHatEnterpriseLinux7 --arch x86_64

bkr system-exclusion-remove --osmajor RedHatEnterpriseLinux7

bkr system-exclusion-add --exclusive --osmajor RedHatEnterpriseLinux6

Also the --arch should really be optional in all of the above. Most systems only support one arch anyway, and even when there is two arches in 99% of the cases the exclusion applies to all the arches anyway.

I want to quickly recapitulate what has been worked on so far:

I started of with the idea of implementing API endpoints which would manipulate a flat list of dictionaries:

[{'arch': XXX, 'osmajor': XXX}]

I realised, that these items don't translate well into backbone. Models should use a uniquely identifiable attribute, which these items lack off. The choice then is to simply add methods to the system model and use jQuery to persist each change in the UI.

However, each model in the database (ExcludeOSMajor, ExcludeOSVersion) does have a unique id (the database id). So my thinking was, since they are uniquely identifiable within their instances and Backbone does have some kind of support for polymorphism (http://backbonejs.org/#Collection-model) all it needs to transparently display the different instances. That indeed did work well up to the point in which I realised that the polymorphism isn't really supported. Each model, no matter what instantiated it, needs a unique ID to be identifiable in the collection.

I could now hack up the collection and models to compute a different identifier, but that sort of defeats the purpose how I started of with: to use Backbone models, which map directly back to our database identifiers and just display them transparently. One additional downside is now to use more than one request to persist models in the database if the system supports more than one architecture. With more than one architecture to support, it also makes displaying tricky. If we want to stop repetition (displaying the same exclusion for each architecture), we need to encode a way to weed out repeating elements, but handle a click for each architecture the element supports.

All this considered, I realised what we want is not a flat list of items like the one stated above, but rather grouped by architecture like so:

[{'arches': [xx], 'osmajor': XXX}]

That eliminates all the work which needs to happen to weed out "duplicate" visual items, and allows to do the hard work of adding/removing exclusions for each architecture at the server end.

Unfortunately that means I'll have to rework parts of my API again, as well as the tests and the UI.