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

redhat_subscription: stop manual unsubscribing on unregistration #9578

Conversation

ptoscano
Copy link
Contributor

SUMMARY

Unregistering a system also drops all the resources for it automatically, so there is no need to manually unsubscribing (which actually means removing all the subscriptions).

In addition to that, newer versions of subscription-manager drop all the support for entitlements, so the "remove" subcommand (used by unsubscribe()) does not exist anymore, and thus the unregistration fails with those versions.

This fixes the registration on EL 10 systems, and Fedora 41 and greater.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

redhat_subscription

@ptoscano ptoscano force-pushed the redhat_subscription-no-remove-on-unregister branch from 1cf9888 to ad2e63f Compare January 16, 2025 10:28
@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module plugins plugin (any type) redhat_subscription tests tests unit tests/unit labels Jan 16, 2025
@richm
Copy link

richm commented Jan 16, 2025

Should the code be conditional then? e.g.

                if subman_version < X.Y:
                    rhsm.unsubscribe()
                rhsm.unregister()

?

That is - if I am a customer using this module to manage an EL9 and an EL10 system, how should I code my play to work correctly on both of those systems?

Am I correct in assuming that it is correct to do the rhsm.unsubscribe() when using earlier versions of subman, but incorrect when using later versions?

@ptoscano
Copy link
Contributor Author

Should the code be conditional then?

No, it shouldn't be needed, that's why I simply dropped the call.

@russoz
Copy link
Collaborator

russoz commented Jan 18, 2025

Should the code be conditional then?

No, it shouldn't be needed, that's why I simply dropped the call.

Hi @ptoscano would care to expand? The concern expressed by @richm seems legit: AFAICT this will break backward compatibility for people using the module in RHEL9 or earlier. But I have not tested that.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-9 Automatically create a backport for the stable-9 branch backport-10 Automatically create a backport for the stable-10 branch labels Jan 18, 2025
@ptoscano
Copy link
Contributor Author

Should the code be conditional then?

No, it shouldn't be needed, that's why I simply dropped the call.

Hi @ptoscano would care to expand? The concern expressed by @richm seems legit: AFAICT this will break backward compatibility for people using the module in RHEL9 or earlier.

No, it will not break anything. Unregistering removes all the bits of a system from the registration server, so removing some manually first is a redundant step.

This is even in very old RHEL's documentation:
https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/5/html/deployment_guide/un-registering#un-registering

The only thing required to unregister a machine is to run the unregister command. This removes the system's entry from the subscription service, removes any subscriptions, and, locally, deletes its identity and subscription certificates.

Emphasis mine on "removes any subscriptions", which is what subscription-manager removes --all (removed by this patch) does.

Unregistering a system also drops all the resources for it
automatically, so there is no need to manually unsubscribing (which
actually means removing all the subscriptions).

In addition to that, newer versions of subscription-manager drop all the
support for entitlements, so the "remove" subcommand (used by
unsubscribe()) does not exist anymore, and thus the unregistration fails
with those versions.

This fixes the registration on EL 10 systems, and Fedora 41 and greater.
@ptoscano ptoscano force-pushed the redhat_subscription-no-remove-on-unregister branch from ad2e63f to 795e5c7 Compare January 20, 2025 14:23
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

Thanks for clarifying. In that case, it LGTM.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jan 20, 2025
@felixfontein felixfontein merged commit bcc92e8 into ansible-collections:main Jan 20, 2025
138 checks passed
Copy link

patchback bot commented Jan 20, 2025

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/bcc92e8aac4397117e14386ff462c7399bd7db6e/pr-9578

Backported as #9589

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 20, 2025
Unregistering a system also drops all the resources for it
automatically, so there is no need to manually unsubscribing (which
actually means removing all the subscriptions).

In addition to that, newer versions of subscription-manager drop all the
support for entitlements, so the "remove" subcommand (used by
unsubscribe()) does not exist anymore, and thus the unregistration fails
with those versions.

This fixes the registration on EL 10 systems, and Fedora 41 and greater.

(cherry picked from commit bcc92e8)
@felixfontein
Copy link
Collaborator

@ptoscano thanks for your contribution!
@richm @russoz thanks for reviewing!

Copy link

patchback bot commented Jan 20, 2025

Backport to stable-10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-10/bcc92e8aac4397117e14386ff462c7399bd7db6e/pr-9578

Backported as #9590

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 20, 2025
Unregistering a system also drops all the resources for it
automatically, so there is no need to manually unsubscribing (which
actually means removing all the subscriptions).

In addition to that, newer versions of subscription-manager drop all the
support for entitlements, so the "remove" subcommand (used by
unsubscribe()) does not exist anymore, and thus the unregistration fails
with those versions.

This fixes the registration on EL 10 systems, and Fedora 41 and greater.

(cherry picked from commit bcc92e8)
felixfontein pushed a commit that referenced this pull request Jan 20, 2025
…al unsubscribing on unregistration (#9589)

redhat_subscription: stop manual unsubscribing on unregistration (#9578)

Unregistering a system also drops all the resources for it
automatically, so there is no need to manually unsubscribing (which
actually means removing all the subscriptions).

In addition to that, newer versions of subscription-manager drop all the
support for entitlements, so the "remove" subcommand (used by
unsubscribe()) does not exist anymore, and thus the unregistration fails
with those versions.

This fixes the registration on EL 10 systems, and Fedora 41 and greater.

(cherry picked from commit bcc92e8)

Co-authored-by: Pino Toscano <[email protected]>
felixfontein pushed a commit that referenced this pull request Jan 20, 2025
…ual unsubscribing on unregistration (#9590)

redhat_subscription: stop manual unsubscribing on unregistration (#9578)

Unregistering a system also drops all the resources for it
automatically, so there is no need to manually unsubscribing (which
actually means removing all the subscriptions).

In addition to that, newer versions of subscription-manager drop all the
support for entitlements, so the "remove" subcommand (used by
unsubscribe()) does not exist anymore, and thus the unregistration fails
with those versions.

This fixes the registration on EL 10 systems, and Fedora 41 and greater.

(cherry picked from commit bcc92e8)

Co-authored-by: Pino Toscano <[email protected]>
@ptoscano ptoscano deleted the redhat_subscription-no-remove-on-unregister branch January 21, 2025 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-9 Automatically create a backport for the stable-9 branch backport-10 Automatically create a backport for the stable-10 branch bug This issue/PR relates to a bug module module plugins plugin (any type) redhat_subscription tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants