From ecb0b84f112e87f773431197fb794a79f5c00114 Mon Sep 17 00:00:00 2001 From: Paul Belanger Date: Sat, 18 Nov 2017 15:23:29 -0500 Subject: [PATCH] Add support for shared ansible_host in inventory Today it is possible to create the following ansible inventory file: [foo] foo01 ansible_host=192.168.1.1 [bar] bar01 ansible_host=192.168.1.1 Which allows a user to create multiple host aliases for a single connection. This could be done with ansible groups, however there is some functional differences on how ansible runs in that configuration. We could also request 2 nodes from nodepool, however in this case, it would be a waste of CI resources because every alias would need a new node from nodepool. Now, a user is able to alias multiple host names to a single node from nodepool by doing the following: nodeset: nodes: - name: - foo - bar label: ubuntu-xenial This would result in a single node request from nodepool, but create an inventory file with 2 alaises sharing and single ansible_host variable. Change-Id: I674d6baac26852ee1503feb1ed16c279bf773688 Signed-off-by: Paul Belanger --- doc/source/user/config.rst | 10 +++++++++- tests/base.py | 2 +- .../inventory/git/common-config/zuul.yaml | 10 ++++++++++ .../inventory/git/org_project/.zuul.yaml | 1 + tests/unit/test_inventory.py | 20 +++++++++++++++++++ tests/unit/test_nodepool.py | 20 +++++++++---------- zuul/configloader.py | 12 ++++++----- zuul/executor/server.py | 3 ++- zuul/model.py | 7 ++++--- 9 files changed, 64 insertions(+), 21 deletions(-) diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst index fa874a9df8..edd3222cec 100644 --- a/doc/source/user/config.rst +++ b/doc/source/user/config.rst @@ -1201,7 +1201,9 @@ configuration may be simplified. label: controller-label - name: compute1 label: compute-label - - name: compute2 + - name: + - compute2 + - web label: compute-label groups: - name: ceph-osd @@ -1212,6 +1214,9 @@ configuration may be simplified. - controller - compute1 - compute2 + - name: ceph-web + nodes: + - web .. attr:: nodeset @@ -1233,6 +1238,9 @@ configuration may be simplified. The name of the node. This will appear in the Ansible inventory for the job. + This can also be as a list of strings. If so, then the list of hosts in + the Ansible inventory will share a common ansible_host address. + .. attr:: label :required: diff --git a/tests/base.py b/tests/base.py index 176c53528a..a683426329 100755 --- a/tests/base.py +++ b/tests/base.py @@ -1412,7 +1412,7 @@ class RecordingAnsibleJob(zuul.executor.server.AnsibleJob): host['host_vars']['ansible_connection'] = 'local' hosts.append(dict( - name='localhost', + name=['localhost'], host_vars=dict(ansible_connection='local'), host_keys=[])) return hosts diff --git a/tests/fixtures/config/inventory/git/common-config/zuul.yaml b/tests/fixtures/config/inventory/git/common-config/zuul.yaml index 900abd6697..e5e2ba03c9 100644 --- a/tests/fixtures/config/inventory/git/common-config/zuul.yaml +++ b/tests/fixtures/config/inventory/git/common-config/zuul.yaml @@ -43,6 +43,16 @@ label: ubuntu-xenial run: playbooks/single-inventory.yaml +- job: + name: single-inventory-list + nodeset: + nodes: + - name: + - compute + - controller + label: ubuntu-xenial + run: playbooks/single-inventory.yaml + - job: name: group-inventory nodeset: nodeset1 diff --git a/tests/fixtures/config/inventory/git/org_project/.zuul.yaml b/tests/fixtures/config/inventory/git/org_project/.zuul.yaml index 26310a0b59..689d7716bb 100644 --- a/tests/fixtures/config/inventory/git/org_project/.zuul.yaml +++ b/tests/fixtures/config/inventory/git/org_project/.zuul.yaml @@ -3,4 +3,5 @@ check: jobs: - single-inventory + - single-inventory-list - group-inventory diff --git a/tests/unit/test_inventory.py b/tests/unit/test_inventory.py index 2835d30678..71cb05e5a4 100644 --- a/tests/unit/test_inventory.py +++ b/tests/unit/test_inventory.py @@ -57,6 +57,26 @@ class TestInventory(ZuulTestCase): self.executor_server.release() self.waitUntilSettled() + def test_single_inventory_list(self): + + inventory = self._get_build_inventory('single-inventory-list') + + all_nodes = ('compute', 'controller') + self.assertIn('all', inventory) + self.assertIn('hosts', inventory['all']) + self.assertIn('vars', inventory['all']) + for node_name in all_nodes: + self.assertIn(node_name, inventory['all']['hosts']) + self.assertIn('zuul', inventory['all']['vars']) + z_vars = inventory['all']['vars']['zuul'] + self.assertIn('executor', z_vars) + self.assertIn('src_root', z_vars['executor']) + self.assertIn('job', z_vars) + self.assertEqual(z_vars['job'], 'single-inventory-list') + + self.executor_server.release() + self.waitUntilSettled() + def test_group_inventory(self): inventory = self._get_build_inventory('group-inventory') diff --git a/tests/unit/test_nodepool.py b/tests/unit/test_nodepool.py index d3f9ddbd73..aa0f082626 100644 --- a/tests/unit/test_nodepool.py +++ b/tests/unit/test_nodepool.py @@ -67,8 +67,8 @@ class TestNodepool(BaseTestCase): # Test a simple node request nodeset = model.NodeSet() - nodeset.addNode(model.Node('controller', 'ubuntu-xenial')) - nodeset.addNode(model.Node('compute', 'ubuntu-xenial')) + nodeset.addNode(model.Node(['controller', 'foo'], 'ubuntu-xenial')) + nodeset.addNode(model.Node(['compute'], 'ubuntu-xenial')) job = model.Job('testjob') job.nodeset = nodeset request = self.nodepool.requestNodes(None, job) @@ -99,8 +99,8 @@ class TestNodepool(BaseTestCase): # Test that node requests are re-submitted after disconnect nodeset = model.NodeSet() - nodeset.addNode(model.Node('controller', 'ubuntu-xenial')) - nodeset.addNode(model.Node('compute', 'ubuntu-xenial')) + nodeset.addNode(model.Node(['controller'], 'ubuntu-xenial')) + nodeset.addNode(model.Node(['compute'], 'ubuntu-xenial')) job = model.Job('testjob') job.nodeset = nodeset self.fake_nodepool.paused = True @@ -116,8 +116,8 @@ class TestNodepool(BaseTestCase): # Test that node requests can be canceled nodeset = model.NodeSet() - nodeset.addNode(model.Node('controller', 'ubuntu-xenial')) - nodeset.addNode(model.Node('compute', 'ubuntu-xenial')) + nodeset.addNode(model.Node(['controller'], 'ubuntu-xenial')) + nodeset.addNode(model.Node(['compute'], 'ubuntu-xenial')) job = model.Job('testjob') job.nodeset = nodeset self.fake_nodepool.paused = True @@ -131,8 +131,8 @@ class TestNodepool(BaseTestCase): # Test that a resubmitted request would not lock nodes nodeset = model.NodeSet() - nodeset.addNode(model.Node('controller', 'ubuntu-xenial')) - nodeset.addNode(model.Node('compute', 'ubuntu-xenial')) + nodeset.addNode(model.Node(['controller'], 'ubuntu-xenial')) + nodeset.addNode(model.Node(['compute'], 'ubuntu-xenial')) job = model.Job('testjob') job.nodeset = nodeset request = self.nodepool.requestNodes(None, job) @@ -152,8 +152,8 @@ class TestNodepool(BaseTestCase): # Test that a lost request would not lock nodes nodeset = model.NodeSet() - nodeset.addNode(model.Node('controller', 'ubuntu-xenial')) - nodeset.addNode(model.Node('compute', 'ubuntu-xenial')) + nodeset.addNode(model.Node(['controller'], 'ubuntu-xenial')) + nodeset.addNode(model.Node(['compute'], 'ubuntu-xenial')) job = model.Job('testjob') job.nodeset = nodeset request = self.nodepool.requestNodes(None, job) diff --git a/zuul/configloader.py b/zuul/configloader.py index 99f10f6b37..98bb23f0a9 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -340,7 +340,7 @@ class PragmaParser(object): class NodeSetParser(object): @staticmethod def getSchema(anonymous=False): - node = {vs.Required('name'): str, + node = {vs.Required('name'): to_list(str), vs.Required('label'): str, } @@ -365,11 +365,13 @@ class NodeSetParser(object): node_names = set() group_names = set() for conf_node in as_list(conf['nodes']): - if conf_node['name'] in node_names: - raise DuplicateNodeError(conf['name'], conf_node['name']) - node = model.Node(conf_node['name'], conf_node['label']) + for name in as_list(conf_node['name']): + if name in node_names: + raise DuplicateNodeError(name, conf_node['name']) + node = model.Node(as_list(conf_node['name']), conf_node['label']) ns.addNode(node) - node_names.add(conf_node['name']) + for name in as_list(conf_node['name']): + node_names.add(name) for conf_group in as_list(conf.get('groups', [])): for node_name in as_list(conf_group['nodes']): if node_name not in node_names: diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 469d6f3508..928d802626 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -495,7 +495,8 @@ def make_inventory_dict(nodes, groups, all_vars): hosts = {} for node in nodes: - hosts[node['name']] = node['host_vars'] + for name in node['name']: + hosts[name] = node['host_vars'] inventory = { 'all': { diff --git a/zuul/model.py b/zuul/model.py index b027c534fe..695212c3ff 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -497,9 +497,10 @@ class NodeSet(object): return n def addNode(self, node): - if node.name in self.nodes: - raise Exception("Duplicate node in %s" % (self,)) - self.nodes[node.name] = node + for name in node.name: + if name in self.nodes: + raise Exception("Duplicate node in %s" % (self,)) + self.nodes[tuple(node.name)] = node def getNodes(self): return list(self.nodes.values())