diff --git a/manila/share/drivers/service_instance.py b/manila/share/drivers/service_instance.py index a7e80cf19f..77bd1e8616 100644 --- a/manila/share/drivers/service_instance.py +++ b/manila/share/drivers/service_instance.py @@ -152,6 +152,7 @@ class ServiceInstanceManager(object): 3. delete_service_instance: removes service instance and network infrastructure. """ + _INSTANCE_CONNECTION_PROTO = "SSH" def get_config_option(self, key): """Returns value of config option. @@ -390,6 +391,19 @@ class ServiceInstanceManager(object): instance_name = network_info['server_id'] server = self._create_service_instance( context, instance_name, network_info) + instance_details = self._get_new_instance_details(server) + + if not self._check_server_availability(instance_details): + raise exception.ServiceInstanceException( + _('%(conn_proto)s connection has not been ' + 'established to %(server)s in %(time)ss. Giving up.') % { + 'conn_proto': self._INSTANCE_CONNECTION_PROTO, + 'server': server['ip'], + 'time': self.max_time_to_build_instance}) + + return instance_details + + def _get_new_instance_details(self, server): instance_details = { 'instance_id': server['id'], 'ip': server['ip'], @@ -408,14 +422,6 @@ class ServiceInstanceManager(object): for key in ('password', 'pk_path', 'subnet_id'): if not instance_details[key]: instance_details.pop(key) - - if not self._check_server_availability(server): - raise exception.ServiceInstanceException( - _('SSH connection has not been ' - 'established to %(server)s in %(time)ss. Giving up.') % { - 'server': server['ip'], - 'time': self.max_time_to_build_instance}) - return instance_details @utils.synchronized("service_instance_get_key", external=True) @@ -495,40 +501,21 @@ class ServiceInstanceManager(object): fail_safe_data['public_port_id'] = ( network_data['public_port']['id']) try: + create_kwargs = self._get_service_instance_create_kwargs() service_instance = self.compute_api.server_create( context, name=instance_name, image=service_image_id, flavor=self.get_config_option("service_instance_flavor_id"), key_name=key_name, - nics=network_data['nics']) + nics=network_data['nics'], + **create_kwargs) fail_safe_data['instance_id'] = service_instance['id'] - t = time.time() - while time.time() - t < self.max_time_to_build_instance: - # NOTE(vponomaryov): emptiness of 'networks' field checked as - # workaround for nova/neutron bug #1210483. - if (service_instance['status'] == 'ACTIVE' and - service_instance.get('networks', {})): - break - if service_instance['status'] == 'ERROR': - raise exception.ServiceInstanceException( - _("Failed to build service instance '%s'.") % - service_instance['id']) - time.sleep(1) - try: - service_instance = self.compute_api.server_get( - context, - service_instance['id']) - except exception.InstanceNotFound as e: - LOG.debug(e) - else: - raise exception.ServiceInstanceException( - _("Instance '%(ins)s' has not been spawned in %(timeout)s " - "seconds. Giving up.") % dict( - ins=service_instance['id'], - timeout=self.max_time_to_build_instance)) + service_instance = self.wait_for_instance_to_be_active( + service_instance['id'], + self.max_time_to_build_instance) security_group = self._get_or_create_security_group(context) if security_group: @@ -569,21 +556,36 @@ class ServiceInstanceManager(object): return service_instance - def _check_server_availability(self, server): + def _get_service_instance_create_kwargs(self): + """Specify extra arguments used when creating the service instance. + + Classes inheriting the service instance manager can use this to easily + pass extra arguments such as user data or metadata. + """ + return {} + + def _check_server_availability(self, instance_details): t = time.time() while time.time() - t < self.max_time_to_build_instance: - LOG.debug('Checking service VM availability.') - try: - socket.socket().connect((server['ip'], 22)) - LOG.debug('Service VM is available via SSH.') - return True - except socket.error as e: - LOG.debug(e) - LOG.debug("Server %s is not available via SSH. Waiting...", - server['ip']) + LOG.debug('Checking server availability.') + if not self._test_server_connection(instance_details): time.sleep(5) + else: + return True return False + def _test_server_connection(self, server): + try: + socket.socket().connect((server['ip'], 22)) + LOG.debug('Server %s is available via SSH.', + server['ip']) + return True + except socket.error as e: + LOG.debug(e) + LOG.debug("Server %s is not available via SSH. Waiting...", + server['ip']) + return False + def delete_service_instance(self, context, server_details): """Removes share infrastructure. @@ -593,6 +595,40 @@ class ServiceInstanceManager(object): self._delete_server(context, instance_id) self.network_helper.teardown_network(server_details) + def wait_for_instance_to_be_active(self, instance_id, timeout): + t = time.time() + while time.time() - t < timeout: + try: + service_instance = self.compute_api.server_get( + self.admin_context, + instance_id) + except exception.InstanceNotFound as e: + LOG.debug(e) + time.sleep(1) + continue + + instance_status = service_instance['status'] + # NOTE(vponomaryov): emptiness of 'networks' field checked as + # workaround for nova/neutron bug #1210483. + if (instance_status == 'ACTIVE' and + service_instance.get('networks', {})): + return service_instance + elif service_instance['status'] == 'ERROR': + break + + LOG.debug("Waiting for instance %(instance_id)s to be active. " + "Current status: %(instance_status)s." % + dict(instance_id=instance_id, + instance_status=instance_status)) + time.sleep(1) + raise exception.ServiceInstanceException( + _("Instance %(instance_id)s failed to reach active state " + "in %(timeout)s seconds. " + "Current status: %(instance_status)s.") % + dict(instance_id=instance_id, + timeout=timeout, + instance_status=instance_status)) + @six.add_metaclass(abc.ABCMeta) class BaseNetworkhelper(object): diff --git a/manila/tests/share/drivers/test_service_instance.py b/manila/tests/share/drivers/test_service_instance.py index 86d163ea2b..4324327a85 100644 --- a/manila/tests/share/drivers/test_service_instance.py +++ b/manila/tests/share/drivers/test_service_instance.py @@ -511,7 +511,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager.admin_context, fake_network_info['server_id'], fake_network_info) self._manager._check_server_availability.assert_called_once_with( - fake_server) + expected_details) self.assertEqual(expected_details, result) def test_set_up_service_instance_not_available(self): @@ -520,6 +520,9 @@ class ServiceInstanceManagerTestCase(test.TestCase): id='fake', ip='1.2.3.4', public_address='1.2.3.4', pk_path=None, subnet_id='fake-subnet-id', router_id='fake-router-id', username=self._manager.get_config_option('service_instance_user')) + expected_details = fake_server.copy() + expected_details.pop('pk_path') + expected_details['instance_id'] = expected_details.pop('id') self.mock_object(self._manager, '_create_service_instance', mock.Mock(return_value=fake_server)) self.mock_object(self._manager, '_check_server_availability', @@ -534,7 +537,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager.admin_context, fake_network_info['server_id'], fake_network_info) self._manager._check_server_availability.assert_called_once_with( - fake_server) + expected_details) def test_ensure_server(self): server_details = {'instance_id': 'fake_inst_id', 'ip': '1.2.3.4'} @@ -1019,62 +1022,6 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager.network_helper.get_network_name.\ assert_called_once_with(network_info) - def test___create_service_instance_not_found_after_creation(self): - def fake_sleep(time): - self.fake_time += time - - def fake_time(): - if not self.fake_time: - return 0 - return self.fake_time - - self.fake_time, self._manager.max_time_to_build_instance = 0, 1 - server_create = dict(id='fakeid', status='CREATING', networks=dict()) - net_name = self._manager.get_config_option("service_network_name") - server_get = dict( - id='fakeid', status='ACTIVE', networks={net_name: 'bar'}) - service_image_id = 'fake_service_image_id' - key_data = 'fake_key_name', None - instance_name = 'fake_instance_name' - network_info = dict() - network_data = dict(nics=['fake_nic1', 'fake_nic2']) - self.mock_object(self._manager.network_helper, 'setup_network', - mock.Mock(return_value=network_data)) - self.mock_object(self._manager, '_get_service_image', - mock.Mock(return_value=service_image_id)) - self.mock_object(self._manager, '_get_key', - mock.Mock(return_value=key_data)) - self.mock_object(self._manager.compute_api, 'server_create', - mock.Mock(return_value=server_create)) - self.mock_object(self._manager.compute_api, 'server_get', - mock.Mock(side_effect=exception.InstanceNotFound( - instance_id=server_get['id']))) - self.mock_object(service_instance.time, 'sleep', - mock.Mock(side_effect=fake_sleep)) - self.mock_object(service_instance.time, 'time', - mock.Mock(side_effect=fake_time)) - - self.assertRaises( - exception.ServiceInstanceException, - self._manager._create_service_instance, - self._manager.admin_context, instance_name, network_info) - - self.assertTrue(service_instance.time.time.called) - self._manager.network_helper.setup_network.assert_called_once_with( - network_info) - self._manager._get_service_image.assert_called_once_with( - self._manager.admin_context) - self._manager._get_key.assert_called_once_with( - self._manager.admin_context) - self._manager.compute_api.server_create.assert_called_once_with( - self._manager.admin_context, name=instance_name, - image=service_image_id, flavor=100, - key_name=key_data[0], nics=network_data['nics']) - self._manager.compute_api.server_get.assert_called_once_with( - self._manager.admin_context, server_create['id']) - self.assertTrue(service_instance.time.sleep.called) - self.assertTrue(service_instance.time.sleep.called) - @ddt.data( dict( instance_id_included=False, @@ -1100,6 +1047,9 @@ class ServiceInstanceManagerTestCase(test.TestCase): mock.Mock(return_value=key_data)) self.mock_object( self._manager.compute_api, 'server_create', mockobj) + self.mock_object( + self._manager, 'wait_for_instance_to_be_active', + mock.Mock(side_effect=exception.ServiceInstanceException)) try: self._manager._create_service_instance( @@ -1125,19 +1075,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): image=service_image_id, flavor=100, key_name=key_data[0], nics=network_data['nics']) - @ddt.data( - mock.Mock(return_value=dict(id='fakeid', status='ERROR')), - mock.Mock(side_effect=exception.ServiceInstanceException)) - def test___create_service_instance_failed_to_build(self, mock_for_get): - def fake_sleep(time): - self.fake_time += time - - def fake_time(): - if not self.fake_time: - return 0 - return self.fake_time - - self.fake_time, self._manager.max_time_to_build_instance = 0, 2 + def test___create_service_instance_failed_to_build(self): server_create = dict(id='fakeid', status='CREATING', networks=dict()) service_image_id = 'fake_service_image_id' key_data = 'fake_key_name', 'fake_key_path' @@ -1155,11 +1093,8 @@ class ServiceInstanceManagerTestCase(test.TestCase): self.mock_object(self._manager.compute_api, 'server_create', mock.Mock(return_value=server_create)) self.mock_object( - self._manager.compute_api, 'server_get', mock_for_get) - self.mock_object(service_instance.time, 'sleep', - mock.Mock(side_effect=fake_sleep)) - self.mock_object(service_instance.time, 'time', - mock.Mock(side_effect=fake_time)) + self._manager, 'wait_for_instance_to_be_active', + mock.Mock(side_effect=exception.ServiceInstanceException)) try: self._manager._create_service_instance( @@ -1183,9 +1118,6 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager.admin_context, name=instance_name, image=service_image_id, flavor=100, key_name=key_data[0], nics=network_data['nics']) - self._manager.compute_api.server_get.assert_called_once_with( - self._manager.admin_context, server_create['id']) - self.assertTrue(service_instance.time.sleep.called) @ddt.data( dict(name=None, path=None), @@ -1207,6 +1139,77 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager._get_key.assert_called_once_with( self._manager.admin_context) + @mock.patch('time.sleep') + @mock.patch('time.time') + def _test_wait_for_instance(self, mock_time, mock_sleep, + server_get_side_eff=None, + expected_try_count=1, + expected_sleep_count=0, + expected_ret_val=None, + expected_exc=None): + mock_server_get = mock.Mock(side_effect=server_get_side_eff) + self.mock_object(self._manager.compute_api, 'server_get', + mock_server_get) + + self.fake_time = 0 + + def fake_time(): + return self.fake_time + + def fake_sleep(sleep_time): + self.fake_time += sleep_time + + # Note(lpetrut): LOG methods can call time.time + mock_time.side_effect = fake_time + mock_sleep.side_effect = fake_sleep + timeout = 3 + + if expected_exc: + self.assertRaises( + expected_exc, + self._manager.wait_for_instance_to_be_active, + instance_id=mock.sentinel.instance_id, + timeout=timeout) + else: + instance = self._manager.wait_for_instance_to_be_active( + instance_id=mock.sentinel.instance_id, + timeout=timeout) + self.assertEqual(expected_ret_val, instance) + + mock_server_get.assert_has_calls( + [mock.call(self._manager.admin_context, + mock.sentinel.instance_id)] * expected_try_count) + mock_sleep.assert_has_calls([mock.call(1)] * expected_sleep_count) + + def test_wait_for_instance_timeout(self): + server_get_side_eff = [ + exception.InstanceNotFound( + instance_id=mock.sentinel.instance_id), + {'status': 'BUILDING'}, + {'status': 'ACTIVE'}] + # Note that in this case, although the status is active, the + # 'networks' field is missing. + self._test_wait_for_instance( + server_get_side_eff=server_get_side_eff, + expected_exc=exception.ServiceInstanceException, + expected_try_count=3, + expected_sleep_count=3) + + def test_wait_for_instance_error_state(self): + mock_instance = {'status': 'ERROR'} + self._test_wait_for_instance( + server_get_side_eff=[mock_instance], + expected_exc=exception.ServiceInstanceException, + expected_try_count=1) + + def test_wait_for_instance_available(self): + mock_instance = {'status': 'ACTIVE', + 'networks': mock.sentinel.networks} + self._test_wait_for_instance( + server_get_side_eff=[mock_instance], + expected_try_count=1, + expected_ret_val=mock_instance) + class BaseNetworkHelperTestCase(test.TestCase): """Tests Base network helper for service instance."""