From 494043947d174296335f4b649412344cd90148c3 Mon Sep 17 00:00:00 2001 From: smoothify Date: Tue, 20 Aug 2013 10:11:39 +0100 Subject: [PATCH 01/15] Add support for role defaults. These are variables on a per role basis with lowest precedence. --- lib/ansible/playbook/play.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 03216fe1f14..aeb95558111 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -183,6 +183,20 @@ class Play(object): dep_stack.append([role,role_path,role_vars]) return dep_stack + def _load_role_defaults(self, defaults_files): + # process default variables + default_vars = {} + for filename in defaults_files: + if os.path.exists(filename): + new_default_vars = utils.parse_yaml_from_file(filename) + if new_default_vars: + if type(new_default_vars) != dict: + raise errors.AnsibleError("%s must be stored as dictonary/hash: %s" % (filename, type(new_default_vars))) + + default_vars = utils.combine_vars(default_vars, new_default_vars) + + return default_vars + def _load_roles(self, roles, ds): # a role is a name that auto-includes the following if they exist # /tasks/main.yml @@ -199,6 +213,7 @@ class Play(object): new_tasks = [] new_handlers = [] new_vars_files = [] + defaults_files = [] pre_tasks = ds.get('pre_tasks', None) if type(pre_tasks) != list: @@ -222,10 +237,13 @@ class Play(object): task_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'tasks')) handler_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'handlers')) vars_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'vars')) + defaults_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'defaults')) task = self._resolve_main(task_basepath) handler = self._resolve_main(handler_basepath) vars_file = self._resolve_main(vars_basepath) + defaults_file = self._resolve_main(defaults_basepath) + library = utils.path_dwim(self.basedir, os.path.join(role_path, 'library')) if not os.path.isfile(task) and not os.path.isfile(handler) and not os.path.isfile(vars_file) and not os.path.isdir(library): @@ -244,6 +262,8 @@ class Play(object): new_handlers.append(nt) if os.path.isfile(vars_file): new_vars_files.append(vars_file) + if os.path.isfile(defaults_file): + defaults_files.append(defaults_file) if os.path.isdir(library): utils.plugins.module_finder.add_directory(library) @@ -275,6 +295,11 @@ class Play(object): ds['handlers'] = new_handlers ds['vars_files'] = new_vars_files + defaults = self._load_role_defaults(defaults_files) + # merge default vars with self.vars, with vars taking precedence. + if defaults: + self.vars = utils.combine_vars(defaults, self.vars) + return ds # ************************************************* From 461858e476f31aa764018549348d3a818e649f80 Mon Sep 17 00:00:00 2001 From: Raul Melo Date: Tue, 27 Aug 2013 16:17:33 +0200 Subject: [PATCH 02/15] Fix issue 3908. There was some ilegal operations over the sets --- library/system/user | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/library/system/user b/library/system/user index 26b16f0ccf7..49dfcac9eca 100644 --- a/library/system/user +++ b/library/system/user @@ -1273,20 +1273,24 @@ class AIX(User): if self.groups is not None: current_groups = self.user_group_membership() - groups = self.get_groups_set() - group_diff = set(current_groups).symmetric_difference(groups) groups_need_mod = False + groups = [] - if group_diff: - if self.append: - for g in groups: - if g in group_diff: - groups.extend(current_groups) - set(groups) - groups_need_mod = True - break - else: + if self.groups == '': + if current_groups and not self.append: groups_need_mod = True + else: + groups = self.get_groups_set() + group_diff = set(current_groups).symmetric_difference(groups) + + if group_diff: + if self.append: + for g in groups: + if g in group_diff: + groups_need_mod = True + break + else: + groups_need_mod = True if groups_need_mod: cmd.append('-G') From faf82bf84184872f25f27e6e2e65d766384f5127 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 28 Aug 2013 13:22:24 -0500 Subject: [PATCH 03/15] Fix bug with fetch when using sudo: true Fixes #3111 --- lib/ansible/runner/action_plugins/fetch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/runner/action_plugins/fetch.py b/lib/ansible/runner/action_plugins/fetch.py index 4f8de7e9dc2..a07db8162c8 100644 --- a/lib/ansible/runner/action_plugins/fetch.py +++ b/lib/ansible/runner/action_plugins/fetch.py @@ -73,7 +73,7 @@ class ActionModule(object): # use slurp if sudo and permissions are lacking remote_data = None - if remote_md5 in ('1', '2') and self.runner.sudo: + if remote_md5 in ('1', '2') or self.runner.sudo: slurpres = self.runner._execute_module(conn, tmp, 'slurp', 'src=%s' % source, inject=inject) if slurpres.is_successful(): if slurpres.result['encoding'] == 'base64': From 993413e706797f4889e374b0fa89e0f38d1dcf14 Mon Sep 17 00:00:00 2001 From: Darragh O'Reilly Date: Wed, 28 Aug 2013 21:19:24 +0100 Subject: [PATCH 04/15] quantum_network: fix some doc mistakes - tenant_name was missing. - comments were on wrong tasks. - error message had a reference to glance. --- library/cloud/quantum_network | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/library/cloud/quantum_network b/library/cloud/quantum_network index 42261e1a0d7..c0e901197ff 100644 --- a/library/cloud/quantum_network +++ b/library/cloud/quantum_network @@ -44,6 +44,11 @@ options: - The tenant name of the login user required: true default: 'yes' + tenant_name: + description: + - The name of the tenant for whom the network is created + required: false + default: None auth_url: description: - The keystone url for authentication @@ -99,15 +104,15 @@ requirements: ["quantumclient", "keystoneclient"] ''' EXAMPLES = ''' -# Creates an external,public network -- quantum_network: state=present login_username=admin login_password=admin - provider_network_type=gre login_tenant_name=admin - provider_segmentation_id=1 tenant_name=tenant1 name=t1network" +# Create a GRE backed Quantum network with tunnel id 1 for tenant1 +- quantum_network: name=t1network tenant_name=tenant1 state=present + provider_network_type=gre provider_segmentation_id=1 + login_username=admin login_password=admin login_tenant_name=admin -# Createss a GRE nework with tunnel id of 1 for tenant 1 -- quantum_network: state=present login_username=admin login_password=admin - provider_network_type=local login_tenant_name=admin - provider_segmentation_id=1 router_external=yes name=external_network +# Create an external network +- quantum_network: name=external_network state=present + provider_network_type=local router_external=yes + login_username=admin login_password=admin login_tenant_name=admin ''' _os_keystone = None @@ -130,7 +135,7 @@ def _get_endpoint(module, ksclient): try: endpoint = ksclient.service_catalog.url_for(service_type='network', endpoint_type='publicURL') except Exception as e: - module.fail_json(msg = "Error getting endpoint for glance: %s " %e.message) + module.fail_json(msg = "Error getting endpoint for Quantum: %s " %e.message) return endpoint def _get_quantum_client(module, kwargs): From 9806f89a049e48a0c8606830be589954cf8b3f32 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Thu, 29 Aug 2013 10:59:34 -0500 Subject: [PATCH 05/15] Revert "Construct the multiprocessing manager only once." This reverts commit 1d13ec2da358b3198f78370a0fe02e1ee9893c64. --- lib/ansible/runner/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index 45e6cb8d07a..1d993d4cc30 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -55,7 +55,6 @@ multiprocessing_runner = None OUTPUT_LOCKFILE = tempfile.TemporaryFile() PROCESS_LOCKFILE = tempfile.TemporaryFile() -MULTIPROCESSING_MANAGER = multiprocessing.Manager() ################################################ @@ -830,7 +829,7 @@ class Runner(object): def _parallel_exec(self, hosts): ''' handles mulitprocessing when more than 1 fork is required ''' - manager = MULTIPROCESSING_MANAGER + manager = multiprocessing.Manager() job_queue = manager.Queue() for host in hosts: job_queue.put(host) From 637d3070dc1cd7ad801635df77b1b5d1fb530cd3 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Thu, 29 Aug 2013 17:21:28 -0500 Subject: [PATCH 06/15] Allow default variables to be overridden by inventory variables --- lib/ansible/playbook/__init__.py | 4 ++-- lib/ansible/playbook/play.py | 9 +++------ lib/ansible/playbook/task.py | 7 ++++--- lib/ansible/runner/__init__.py | 4 ++++ 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/ansible/playbook/__init__.py b/lib/ansible/playbook/__init__.py index 3f9130e1539..b8bc24b059a 100644 --- a/lib/ansible/playbook/__init__.py +++ b/lib/ansible/playbook/__init__.py @@ -307,7 +307,7 @@ class PlayBook(object): remote_pass=self.remote_pass, module_path=self.module_path, timeout=self.timeout, remote_user=task.play.remote_user, remote_port=task.play.remote_port, module_vars=task.module_vars, - private_key_file=self.private_key_file, + default_vars=task.default_vars, private_key_file=self.private_key_file, setup_cache=self.SETUP_CACHE, basedir=task.play.basedir, conditional=task.only_if, callbacks=self.runner_callbacks, sudo=task.sudo, sudo_user=task.sudo_user, @@ -447,7 +447,7 @@ class PlayBook(object): remote_pass=self.remote_pass, remote_port=play.remote_port, private_key_file=self.private_key_file, setup_cache=self.SETUP_CACHE, callbacks=self.runner_callbacks, sudo=play.sudo, sudo_user=play.sudo_user, transport=play.transport, sudo_pass=self.sudo_pass, is_playbook=True, module_vars=play.vars, - check=self.check, diff=self.diff + default_vars=play.default_vars, check=self.check, diff=self.diff ).run() self.stats.compute(setup_results, setup=True) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index d204980b9da..68563d655ae 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -28,7 +28,7 @@ import os class Play(object): __slots__ = [ - 'hosts', 'name', 'vars', 'vars_prompt', 'vars_files', + 'hosts', 'name', 'vars', 'default_vars', 'vars_prompt', 'vars_files', 'handlers', 'remote_user', 'remote_port', 'sudo', 'sudo_user', 'transport', 'playbook', 'tags', 'gather_facts', 'serial', '_ds', '_handlers', '_tasks', @@ -70,7 +70,7 @@ class Play(object): self.tags = [] ds = self._load_roles(self.roles, ds) - self.vars_files = ds.get('vars_files', []) + self.vars_files = ds.get('vars_files', []) self._update_vars_files_for_host(None) @@ -295,10 +295,7 @@ class Play(object): ds['handlers'] = new_handlers ds['vars_files'] = new_vars_files - defaults = self._load_role_defaults(defaults_files) - # merge default vars with self.vars, with vars taking precedence. - if defaults: - self.vars = utils.combine_vars(defaults, self.vars) + self.default_vars = self._load_role_defaults(defaults_files) return ds diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index a4f4911fba7..6ff409eab00 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -24,7 +24,7 @@ class Task(object): __slots__ = [ 'name', 'meta', 'action', 'only_if', 'when', 'async_seconds', 'async_poll_interval', - 'notify', 'module_name', 'module_args', 'module_vars', + 'notify', 'module_name', 'module_args', 'module_vars', 'default_vars', 'play', 'notified_by', 'tags', 'register', 'delegate_to', 'first_available_file', 'ignore_errors', 'local_action', 'transport', 'sudo', 'sudo_user', 'sudo_pass', @@ -100,8 +100,9 @@ class Task(object): elif not x in Task.VALID_KEYS: raise errors.AnsibleError("%s is not a legal parameter in an Ansible task or handler" % x) - self.module_vars = module_vars - self.play = play + self.module_vars = module_vars + self.play = play + self.default_vars = play.default_vars # load various attributes self.name = ds.get('name', None) diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index 45e6cb8d07a..63e99c7a947 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -131,6 +131,7 @@ class Runner(object): sudo=False, # whether to run sudo or not sudo_user=C.DEFAULT_SUDO_USER, # ex: 'root' module_vars=None, # a playbooks internals thing + default_vars=None, # ditto is_playbook=False, # running from playbook or not? inventory=None, # reference to Inventory object subset=None, # subset pattern @@ -159,6 +160,7 @@ class Runner(object): self.inventory = utils.default(inventory, lambda: ansible.inventory.Inventory(host_list)) self.module_vars = utils.default(module_vars, lambda: {}) + self.default_vars = utils.default(default_vars, lambda: {}) self.always_run = None self.connector = connection.Connection(self) self.conditional = conditional @@ -405,6 +407,7 @@ class Runner(object): port = self.remote_port inject = {} + inject = utils.combine_vars(inject, self.default_vars) inject = utils.combine_vars(inject, host_variables) inject = utils.combine_vars(inject, self.module_vars) inject = utils.combine_vars(inject, self.setup_cache[host]) @@ -413,6 +416,7 @@ class Runner(object): inject['group_names'] = host_variables.get('group_names', []) inject['groups'] = self.inventory.groups_list() inject['vars'] = self.module_vars + inject['defaults'] = self.default_vars inject['environment'] = self.environment if self.inventory.basedir() is not None: From 25e3eed519b723a0c8017c67dee1eda35bcbb0b6 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Fri, 30 Aug 2013 00:32:20 -0500 Subject: [PATCH 07/15] Fixing a bug in variable precedence for roles and dependencies --- lib/ansible/playbook/play.py | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 68563d655ae..8a13902cb0c 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -69,9 +69,17 @@ class Play(object): elif type(self.tags) != list: self.tags = [] - ds = self._load_roles(self.roles, ds) + # We first load the vars files from the datastructure + # so we have the default variables to pass into the roles self.vars_files = ds.get('vars_files', []) + self._update_vars_files_for_host(None) + # now we load the roles into the datastructure + ds = self._load_roles(self.roles, ds) + + # and finally re-process the vars files as they may have + # been updated by the included roles + self.vars_files = ds.get('vars_files', []) self._update_vars_files_for_host(None) # template everything to be efficient, but do not pre-mature template @@ -153,6 +161,13 @@ class Play(object): raise errors.AnsibleError("too many levels of recursion while resolving role dependencies") for role in roles: role_path,role_vars = self._get_role_path(role) + role_vars = utils.combine_vars(role_vars, passed_vars) + vars = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'vars'))) + vars_data = {} + if os.path.isfile(vars): + vars_data = utils.parse_yaml_from_file(vars) + if vars_data: + role_vars = utils.combine_vars(vars_data, role_vars) # the meta directory contains the yaml that should # hold the list of dependencies (if any) meta = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'meta'))) @@ -162,17 +177,14 @@ class Play(object): dependencies = data.get('dependencies',[]) for dep in dependencies: (dep_path,dep_vars) = self._get_role_path(dep) + dep_vars = utils.combine_vars(passed_vars, dep_vars) + dep_vars = utils.combine_vars(role_vars, dep_vars) vars = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(dep_path, 'vars'))) vars_data = {} if os.path.isfile(vars): vars_data = utils.parse_yaml_from_file(vars) - dep_vars.update(role_vars) - for k in passed_vars.keys(): - if not k in dep_vars: - dep_vars[k] = passed_vars[k] - for k in vars_data.keys(): - if not k in dep_vars: - dep_vars[k] = vars_data[k] + if vars_data: + dep_vars = utils.combine_vars(vars_data, dep_vars) if 'role' in dep_vars: del dep_vars['role'] self._build_role_dependencies([dep], dep_stack, passed_vars=dep_vars, level=level+1) From 736c8b19d350aa9d1160fa6d8e1d0bd12f387632 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Fri, 30 Aug 2013 11:35:35 -0500 Subject: [PATCH 08/15] Added ability to limit role dependencies to just one inclusion --- docsite/latest/rst/playbooks.rst | 18 +++++++++++++----- lib/ansible/playbook/play.py | 13 ++++++++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/docsite/latest/rst/playbooks.rst b/docsite/latest/rst/playbooks.rst index 3bd35fdc780..005c1fb52e3 100644 --- a/docsite/latest/rst/playbooks.rst +++ b/docsite/latest/rst/playbooks.rst @@ -568,10 +568,10 @@ Role dependencies can also be specified as a full path, just like top level role dependencies: - { role: '/path/to/common/roles/foo', x: 1 } -Roles dependencies are always executed before the role that includes them, and are recursive. - -Role dependencies may be included more than once. Continuing the above example, the 'car' role could -add 'wheel' dependencies as follows:: +Roles dependencies are always executed before the role that includes them, and are recursive. By default, +roles can also only be added as a dependency once - if another role also lists it as a dependency it will +not be run again. This behavior can be overridden by adding `allow_duplicates: yes` to the `meta/main.yml` file. +For example, a role named 'car' could add a role named 'wheel' to its dependencies as follows:: --- dependencies: @@ -580,7 +580,15 @@ add 'wheel' dependencies as follows:: - { role: wheel, n: 3 } - { role: wheel, n: 4 } -If the wheel role required tire and brake in turn, this would result in the following execution order:: +And the `meta/main.yml` for wheel contained the following:: + + --- + allow_duplicates: yes + dependencies: + - { role: tire } + - { role: brake } + +The resulting order of execution would be as follows:: tire(n=1) brake(n=1) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 8a13902cb0c..a7ca1236cff 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -29,7 +29,7 @@ class Play(object): __slots__ = [ 'hosts', 'name', 'vars', 'default_vars', 'vars_prompt', 'vars_files', - 'handlers', 'remote_user', 'remote_port', + 'handlers', 'remote_user', 'remote_port', 'included_roles', 'sudo', 'sudo_user', 'transport', 'playbook', 'tags', 'gather_facts', 'serial', '_ds', '_handlers', '_tasks', 'basedir', 'any_errors_fatal', 'roles', 'max_fail_pct' @@ -75,6 +75,7 @@ class Play(object): self._update_vars_files_for_host(None) # now we load the roles into the datastructure + self.included_roles = [] ds = self._load_roles(self.roles, ds) # and finally re-process the vars files as they may have @@ -177,6 +178,16 @@ class Play(object): dependencies = data.get('dependencies',[]) for dep in dependencies: (dep_path,dep_vars) = self._get_role_path(dep) + meta = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(dep_path, 'meta'))) + if os.path.isfile(meta): + meta_data = utils.parse_yaml_from_file(meta) + if meta_data: + allow_dupes = utils.boolean(meta_data.get('allow_duplicates','')) + if not allow_dupes: + if dep.get('role') in self.included_roles: + continue + else: + self.included_roles.append(dep.get('role')) dep_vars = utils.combine_vars(passed_vars, dep_vars) dep_vars = utils.combine_vars(role_vars, dep_vars) vars = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(dep_path, 'vars'))) From ad75350919cfb762fb25aa522468fe9db05f2bb1 Mon Sep 17 00:00:00 2001 From: Jeremy Price Date: Sat, 31 Aug 2013 17:58:37 -0400 Subject: [PATCH 09/15] adding regions_exclude parameter to be able to do subtractive region exclusion. Defaults to us-gov-west-1 so as to not cause errors for people who don't have govcloud credentials but get it in their regions list --- plugins/inventory/ec2.ini | 1 + plugins/inventory/ec2.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/inventory/ec2.ini b/plugins/inventory/ec2.ini index 3e4f08fb5f8..703f07081a5 100644 --- a/plugins/inventory/ec2.ini +++ b/plugins/inventory/ec2.ini @@ -12,6 +12,7 @@ # in AWS and merge the results together. Alternatively, set this to a comma # separated list of regions. E.g. 'us-east-1,us-west-1,us-west-2' regions = all +regions_exclude = us-gov-west-1 # When generating inventory, Ansible needs to know how to address a server. # Each EC2 instance has a lot of variables associated with it. Here is the list: diff --git a/plugins/inventory/ec2.py b/plugins/inventory/ec2.py index eb1d6595f6e..913ddb5f84a 100755 --- a/plugins/inventory/ec2.py +++ b/plugins/inventory/ec2.py @@ -189,12 +189,14 @@ class Ec2Inventory(object): # Regions self.regions = [] configRegions = config.get('ec2', 'regions') + configRegions_exclude = config.get('ec2', 'regions_exclude') if (configRegions == 'all'): if self.eucalyptus_host: self.regions.append(boto.connect_euca(host=self.eucalyptus_host).region.name) else: for regionInfo in ec2.regions(): - self.regions.append(regionInfo.name) + if regionInfo.name not in configRegions_exclude: + self.regions.append(regionInfo.name) else: self.regions = configRegions.split(",") From e0df5b58889d0d8551b556630d97b8a3debcc8f1 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Sun, 1 Sep 2013 00:37:12 -0500 Subject: [PATCH 10/15] A couple more tweaks to role default variables/dependencies * Default variables are now fed directly into roles, just like the other variables, so that roles see their unique values rather than those set at the global level. * Role dependency duplicates are now determined by checking the params used when specifying them as dependencies rather than just on the name of the role. For example, the following would be included twice without having to specify "allow_duplicates: true": dependencies: - { role: foo, x: 1 } - { role: foo, x: 2 } --- lib/ansible/playbook/play.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index a7ca1236cff..8a83d173081 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -169,6 +169,12 @@ class Play(object): vars_data = utils.parse_yaml_from_file(vars) if vars_data: role_vars = utils.combine_vars(vars_data, role_vars) + defaults = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'defaults'))) + defs_data = {} + if os.path.isfile(defaults): + defs_data = utils.parse_yaml_from_file(defaults) + if defs_data: + role_vars = utils.combine_vars(role_vars, defs_data) # the meta directory contains the yaml that should # hold the list of dependencies (if any) meta = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'meta'))) @@ -184,10 +190,10 @@ class Play(object): if meta_data: allow_dupes = utils.boolean(meta_data.get('allow_duplicates','')) if not allow_dupes: - if dep.get('role') in self.included_roles: + if dep in self.included_roles: continue else: - self.included_roles.append(dep.get('role')) + self.included_roles.append(dep) dep_vars = utils.combine_vars(passed_vars, dep_vars) dep_vars = utils.combine_vars(role_vars, dep_vars) vars = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(dep_path, 'vars'))) @@ -196,6 +202,12 @@ class Play(object): vars_data = utils.parse_yaml_from_file(vars) if vars_data: dep_vars = utils.combine_vars(vars_data, dep_vars) + defaults = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(dep_path, 'defaults'))) + defs_data = {} + if os.path.isfile(defaults): + defs_data = utils.parse_yaml_from_file(defaults) + if defs_data: + dep_vars = utils.combine_vars(dep_vars, defs_data) if 'role' in dep_vars: del dep_vars['role'] self._build_role_dependencies([dep], dep_stack, passed_vars=dep_vars, level=level+1) From 633eda957d061ec4629452234fc9b6e3a9f54803 Mon Sep 17 00:00:00 2001 From: kiri Date: Sat, 31 Aug 2013 12:33:50 +0900 Subject: [PATCH 11/15] fix JSON extra vars quotation. --- docsite/latest/rst/playbooks2.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docsite/latest/rst/playbooks2.rst b/docsite/latest/rst/playbooks2.rst index 5c006c0edfb..5b8a30b8c32 100644 --- a/docsite/latest/rst/playbooks2.rst +++ b/docsite/latest/rst/playbooks2.rst @@ -285,7 +285,7 @@ Example:: As of Ansible 1.2, you can also pass in extra vars as quoted JSON, like so:: - --extra-vars "{'pacman':'mrs','ghosts':['inky','pinky','clyde','sue']}" + --extra-vars '{"pacman":"mrs","ghosts":["inky","pinky","clyde","sue"]}' The key=value form is obviously simpler, but it's there if you need it! From 47a89a57fa05dda14ded2efc479ad88095371e0d Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Sun, 1 Sep 2013 08:49:41 -0500 Subject: [PATCH 12/15] Fixing bug in playbook use of default variables in roles --- lib/ansible/playbook/play.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 8a83d173081..6c697e684d3 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -174,7 +174,7 @@ class Play(object): if os.path.isfile(defaults): defs_data = utils.parse_yaml_from_file(defaults) if defs_data: - role_vars = utils.combine_vars(role_vars, defs_data) + role_vars = utils.combine_vars(defs_data, role_vars) # the meta directory contains the yaml that should # hold the list of dependencies (if any) meta = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'meta'))) @@ -207,7 +207,7 @@ class Play(object): if os.path.isfile(defaults): defs_data = utils.parse_yaml_from_file(defaults) if defs_data: - dep_vars = utils.combine_vars(dep_vars, defs_data) + dep_vars = utils.combine_vars(defs_data, dep_vars) if 'role' in dep_vars: del dep_vars['role'] self._build_role_dependencies([dep], dep_stack, passed_vars=dep_vars, level=level+1) From 266d2008d8a97494d159d8e3a907e6c01e12c8a6 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Sun, 1 Sep 2013 08:53:59 -0500 Subject: [PATCH 13/15] Reverting the role default variables change Loading the default variables in _build_role_dependencies() lead to a side-effect where those variables were over-riding inventory variables. --- lib/ansible/playbook/play.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 6c697e684d3..cca8acbfb67 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -169,12 +169,6 @@ class Play(object): vars_data = utils.parse_yaml_from_file(vars) if vars_data: role_vars = utils.combine_vars(vars_data, role_vars) - defaults = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'defaults'))) - defs_data = {} - if os.path.isfile(defaults): - defs_data = utils.parse_yaml_from_file(defaults) - if defs_data: - role_vars = utils.combine_vars(defs_data, role_vars) # the meta directory contains the yaml that should # hold the list of dependencies (if any) meta = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'meta'))) @@ -202,12 +196,6 @@ class Play(object): vars_data = utils.parse_yaml_from_file(vars) if vars_data: dep_vars = utils.combine_vars(vars_data, dep_vars) - defaults = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(dep_path, 'defaults'))) - defs_data = {} - if os.path.isfile(defaults): - defs_data = utils.parse_yaml_from_file(defaults) - if defs_data: - dep_vars = utils.combine_vars(defs_data, dep_vars) if 'role' in dep_vars: del dep_vars['role'] self._build_role_dependencies([dep], dep_stack, passed_vars=dep_vars, level=level+1) From 02b7b79d7e7ba8b923052ac7ca99fb35954129b2 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Sun, 1 Sep 2013 10:52:42 -0500 Subject: [PATCH 14/15] Re-adding capability of tasks to see a unique view of their own defaults --- lib/ansible/playbook/play.py | 33 +++++++++++++++++++++------------ lib/ansible/playbook/task.py | 4 ++-- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index cca8acbfb67..a1c621ef560 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -169,6 +169,10 @@ class Play(object): vars_data = utils.parse_yaml_from_file(vars) if vars_data: role_vars = utils.combine_vars(vars_data, role_vars) + defaults = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'defaults'))) + defs_data = {} + if os.path.isfile(defaults): + defs_data = utils.parse_yaml_from_file(defaults) # the meta directory contains the yaml that should # hold the list of dependencies (if any) meta = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'meta'))) @@ -196,14 +200,18 @@ class Play(object): vars_data = utils.parse_yaml_from_file(vars) if vars_data: dep_vars = utils.combine_vars(vars_data, dep_vars) + defaults = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(dep_path, 'defaults'))) + dep_defs_data = {} + if os.path.isfile(defaults): + dep_defs_data = utils.parse_yaml_from_file(defaults) if 'role' in dep_vars: del dep_vars['role'] self._build_role_dependencies([dep], dep_stack, passed_vars=dep_vars, level=level+1) - dep_stack.append([dep,dep_path,dep_vars]) + dep_stack.append([dep,dep_path,dep_vars,dep_defs_data]) # only add the current role when we're at the top level, # otherwise we'll end up in a recursive loop if level == 0: - dep_stack.append([role,role_path,role_vars]) + dep_stack.append([role,role_path,role_vars,defs_data]) return dep_stack def _load_role_defaults(self, defaults_files): @@ -249,7 +257,7 @@ class Play(object): roles = self._build_role_dependencies(roles, [], self.vars) - for role,role_path,role_vars in roles: + for (role,role_path,role_vars,def_vars) in roles: # special vars must be extracted from the dict to the included tasks special_keys = [ "sudo", "sudo_user", "when", "with_items" ] special_vars = {} @@ -257,10 +265,10 @@ class Play(object): if k in role_vars: special_vars[k] = role_vars[k] - task_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'tasks')) - handler_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'handlers')) - vars_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'vars')) - defaults_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'defaults')) + task_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'tasks')) + handler_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'handlers')) + vars_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'vars')) + defaults_basepath = utils.path_dwim(self.basedir, os.path.join(role_path, 'defaults')) task = self._resolve_main(task_basepath) handler = self._resolve_main(handler_basepath) @@ -272,7 +280,7 @@ class Play(object): if not os.path.isfile(task) and not os.path.isfile(handler) and not os.path.isfile(vars_file) and not os.path.isdir(library): raise errors.AnsibleError("found role at %s, but cannot find %s or %s or %s or %s" % (role_path, task, handler, vars_file, library)) if os.path.isfile(task): - nt = dict(include=pipes.quote(task), vars=role_vars) + nt = dict(include=pipes.quote(task), vars=role_vars, def_vars=def_vars) for k in special_keys: if k in special_vars: nt[k] = special_vars[k] @@ -342,7 +350,7 @@ class Play(object): # ************************************************* - def _load_tasks(self, tasks, vars={}, sudo_vars={}, additional_conditions=[], original_file=None): + def _load_tasks(self, tasks, vars={}, def_vars={}, sudo_vars={}, additional_conditions=[], original_file=None): ''' handle task and handler include statements ''' results = [] @@ -388,11 +396,12 @@ class Play(object): included_additional_conditions.insert(0, utils.compile_when_to_only_if("%s %s" % (k[5:], x[k]))) elif k == 'when': included_additional_conditions.insert(0, utils.compile_when_to_only_if("jinja2_compare %s" % x[k])) - elif k in ("include", "vars", "only_if", "sudo", "sudo_user"): + elif k in ("include", "vars", "def_vars", "only_if", "sudo", "sudo_user"): pass else: raise errors.AnsibleError("parse error: task includes cannot be used with other directives: %s" % k) + def_vars = utils.combine_vars(self.default_vars, x.get('def_vars', {})) if 'vars' in x: task_vars = utils.combine_vars(task_vars, x['vars']) if 'only_if' in x: @@ -410,9 +419,9 @@ class Play(object): include_file = template(dirname, tokens[0], mv) include_filename = utils.path_dwim(dirname, include_file) data = utils.parse_yaml_from_file(include_filename) - results += self._load_tasks(data, mv, included_sudo_vars, included_additional_conditions, original_file=include_filename) + results += self._load_tasks(data, mv, def_vars, included_sudo_vars, included_additional_conditions, original_file=include_filename) elif type(x) == dict: - results.append(Task(self,x,module_vars=task_vars, additional_conditions=additional_conditions)) + results.append(Task(self,x,module_vars=task_vars,default_vars=def_vars,additional_conditions=additional_conditions)) else: raise Exception("unexpected task type") diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 6ff409eab00..ba7599772e3 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -41,7 +41,7 @@ class Task(object): 'any_errors_fatal', 'changed_when', 'always_run' ] - def __init__(self, play, ds, module_vars=None, additional_conditions=None): + def __init__(self, play, ds, module_vars=None, default_vars=None, additional_conditions=None): ''' constructor loads from a task or handler datastructure ''' # meta directives are used to tell things like ansible/playbook to run @@ -101,8 +101,8 @@ class Task(object): raise errors.AnsibleError("%s is not a legal parameter in an Ansible task or handler" % x) self.module_vars = module_vars + self.default_vars = default_vars self.play = play - self.default_vars = play.default_vars # load various attributes self.name = ds.get('name', None) From 50f54f6bdad17896fe1a51b252ddf84a3069b730 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Mon, 2 Sep 2013 21:35:47 -0500 Subject: [PATCH 15/15] Making variable names more descriptive for the default variables work --- lib/ansible/playbook/play.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index a1c621ef560..040e0b6213c 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -170,9 +170,9 @@ class Play(object): if vars_data: role_vars = utils.combine_vars(vars_data, role_vars) defaults = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'defaults'))) - defs_data = {} + defaults_data = {} if os.path.isfile(defaults): - defs_data = utils.parse_yaml_from_file(defaults) + defaults_data = utils.parse_yaml_from_file(defaults) # the meta directory contains the yaml that should # hold the list of dependencies (if any) meta = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(role_path, 'meta'))) @@ -201,17 +201,17 @@ class Play(object): if vars_data: dep_vars = utils.combine_vars(vars_data, dep_vars) defaults = self._resolve_main(utils.path_dwim(self.basedir, os.path.join(dep_path, 'defaults'))) - dep_defs_data = {} + dep_defaults_data = {} if os.path.isfile(defaults): - dep_defs_data = utils.parse_yaml_from_file(defaults) + dep_defaults_data = utils.parse_yaml_from_file(defaults) if 'role' in dep_vars: del dep_vars['role'] self._build_role_dependencies([dep], dep_stack, passed_vars=dep_vars, level=level+1) - dep_stack.append([dep,dep_path,dep_vars,dep_defs_data]) + dep_stack.append([dep,dep_path,dep_vars,dep_defaults_data]) # only add the current role when we're at the top level, # otherwise we'll end up in a recursive loop if level == 0: - dep_stack.append([role,role_path,role_vars,defs_data]) + dep_stack.append([role,role_path,role_vars,defaults_data]) return dep_stack def _load_role_defaults(self, defaults_files): @@ -257,7 +257,7 @@ class Play(object): roles = self._build_role_dependencies(roles, [], self.vars) - for (role,role_path,role_vars,def_vars) in roles: + for (role,role_path,role_vars,default_vars) in roles: # special vars must be extracted from the dict to the included tasks special_keys = [ "sudo", "sudo_user", "when", "with_items" ] special_vars = {} @@ -280,7 +280,7 @@ class Play(object): if not os.path.isfile(task) and not os.path.isfile(handler) and not os.path.isfile(vars_file) and not os.path.isdir(library): raise errors.AnsibleError("found role at %s, but cannot find %s or %s or %s or %s" % (role_path, task, handler, vars_file, library)) if os.path.isfile(task): - nt = dict(include=pipes.quote(task), vars=role_vars, def_vars=def_vars) + nt = dict(include=pipes.quote(task), vars=role_vars, default_vars=default_vars) for k in special_keys: if k in special_vars: nt[k] = special_vars[k] @@ -350,7 +350,7 @@ class Play(object): # ************************************************* - def _load_tasks(self, tasks, vars={}, def_vars={}, sudo_vars={}, additional_conditions=[], original_file=None): + def _load_tasks(self, tasks, vars={}, default_vars={}, sudo_vars={}, additional_conditions=[], original_file=None): ''' handle task and handler include statements ''' results = [] @@ -396,12 +396,12 @@ class Play(object): included_additional_conditions.insert(0, utils.compile_when_to_only_if("%s %s" % (k[5:], x[k]))) elif k == 'when': included_additional_conditions.insert(0, utils.compile_when_to_only_if("jinja2_compare %s" % x[k])) - elif k in ("include", "vars", "def_vars", "only_if", "sudo", "sudo_user"): + elif k in ("include", "vars", "default_vars", "only_if", "sudo", "sudo_user"): pass else: raise errors.AnsibleError("parse error: task includes cannot be used with other directives: %s" % k) - def_vars = utils.combine_vars(self.default_vars, x.get('def_vars', {})) + default_vars = utils.combine_vars(self.default_vars, x.get('default_vars', {})) if 'vars' in x: task_vars = utils.combine_vars(task_vars, x['vars']) if 'only_if' in x: @@ -419,9 +419,9 @@ class Play(object): include_file = template(dirname, tokens[0], mv) include_filename = utils.path_dwim(dirname, include_file) data = utils.parse_yaml_from_file(include_filename) - results += self._load_tasks(data, mv, def_vars, included_sudo_vars, included_additional_conditions, original_file=include_filename) + results += self._load_tasks(data, mv, default_vars, included_sudo_vars, included_additional_conditions, original_file=include_filename) elif type(x) == dict: - results.append(Task(self,x,module_vars=task_vars,default_vars=def_vars,additional_conditions=additional_conditions)) + results.append(Task(self,x,module_vars=task_vars,default_vars=default_vars,additional_conditions=additional_conditions)) else: raise Exception("unexpected task type")