From 46b59b02edbcbd1257c77f096028325c0f7dbc8b Mon Sep 17 00:00:00 2001 From: Will Thames Date: Fri, 15 Aug 2014 21:19:40 +1000 Subject: [PATCH] Friendly Role Names and roles from URLs * Roles can now be given a friendly name as third field in role spec csv * Roles can be installed from URL (not just from archived SCMs) * Integration tests to demonstrate this * Unit tests to ensure that role spec parsing works as expected --- bin/ansible-galaxy | 93 ++++++++++++---------------- lib/ansible/playbook/play.py | 19 ++---- lib/ansible/utils/__init__.py | 23 +++++++ test/integration/galaxy_playbook.yml | 1 + test/integration/galaxy_rolesfile | 3 +- test/units/TestUtils.py | 9 +++ 6 files changed, 81 insertions(+), 67 deletions(-) diff --git a/bin/ansible-galaxy b/bin/ansible-galaxy index a5fd6e1ae64..afc2d9de85d 100755 --- a/bin/ansible-galaxy +++ b/bin/ansible-galaxy @@ -332,13 +332,12 @@ def api_get_list(api_server, what): # scm repo utility functions #------------------------------------------------------------------------------------- -def scm_archive_role(scm, role_url, role_version): +def scm_archive_role(scm, role_url, role_version, role_name): if scm not in ['hg', 'git']: print "SCM %s is not currently supported" % scm return False tempdir = tempfile.mkdtemp() - role_name = ansible.utils.repo_url_to_role_name(role_url) - clone_cmd = [scm, 'clone', role_url] + clone_cmd = [scm, 'clone', role_url, role_name] with open('/dev/null', 'w') as devnull: popen = subprocess.Popen(clone_cmd, cwd=tempdir, stdout=devnull, stderr=devnull) rc = popen.wait() @@ -464,7 +463,10 @@ def fetch_role(role_name, target, role_data, options): """ # first grab the file and save it to a temp location - archive_url = 'https://github.com/%s/%s/archive/%s.tar.gz' % (role_data["github_user"], role_data["github_repo"], target) + if '://' in role_name: + archive_url = role_name + else: + archive_url = 'https://github.com/%s/%s/archive/%s.tar.gz' % (role_data["github_user"], role_data["github_repo"], target) print " - downloading role from %s" % archive_url try: @@ -706,39 +708,24 @@ def execute_install(args, options, parser): while len(roles_left) > 0: # query the galaxy API for the role data - role_name = roles_left.pop(0).strip() - role_version = None + (scm, role_src, role_version, role_name) = ansible.utils.role_spec_parse(roles_left.pop(0)) + role_data = None - if role_name == "" or role_name.startswith("#"): - continue - elif ',' in role_name: - role_name,role_version = role_name.split(',',1) - role_name = role_name.strip() - role_version = role_version.strip() - - if os.path.isfile(role_name): + if os.path.isfile(role_src): # installing a local tar.gz - tar_file = role_name - role_name = os.path.basename(role_name).replace('.tar.gz','') - if tarfile.is_tarfile(tar_file): - print " - installing %s as %s" % (tar_file, role_name) - if not install_role(role_name, role_version, tar_file, options): - exit_without_ignore(options) - else: - print "%s (%s) was NOT installed successfully." % (role_name,tar_file) - exit_without_ignore(options) + tmp_file = role_src else: - if '+' in role_name: - (scm, role_url) = role_name.split('+',1) - role_name = ansible.utils.repo_url_to_role_name(role_name) + if scm: # create tar file from scm url - tmp_file = scm_archive_role(scm, role_url, role_version) - role_data = None + tmp_file = scm_archive_role(scm, role_src, role_version, role_name) + elif '://' in role_src: + # just download a URL - version will probably be in the URL + tmp_file = fetch_role(role_src, None, None, options) else: - # installing remotely - role_data = api_lookup_role_by_name(api_server, role_name) + # installing from galaxy + role_data = api_lookup_role_by_name(api_server, role_src) if not role_data: - print "Sorry, %s was not found on %s." % (role_name, api_server) + print "Sorry, %s was not found on %s." % (role_src, api_server) continue role_versions = api_fetch_role_related(api_server, 'versions', role_data['id']) @@ -762,29 +749,29 @@ def execute_install(args, options, parser): # download the role. if --no-deps was specified, we stop here, # otherwise we recursively grab roles and all of their deps. - tmp_file = fetch_role(role_name, role_version, role_data, options) - if tmp_file and install_role(role_name, role_version, tmp_file, options): - # we're done with the temp file, clean it up - os.unlink(tmp_file) - # install dependencies, if we want them - if not no_deps: - if not role_data: - role_data = get_role_metadata(role_name, options) - role_dependencies = role_data['dependencies'] + tmp_file = fetch_role(role_src, role_version, role_data, options) + if tmp_file and install_role(role_name, role_version, tmp_file, options): + # we're done with the temp file, clean it up + os.unlink(tmp_file) + # install dependencies, if we want them + if not no_deps: + if not role_data: + role_data = get_role_metadata(role_name, options) + role_dependencies = role_data['dependencies'] + else: + role_dependencies = role_data['summary_fields']['dependencies'] # api_fetch_role_related(api_server, 'dependencies', role_data['id']) + for dep_name in role_dependencies: + #dep_name = "%s.%s" % (dep['owner'], dep['name']) + if not get_role_metadata(dep_name.split('/')[-1], options): + print ' adding dependency: %s' % dep_name + roles_left.append(dep_name) else: - role_dependencies = role_data['summary_fields']['dependencies'] # api_fetch_role_related(api_server, 'dependencies', role_data['id']) - for dep_name in role_dependencies: - #dep_name = "%s.%s" % (dep['owner'], dep['name']) - if not get_role_metadata(dep_name.split('/')[-1], options): - print ' adding dependency: %s' % dep_name - roles_left.append(dep_name) - else: - print ' dependency %s is already installed, skipping.' % dep_name - else: - if tmp_file: - os.unlink(tmp_file) - print "%s was NOT installed successfully." % role_name - exit_without_ignore(options) + print ' dependency %s is already installed, skipping.' % dep_name + else: + if tmp_file: + os.unlink(tmp_file) + print "%s was NOT installed successfully." % role_name + exit_without_ignore(options) sys.exit(0) def execute_remove(args, options, parser): diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index c4323f52d04..cdbab8c59e9 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -178,13 +178,6 @@ class Play(object): returns any variables that were included with the role """ orig_path = template(self.basedir,role,self.vars) - if '+' in orig_path and '://' in orig_path: - # dependency name pointing to SCM URL - # assume role name is last part of the URL - orig_path = utils.repo_url_to_role_name(orig_path) - if ',' in orig_path: - # version information for role dependency used by galaxy - orig_path = orig_path.split(',')[0] role_vars = {} if type(orig_path) == dict: @@ -193,20 +186,21 @@ class Play(object): if role_name is None: raise errors.AnsibleError("expected a role name in dictionary: %s" % orig_path) role_vars = orig_path - orig_path = role_name + else: + (scm, role_src, role_version, role_name) = utils.role_spec_parse(orig_path) role_path = None possible_paths = [ - utils.path_dwim(self.basedir, os.path.join('roles', orig_path)), - utils.path_dwim(self.basedir, orig_path) + utils.path_dwim(self.basedir, os.path.join('roles', role_name)), + utils.path_dwim(self.basedir, role_name) ] if C.DEFAULT_ROLES_PATH: search_locations = C.DEFAULT_ROLES_PATH.split(os.pathsep) for loc in search_locations: loc = os.path.expanduser(loc) - possible_paths.append(utils.path_dwim(loc, orig_path)) + possible_paths.append(utils.path_dwim(loc, role_name)) for path_option in possible_paths: if os.path.isdir(path_option): @@ -419,8 +413,7 @@ class Play(object): role_name = role['role'] else: role_name = role - if '+' in role_name and '://' in role_name: - role_name = utils.repo_url_to_role_name(role_name) + (scm, role_src, role_version, role_name) = utils.role_spec_parse(role_name) role_names.append(role_name) if os.path.isfile(task): diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index af2bfa1919d..65dc80150b5 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -356,10 +356,33 @@ def repo_url_to_role_name(repo_url): trailing_path = repo_url.split('/')[-1] if trailing_path.endswith('.git'): trailing_path = trailing_path[:-4] + if trailing_path.endswith('.tar.gz'): + trailing_path = trailing_path[:-7] if ',' in trailing_path: trailing_path = trailing_path.split(',')[0] return trailing_path + +def role_spec_parse(role_spec): + role_spec = role_spec.strip() + role_version = '' + if role_spec == "" or role_spec.startswith("#"): + return (None, None, None, None) + tokens = map(lambda s: s.strip(), role_spec.split(',')) + if '+' in tokens[0]: + (scm, role_url) = tokens[0].split('+') + else: + scm = None + role_url = tokens[0] + if len(tokens) >= 2: + role_version = tokens[1] + if len(tokens) == 3: + role_name = tokens[2] + else: + role_name = repo_url_to_role_name(tokens[0]) + return (scm, role_url, role_version, role_name) + + def json_loads(data): ''' parse a JSON string and return a data structure ''' diff --git a/test/integration/galaxy_playbook.yml b/test/integration/galaxy_playbook.yml index 1d9b03b22a2..d7adebf3400 100644 --- a/test/integration/galaxy_playbook.yml +++ b/test/integration/galaxy_playbook.yml @@ -3,3 +3,4 @@ roles: - "git-ansible-galaxy" + - "http-role" diff --git a/test/integration/galaxy_rolesfile b/test/integration/galaxy_rolesfile index 43fb49e9243..01aa27a52ac 100644 --- a/test/integration/galaxy_rolesfile +++ b/test/integration/galaxy_rolesfile @@ -1,2 +1,3 @@ -git+http://bitbucket.org/willthames/git-ansible-galaxy,v1.3 +git+http://bitbucket.org/willthames/git-ansible-galaxy,v1.4 hg+ssh://hg@bitbucket.org/willthames/hg-ansible-galaxy +https://bitbucket.org/willthames/http-ansible-galaxy/get/master.tar.gz,,http-role diff --git a/test/units/TestUtils.py b/test/units/TestUtils.py index 8593ca3de2c..0c7bc4c4c90 100644 --- a/test/units/TestUtils.py +++ b/test/units/TestUtils.py @@ -784,3 +784,12 @@ class TestUtils(unittest.TestCase): ("ssh://git@git.example.com:repos/role-name,v0.1", "role-name")] for (url, result) in tests: self.assertEqual(ansible.utils.repo_url_to_role_name(url), result) + + def test_role_spec_parse(self): + tests = [("git+http://git.example.com/repos/repo.git,v1.0", ('git', 'http://git.example.com/repos/repo.git', 'v1.0', 'repo')), + ("http://repo.example.com/download/tarfile.tar.gz", (None, 'http://repo.example.com/download/tarfile.tar.gz', '', 'tarfile')), + ("http://repo.example.com/download/tarfile.tar.gz,,nicename", (None, 'http://repo.example.com/download/tarfile.tar.gz', '', 'nicename')), + ("git+http://git.example.com/repos/repo.git,v1.0,awesome", ('git', 'http://git.example.com/repos/repo.git', 'v1.0', 'awesome'))] + for (spec, result) in tests: + self.assertEqual(ansible.utils.role_spec_parse(spec), result) +