diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index f66008ebe5..e4395ed250 100644 --- a/ironic/drivers/modules/agent_client.py +++ b/ironic/drivers/modules/agent_client.py @@ -60,9 +60,9 @@ class AgentClient(object): """Get URL endpoint for agent command request""" agent_url = node.driver_internal_info.get('agent_url') if not agent_url: - raise exception.IronicException(_('Agent driver requires ' - 'agent_url in ' - 'driver_internal_info')) + raise exception.AgentConnectionFailed(_('Agent driver requires ' + 'agent_url in ' + 'driver_internal_info')) return ('%(agent_url)s/%(api_version)s/commands/' % {'agent_url': agent_url, 'api_version': CONF.agent.agent_api_version}) @@ -219,14 +219,14 @@ class AgentClient(object): return result @METRICS.timer('AgentClient.get_commands_status') - @retrying.retry( - retry_on_exception=( - lambda e: isinstance(e, exception.AgentConnectionFailed)), - stop_max_attempt_number=CONF.agent.max_command_attempts) - def get_commands_status(self, node): + def get_commands_status(self, node, retry_connection=True, + expect_errors=False): """Get command status from agent. :param node: A Node object. + :param retry_connection: Whether to retry connection problems. + :param expect_errors: If True, do not log connection problems as + errors. :return: A list of command results, each result is related to a command been issued to agent. A typical result can be: @@ -255,17 +255,27 @@ class AgentClient(object): """ url = self._get_command_url(node) LOG.debug('Fetching status of agent commands for node %s', node.uuid) - try: - resp = self.session.get(url, timeout=CONF.agent.command_timeout) - except (requests.ConnectionError, requests.Timeout) as e: - msg = (_('Failed to connect to the agent running on node %(node)s ' - 'to collect commands status. ' - 'Error: %(error)s') % - {'node': node.uuid, 'error': e}) - LOG.error(msg) - raise exception.AgentConnectionFailed(reason=msg) - result = resp.json()['commands'] + def _get(): + try: + return self.session.get(url, + timeout=CONF.agent.command_timeout) + except (requests.ConnectionError, requests.Timeout) as e: + msg = (_('Failed to connect to the agent running on node ' + '%(node)s to collect commands status. ' + 'Error: %(error)s') % + {'node': node.uuid, 'error': e}) + logging_call = LOG.debug if expect_errors else LOG.error + logging_call(msg) + raise exception.AgentConnectionFailed(reason=msg) + + if retry_connection: + _get = retrying.retry( + retry_on_exception=( + lambda e: isinstance(e, exception.AgentConnectionFailed)), + stop_max_attempt_number=CONF.agent.max_command_attempts)(_get) + + result = _get().json()['commands'] status = '; '.join('%(cmd)s: result "%(res)s", error "%(err)s"' % {'cmd': r.get('command_name'), 'res': r.get('command_result'), diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py index 22c8575555..69aa03495f 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_client.py +++ b/ironic/tests/unit/drivers/modules/test_agent_client.py @@ -92,7 +92,7 @@ class TestAgentClient(base.TestCase): def test__get_command_url_fail(self): del self.node.driver_internal_info['agent_url'] - self.assertRaises(exception.IronicException, + self.assertRaises(exception.AgentConnectionFailed, self.client._get_command_url, self.node) @@ -162,7 +162,7 @@ class TestAgentClient(base.TestCase): method = 'foo.bar' params = {} - self.client._get_command_url(self.node) + url = self.client._get_command_url(self.node) self.client._get_command_body(method, params) e = self.assertRaises(exception.AgentConnectionFailed, @@ -173,6 +173,12 @@ class TestAgentClient(base.TestCase): 'command %(method)s. Error: %(error)s' % {'method': method, 'node': self.node.uuid, 'error': error}, str(e)) + self.client.session.post.assert_called_with( + url, + data=mock.ANY, + params={'wait': 'false'}, + timeout=60) + self.assertEqual(3, self.client.session.post.call_count) def test__command_error_code(self): response_text = {"faultstring": "you dun goofd"} @@ -259,15 +265,21 @@ class TestAgentClient(base.TestCase): timeout=CONF.agent.command_timeout) def test_get_commands_status_retries(self): - with mock.patch.object(self.client.session, 'get', - autospec=True) as mock_get: - res = mock.MagicMock(spec_set=['json']) - res.json.return_value = {'commands': []} - mock_get.side_effect = [ - requests.ConnectionError('boom'), - res] - self.assertEqual([], self.client.get_commands_status(self.node)) - self.assertEqual(2, mock_get.call_count) + res = mock.MagicMock(spec_set=['json']) + res.json.return_value = {'commands': []} + self.client.session.get.side_effect = [ + requests.ConnectionError('boom'), + res + ] + self.assertEqual([], self.client.get_commands_status(self.node)) + self.assertEqual(2, self.client.session.get.call_count) + + def test_get_commands_status_no_retries(self): + self.client.session.get.side_effect = requests.ConnectionError('boom') + self.assertRaises(exception.AgentConnectionFailed, + self.client.get_commands_status, self.node, + retry_connection=False) + self.assertEqual(1, self.client.session.get.call_count) def test_prepare_image(self): self.client._command = mock.MagicMock(spec_set=[])