Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Guest OS additions and mappings #3609

Closed
rohityadavcloud opened this issue Sep 25, 2019 · 16 comments · Fixed by #6979
Closed

Improve Guest OS additions and mappings #3609

rohityadavcloud opened this issue Sep 25, 2019 · 16 comments · Fixed by #6979
Assignees
Labels
complexity:complex a few weeks to a few months of work with a lot of interaction Severity:Minor type:improvement
Milestone

Comments

@rohityadavcloud
Copy link
Member

When adding support for new guest OS and their hypervisor mappings, the upgrade path(s) have historically assumed static guest OS id in the schema/upgrade paths even for id which is an AUTO_INCREMENT column. This can break on environments who add their custom guest OS and hypervisor mappings. The implementation should be improved to allow insertion of guest OS either via APIs or via schema upgrade paths without making such static assumptions.

ISSUE TYPE
  • Improvement Request
COMPONENT NAME
Guest OS mappings
CLOUDSTACK VERSION
4.11.x, 4.13.x, all previous
@andrijapanicsb
Copy link
Contributor

andrijapanicsb commented Sep 30, 2019

I believe we effectively need to handle 2 tables: guest_os and guest_os_hypervisor (expecting that end-users will NOT have a need to change guest_os_category or guest_os_details)

Current code behaviour (tested with 4.11/4.13):

  • from the guest_os table, show all duplicates (if any, should be none usually) - this is what's visible in the listOsTypes or in the GUI (and it does show duplicates when testing)
  • from the guest_os_hypervisor table, show the latest (last row) matching hypervisor os mappings if there are any duplicates (i.e. there is at least one duplicate example for Windows Server 2016 for VMware 6.5 and later, which you can observe with:
    SELECT * FROM guest_os_hypervisor WHERE hypervisor_type="VMware" and hypervisor_version="6.5" and guest_os_id IN (SELECT id FROM guest_os WHERE display_name="Windows Server 2016 (64-bit)");

Possible improvements as following:

guest_os table

  • when inserting new guest OSes, simply SKIP defining the ID, and this will use the next ID available
  • make sure to "read" the "id" assigned to that record, since we'll probably need it to add new referencing records in the guest_os_hypervisor table later

guest_os_hypervisor table

  • skip defining the ID explicitly while adding new mappings, let auto-increment work its stuff
  • keep the current behaviour of using the last (with highest ID) records from this table (if there are duplicates), which allows us to "override" identical mapping that the end-users might have created previously (before the upgrade)
  • make sure to reference the correct ID from the guest_os table that was auto-incremented - this seems to be the only "complexity" here

This would allow for:

  • end-users to add their own OS and OS mappings with whatever IDs they might use
  • during schema upgrades, allow us to "override" this by adding identical records as the part of the upgrade path with higher IDs, which will be the only one "honored", thus obsoleting the previously added user-records

FTR:
image

@PaulAngus
Copy link
Member

I believe that the entire methodology around supporting hypervisor versions needs refactoring. Users/operators should not need to be independently adding OSes.
I would like to see some kind of file (probably json) which describes a hypervisor type+version, it's capabilities, it's supported GuestOSes, and any string mappings required.
Operators would then 'import/activate' the 'file' for the hypervisors that they are supporting, and new versions of hypervisor (which don't require core code changes) could be provided outside of CloudStack's release cycle.

CloudStack would also only 'present' the Guest OS lists supplied by the Hypervisor (via the json file) rather than an 'all-encompassing' list of hundreds of OS types.

for example wrt CentOS, vSphere and XenServer only cares 5.x, 6.x 7.x, (64bit vs 32 bit) KVM doesn't care at all. So 10s of CentOS versions is pointless....

@svenvogel
Copy link
Contributor

for example wrt CentOS, vSphere and XenServer only cares 5.x, 6.x 7.x, (64bit vs 32 bit) KVM doesn't care at all. So 10s of CentOS versions is pointless....

@PaulAngus Sounds good especially the "10s of CentOS" ... It would make the work of the operator easier.

@DaanHoogland
Copy link
Contributor

a middle ground solution could be to start IDs for new guest osses and - mappings at 10000 from now on autoadding them as we go on and leave old mapping at lower IDs. Users should not be bothered indeed but operators should have some control thopugh automation is best. It should be possible for an operator to dissallow an OS by removing its mapping or definition alltogether.
€0,02

@DaanHoogland DaanHoogland added complexity:complex a few weeks to a few months of work with a lot of interaction Severity:Minor and removed complexity:normal labels Jan 4, 2021
@DaanHoogland DaanHoogland added this to the 4.16.0.0 milestone Jan 4, 2021
@DaanHoogland
Copy link
Contributor

@PaulAngus' suggestion has merit, adding the guest os map as json, but that would have to be on a either or basis. It does make the solution more solid but adds complexity to the change.

side note: each os would need a List<String> recognised_as for convenience in the hypervisor plugins.
side note2: an export would help administrating during updates and upgrades.

@rohityadavcloud
Copy link
Member Author

cc @harikrishna-patnala

@GutoVeronezi
Copy link
Contributor

@rhtyd There is a PR (#4699) that I think would solve this situation.
It was created a procedure that will add guest_os + hypervisor mapping;

  • To each new map, call the procedure;
  • If the map already exists (validating name and category), it will skip it;
  • The procedure respects the AUTO_INCREMENT columns;
  • After generate the guest_os.id, it will retrieve it to use in guest_os_hypervisor;

If this PR (#4699) is accepted, then we need to ensure that every PR, that add new guest_os + hypervisor mappings, uses the procedure.

Could you take a look a it?

@rohityadavcloud
Copy link
Member Author

Thanks @GutoVeronezi I looked at the PR and I think it doesn't fully satisfy the requirements intended with this issue. I've asked my colleagues if they can look into your solution and expand on them since you don't want to expand scope of your work that would include the full set of changes I was looking for.

@harikrishna-patnala/@Spaceman1984 are looking into the full feature who may update scope of their changes, I've been communicated at $dayjob that their feature will cover all my requirements and some additional scope.

@Spaceman1984
Copy link
Contributor

That's correct @rhtyd

@rohityadavcloud rohityadavcloud modified the milestones: 4.16.0.0, 4.16.1.0 Sep 8, 2021
@rohityadavcloud
Copy link
Member Author

Moving this to next minor milestone, possibly to major in future.

@rohityadavcloud
Copy link
Member Author

@nvazquez can you review if this needs any work or can be closed? cc @sureshanaparti

@nvazquez
Copy link
Contributor

nvazquez commented Mar 2, 2022

@sureshanaparti is planning to work on it @rohityadavcloud

@nvazquez
Copy link
Contributor

Moving to the next milestone as advised by @sureshanaparti

@rohityadavcloud
Copy link
Member Author

Has this been fixed with Suresh's PR now @harikrishna-patnala @shwstppr ?

@aarushisoni
Copy link

Hi my name is Aarushi Soni . I want to contribute to this issue . Is this issue still open ?
Please guide me through this process.

@DaanHoogland
Copy link
Contributor

@aarushisoni please have a look at https://github.com/shapeblue/hackerbook to get acquaited with the project.
Once you're comfortable just submit a pr referencing any issue if applicable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity:complex a few weeks to a few months of work with a lot of interaction Severity:Minor type:improvement
Projects
None yet