From 93da16d88f164dcbbc040e1fb627ab24f22d230b Mon Sep 17 00:00:00 2001 From: Zhenguo Niu Date: Thu, 7 Sep 2017 09:20:15 +0000 Subject: [PATCH] Revert "Return node(name or uuid) with server instead of node_uuid" This reverts commit 5124ae1d44429f246ce8687e8143d75dc75131d6. Change-Id: I0ff6f7e22afbe8833bbc6044b8b7f5c768c9b940 --- api-ref/source/v1/parameters.yaml | 4 +- api-ref/source/v1/servers.inc | 5 +- mogan/api/controllers/v1/servers.py | 6 +-- mogan/baremetal/driver.py | 8 +-- mogan/baremetal/ironic/driver.py | 54 +++++++++---------- .../91941bf1ebc9_initial_migration.py | 2 +- mogan/db/sqlalchemy/api.py | 4 +- mogan/db/sqlalchemy/models.py | 2 +- mogan/engine/flows/create_server.py | 11 ++-- mogan/engine/manager.py | 8 +-- mogan/objects/server.py | 2 +- mogan/scheduler/filter_scheduler.py | 4 +- mogan/tests/tempest/service/client.py | 4 +- mogan/tests/unit/api/v1/test_server.py | 4 +- mogan/tests/unit/db/utils.py | 3 +- mogan/tests/unit/engine/test_manager.py | 4 +- mogan/tests/unit/objects/test_objects.py | 2 +- 17 files changed, 62 insertions(+), 65 deletions(-) diff --git a/api-ref/source/v1/parameters.yaml b/api-ref/source/v1/parameters.yaml index 7393738e..7186f588 100644 --- a/api-ref/source/v1/parameters.yaml +++ b/api-ref/source/v1/parameters.yaml @@ -538,9 +538,9 @@ nics: in: body required: true type: object -node: +node_uuid: description: | - The name of the node which our server associated with. Only visible for admin users. + The UUID of the node which our server associated with. Only visible for admin users. in: body required: false type: string diff --git a/api-ref/source/v1/servers.inc b/api-ref/source/v1/servers.inc index de1bebd5..9d468686 100644 --- a/api-ref/source/v1/servers.inc +++ b/api-ref/source/v1/servers.inc @@ -62,7 +62,6 @@ Response - image_uuid: imageRef - availability_zone: availability_zone - addresses: addresses - - node: node - links: links - uuid: server_uuid - status: server_status @@ -215,7 +214,6 @@ Response - image_uuid: imageRef - availability_zone: availability_zone - addresses: addresses - - node: node - links: links - uuid: server_uuid - status: server_status @@ -269,7 +267,7 @@ Response - image_uuid: imageRef - availability_zone: availability_zone - addresses: addresses - - node: node + - node_uuid: node_uuid - links: links - uuid: server_uuid - status: server_status @@ -334,7 +332,6 @@ Response - image_uuid: imageRef - availability_zone: availability_zone - addresses: addresses - - node: node - links: links - uuid: server_uuid - status: server_status diff --git a/mogan/api/controllers/v1/servers.py b/mogan/api/controllers/v1/servers.py index f650643f..d7e78225 100644 --- a/mogan/api/controllers/v1/servers.py +++ b/mogan/api/controllers/v1/servers.py @@ -46,7 +46,7 @@ import re _DEFAULT_SERVER_RETURN_FIELDS = ('uuid', 'name', 'description', 'status', 'power_state') -_ONLY_ADMIN_VISIBLE_SEVER_FIELDS = ('node', 'affinity_zone',) +_ONLY_ADMIN_VISIBLE_SEVER_FIELDS = ('node_uuid', 'affinity_zone',) LOG = log.getLogger(__name__) @@ -450,8 +450,8 @@ class Server(base.APIBase): fault = {wtypes.text: types.jsontype} """The fault of the server""" - node = wtypes.text - """The backend node of the server""" + node_uuid = types.uuid + """The node UUID of the server""" affinity_zone = wtypes.text """The affinity zone of the server""" diff --git a/mogan/baremetal/driver.py b/mogan/baremetal/driver.py index 4311e6f0..23161a90 100644 --- a/mogan/baremetal/driver.py +++ b/mogan/baremetal/driver.py @@ -52,18 +52,18 @@ class BaseEngineDriver(object): """ raise NotImplementedError() - def set_power_state(self, context, node, state): + def set_power_state(self, context, node_uuid, state): """Set a node's power state. - :param node: node name or id to change power state. + :param node_uuid: node id to change power state. :param state: mogan states to change to. """ raise NotImplementedError() - def get_ports_from_node(self, node, detail=True): + def get_ports_from_node(self, node_uuid, detail=True): """Get a node's ports info. - :param node: node name or id to get ports info. + :param node_uuid: node id to get ports info. :param detail: whether to get detailed info of all the ports, default to False. """ diff --git a/mogan/baremetal/ironic/driver.py b/mogan/baremetal/ironic/driver.py index 49599e9f..02b34fbd 100644 --- a/mogan/baremetal/ironic/driver.py +++ b/mogan/baremetal/ironic/driver.py @@ -88,9 +88,9 @@ class IronicDriver(base_driver.BaseEngineDriver): super(IronicDriver, self).__init__() self.ironicclient = ironic.IronicClientWrapper() - def _get_node(self, node): - """Get a node by its name.""" - return self.ironicclient.call('node.get', node, + def _get_node(self, node_uuid): + """Get a node by its UUID.""" + return self.ironicclient.call('node.get', node_uuid, fields=_NODE_FIELDS) def _validate_server_and_node(self, server): @@ -227,16 +227,16 @@ class IronicDriver(base_driver.BaseEngineDriver): _log_ironic_polling(message, node, server) - def get_ports_from_node(self, node, detail=True): + def get_ports_from_node(self, node_uuid, detail=True): """List the MAC addresses and the port types from a node.""" ports = self.ironicclient.call("node.list_ports", - node, detail=detail) - portgroups = self.ironicclient.call("portgroup.list", node=node, + node_uuid, detail=detail) + portgroups = self.ironicclient.call("portgroup.list", node=node_uuid, detail=detail) return ports + portgroups - def plug_vif(self, node, port_id): - self.ironicclient.call("node.vif_attach", node, port_id) + def plug_vif(self, node_uuid, port_id): + self.ironicclient.call("node.vif_attach", node_uuid, port_id) def unplug_vif(self, context, server, port_id): LOG.debug("unplug: server_uuid=%(uuid)s vif=%(server_nics)s " @@ -244,7 +244,7 @@ class IronicDriver(base_driver.BaseEngineDriver): {'uuid': server.uuid, 'server_nics': str(server.nics), 'port_id': port_id}) - node = self._get_node(server.node) + node = self._get_node(server.node_uuid) self._unplug_vif(node, port_id) def _unplug_vif(self, node, port_id): @@ -268,30 +268,29 @@ class IronicDriver(base_driver.BaseEngineDriver): # The engine manager is meant to know the node uuid, so missing uuid # is a significant issue. It may mean we've been passed the wrong data. - node_ident = server.node - if not node_ident: + node_uuid = server.node_uuid + if not node_uuid: raise ironic_exc.BadRequest( _("Ironic node uuid not supplied to " "driver for server %s.") % server.uuid) # add server info to node - node = self._get_node(node_ident) + node = self._get_node(node_uuid) self._add_server_info_to_node(node, server, partitions) - # validate we are ready to do the deploy - validate_chk = self.ironicclient.call("node.validate", node_ident) + validate_chk = self.ironicclient.call("node.validate", node_uuid) if (not validate_chk.deploy.get('result') or not validate_chk.power.get('result')): raise exception.ValidationError(_( "Ironic node: %(id)s failed to validate." " (deploy: %(deploy)s, power: %(power)s)") - % {'id': server.node, + % {'id': server.node_uuid, 'deploy': validate_chk.deploy, 'power': validate_chk.power}) # trigger the node deploy try: - self.ironicclient.call("node.set_provision_state", node_ident, + self.ironicclient.call("node.set_provision_state", node_uuid, ironic_states.ACTIVE, configdrive=configdrive_value) except Exception as e: @@ -313,7 +312,7 @@ class IronicDriver(base_driver.BaseEngineDriver): LOG.error("Error deploying server %(server)s on " "baremetal node %(node)s.", {'server': server.uuid, - 'node': node_ident}) + 'node': node_uuid}) def _unprovision(self, server, node): """This method is called from destroy() to unprovision @@ -538,14 +537,13 @@ class IronicDriver(base_driver.BaseEngineDriver): """ LOG.debug('Rebuild called for server', server=server) - node_ident = server.node_ident - node = self._get_node(node_ident) + node_uuid = server.node_uuid + node = self._get_node(node_uuid) self._add_server_info_to_node(node, server, preserve_ephemeral) - # trigger the node rebuild try: self.ironicclient.call("node.set_provision_state", - node_ident, + node_uuid, ironic_states.REBUILD) except (ironic_exc.InternalServerError, ironic_exc.BadRequest) as e: @@ -575,17 +573,17 @@ class IronicDriver(base_driver.BaseEngineDriver): for the server """ node = self._validate_server_and_node(server) - node_ident = node.uuid + node_uuid = node.uuid def _get_console(): """Request ironicclient to acquire node console.""" try: - return self.ironicclient.call('node.get_console', node_ident) + return self.ironicclient.call('node.get_console', node_uuid) except (ironic_exc.InternalServerError, ironic_exc.BadRequest) as e: LOG.error('Failed to acquire console information for ' 'node %(server)s: %(reason)s', - {'server': node_ident, + {'server': node_uuid, 'reason': e}) raise exception.ConsoleNotAvailable() @@ -604,13 +602,13 @@ class IronicDriver(base_driver.BaseEngineDriver): """Request ironicclient to enable/disable node console.""" try: self.ironicclient.call( - 'node.set_console_mode', node_ident, mode) + 'node.set_console_mode', node_uuid, mode) except (ironic_exc.InternalServerError, # Validations ironic_exc.BadRequest) as e: # Maintenance LOG.error('Failed to set console mode to "%(mode)s" ' 'for node %(node)s: %(reason)s', {'mode': mode, - 'node': node_ident, + 'node': node_uuid, 'reason': e}) raise exception.ConsoleNotAvailable() @@ -623,7 +621,7 @@ class IronicDriver(base_driver.BaseEngineDriver): LOG.error('Timeout while waiting for console mode to be ' 'set to "%(mode)s" on node %(node)s', {'mode': mode, - 'node': node_ident}) + 'node': node_uuid}) raise exception.ConsoleNotAvailable() # Acquire the console @@ -651,7 +649,7 @@ class IronicDriver(base_driver.BaseEngineDriver): return {'node': node, 'console_info': console['console_info']} else: - LOG.debug('Console is disabled for node %s', node_ident) + LOG.debug('Console is disabled for node %s', node_uuid) raise exception.ConsoleNotAvailable() def get_serial_console(self, context, server, console_type): diff --git a/mogan/db/sqlalchemy/alembic/versions/91941bf1ebc9_initial_migration.py b/mogan/db/sqlalchemy/alembic/versions/91941bf1ebc9_initial_migration.py index 753035a2..0bf61a49 100644 --- a/mogan/db/sqlalchemy/alembic/versions/91941bf1ebc9_initial_migration.py +++ b/mogan/db/sqlalchemy/alembic/versions/91941bf1ebc9_initial_migration.py @@ -74,7 +74,7 @@ def upgrade(): sa.Column('image_uuid', sa.String(length=36), nullable=True), sa.Column('launched_at', sa.DateTime(), nullable=True), sa.Column('availability_zone', sa.String(length=255), nullable=True), - sa.Column('node', sa.String(length=255), nullable=True), + sa.Column('node_uuid', sa.String(length=36), nullable=True), sa.Column('extra', sa.Text(), nullable=True), sa.Column('partitions', sa.Text(), nullable=True), sa.Column('locked', sa.Boolean(), nullable=True), diff --git a/mogan/db/sqlalchemy/api.py b/mogan/db/sqlalchemy/api.py index 47ae0195..9a51d19a 100644 --- a/mogan/db/sqlalchemy/api.py +++ b/mogan/db/sqlalchemy/api.py @@ -127,8 +127,8 @@ class Connection(api.Connection): query = query.filter_by(flavor_uuid=filters['flavor_uuid']) if 'image_uuid' in filters: query = query.filter_by(image_uuid=filters['image_uuid']) - if 'node' in filters: - query = query.filter_by(node=filters['node']) + if 'node_uuid' in filters: + query = query.filter_by(node_uuid=filters['node_uuid']) return query @oslo_db_api.retry_on_deadlock diff --git a/mogan/db/sqlalchemy/models.py b/mogan/db/sqlalchemy/models.py index 11bf5ab8..30fbdd90 100644 --- a/mogan/db/sqlalchemy/models.py +++ b/mogan/db/sqlalchemy/models.py @@ -85,7 +85,7 @@ class Server(Base): flavor_uuid = Column(String(36), nullable=True) availability_zone = Column(String(255), nullable=True) image_uuid = Column(String(36), nullable=True) - node = Column(String(255), nullable=True) + node_uuid = Column(String(36), nullable=True) launched_at = Column(DateTime, nullable=True) extra = Column(db_types.JsonEncodedDict) partitions = Column(db_types.JsonEncodedDict) diff --git a/mogan/engine/flows/create_server.py b/mogan/engine/flows/create_server.py index 079c5279..f6f72eb9 100644 --- a/mogan/engine/flows/create_server.py +++ b/mogan/engine/flows/create_server.py @@ -102,7 +102,7 @@ class OnFailureRescheduleTask(flow_utils.MoganTask): def revert(self, context, result, flow_failures, server, **kwargs): # Clean up associated node uuid - server.node = None + server.node_uuid = None server.save() # Check if we have a cause which can tell us not to reschedule and @@ -137,13 +137,13 @@ class BuildNetworkTask(flow_utils.MoganTask): # TODO(zhenguo): This seems not needed as our scheduler has already # guaranteed this. - ports = self.manager.driver.get_ports_from_node(server.node) + ports = self.manager.driver.get_ports_from_node(server.node_uuid) if len(requested_networks) > len(ports): raise exception.InterfacePlugException(_( "Ironic node: %(id)s virtual to physical interface count" " mismatch" " (Vif count: %(vif_count)d, Pif count: %(pif_count)d)") - % {'id': server.node, + % {'id': server.node_uuid, 'vif_count': len(requested_networks), 'pif_count': len(ports)}) @@ -167,7 +167,8 @@ class BuildNetworkTask(flow_utils.MoganTask): server_nic = objects.ServerNic(context, **nic_dict) nics_obj.objects.append(server_nic) - self.manager.driver.plug_vif(server.node, port['id']) + self.manager.driver.plug_vif(server.node_uuid, + port['id']) # Get updated VIF info port_dict = self.manager.network_api.show_port( context, port.get('id')) @@ -267,7 +268,7 @@ class CreateServerTask(flow_utils.MoganTask): configdrive_value = configdrive.get('value') self.driver.spawn(context, server, configdrive_value, partitions) LOG.info('Successfully provisioned Ironic node %s', - server.node) + server.node_uuid) def revert(self, context, result, flow_failures, server, **kwargs): LOG.debug("Server %s: destroy backend node", server.uuid) diff --git a/mogan/engine/manager.py b/mogan/engine/manager.py index 25b03f21..fe3725ad 100644 --- a/mogan/engine/manager.py +++ b/mogan/engine/manager.py @@ -109,7 +109,7 @@ class EngineManager(base_manager.BaseEngineManager): for rp in all_rps: if rp['uuid'] not in node_uuids: server_by_node = objects.Server.list( - context, filters={'node': rp['name']}) + context, filters={'node_uuid': rp['uuid']}) if server_by_node: continue self.scheduler_client.reportclient.delete_resource_provider( @@ -345,7 +345,7 @@ class EngineManager(base_manager.BaseEngineManager): {"nodes": nodes}) for (server, node) in six.moves.zip(servers, nodes): - server.node = node + server.node_uuid = node server.save() # Add a retry entry for the selected node retry_nodes = retry['nodes'] @@ -469,7 +469,7 @@ class EngineManager(base_manager.BaseEngineManager): # Issue delete request to driver only if server is associated with # a underlying node. - if server.node: + if server.node_uuid: do_delete_server(server) server.power_state = states.NOSTATE @@ -594,7 +594,7 @@ class EngineManager(base_manager.BaseEngineManager): try: vif = self.network_api.bind_port(context, vif_port['id'], server) vif_port = vif['port'] - self.driver.plug_vif(server.node, vif_port['id']) + self.driver.plug_vif(server.node_uuid, vif_port['id']) nics_obj = objects.ServerNics(context) nic_dict = {'port_id': vif_port['id'], 'network_id': vif_port['network_id'], diff --git a/mogan/objects/server.py b/mogan/objects/server.py index 72ad31b8..2a5c99b1 100644 --- a/mogan/objects/server.py +++ b/mogan/objects/server.py @@ -49,7 +49,7 @@ class Server(base.MoganObject, object_base.VersionedObjectDictCompat): 'image_uuid': object_fields.UUIDField(nullable=True), 'nics': object_fields.ObjectField('ServerNics', nullable=True), 'fault': object_fields.ObjectField('ServerFault', nullable=True), - 'node': object_fields.StringField(nullable=True), + 'node_uuid': object_fields.UUIDField(nullable=True), 'launched_at': object_fields.DateTimeField(nullable=True), 'metadata': object_fields.FlexibleDictField(nullable=True), 'partitions': object_fields.FlexibleDictField(nullable=True), diff --git a/mogan/scheduler/filter_scheduler.py b/mogan/scheduler/filter_scheduler.py index 39d06a8e..1f258fdc 100644 --- a/mogan/scheduler/filter_scheduler.py +++ b/mogan/scheduler/filter_scheduler.py @@ -128,7 +128,7 @@ class FilterScheduler(driver.Scheduler): agg_uuids = [agg.uuid for agg in aggregates] query_filters = {'member_of': 'in:' + ','.join(agg_uuids)} rps = self.reportclient.get_filtered_resource_providers(query_filters) - return [rp['name'] for rp in rps] + return [rp['uuid'] for rp in rps] def _get_filtered_affzs_nodes(self, context, server_group, filtered_nodes, num_servers): @@ -260,7 +260,7 @@ class FilterScheduler(driver.Scheduler): query_filters = {'resources': resources_filter} filtered_nodes = self.reportclient.\ get_filtered_resource_providers(query_filters) - return [node['name'] for node in filtered_nodes] + return [node['uuid'] for node in filtered_nodes] return list(filtered_nodes) diff --git a/mogan/tests/tempest/service/client.py b/mogan/tests/tempest/service/client.py index f3c80281..e3a9247d 100644 --- a/mogan/tests/tempest/service/client.py +++ b/mogan/tests/tempest/service/client.py @@ -315,12 +315,12 @@ class BaremetalNodeClient(rest_client.RestClient): body = self.deserialize(body)['nodes'] return rest_client.ResponseBodyList(resp, body) - def show_bm_node(self, node_ident=None, service_id=None): + def show_bm_node(self, node_uuid=None, service_id=None): if service_id: uri = '%s/nodes/detail?instance_uuid=%s' % (self.uri_prefix, service_id) else: - uri = '%s/nodes/%s' % (self.uri_prefix, node_ident) + uri = '%s/nodes/%s' % (self.uri_prefix, node_uuid) resp, body = self.get(uri) self.expected_success(200, resp.status) body = self.deserialize(body) diff --git a/mogan/tests/unit/api/v1/test_server.py b/mogan/tests/unit/api/v1/test_server.py index 0e7be9f5..7230d842 100644 --- a/mogan/tests/unit/api/v1/test_server.py +++ b/mogan/tests/unit/api/v1/test_server.py @@ -138,7 +138,7 @@ class TestServerAuthorization(v1_test.APITestV1): headers = self.gen_headers(self.context, roles="no-admin") resp = self.get_json('/servers/%s' % self.server1.uuid, headers=headers) - self.assertNotIn('node', resp) + self.assertNotIn('node_uuid', resp) def test_server_get_one_by_admin(self): # when the evil tenant is admin, he can do everything. @@ -146,7 +146,7 @@ class TestServerAuthorization(v1_test.APITestV1): headers = self.gen_headers(self.context, roles="admin") resp = self.get_json('/servers/%s' % self.server1.uuid, headers=headers) - self.assertIn('node', resp) + self.assertIn('node_uuid', resp) def test_server_get_one_unauthorized(self): # not admin and not the owner diff --git a/mogan/tests/unit/db/utils.py b/mogan/tests/unit/db/utils.py index 7d40d0a5..44b18a49 100644 --- a/mogan/tests/unit/db/utils.py +++ b/mogan/tests/unit/db/utils.py @@ -54,7 +54,8 @@ def get_test_server(**kw): 'image_uuid': kw.get('image_uuid', 'ac3b2291-b9ef-45f6-8eeb-21ac568a64a5'), 'nics': kw.get('nics', fake_server_nics), - 'node': kw.get('node', 'node-0'), + 'node_uuid': kw.get('node_uuid', + 'f978ef48-d4af-4dad-beec-e6174309bc71'), 'launched_at': kw.get('launched_at'), 'extra': kw.get('extra', {}), 'partitions': kw.get('partitions', {}), diff --git a/mogan/tests/unit/engine/test_manager.py b/mogan/tests/unit/engine/test_manager.py index ac851c1b..8bc8ba0d 100644 --- a/mogan/tests/unit/engine/test_manager.py +++ b/mogan/tests/unit/engine/test_manager.py @@ -98,7 +98,7 @@ class ManageServerTestCase(mgr_utils.ServiceSetUpMixin, fake_node = mock.MagicMock() fake_node.provision_state = ironic_states.ACTIVE server = obj_utils.create_test_server( - self.context, status=states.DELETING, node=None) + self.context, status=states.DELETING, node_uuid=None) self._start_service() self.service.delete_server(self.context, server) @@ -204,7 +204,7 @@ class ManageServerTestCase(mgr_utils.ServiceSetUpMixin, fake_node = mock.MagicMock() fake_node.provision_state = ironic_states.ACTIVE server = obj_utils.create_test_server( - self.context, status=states.ACTIVE, node=None) + self.context, status=states.ACTIVE, node_uuid=None) port_id = server['nics'][0]['port_id'] self._start_service() self.service.detach_interface(self.context, server, port_id) diff --git a/mogan/tests/unit/objects/test_objects.py b/mogan/tests/unit/objects/test_objects.py index e4f2d5d9..337317ed 100644 --- a/mogan/tests/unit/objects/test_objects.py +++ b/mogan/tests/unit/objects/test_objects.py @@ -382,7 +382,7 @@ class _TestObject(object): # version bump. It is md5 hash of object fields and remotable methods. # The fingerprint values should only be changed if there is a version bump. expected_object_fingerprints = { - 'Server': '1.0-9063f3b9d00d4635c4d1f53183da848b', + 'Server': '1.0-2ede97617e84ef9eed6ac61e67b13167', 'ServerFault': '1.0-74349ff701259e4834b4e9dc2dac1b12', 'ServerFaultList': '1.0-43e8aad0258652921f929934e9e048fd', 'Flavor': '1.0-9f7166aa387d89ec40cd699019d0c9a9',