From 4107f98669b30ce37d750045e748c24d74efe4d3 Mon Sep 17 00:00:00 2001 From: Yaroslav Lobankov Date: Thu, 6 Oct 2016 21:29:31 +0300 Subject: [PATCH] Fix the issue with getting nodes by FQDNs This patch deletes hard-coded FQDN pattern for nodes and fixes the issue. Closes-Bug: #1630531 Change-Id: If6fe899415372aab9fd6b8d03da91ecfee142cde --- os_faults/ansible/executor.py | 12 +++++++- os_faults/drivers/fuel.py | 23 ++++++--------- os_faults/tests/unit/ansible/test_executor.py | 20 +++++++++++++ .../unit/drivers/test_fuel_management.py | 29 ++++++++++--------- .../tests/unit/drivers/test_fuel_service.py | 24 ++++++++------- 5 files changed, 68 insertions(+), 40 deletions(-) diff --git a/os_faults/ansible/executor.py b/os_faults/ansible/executor.py index 2d07330..df8bfbf 100644 --- a/os_faults/ansible/executor.py +++ b/os_faults/ansible/executor.py @@ -12,6 +12,7 @@ # limitations under the License. from collections import namedtuple +import copy import os from ansible.executor import task_queue_manager @@ -33,6 +34,8 @@ DEFAULT_ERROR_STATUSES = {STATUS_FAILED, STATUS_UNREACHABLE} SSH_COMMON_ARGS = '-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no' +STDOUT_LIMIT = 4096 # Symbols count + class AnsibleExecutionException(Exception): pass @@ -198,6 +201,13 @@ class AnsibleRunner(object): else AnsibleExecutionException) raise ek(msg) - LOG.debug('Execution result: %s' % result) + log_result = copy.deepcopy(result) + LOG.debug('Execution completed with %s result(s):' % len(log_result)) + for lr in log_result: + if len(lr.payload['stdout']) > STDOUT_LIMIT: + lr.payload['stdout'] = ( + lr.payload['stdout'][:STDOUT_LIMIT] + '... ') + del lr.payload['stdout_lines'] + LOG.debug(lr) return result diff --git a/os_faults/drivers/fuel.py b/os_faults/drivers/fuel.py index 2626780..eeaf502 100644 --- a/os_faults/drivers/fuel.py +++ b/os_faults/drivers/fuel.py @@ -474,9 +474,12 @@ class FuelManagement(cloud_management.CloudManagement): def _get_cloud_hosts(self): if not self.cached_cloud_hosts: - task = {'command': 'fuel2 node list -f json'} + task = {'command': 'fuel node --json'} result = self.execute_on_master_node(task) - return json.loads(result[0].payload['stdout']) + for r in json.loads(result[0].payload['stdout']): + host = {'ip': r['ip'], 'mac': r['mac'], 'fqdn': r['fqdn']} + self.cached_cloud_hosts.append(host) + self.fqdn_to_hosts[host['fqdn']] = host return self.cached_cloud_hosts @@ -501,24 +504,18 @@ class FuelManagement(cloud_management.CloudManagement): else: return self.cloud_executor.execute(hosts, task, []) - def _retrieve_hosts_fqdn(self): - for host in self._get_cloud_hosts(): - host['fqdn'] = 'node-%s.domain.tld' % host['id'] - self.fqdn_to_hosts[host['fqdn']] = host - def get_nodes(self, fqdns=None): """Get nodes in the cloud This function returns NodesCollection representing all nodes in the - cloud or only those that has specified FQDNs. + cloud or only those that were specified by FQDNs. :param fqdns: list of FQDNs or None to retrieve all nodes :return: NodesCollection """ + hosts = self._get_cloud_hosts() + if fqdns: - # return only specified logging.debug('Trying to find nodes with FQDNs: %s', fqdns) - if not self.fqdn_to_hosts: - self._retrieve_hosts_fqdn() hosts = list() for fqdn in fqdns: if fqdn in self.fqdn_to_hosts: @@ -527,9 +524,7 @@ class FuelManagement(cloud_management.CloudManagement): raise error.NodeCollectionError( 'Node with FQDN \'%s\' not found!' % fqdn) logging.debug('The following nodes were found: %s', hosts) - else: - # return all nodes - hosts = self._get_cloud_hosts() + return FuelNodeCollection(cloud_management=self, power_management=self.power_management, hosts=hosts) diff --git a/os_faults/tests/unit/ansible/test_executor.py b/os_faults/tests/unit/ansible/test_executor.py index 4c76975..8328b63 100644 --- a/os_faults/tests/unit/ansible/test_executor.py +++ b/os_faults/tests/unit/ansible/test_executor.py @@ -205,3 +205,23 @@ class AnsibleRunnerTestCase(test.TestCase): err = self.assertRaises(executor.AnsibleExecutionException, ex.execute, my_hosts, my_tasks, my_statuses) self.assertEqual(type(err), executor.AnsibleExecutionException) + + @mock.patch('copy.deepcopy') + @mock.patch('os_faults.ansible.executor.AnsibleRunner.run_playbook') + def test_execute_stdout_is_more_than_stdout_limit( + self, mock_run_playbook, mock_deepcopy): + result = mock.Mock() + result.payload = {'stdout': 'a' * (executor.STDOUT_LIMIT + 1), + 'stdout_lines': 'a' * (executor.STDOUT_LIMIT + 1)} + mock_run_playbook.return_value = [result] + + mock_deepcopy.return_value = [result] + log_result = mock_deepcopy.return_value[0] + + my_hosts = ['0.0.0.0', '255.255.255.255'] + my_tasks = 'my_task' + ex = executor.AnsibleRunner() + ex.execute(my_hosts, my_tasks) + + self.assertEqual('a' * executor.STDOUT_LIMIT + '... ', + log_result.payload['stdout']) diff --git a/os_faults/tests/unit/drivers/test_fuel_management.py b/os_faults/tests/unit/drivers/test_fuel_management.py index e81ddad..11fec8c 100644 --- a/os_faults/tests/unit/drivers/test_fuel_management.py +++ b/os_faults/tests/unit/drivers/test_fuel_management.py @@ -27,8 +27,10 @@ class FuelManagementTestCase(test.TestCase): super(FuelManagementTestCase, self).setUp() self.fake_ansible_result = fakes.FakeAnsibleResult( - payload={'stdout': '[{"ip": "10.0.0.2", "mac": "02", "id": "2"},' - ' {"ip": "10.0.0.3", "mac": "03", "id": "3"}]'}) + payload={ + 'stdout': '[{"ip": "10.0.0.2", "mac": "02", "fqdn": "node-2"},' + ' {"ip": "10.0.0.3", "mac": "03", "fqdn": "node-3"}]' + }) @mock.patch('os_faults.ansible.executor.AnsibleRunner', autospec=True) def test_verify(self, mock_ansible_runner): @@ -45,7 +47,7 @@ class FuelManagementTestCase(test.TestCase): fuel_managment.verify() ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': 'hostname'}), ]) @@ -60,12 +62,12 @@ class FuelManagementTestCase(test.TestCase): nodes = fuel_managment.get_nodes() ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), ]) self.assertEqual(nodes.hosts, - [{'ip': '10.0.0.2', 'mac': '02', "id": "2"}, - {'ip': '10.0.0.3', 'mac': '03', "id": "3"}]) + [{'ip': '10.0.0.2', 'mac': '02', "fqdn": "node-2"}, + {'ip': '10.0.0.3', 'mac': '03', "fqdn": "node-3"}]) @mock.patch('os_faults.ansible.executor.AnsibleRunner', autospec=True) def test_execute_on_cloud(self, mock_ansible_runner): @@ -84,7 +86,7 @@ class FuelManagementTestCase(test.TestCase): nodes.get_ips(), {'command': 'mycmd'}, raise_on_error=False) ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': 'mycmd'}, []), ]) @@ -100,11 +102,10 @@ class FuelManagementTestCase(test.TestCase): 'address': 'fuel.local', 'username': 'root', }) - nodes = fuel_managment.get_nodes(fqdns=['node-3.domain.tld']) + nodes = fuel_managment.get_nodes(fqdns=['node-3']) - self.assertEqual(nodes.hosts, [{'ip': '10.0.0.3', - 'mac': '03', 'id': '3', - 'fqdn': 'node-3.domain.tld'}]) + self.assertEqual(nodes.hosts, + [{'ip': '10.0.0.3', 'mac': '03', 'fqdn': 'node-3'}]) @mock.patch('os_faults.ansible.executor.AnsibleRunner', autospec=True) @ddt.data(('keystone', fuel.KeystoneService), @@ -136,9 +137,9 @@ class FuelManagementTestCase(test.TestCase): nodes = service.get_nodes() ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service_cls.GET_NODES_CMD}, []), ]) - self.assertEqual(nodes.hosts, [{'ip': '10.0.0.3', - 'mac': '03', 'id': '3'}]) + self.assertEqual(nodes.hosts, + [{'ip': '10.0.0.3', 'mac': '03', "fqdn": "node-3"}]) diff --git a/os_faults/tests/unit/drivers/test_fuel_service.py b/os_faults/tests/unit/drivers/test_fuel_service.py index 48f3239..55e2f45 100644 --- a/os_faults/tests/unit/drivers/test_fuel_service.py +++ b/os_faults/tests/unit/drivers/test_fuel_service.py @@ -28,8 +28,10 @@ class FuelServiceTestCase(test.TestCase): super(FuelServiceTestCase, self).setUp() self.conf = {'address': 'fuel.local', 'username': 'root'} self.fake_ansible_result = fakes.FakeAnsibleResult( - payload={'stdout': '[{"ip": "10.0.0.2", "mac": "02", "id": "2"},' - ' {"ip": "10.0.0.3", "mac": "03", "id": "3"}]'}) + payload={ + 'stdout': '[{"ip": "10.0.0.2", "mac": "02", "fqdn": "node-2"},' + ' {"ip": "10.0.0.3", "mac": "03", "fqdn": "node-3"}]' + }) @mock.patch('os_faults.ansible.executor.AnsibleRunner', autospec=True) @ddt.data(('keystone', fuel.KeystoneService), @@ -66,7 +68,7 @@ class FuelServiceTestCase(test.TestCase): service.kill() ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service_cls.GET_NODES_CMD}, []), mock.call(['10.0.0.2', '10.0.0.3'], @@ -108,7 +110,7 @@ class FuelServiceTestCase(test.TestCase): service.freeze() ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service_cls.GET_NODES_CMD}, []), mock.call(['10.0.0.2', '10.0.0.3'], @@ -151,7 +153,7 @@ class FuelServiceTestCase(test.TestCase): delay_sec = 10 service.freeze(nodes=None, sec=delay_sec) ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service_cls.GET_NODES_CMD}, []), mock.call(['10.0.0.2', '10.0.0.3'], @@ -194,7 +196,7 @@ class FuelServiceTestCase(test.TestCase): service.unfreeze() ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service_cls.GET_NODES_CMD}, []), mock.call(['10.0.0.2', '10.0.0.3'], @@ -225,7 +227,7 @@ class FuelServiceTestCase(test.TestCase): service.unplug() ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service_cls.GET_NODES_CMD}, []), mock.call(['10.0.0.2', '10.0.0.3'], @@ -257,7 +259,7 @@ class FuelServiceTestCase(test.TestCase): service.plug() ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service_cls.GET_NODES_CMD}, []), mock.call(['10.0.0.2', '10.0.0.3'], @@ -298,7 +300,7 @@ class FuelServiceTestCase(test.TestCase): service.restart() ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service_cls.GET_NODES_CMD}, []), mock.call(['10.0.0.2', '10.0.0.3'], @@ -328,7 +330,7 @@ class FuelServiceTestCase(test.TestCase): self.assertEqual('Task failed on some nodes', str(exception)) ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service.GET_NODES_CMD}, []), mock.call(['10.0.0.2', '10.0.0.3'], @@ -355,7 +357,7 @@ class FuelServiceTestCase(test.TestCase): self.assertEqual('Node collection is empty', str(exception)) ansible_runner_inst.execute.assert_has_calls([ - mock.call(['fuel.local'], {'command': 'fuel2 node list -f json'}), + mock.call(['fuel.local'], {'command': 'fuel node --json'}), mock.call(['10.0.0.2', '10.0.0.3'], {'command': service.GET_NODES_CMD}, []), ])