Fixes #37416 - assign templates for new Debian/Suse OSs

[sbernhard]

What are the changes introduced in this pull request?

Considerations taken when implementing this change?

What are the testing steps for this pull request?

[wbclark]

I didn't test this in the scenario where the OS is automatically created during a puppet check-in (what the commit 5befbde references)

Instead I manually created OSes for Debian "bookworm" 12.5 and SLES 15-SP5, populating only the minimum required fields; after creation, I went to edit the newly created OS and observed that, e.g. "AutoYaST entire SCSI disk" and "AutoYaST default iPXE" etc. were already selected in the respective Partition Table and Templates tabs, which wasn't the case when I tested without this commit.

The code looks good to me and seems to work as intended.

The existing test at test/concerns/operatingsystem_extensions_test.rb already covers the core logic, and the new bits are relatively simple and just involve returning strings for various cases. It could still potentially be worth adding tests for the new OSes to protect against future bugs, but I could honestly go either way on it... what do you think @sbernhard ?

[sbernhard]

Thanks @wbclark for having a look at it already. Actually,I think it would be even better to use templates assignment from the past if a new OS is auto-generated. Like, if the OS "Ubuntu 24.4.1" is auto-generated, it should use the OS "Ubuntu 23.4" (without minor) and re-use the template associatons. Only, if no data from the past can be re-used, take the hard-coded templates - which is not perfect either but better than nothing. Therefore, I set this PR to draft again.

What are your thoughts about this behavior @wbclark? Does it make sense?

[sbernhard]

Updated the PR @wbclark with the mentioned assign templates from related os.

(Tests are missing right now)

[wbclark]

What are your thoughts about this behavior @wbclark? Does it make sense?

Hmmm, it's a neat idea. My initial questions and thoughts about it are

1. How do you propose to determine exactly one prior version to inherit from? First match OS family, then match OS name, then same major release and prior minor release, falling back to match on latest minor release on prior major release, finally falling back to the defaults if none can be found? Or some other strategy or logic to make the determination? What do you suppose is the best strategy in testing, to handle the additional complexity it might introduce?

2. As you know, for Red Hat OSes, we have settings to determine the default templates when a new OS in the family gets created. What do you think about potential cognitive overhead for the user of having different behaviors per OS family? Do you have a strong preference for your proposed improvement, vs. implementing similar settings for other OSes, and why or why not?

3. In some cases there may be a bug with (z-1) that requires using a modified template as a workaround, for that release only. In such cases, this would require a 2nd manual intervention to restore the original template on the z release after that OS gets created. Do you see any other cases where this inheritance behavior may not be desired?

4. The change you have already represents an incremental improvement in functionality, and IMO the code is a little bit nicer too for reading, maintenance, etc. Do you feel strongly about doing it all in a single PR or are you open to doing the initially proposed enhancement first, and working on the proposed inheritance behavior in a separate PR?

[wbclark]

Aha, @sbernhard you were a bit faster than me. :-)

Would you kindly take a look at my above questions about your new proposal whenever you have a few minutes to do so, and in the meantime, I'll familiarize myself with the new changes and solicit some feedback about the idea from my team at our standup tomorrow.

Thanks!

[goarsna]

I had a look at the changes and on my first tests everything worked as expected.

[wbclark]

Hey @sbernhard , thanks for your patience. Consensus from the colleagues was that as long as it only affects Debian and SUSE, this should be totally fine 👍

[sbernhard]

[test foreman]

[sbernhard]

The change is ready @wbclark

[wbclark]

Thanks @sbernhard -- I'm returning from vacation today and will take a fresh look after I get through some meetings. :)

[wbclark]

@sbernhard I couldn't make this work at present.

My steps were

1. Rebased the commit on master
2. Created an SLES OS and changed the default partition table to AutoYaST LVM
3. Created a new SLES OS and observed that the default partition table was still AutoYaST entire SCSI disk

I tried it a few different ways, and added some debugging, and found that the find_related_os method was indeed finding the prior OSes to inherit from:

21:50:48 rails.1   | 2024-05-29T21:50:48 [W|app|6cec470c] !!!! ALL_RELATED_OS: [{"id"=>17, "major"=>"15", "name"=>"SLES", "minor"=>"5", "nameindicator"=>nil, "created_at"=>"2024-05-29T21:48:06.770-04:00", "updated_at"=>"2024-05-29T21:48:06.770-04:00", "release_name"=>"", "description"=>"", "password_hash"=>"SHA256", "title"=>"SLES 15.5"}, {"id"=>16, "major"=>"15", "name"=>"SLES", "minor"=>"4", "nameindicator"=>nil, "created_at"=>"2024-05-29T21:43:50.634-04:00", "updated_at"=>"2024-05-29T21:43:50.634-04:00", "release_name"=>"", "description"=>"", "password_hash"=>"SHA256", "title"=>"SLES 15.4"}, {"id"=>15, "major"=>"15", "name"=>"SLES", "minor"=>"3", "nameindicator"=>nil, "created_at"=>"2024-05-29T21:34:33.190-04:00", "updated_at"=>"2024-05-29T21:34:33.190-04:00", "release_name"=>"", "description"=>"", "password_hash"=>"SHA256", "title"=>"SLES 15.3"}, {"id"=>14, "major"=>"15", "name"=>"SLES", "minor"=>"2", "nameindicator"=>nil, "created_at"=>"2024-05-29T21:33:42.363-04:00", "updated_at"=>"2024-05-29T21:33:42.363-04:00", "release_name"=>"", "description"=>"", "password_hash"=>"SHA256", "title"=>"SLES 15.2"}, {"id"=>12, "major"=>"15", "name"=>"SLES", "minor"=>"", "nameindicator"=>nil, "created_at"=>"2024-05-29T21:31:02.558-04:00", "updated_at"=>"2024-05-29T21:31:02.558-04:00", "release_name"=>"", "description"=>"", "password_hash"=>"SHA256", "title"=>"SLES 15"}] !!!!

Could you confirm I understood the intent correctly -- that my overridden default partition table should then have been inherited by the newly created OS in the same family?

In that case, I think the issue should be somewhere in the assign_related_os_templates method

[wbclark]

Suggested change

	#false encoding: utf-8
	# encoding: utf-8

A find-and-replace editor mishap? :D