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

When migrating compare product classes #1281

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

When migrating compare product classes #1281

wants to merge 2 commits into from

Conversation

jesusbv
Copy link
Collaborator

@jesusbv jesusbv commented Feb 5, 2025

Description

Because of the change from SLE Micro < 6.0 to SL Micro 6+ comparing the identifier no longer works when migrating

This fixes bsc#1236816

How to test

Start a Micro 5.5 instance, run transactional-update migration , choose any of the available options, i.e.

Available migrations:

    1 |SUSE Linux Micro 6.1 x86_64

    2 |SUSE Linux Micro 6.0 x86_64


[num/q]: 1

Migration should succeed

Change Type

Please select the correct option.

  • Bug Fix (a non-breaking change which fixes an issue)
  • New Feature (a non-breaking change which adds new functionality)
  • Documentation Update (a change which only updates documentation)

Checklist

Please check off each item if the requirement is met.

  • I have reviewed my own code and believe that it's ready for an external review.
  • I have provided comments for any hard-to-understand code.
  • I have documented the MANUAL.md file with any changes to the user experience.
  • If my changes are non-trivial, I have added a changelog entry to notify users at package/obs/rmt-server.changes.

Review

Please check out our review guidelines
and get in touch with the author to get a shared understanding of the change.

Because of the change from SLE Micro < 6.0 to SL Micro 6+
comparing the identifier no longer works when migrating

This fixes bsc#1236816
@jesusbv jesusbv self-assigned this Feb 5, 2025
@jesusbv jesusbv added the 2.22 label Feb 5, 2025
@@ -122,7 +122,7 @@ def verify_base_product_upgrade

activated_bases = @system.products.where(product_type: 'base')
activated_bases.each do |base_product|
return true if (base_product.identifier == upgrade_product.identifier)
return true if base_product.product_class == upgrade_product.product_class
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe base_product.identifier == upgrade_product.identifier || base_product.product_class == upgrade_product.product_class is more safe.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that option but isn't product_identifer matching a subset of product_class matching ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The product identifier can change across versions (as happened with Micro), but also the product class can change. So the suggested condition would be more safe.
In the end what we do here is a hack because it's not based on the subscription. In SCC, the migration would check if the target product is included in the system's subscription.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so, similar to the issue with SUMA ARM64, a better check on subscription is recommended/encouraged

I will do that instead of the double hack (product identifier + product class)

Thoughts ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, it would be useful if you have a way also for PAYG to find out the used subscription, to use it's settings to determine which products a system should have access to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the fix/new check would be for PAYG as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants