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

alternatives: Fix bug with priority default #4810

Merged
merged 6 commits into from
Jun 13, 2022

Conversation

jiuka
Copy link
Contributor

@jiuka jiuka commented Jun 8, 2022

If neigther the priority nor the subcommands where specified the module decided to update the priority with the default value anyway. This resulted in the bug #4803 and #4804 when the subcommands have not been specified.

SUMMARY

Do not set the priority if the default value if the priority is not set.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

alternatives

ADDITIONAL INFORMATION

If neigther the priority nor the subcommands where specified the module decided to update the priority with the default value anyway. This resulted in bug ansible-collections#4803 and ansible-collections#4804
@ansibullbot
Copy link
Collaborator

cc @mulby
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug integration tests/integration module module plugins plugin (any type) system tests tests labels Jun 8, 2022
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.

Thanks! Please add a changelog fragment.

Copy link
Contributor

@pilou- pilou- left a comment

Choose a reason for hiding this comment

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

I added a very last nitpick but LGTM.

@felixfontein
Copy link
Collaborator

Hmm, when looking at the fix and the issues, I don't think this fixes #4803 or #4804. The problem seems to be that link for the subcommands is not correctly identified. For example, on RHEL 9, I get

$ alternatives --display man
man - status is auto.
 link currently points to /usr/bin/man.man-db
/usr/bin/man.man-db - priority 300
 slave apropos: /usr/bin/apropos.man-db
 slave whatis: /usr/bin/whatis.man-db
 slave man.1.gz: /usr/share/man/man1/man.man-db.1.gz
 slave apropos.1.gz: /usr/share/man/man1/apropos.man-db.1.gz
 slave whatis.1.gz: /usr/share/man/man1/whatis.man-db.1.gz
Current `best' version is /usr/bin/man.man-db.

This gives:

>>> alternative_regex.findall(out)
[('/usr/bin/man.man-db', '300', '\n slave apropos: /usr/bin/apropos.man-db\n slave whatis: /usr/bin/whatis.man-db\n slave man.1.gz: /usr/share/man/man1/man.man-db.1.gz\n slave apropos.1.gz: /usr/share/man/man1/apropos.man-db.1.gz\n slave whatis.1.gz: /usr/share/man/man1/whatis.man-db.1.gz')]
>>> path, prio, subcmd = alternative_regex.findall(out)[0]
>>> path
'/usr/bin/man.man-db'
>>> prio
'300'
>>> subcmd
'\n slave apropos: /usr/bin/apropos.man-db\n slave whatis: /usr/bin/whatis.man-db\n slave man.1.gz: /usr/share/man/man1/man.man-db.1.gz\n slave apropos.1.gz: /usr/share/man/man1/apropos.man-db.1.gz\n slave whatis.1.gz: /usr/share/man/man1/whatis.man-db.1.gz'
>>> subcmd_regex.findall(subcmd)
[('apropos', '/usr/bin/apropos.man-db'), ('whatis', '/usr/bin/whatis.man-db'), ('man.1.gz', '/usr/share/man/man1/man.man-db.1.gz'), ('apropos.1.gz', '/usr/share/man/man1/apropos.man-db.1.gz'), ('whatis.1.gz', '/usr/share/man/man1/whatis.man-db.1.gz')]
>>> subcmd_path_link_regex = re.compile(r'^\s*slave (\S+) is (.*)$', re.MULTILINE)
>>> subcmd_path_link_regex.findall(out)
[]

Thus subcmd_path_map is an empty map, and

        for path, prio, subcmd in alternative_regex.findall(display_output):
            self.current_alternatives[path] = dict(
                priority=int(prio),
                subcommands=[dict(
                    name=name,
                    path=spath,
                    link=subcmd_path_map.get(name)
                ) for name, spath in subcmd_regex.findall(subcmd) if spath != '(null)']
            )

produces dictionaries with link=None. The None entries are removed by module.run_command(), resulting in the missing options reported in the issues.

The output was a bit more interesting on RHEL8.5 we use in CI for python3:

$ alternatives --display python3
python3 - status is auto.
 link currently points to /usr/bin/python3.6
/usr/bin/python3.6 - priority 1000000
 slave easy_install-3: /usr/bin/easy_install-3.6
 slave pip-3: /usr/bin/pip-3.6
 slave pip3: /usr/bin/pip3.6
 slave pydoc-3: /usr/bin/pydoc3.6
 slave pydoc3: /usr/bin/pydoc3.6
 slave python3-config: /usr/bin/python3.6-config
 slave pyvenv-3: /usr/bin/pyvenv-3.6
 slave python3-man: /usr/share/man/man1/python3.6.1.gz
/usr/bin/python3.8 - priority 3800
 slave easy_install-3: /usr/bin/easy_install-3.8
 slave pip-3: /usr/bin/pip-3.8
 slave pip3: /usr/bin/pip3.8
 slave pydoc-3: /usr/bin/pydoc3.8
 slave pydoc3: /usr/bin/pydoc3.8
 slave python3-config: /usr/bin/python3.8-config
 slave pyvenv-3: (null)
 slave python3-man: /usr/share/man/man1/python3.8.1.gz
Current `best' version is /usr/bin/python3.6.

The output on Ubuntu 20.04 and 22.04 is different:

$ update-alternatives --display java
java - auto mode
  link best version is /usr/lib/jvm/java-11-openjdk-amd64/bin/java
  link currently points to /usr/lib/jvm/java-11-openjdk-amd64/bin/java
  link java is /usr/bin/java
  slave java.1.gz is /usr/share/man/man1/java.1.gz
/usr/lib/jvm/java-11-openjdk-amd64/bin/java - priority 1111
  slave java.1.gz: /usr/lib/jvm/java-11-openjdk-amd64/man/man1/java.1.gz
/usr/lib/jvm/java-8-openjdk-amd64/jre/bin/java - priority 1081
  slave java.1.gz: /usr/lib/jvm/java-8-openjdk-amd64/jre/man/man1/java.1.gz

So the problem here isn't the default value for priority, but that on RHEL the module cannot identify the links it needs.

@felixfontein
Copy link
Collaborator

Ok, so this does fix a bug, but not the bug reported in the issues. Let's merge this anyway, but we need a fix for these issues as well...

@felixfontein felixfontein merged commit 57e83ac into ansible-collections:main Jun 13, 2022
@patchback
Copy link

patchback bot commented Jun 13, 2022

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/57e83ac80b727c472d0c3911d9189ae667feb110/pr-4810

Backported as #4835

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

@felixfontein
Copy link
Collaborator

Let's fix the issues in another PR.

patchback bot pushed a commit that referenced this pull request Jun 13, 2022
* alternatives: Fix bug with priority default

If neigther the priority nor the subcommands where specified the module decided to update the priority with the default value anyway. This resulted in bug #4803 and #4804

* Add changelog fragment.

* Distinguish None from 0.

* Address review comments.

* Update plugins/modules/system/alternatives.py

Co-authored-by: Pilou <[email protected]>

* Remove unrelated issues from changelog.

Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: Pilou <[email protected]>
(cherry picked from commit 57e83ac)
This was referenced Nov 20, 2022
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 has_issue integration tests/integration module module plugins plugin (any type) system tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants