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

Improve ansible-galaxy handling of role versions #12904

Merged
merged 2 commits into from
Jan 17, 2017

Conversation

willthames
Copy link
Contributor

Ensure that role versions are considered when deciding
whether or not to (re-)install a role.

Issue a warning when the version of a dependency conflicts
with the version of an already installed role

Display what version of a role is being installed
Show the versions when upgrading/downgrading a role.

Implements #11266

[will@dingo tmp]$ ansible-galaxy install -r ~/src/ansible/test/integration/galaxy_roles.yml -p roles
- downloading role 'oracle_java7', owned by briancoca
- downloading role from https://github.com/bcoca/ansible-oracle_java7-role/archive/v1.0.tar.gz
- extracting oracle_java7 to roles/oracle_java7
- oracle_java7 (v1.0) was installed successfully
- extracting git-ansible-galaxy to roles/git-ansible-galaxy
- git-ansible-galaxy (v1.6) was installed successfully
- adding dependency: git-gal-dep (v0.2)
- extracting hg-ansible-galaxy to roles/hg-ansible-galaxy
- hg-ansible-galaxy was installed successfully
- downloading role from https://bitbucket.org/willthames/http-ansible-galaxy/get/master.tar.gz
- extracting http-role to roles/http-role
- http-role was installed successfully
- extracting php to roles/php
- php was installed successfully
- extracting git-gal-dep to roles/git-gal-dep
- git-gal-dep (v0.2) was installed successfully
[will@dingo tmp]$ ansible-galaxy install -r ~/src/ansible/test/integration/galaxy_roles.yml -p roles
- changing role oracle_java7 from v1.0 to unspecified
- downloading role 'oracle_java7', owned by briancoca
- downloading role from https://github.com/bcoca/ansible-oracle_java7-role/archive/v1.0.tar.gz
- extracting oracle_java7 to roles/oracle_java7
- oracle_java7 (v1.0) was installed successfully
- git-ansible-galaxy (v1.6) is already installed, skipping.
- hg-ansible-galaxy is already installed, skipping.
- http-role is already installed, skipping.
- php is already installed, skipping.

Note that the behaviour with galaxy roles with an unspecified version is a little odd. This is because meta/.galaxy_install_info gets told the version of the role that actually gets installed when installing under galaxy. However, if you're not fixing versions in your rolesfile, then it's fine to expect that the role will be reinstalled each time you run ansible-galaxy install. (This is similar to the behaviour of yum state=latest, for example). One fix for this would be for .galaxy_install_info to be told the version of the role specified, rather than the version of the role installed.

"""
if self.version:
return "%s (%s)" % (self.name, self.version)
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

imho, this else is not usefull ?.

if self.version:
    return xxxx
return xxxx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Lujeni meh, it's really not that important. I prefer it the way it is, and you're entitled to your preference.

Unless you can point to actual documentation backing up your preferences though (PEP-8, Ansible coding standards, etc.), I'm unlikely to change my code.

@chouseknecht
Copy link
Contributor

+1

@willthames willthames mentioned this pull request Apr 16, 2016
@geerlingguy
Copy link
Contributor

This is helpful towards solving both #11266 and ansible/galaxy-issues#49

@@ -318,3 +318,14 @@ def spec(self):
}
"""
return dict(scm=self.scm, src=self.src, version=self.version, name=self.name)

@property
def version_string(self):
Copy link
Member

Choose a reason for hiding this comment

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

not really version_string .. full_name? name_and_version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this can be __repr__ rather than version_string :)

@willthames
Copy link
Contributor Author

@bcoca should be ready for re-review now, I believe I've addressed your concerns.

Ensure that role versions are considered when deciding
whether or not to (re-)install a role.

Issue a warning when the version of a dependency conflicts
with the version of an already installed role

Display what version of a role is being installed
Show the versions when upgrading/downgrading a role.

Implements ansible#11266
Ensure that force is required to change role versions
@ansibot ansibot added affects_2.3 This issue/PR affects Ansible v2.3 needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 13, 2016
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Dec 17, 2016
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 29, 2016
@swalkinshaw
Copy link

👋 apologies for the bump, but this PR deserves some attention as it implements a much requested feature.

@bcoca bcoca merged commit 5ee5593 into ansible:devel Jan 17, 2017
@geerlingguy
Copy link
Contributor

Ooh, it's merged!

mscherer added a commit to mscherer/ansible that referenced this pull request Apr 7, 2017
In current stable (2.2), ansible galaxy install --force do erase
a role, even if the version is not set. This commit should restore
that specific behavior, in accordance to people reports:
  ansible#11266 (comment)

It was also the behavior planned in the initial discussion:
"if you're not fixing versions in your roles file, then it's fine
to expect that the role will be reinstalled each time you run
ansible-galaxy install.", cf ansible#12904
chouseknecht pushed a commit that referenced this pull request Apr 21, 2017
In current stable (2.2), ansible galaxy install --force do erase
a role, even if the version is not set. This commit should restore
that specific behavior, in accordance to people reports:
  #11266 (comment)

It was also the behavior planned in the initial discussion:
"if you're not fixing versions in your roles file, then it's fine
to expect that the role will be reinstalled each time you run
ansible-galaxy install.", cf #12904
abadger pushed a commit that referenced this pull request Jun 7, 2017
In current stable (2.2), ansible galaxy install --force do erase
a role, even if the version is not set. This commit should restore
that specific behavior, in accordance to people reports:
  #11266 (comment)

It was also the behavior planned in the initial discussion:
"if you're not fixing versions in your roles file, then it's fine
to expect that the role will be reinstalled each time you run
ansible-galaxy install.", cf #12904
(cherry picked from commit 78836ec)
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.3 This issue/PR affects Ansible v2.3 c:cli/ c:galaxy/ feature This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants