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

Fix #5313: redhat_subscription module is not idempotent when pool_ids are used #5319

Conversation

cfiehe
Copy link
Contributor

@cfiehe cfiehe commented Sep 29, 2022

SUMMARY

This fix ensures the idempotency of the redhat_subscription module when pool_ids are used. The main problem was, that a 'None' quantity was not properly handled and that the quantity check compared a string with an integer.

Signed-off-by: Christoph Fiehe [email protected]

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

redhat_subscription

ADDITIONAL INFORMATION

@cfiehe
Copy link
Contributor Author

cfiehe commented Sep 29, 2022

Sorry, I had some trouble with rebasing the branch. I have modified the changelog fragment and created a new pull request.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug 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 os packaging plugins plugin (any type) labels Sep 29, 2022
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-4 labels Sep 29, 2022
@felixfontein
Copy link
Collaborator

Continues #5314.

@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Sep 29, 2022
@cfiehe
Copy link
Contributor Author

cfiehe commented Oct 2, 2022

Yes, I think you are right. We can remove the first conditional part quantity == 0 and use quantity is not None and quantity != quantity_used solely 👍. Of course, quantity == 0 implies quantity is not None so the check quantity != quantity_used can also handle this case.

@cfiehe
Copy link
Contributor Author

cfiehe commented Oct 2, 2022

Should I modify the pull request?

@felixfontein
Copy link
Collaborator

I think so.

@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Oct 2, 2022
@cfiehe cfiehe force-pushed the rhsm_fix_idempotency_with_pool_ids branch from e89e1e6 to 7512089 Compare October 2, 2022 20:08
@cfiehe
Copy link
Contributor Author

cfiehe commented Oct 2, 2022

I think so.

Done 🙂.

@ansibullbot ansibullbot removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Oct 2, 2022
@cfiehe cfiehe force-pushed the rhsm_fix_idempotency_with_pool_ids branch from 7512089 to d2d4d2a Compare October 2, 2022 20:42
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

I think this looks correct now. I assume you did also test the latest version whether it really does what it should :)

I'll merge tomorrow evening if nobody complains by then.

@cfiehe
Copy link
Contributor Author

cfiehe commented Oct 2, 2022

I think this looks correct now. I assume you did also test the latest version whether it really does what it should :)

I must test it tomorrow. RHEL testing is always more complicated. Please wait for my feedback.

@cfiehe
Copy link
Contributor Author

cfiehe commented Oct 3, 2022

I have tested the modifications. They work like a charm 😊.

Thanks for your support.

@cfiehe cfiehe force-pushed the rhsm_fix_idempotency_with_pool_ids branch from d2d4d2a to 4cec80e Compare October 3, 2022 07:17
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Oct 3, 2022
@cfiehe cfiehe force-pushed the rhsm_fix_idempotency_with_pool_ids branch from 4cec80e to 4dc6790 Compare October 3, 2022 08:15
@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Oct 3, 2022
…otent when pool_ids

This fix ensures the idempotency of the redhat_subscription module when pool_ids are used. The main problem was, that a 'None' quantity was not properly handled and that the quantity check compared a string with an integer.

Signed-off-by: Christoph Fiehe <[email protected]>
@cfiehe cfiehe force-pushed the rhsm_fix_idempotency_with_pool_ids branch from 4dc6790 to 1e66d0f Compare October 3, 2022 08:21
@cfiehe cfiehe changed the title Fix #5313: redhat_subscription module is not idempotent when pool_ids Fix #5313: redhat_subscription module is not idempotent when pool_ids are used Oct 3, 2022
@felixfontein felixfontein merged commit 6fe2a84 into ansible-collections:main Oct 3, 2022
@patchback
Copy link

patchback bot commented Oct 3, 2022

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/6fe2a84e8733c4f27d6aca7b77dbdb9bb6e173be/pr-5319

Backported as #5329

🤖 @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 Oct 3, 2022
…#5319)

This fix ensures the idempotency of the redhat_subscription module when pool_ids are used. The main problem was, that a 'None' quantity was not properly handled and that the quantity check compared a string with an integer.

Signed-off-by: Christoph Fiehe <[email protected]>

Signed-off-by: Christoph Fiehe <[email protected]>
Co-authored-by: Christoph Fiehe <[email protected]>
(cherry picked from commit 6fe2a84)
@felixfontein
Copy link
Collaborator

@cfiehe thanks for your contribution!

@patchback
Copy link

patchback bot commented Oct 3, 2022

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/6fe2a84e8733c4f27d6aca7b77dbdb9bb6e173be/pr-5319

Backported as #5330

🤖 @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 Oct 3, 2022
…#5319)

This fix ensures the idempotency of the redhat_subscription module when pool_ids are used. The main problem was, that a 'None' quantity was not properly handled and that the quantity check compared a string with an integer.

Signed-off-by: Christoph Fiehe <[email protected]>

Signed-off-by: Christoph Fiehe <[email protected]>
Co-authored-by: Christoph Fiehe <[email protected]>
(cherry picked from commit 6fe2a84)
@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Oct 3, 2022
felixfontein pushed a commit that referenced this pull request Oct 3, 2022
…#5319) (#5329)

This fix ensures the idempotency of the redhat_subscription module when pool_ids are used. The main problem was, that a 'None' quantity was not properly handled and that the quantity check compared a string with an integer.

Signed-off-by: Christoph Fiehe <[email protected]>

Signed-off-by: Christoph Fiehe <[email protected]>
Co-authored-by: Christoph Fiehe <[email protected]>
(cherry picked from commit 6fe2a84)

Co-authored-by: cfiehe <[email protected]>
felixfontein pushed a commit that referenced this pull request Oct 3, 2022
…#5319) (#5330)

This fix ensures the idempotency of the redhat_subscription module when pool_ids are used. The main problem was, that a 'None' quantity was not properly handled and that the quantity check compared a string with an integer.

Signed-off-by: Christoph Fiehe <[email protected]>

Signed-off-by: Christoph Fiehe <[email protected]>
Co-authored-by: Christoph Fiehe <[email protected]>
(cherry picked from commit 6fe2a84)

Co-authored-by: cfiehe <[email protected]>
v1v added a commit to v1v/community.general that referenced this pull request Oct 7, 2022
* upstream/main: (203 commits)
  Make pfexec become usable for illumos (ansible-collections#3889)
  znode: add options for authentication (ansible-collections#5306)
  keycloak_user_federation: add explanation and example to vendor option (ansible-collections#4893)
  Next expected release is 5.8.0.
  Allow terraform module to specify complex variable structures (ansible-collections#4797)
  Fix ansible-collections#5313: redhat_subscription module is not idempotent when pool_ids (ansible-collections#5319)
  bitwarden: Add field to search for all item attributes, instead of on… (ansible-collections#5297)
  New Module: Keycloak User Rolemapping (ansible-collections#4898)
  chore: Update lxc_container to support py3 (ansible-collections#5304)
  terraform: run `init` with no-color, too (ansible-collections#5147)
  nmcli: fix error when setting previously unset mac address (ansible-collections#5291)
  [feat] proxmox_snap: snapshot containers with configured mountpoints (ansible-collections#5274)
  machinectl: include the success command (ansible-collections#5287)
  Add SetSessionService to redfish_config (ansible-collections#5009)
  locale_gen: fix UbuntuMode (ansible-collections#5282)
  ini_file: fix lint error (ansible-collections#5307)
  netcup_dnsapi: Add timeout paramter (ansible-collections#5301)
  stable-2.14 is now default.
  Add stable-2.14 to CI, adjust to devel version bump (ansible-collections#5298)
  Try to run reuse workflow without explicitly allowing it for new contributors. (ansible-collections#5296)
  ...
bratwurzt pushed a commit to bratwurzt/community.general that referenced this pull request Nov 7, 2022
…otent when pool_ids (ansible-collections#5319)

This fix ensures the idempotency of the redhat_subscription module when pool_ids are used. The main problem was, that a 'None' quantity was not properly handled and that the quantity check compared a string with an integer.

Signed-off-by: Christoph Fiehe <[email protected]>

Signed-off-by: Christoph Fiehe <[email protected]>
Co-authored-by: Christoph Fiehe <[email protected]>
bratwurzt pushed a commit to bratwurzt/community.general that referenced this pull request Nov 7, 2022
…otent when pool_ids (ansible-collections#5319)

This fix ensures the idempotency of the redhat_subscription module when pool_ids are used. The main problem was, that a 'None' quantity was not properly handled and that the quantity check compared a string with an integer.

Signed-off-by: Christoph Fiehe <[email protected]>

Signed-off-by: Christoph Fiehe <[email protected]>
Co-authored-by: Christoph Fiehe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor os packaging plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants