8000 Skip attach operation for OS versions that no longer support attach by dvanderveer · Pull Request #10254 · ansible-collections/community.general · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Skip attach operation for OS versions that no longer support attach #10254

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Skip attach operation for OS versions that no longer support attach #10254

wants to merge 11 commits into from

Conversation

dvanderveer
Copy lin 8000 k
@dvanderveer dvanderveer commented Jun 15, 2025 •
edited
Loading
SUMMARY

Skip the attach operation for OS versions where the subscription-manager attach sub-command has been removed.
Fixes #10253.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

redhat_subscription

ADDITIONAL INFORMATION

Before the change, the module would fail with the error seen in #10253 when registering an RHEL 10.0 host with auto_attach enabled. With the change, registration succeeds on RHEL 10 with auto_attach enabled. Also confirmed that registration of an RHEL 9.x host still works as before. Post-registration, confirmed that subscription-manager identity showed org ID and that yum update worked correctly on both RHEL 9.x and 10.0 test hosts.

@ansibullbot
Copy link
Collaborator

cc @cnsnyder @ptoscano
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type) redhat_subscription labels Jun 15, 2025
@dvanderveer
Copy link
Author
dvanderveer commented Jun 15, 2025 •
edited
Loading

From inspecting RHEL 9.x and 10.x hosts, I believe that subscription-manager version 1.29 was the last to include the attach and remove sub-commands.

RHEL 9.x:

# rpm -q subscription-manager
subscription-manager-1.29.42-1.el9.x86_64

RHEL 10.0:

# rpm -q subscription-manager
subscription-manager-1.30.6-1.el10_0.x86_64

I don't have easy access to Fedora hosts for testing, so I'm basing the Fedora version in this PR on what I can see from package lists for Fedora mirrors. Fedora 40 mirrors show subscription-manager-1.29.40-2.fc40.x86_64.rpm, while Fedora 41 mirrors show subscription-manager-1.30.2-1.fc41.x86_64.rpm. Based on those version numbers, I'm inferring that Fedora 41+ will behave the same as RHEL 10.

@ansibullbot

This comment was marked as outdated.

Sign in to view
@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 15, 2025
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 15, 2025
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-10 Automatically create a backport for the stable-10 branch labels Jun 16, 2025
@felixfontein felixfontein added the backport-11 Automatically create a backport for the stable-10 branch label Jun 16, 2025
@dvanderveer dvanderveer changed the title Skip attach operation for OS versions that no longer support it Skip attach and remove operations for OS versions that no longer support them Jun 16, 2025
ptoscano
ptoscano suggested changes Jun 17, 2025
View reviewed changes
Copy link
Contributor
@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

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

Thanks @dvanderveer for the PR!

I have concerns about the changes recently added here about the unregistration; see my notes here and in #10260.

Regarding the changes for auto_attach: the general idea is OK, there are few things to improve (I left notes).

Thanks!

@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 17, 2025
@ansibullbot ansibullbot added the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI label Jun 17, 2025
@dvanderveer dvanderveer changed the title Skip attach and remove operations for OS versions that no longer support them Skip attach operation for OS versions that no longer support it Jun 17, 2025
@dvanderveer dvanderveer changed the title Skip attach operation for OS versions that no longer support it Skip attach operation for OS versions that no longer support attach Jun 17, 2025
@ansibullbot ansibullbot removed the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI label Jun 17, 2025
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 17, 2025
@dvanderveer dvanderveer requested a review from ptoscano June 17, 2025 18:12
felixfontein
felixfontein reviewed Jun 17, 2025
View reviewed changes
@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 17, 2025
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 17, 2025
russoz
russoz reviewed Jun 18, 2025
View reviewed changes
@ptoscano
Copy link
Contributor

Hi @dvanderveer,

I noticed your changes, I haven't forgot about you :)

The version check for auto_attach needs to be properly tested in both cases (i.e. "old" and "new" OSes), as it changes which commands are actually called (see the list of cases & commands expected in tests/unit/plugins/modules/test_redhat_subscription.py). The problem here is that we need to mock also distro, and the current framework in the unit test does not support that. It was not a problem until now because all the version checks are done only when registering via D-Bus, which is not unit-tested at all (yes, I know, it's my fault here; trying to test & mock it was a nightmare, and I cannot find my old failed experiments for it). Also, in general the version situation is a bit of a mess: it started easily, and sadly it needed to be used more and more, and it's my fault again here for not having found a cleaner way.

Please give me some days that I want to think about the testing situation for this a bit more, and see what I can come up with.

@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@russoz russoz russoz left review comments

794A @felixfontein felixfontein felixfontein left review comments

@ptoscano ptoscano Awaiting requested review from ptoscano

Assignees
No one assigned
Labels
backport-10 Automatically create a backport for the stable-10 branch backport-11 Automatically create a backport for the stable-10 branch bug This issue/PR relates to a bug check-before-release PR will be looked at again shortly before release and merged if possible. module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor plugins plugin (any type) redhat_subscription
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

redhat_subscription fails on RHEL 10 when auto_attach is true
5 participants
@dvanderveer @ansibullbot @ptoscano @russoz @felixfontein
0