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

Change how alternatives states check for installed #30017

Merged
merged 1 commit into from Dec 28, 2015

Conversation

Projects
None yet
4 participants
@anlutro
Contributor

anlutro commented Dec 26, 2015

salt.states.alternatives.installed calls salt.modules.alternatives.check_installed to check if an alternative is already installed. However, this module function only returns True if the given alternative is the currently symlinked alternative.

If I have a system with both vim and vim-tiny installed, I want to have a alternatives.installed state for both of them - but I can't, because the check_install function assumes I want both of them to be the currently selected alternative.

I'm not 100% sure if this is a breaking change or a bugfix. I currently have a state that uses alternatives.installed and it reports a change every time I run the highstate. If this can be considered a bugfix, it should be backported to 2015.8.

@anlutro

This comment has been minimized.

Show comment
Hide comment
@anlutro

anlutro Dec 26, 2015

Contributor

Alternatively, I could create a new alternatives.present state which does update-alternatives --install, and change the existing alternatives.installed state to run update-alternatives --set, but that would be somewhat confusing terminology in my opinion.

Contributor

anlutro commented Dec 26, 2015

Alternatively, I could create a new alternatives.present state which does update-alternatives --install, and change the existing alternatives.installed state to run update-alternatives --set, but that would be somewhat confusing terminology in my opinion.

@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Dec 28, 2015

Contributor

I think you make a good argument that this is a bugfix. Let's merge this and backport it.

Contributor

cachedout commented Dec 28, 2015

I think you make a good argument that this is a bugfix. Let's merge this and backport it.

cachedout added a commit that referenced this pull request Dec 28, 2015

Merge pull request #30017 from alprs/fix-alternatives_install
Change how alternatives states check for installed

@cachedout cachedout merged commit 1ffae15 into saltstack:develop Dec 28, 2015

4 of 5 checks passed

default Merged build finished.
Details
jenkins/salt-pr-clone Salt PR - Clone Repository #12358 — SUCCESS
Details
jenkins/salt-pr-linode-ubuntu14.04-n Salt PR - Linode Ubuntu 14.04 #3422 — SUCCESS
Details
jenkins/salt-pr-lint-n Salt PR - Code Lint #12060 — SUCCESS
Details
jenkins/salt-pr-rs-cent7-n Salt PR - RS CentOS 7 #10941 — SUCCESS
Details

cachedout added a commit that referenced this pull request Dec 28, 2015

@rickbradshaw

This comment has been minimized.

Show comment
Hide comment
@rickbradshaw

rickbradshaw Feb 26, 2016

this "fix" actually breaks on CentOS because --list doesn't take any arguments. --display will take an argument though.

[INFO    ] Running state [ld] at time 11:35:34.786627
[INFO    ] Executing state alternatives.install for ld
[INFO    ] Executing command ['alternatives', '--list', 'ld'] in directory '/root'
[ERROR   ] Command ['alternatives', '--list', 'ld'] failed with return code: 2
[ERROR   ] stdout: alternatives version 1.3.61 - Copyright (C) 2001 Red Hat, Inc.
This may be freely redistributed under the terms of the GNU Public License.

usage: alternatives --install <link> <name> <path> <priority>
                    [--initscript <service>]
                    [--slave <link> <name> <path>]*
       alternatives --remove <name> <path>
       alternatives --auto <name>
       alternatives --config <name>
       alternatives --display <name>
       alternatives --set <name> <path>
       alternatives --list

common options: --verbose --test --help --usage --version
                --altdir <directory> --admindir <directory>
[ERROR   ] retcode: 2
[INFO    ] Executing command ['alternatives', '--install', '/usr/bin/ld', 'ld', '/usr/bin/ld.gold', '100'] in directory '/root'
[INFO    ] {'priority': 100, 'path': '/usr/bin/ld.gold', 'link': '/usr/bin/ld', 'name': 'ld'}
[INFO    ] Completed state [ld] at time 11:35:34.807549

I should also note that the --install doesn't seem to do the trick either as after the run ld is still pointing at the wrong binary.

#the command from above. ( INFO log )
[root@ckx-bld-commit01 ~]# alternatives --install /usr/bin/ld ld /usr/bin/ld.gold 100
[root@ckx-bld-commit01 ~]# ls -al /etc/alternatives/ld
lrwxrwxrwx 1 root root 15 Feb 26 11:35 /etc/alternatives/ld -> /usr/bin/ld.bfd
[root@ckx-bld-commit01 ~]# alternatives --display ld
ld - status is manual.
 link currently points to /usr/bin/ld.bfd
/usr/bin/ld.bfd - priority 50
/usr/bin/ld.gold - priority 100
Current `best' version is /usr/bin/ld.gold.

[root@ckx-bld-commit01 ~]# rpm -qa | grep salt
salt-2015.8.5-1.el7.noarch
salt-minion-2015.8.5-1.el7.noarch

so it is updating the alternatives info, but not actually changing the symlink?

rickbradshaw commented Feb 26, 2016

this "fix" actually breaks on CentOS because --list doesn't take any arguments. --display will take an argument though.

[INFO    ] Running state [ld] at time 11:35:34.786627
[INFO    ] Executing state alternatives.install for ld
[INFO    ] Executing command ['alternatives', '--list', 'ld'] in directory '/root'
[ERROR   ] Command ['alternatives', '--list', 'ld'] failed with return code: 2
[ERROR   ] stdout: alternatives version 1.3.61 - Copyright (C) 2001 Red Hat, Inc.
This may be freely redistributed under the terms of the GNU Public License.

usage: alternatives --install <link> <name> <path> <priority>
                    [--initscript <service>]
                    [--slave <link> <name> <path>]*
       alternatives --remove <name> <path>
       alternatives --auto <name>
       alternatives --config <name>
       alternatives --display <name>
       alternatives --set <name> <path>
       alternatives --list

common options: --verbose --test --help --usage --version
                --altdir <directory> --admindir <directory>
[ERROR   ] retcode: 2
[INFO    ] Executing command ['alternatives', '--install', '/usr/bin/ld', 'ld', '/usr/bin/ld.gold', '100'] in directory '/root'
[INFO    ] {'priority': 100, 'path': '/usr/bin/ld.gold', 'link': '/usr/bin/ld', 'name': 'ld'}
[INFO    ] Completed state [ld] at time 11:35:34.807549

I should also note that the --install doesn't seem to do the trick either as after the run ld is still pointing at the wrong binary.

#the command from above. ( INFO log )
[root@ckx-bld-commit01 ~]# alternatives --install /usr/bin/ld ld /usr/bin/ld.gold 100
[root@ckx-bld-commit01 ~]# ls -al /etc/alternatives/ld
lrwxrwxrwx 1 root root 15 Feb 26 11:35 /etc/alternatives/ld -> /usr/bin/ld.bfd
[root@ckx-bld-commit01 ~]# alternatives --display ld
ld - status is manual.
 link currently points to /usr/bin/ld.bfd
/usr/bin/ld.bfd - priority 50
/usr/bin/ld.gold - priority 100
Current `best' version is /usr/bin/ld.gold.

[root@ckx-bld-commit01 ~]# rpm -qa | grep salt
salt-2015.8.5-1.el7.noarch
salt-minion-2015.8.5-1.el7.noarch

so it is updating the alternatives info, but not actually changing the symlink?

@anlutro anlutro deleted the alprs:fix-alternatives_install branch Mar 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment