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.

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

Fix alternatives module #4836

Merged
merged 3 commits into from
Jun 14, 2022
Merged

Fix alternatives module #4836

merged 3 commits into from
Jun 14, 2022

Conversation

felixfontein
Copy link
Collaborator

SUMMARY

Fixes #4803, fixes #4804.

CC @jiuka

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

alternatives

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Jun 13, 2022
This was referenced Jun 13, 2022
Closed
Closed
@felixfontein
Copy link
Collaborator Author

I think that idempotence of the module probably has some problems on RHEL (see #4810 (comment)), but this PR should at least make the module work again when subcommands are not used.

@ansibullbot
Copy link
Collaborator

cc @mulby
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug has_issue module module plugins plugin (any type) system labels Jun 13, 2022
felixfontein
felixfontein commented Jun 13, 2022
View reviewed changes
plugins/modules/system/alternatives.py
@@ -77,6 +77,7 @@
description:
- The path to the symbolic link that should point to the real subcommand executable.
type: path
required: true
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When link wasn't specified, the module generated an invalid argument for alternatives and failed. So this change shouldn't break anything that was working.

@felixfontein
Copy link
Collaborator Author

I did some quick tests with RHEL 8.5 and python3, and it seems to work fine.

@pilou-
Copy link
Contributor

pilou- commented Jun 13, 2022

I did some quick tests with RHEL 8.5 and python3, and it seems to work fine.

Should not this have been caught by the alternatives integration tests run within the CI when subcommands was added (#4654)?

@felixfontein
Copy link
Collaborator Author

The tests do not cover this since they never switch between alternatives when subcommands are present.

@ansibullbot ansibullbot added integration tests/integration tests tests needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 14, 2022
@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 14, 2022
@felixfontein
Copy link
Collaborator Author

The tests should now cover that. I also added some more debug output so that when they start failing in the future, its (hopefully) easier to figure out what goes wrong.

@felixfontein felixfontein merged commit 84d8ca9 into ansible-collections:main Jun 14, 2022
@patchback
Copy link

patchback bot commented Jun 14, 2022 •
edited
Loading

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/84d8ca9234bee2e3926fbc64f6558d8704085fb5/pr-4836

Backported as #4840

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

@felixfontein felixfontein deleted the alternatives branch June 14, 2022 14:02
patchback bot pushed a commit that referenced this pull request Jun 14, 2022
@felixfontein @patchback
Fix alternatives module (#4836)
2a441a0 
* Only pass subcommands when they are specified as module arguments.

* When 'subcommands' is specified, 'link' must be given for every subcommand.

* Extend subcommand tests.

(cherry picked from commit 84d8ca9)
@patchback patchback bot mentioned this pull request Jun 14, 2022
Merged
@felixfontein
Copy link
Collaborator Author

@pilou- thanks a lot for reviewing and testing this!

This was referenced Nov 20, 2022
Closed
Merged
Closed
Closed
Closed
Closed
Merged
Merged
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Open
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Dec 1, 2022
Open
Closed
@renovate renovate bot mentioned this pull request Dec 14, 2022
Closed
1 task
@renovate renovate bot mentioned this pull request Jan 1, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug This issue/PR relates to a bug has_issue integration tests/integration module module plugins plugin (any type) system tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Alternatives subcommand is missing link if not passed explicitly 5.1.0 breaks alternatives on RHEL 8
3 participants
@felixfontein @ansibullbot @pilou-