From 52ce1e4bc0d5fd00251b02e3652fcf8ab6db94d6 Mon Sep 17 00:00:00 2001 From: Lukas Brabec Date: Mar 07 2018 10:37:48 +0000 Subject: [PATCH 1/2] Retry to spawn disposable client --- diff --git a/conf/taskotron.yaml.example b/conf/taskotron.yaml.example index b35bd2f..b0482a4 100644 --- a/conf/taskotron.yaml.example +++ b/conf/taskotron.yaml.example @@ -144,6 +144,9 @@ #default_disposable_flavor: taskotron_cloud #default_disposable_arch: x86_64 +## Number of retries when disposable client fails to boot within timeout +#spawn_vm_retries: 3 + ## Additional repos for minion to install packages from #minion_repos: # - https://server1/repo/ diff --git a/libtaskotron/config_defaults.py b/libtaskotron/config_defaults.py index 8202e00..ebd569c 100644 --- a/libtaskotron/config_defaults.py +++ b/libtaskotron/config_defaults.py @@ -90,6 +90,8 @@ class Config(object): default_disposable_flavor = 'taskotron_cloud' #: default_disposable_arch = 'x86_64' #: + spawn_vm_retries = 3 #: + minion_repos = [] #: log_level_stream = 'INFO' #: diff --git a/libtaskotron/executor.py b/libtaskotron/executor.py index 45015f1..3dd4b57 100644 --- a/libtaskotron/executor.py +++ b/libtaskotron/executor.py @@ -55,14 +55,26 @@ class Executor(object): env = image_utils.devise_environment(self.arg_data) self.task_vm = vm.TestCloudMachine(uuid) - self.task_vm.prepare(**env) - self.task_vm.wait_for_port(22) + retries = config.get_config().spawn_vm_retries - log.debug('Disposable client (%s %s) ready', self.task_vm.instancename, + while retries > 0: + try: + self.task_vm.prepare(**env) + self.task_vm.wait_for_port(22) + + log.debug('Disposable client (%s %s) ready', self.task_vm.instancename, self.task_vm.ipaddr) - return self.task_vm.ipaddr + return self.task_vm.ipaddr + except vm.TestcloudInstanceError, e: + log.info('Disposable client failed to boot: %s, retrying.' % e) + self.task_vm.teardown() + + retries -= 1 + + raise exc.TaskotronRemoteError('Disposable client failed to boot') + def _get_client_ipaddr(self): '''Get an IP address of the machine the task is going to be executed diff --git a/testing/test_executor.py b/testing/test_executor.py index 60a4ede..135296e 100644 --- a/testing/test_executor.py +++ b/testing/test_executor.py @@ -16,6 +16,8 @@ from libtaskotron import executor import libtaskotron.exceptions as exc from libtaskotron.directives import resultsdb_directive from libtaskotron import os_utils +from libtaskotron.ext.disposable import vm +from libtaskotron import config PLAYBOOK=''' - hosts: localhost @@ -269,6 +271,21 @@ class TestExecutor(): mock_report_results.assert_called_once() mock_task_vm.teardown.assert_called_once() + def test_execute_libvirt_retries(self, monkeypatch): + '''Execution using libvirt mode, vm boot retries''' + + mock_vm_prepare = mock.Mock(side_effect=vm.TestcloudInstanceError) + mock_vm = mock.Mock(return_value=mock.Mock(prepare=mock_vm_prepare)) + monkeypatch.setattr(vm, 'TestCloudMachine', mock_vm) + + self.executor.arg_data['local'] = False + self.executor.arg_data['libvirt'] = True + + with pytest.raises(exc.TaskotronRemoteError): + self.executor.execute() + + assert mock_vm_prepare.call_count == config.get_config().spawn_vm_retries + def test_execute_no_playbooks(self, monkeypatch): '''Should raise when there are no playbooks''' mock_run_playbook = mock.Mock() From f97dc3e1ef93e7b5b53bdd1568f00023636e84e7 Mon Sep 17 00:00:00 2001 From: Kamil Páral Date: Mar 07 2018 12:11:33 +0000 Subject: [PATCH 2/2] small improvements during review --- diff --git a/libtaskotron/exceptions.py b/libtaskotron/exceptions.py index 020863a..f5dd479 100644 --- a/libtaskotron/exceptions.py +++ b/libtaskotron/exceptions.py @@ -93,3 +93,8 @@ class TaskotronInterruptError(TaskotronError): def __str__(self): return 'Received system signal %d (%s)' % (self.signum, self.signame) + + +class TaskotronMinionError(TaskotronError): + '''All errors related to persistent or disposable minion handling''' + pass diff --git a/libtaskotron/executor.py b/libtaskotron/executor.py index 3dd4b57..7cc497c 100644 --- a/libtaskotron/executor.py +++ b/libtaskotron/executor.py @@ -59,22 +59,24 @@ class Executor(object): retries = config.get_config().spawn_vm_retries while retries > 0: + retries -= 1 try: self.task_vm.prepare(**env) self.task_vm.wait_for_port(22) - log.debug('Disposable client (%s %s) ready', self.task_vm.instancename, - self.task_vm.ipaddr) + log.debug('Disposable client (%s %s) ready', + self.task_vm.instancename, + self.task_vm.ipaddr) return self.task_vm.ipaddr except vm.TestcloudInstanceError, e: - log.info('Disposable client failed to boot: %s, retrying.' % e) - self.task_vm.teardown() - - retries -= 1 - - raise exc.TaskotronRemoteError('Disposable client failed to boot') - + if retries <= 0: + raise exc.TaskotronMinionError('Disposable client failed ' + 'to boot: %s', e) + else: + log.warning('Disposable client failed to boot, retrying: ' + '%s', e) + self.task_vm.teardown() def _get_client_ipaddr(self): '''Get an IP address of the machine the task is going to be executed diff --git a/testing/test_executor.py b/testing/test_executor.py index 135296e..6b11e29 100644 --- a/testing/test_executor.py +++ b/testing/test_executor.py @@ -18,6 +18,7 @@ from libtaskotron.directives import resultsdb_directive from libtaskotron import os_utils from libtaskotron.ext.disposable import vm from libtaskotron import config +import testcloud.exceptions PLAYBOOK=''' - hosts: localhost @@ -35,7 +36,7 @@ PLAYBOOK=''' class TestExecutor(): @pytest.fixture - def setup(self, tmpdir): + def setup(self, tmpdir, monkeypatch): '''Run this before every test invocation''' self.artifactsdir = tmpdir.mkdir('artifacts') self.taskdir = tmpdir.mkdir('taskdir') @@ -58,6 +59,10 @@ class TestExecutor(): self.ipaddr = '127.0.0.1' self.executor = executor.Executor(self.arg_data) + monkeypatch.setattr(config, '_config', None) + self.conf = config.get_config() + self.conf.spawn_vm_retries = 5 + def test_interrupt_handler(self): '''Raise correct exception for an interrupt''' @@ -271,21 +276,6 @@ class TestExecutor(): mock_report_results.assert_called_once() mock_task_vm.teardown.assert_called_once() - def test_execute_libvirt_retries(self, monkeypatch): - '''Execution using libvirt mode, vm boot retries''' - - mock_vm_prepare = mock.Mock(side_effect=vm.TestcloudInstanceError) - mock_vm = mock.Mock(return_value=mock.Mock(prepare=mock_vm_prepare)) - monkeypatch.setattr(vm, 'TestCloudMachine', mock_vm) - - self.executor.arg_data['local'] = False - self.executor.arg_data['libvirt'] = True - - with pytest.raises(exc.TaskotronRemoteError): - self.executor.execute() - - assert mock_vm_prepare.call_count == config.get_config().spawn_vm_retries - def test_execute_no_playbooks(self, monkeypatch): '''Should raise when there are no playbooks''' mock_run_playbook = mock.Mock() @@ -361,3 +351,30 @@ class TestExecutor(): assert finished == False mock_run_playbook.assert_called_once() mock_report_results.assert_not_called() + + def test_spawn_vm_retries(self, monkeypatch): + '''Spawning VM should be attempted several times on failure''' + mock_vm_prepare = mock.Mock( + side_effect=[testcloud.exceptions.TestcloudInstanceError, + mock.DEFAULT]) + mock_vm = mock.Mock() + vm_instance = mock_vm.return_value + vm_instance.prepare = mock_vm_prepare + vm_instance.ipaddr = '10.11.12.13' + monkeypatch.setattr(vm, 'TestCloudMachine', mock_vm) + + ipaddr = self.executor._spawn_vm(None) + assert ipaddr == '10.11.12.13' + assert mock_vm_prepare.call_count == 2 + + def test_spawn_vm_complete_fail(self, monkeypatch): + '''Exception should be raised when VM can't be spawned using retries''' + mock_vm_prepare = mock.Mock( + side_effect=testcloud.exceptions.TestcloudInstanceError) + mock_vm = mock.Mock(return_value=mock.Mock(prepare=mock_vm_prepare)) + monkeypatch.setattr(vm, 'TestCloudMachine', mock_vm) + + with pytest.raises(exc.TaskotronMinionError): + self.executor._spawn_vm(None) + + assert mock_vm_prepare.call_count == self.conf.spawn_vm_retries