From a4b1ee741f09c73c9771f72c206859aba3b215f8 Mon Sep 17 00:00:00 2001 From: Zhenguo Niu Date: Mon, 11 Sep 2017 10:58:53 +0800 Subject: [PATCH] Return node name with server API object Currently we support to list compute nodes and aggregate nodes with names, but server show will return the node uuid always, which makes the admins have to go to ironic to get the relationship between name and uuid, so this changes to return node name. Change-Id: I6f1e5d0f9cfe277fe952d21b38ebfe68dde4f745 Closes-Bug: #1715036 --- 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 | 7 +++++++ mogan/baremetal/ironic/driver.py | 13 +++++++++++++ .../versions/91941bf1ebc9_initial_migration.py | 1 + mogan/db/sqlalchemy/models.py | 1 + mogan/engine/flows/create_server.py | 3 ++- mogan/engine/manager.py | 1 + mogan/objects/server.py | 1 + mogan/tests/unit/api/v1/test_server.py | 4 ++-- mogan/tests/unit/db/utils.py | 1 + mogan/tests/unit/objects/test_objects.py | 2 +- 13 files changed, 39 insertions(+), 10 deletions(-) diff --git a/api-ref/source/v1/parameters.yaml b/api-ref/source/v1/parameters.yaml index 7186f588..036c9243 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_uuid: +node: description: | - The UUID of the node which our server associated with. Only visible for admin users. + 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 9d468686..de1bebd5 100644 --- a/api-ref/source/v1/servers.inc +++ b/api-ref/source/v1/servers.inc @@ -62,6 +62,7 @@ Response - image_uuid: imageRef - availability_zone: availability_zone - addresses: addresses + - node: node - links: links - uuid: server_uuid - status: server_status @@ -214,6 +215,7 @@ Response - image_uuid: imageRef - availability_zone: availability_zone - addresses: addresses + - node: node - links: links - uuid: server_uuid - status: server_status @@ -267,7 +269,7 @@ Response - image_uuid: imageRef - availability_zone: availability_zone - addresses: addresses - - node_uuid: node_uuid + - node: node - links: links - uuid: server_uuid - status: server_status @@ -332,6 +334,7 @@ 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 d7e78225..f650643f 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_uuid', 'affinity_zone',) +_ONLY_ADMIN_VISIBLE_SEVER_FIELDS = ('node', '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_uuid = types.uuid - """The node UUID of the server""" + node = wtypes.text + """The backend node 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 6ed21798..cd9d9240 100644 --- a/mogan/baremetal/driver.py +++ b/mogan/baremetal/driver.py @@ -139,6 +139,13 @@ class BaseEngineDriver(object): """ raise NotImplementedError() + def get_node_name(node): + """Get the name of a node. + + :param node: the uuid of the node. + """ + raise NotImplementedError() + def get_manageable_nodes(self): """Retrieve all manageable nodes information. diff --git a/mogan/baremetal/ironic/driver.py b/mogan/baremetal/ironic/driver.py index 32ffa6c4..00e3c0f8 100644 --- a/mogan/baremetal/ironic/driver.py +++ b/mogan/baremetal/ironic/driver.py @@ -740,6 +740,19 @@ class IronicDriver(base_driver.BaseEngineDriver): return (not node.instance_uuid and node.provision_state == ironic_states.AVAILABLE) + def get_node_name(self, node): + """Get the name of a node. + + :param node: the uuid of the node. + """ + try: + node = self.ironicclient.call( + 'node.get', node, fields=('name',)) + except Exception: + return None + + return node.name + def get_manageable_nodes(self): nodes = self._get_manageable_nodes() manageable_nodes = [] diff --git a/mogan/db/sqlalchemy/alembic/versions/91941bf1ebc9_initial_migration.py b/mogan/db/sqlalchemy/alembic/versions/91941bf1ebc9_initial_migration.py index 09c407f0..3067286e 100644 --- a/mogan/db/sqlalchemy/alembic/versions/91941bf1ebc9_initial_migration.py +++ b/mogan/db/sqlalchemy/alembic/versions/91941bf1ebc9_initial_migration.py @@ -78,6 +78,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), diff --git a/mogan/db/sqlalchemy/models.py b/mogan/db/sqlalchemy/models.py index d2c186c8..3a2311f7 100644 --- a/mogan/db/sqlalchemy/models.py +++ b/mogan/db/sqlalchemy/models.py @@ -85,6 +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) diff --git a/mogan/engine/flows/create_server.py b/mogan/engine/flows/create_server.py index accb52ea..45a2de25 100644 --- a/mogan/engine/flows/create_server.py +++ b/mogan/engine/flows/create_server.py @@ -101,8 +101,9 @@ class OnFailureRescheduleTask(flow_utils.MoganTask): filter_properties=filter_properties) def revert(self, context, result, flow_failures, server, **kwargs): - # Clean up associated node uuid + # Clean up associated node server.node_uuid = None + server.node = None server.save() # Check if we have a cause which can tell us not to reschedule and diff --git a/mogan/engine/manager.py b/mogan/engine/manager.py index 7dc4780d..71626f86 100644 --- a/mogan/engine/manager.py +++ b/mogan/engine/manager.py @@ -345,6 +345,7 @@ class EngineManager(base_manager.BaseEngineManager): for (server, node) in six.moves.zip(servers, nodes): server.node_uuid = node + server.node = self.driver.get_node_name(node) server.save() # Add a retry entry for the selected node retry_nodes = retry['nodes'] diff --git a/mogan/objects/server.py b/mogan/objects/server.py index 2a5c99b1..f033f740 100644 --- a/mogan/objects/server.py +++ b/mogan/objects/server.py @@ -49,6 +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), diff --git a/mogan/tests/unit/api/v1/test_server.py b/mogan/tests/unit/api/v1/test_server.py index 7230d842..0e7be9f5 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_uuid', resp) + self.assertNotIn('node', 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_uuid', resp) + self.assertIn('node', 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 2cb16a7d..16586736 100644 --- a/mogan/tests/unit/db/utils.py +++ b/mogan/tests/unit/db/utils.py @@ -55,6 +55,7 @@ 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'), diff --git a/mogan/tests/unit/objects/test_objects.py b/mogan/tests/unit/objects/test_objects.py index 337317ed..676e871a 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-2ede97617e84ef9eed6ac61e67b13167', + 'Server': '1.0-6b13b984cd3656a977456a12d3d1c167', 'ServerFault': '1.0-74349ff701259e4834b4e9dc2dac1b12', 'ServerFaultList': '1.0-43e8aad0258652921f929934e9e048fd', 'Flavor': '1.0-9f7166aa387d89ec40cd699019d0c9a9',