From 1f168e96bd9c6f3fdf05fb528e87c08b3e85a295 Mon Sep 17 00:00:00 2001 From: Devananda van der Veen Date: Fri, 14 Jun 2013 08:57:27 -0700 Subject: [PATCH] Add object support to API service. The previous api/controllers/v1.py code was an early straw-man that directly represented db/sqlalchemy/models.py data structures as a proof-of-concept for the API service. This replaces that by using the versioned object models which were recently ported from Nova, wrapping them in an API class to allow for type checking and versioning. This patch also removes other methods which were part of that early proof-of-concept, and which will be implemented according to the v1 API spec in future patches. It also removes the unused api/models/__init__.py and db/models.py files. Partially implements bp:api-v1-impl Change-Id: I2580d863b383e12628821c9156195c00806beebd --- ironic/api/controllers/v1.py | 178 +++++--------- ironic/api/model/__init__.py | 16 -- .../compute/contrib/baremetal_nodes.py | 219 ------------------ ironic/db/models.py | 70 ------ ironic/db/sqlalchemy/api.py | 13 ++ ironic/objects/node.py | 17 +- ironic/objects/utils.py | 14 ++ ironic/tests/objects/test_node.py | 12 +- 8 files changed, 103 insertions(+), 436 deletions(-) delete mode 100644 ironic/api/model/__init__.py delete mode 100644 ironic/api/openstack/compute/contrib/baremetal_nodes.py delete mode 100644 ironic/db/models.py diff --git a/ironic/api/controllers/v1.py b/ironic/api/controllers/v1.py index 2f91ce436a..6ec8153519 100644 --- a/ironic/api/controllers/v1.py +++ b/ironic/api/controllers/v1.py @@ -18,8 +18,11 @@ """ Version 1 of the Ironic API +NOTE: IN PROGRESS AND NOT FULLY IMPLEMENTED. + Should maintain feature parity with Nova Baremetal Extension. -Specification in ironic/doc/api/v1.rst + +Specification can be found at ironic/doc/api/v1.rst """ import pecan @@ -29,24 +32,13 @@ import wsme from wsme import types as wtypes import wsmeext.pecan as wsme_pecan +from ironic.objects import node as node_obj from ironic.openstack.common import log -# TODO(deva): The API shouldn't know what db IMPL is in use. -# Import ironic.db.models once that layer is written. - LOG = log.getLogger(__name__) -class Base(wtypes.Base): - - def __init__(self, **kwargs): - self.fields = list(kwargs) - for k, v in kwargs.iteritems(): - setattr(self, k, v) - - @classmethod - def from_db_model(cls, m): - return cls(**m.as_dict()) +class APIBase(wtypes.Base): def as_dict(self): return dict((k, getattr(self, k)) @@ -55,145 +47,91 @@ class Base(wtypes.Base): getattr(self, k) != wsme.Unset) -class Interface(Base): - """A representation of a network interface for a baremetal node.""" +class Node(APIBase): + """API representation of a bare metal node. - node_id = int - address = wtypes.text - - @classmethod - def sample(cls): - return cls(node_id=1, - address='52:54:00:cf:2d:31', - ) - - -class InterfacesController(rest.RestController): - """REST controller for Interfaces.""" - - @wsme_pecan.wsexpose(Interface, unicode) - def post(self, iface): - """Ceate a new interface.""" - return Interface.sample() - - @wsme_pecan.wsexpose() - def get_all(self): - """Retrieve a list of all interfaces.""" - ifaces = [Interface.sample()] - return [(i.node_id, i.address) for i in ifaces] - - @wsme_pecan.wsexpose(Interface, unicode) - def get_one(self, address): - """Retrieve information about the given interface.""" - r = pecan.request.dbapi.get_iface(address) - return Interface.from_db_model(r) - - @wsme_pecan.wsexpose() - def delete(self, iface_id): - """Delete an interface.""" - pass - - @wsme_pecan.wsexpose() - def put(self, iface_id): - """Update an interface.""" - pass - - -class Node(Base): - """A representation of a bare metal node.""" + This class enforces type checking and value constraints, and converts + between the internal object model and the API representation of a node. + """ + # NOTE: translate 'id' publicly to 'uuid' internally uuid = wtypes.text - cpu_arch = wtypes.text - cpu_num = int - memory = int - local_storage_max = int - task_state = wtypes.text - image_path = wtypes.text instance_uuid = wtypes.text - instance_name = wtypes.text - power_info = wtypes.text - extra = wtypes.text - @classmethod - def sample(cls): - power_info = "{'driver': 'ipmi', 'user': 'fake', " \ - + "'password': 'password', 'address': '1.2.3.4'}" - return cls(uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123', - cpu_arch='x86_64', - cpu_num=4, - memory=16384, - local_storage_max=1000, - task_state='NOSTATE', - image_path='/fake/image/path', - instance_uuid='8227348d-5f1d-4488-aad1-7c92b2d42504', - power_info=power_info, - extra='{}', - ) + # NOTE: task_* fields probably need to be reworked to match API spec + task_state = wtypes.text + task_start = wtypes.text + # NOTE: allow arbitrary dicts for driver_info and extra so that drivers + # and vendors can expand on them without requiring API changes. + # NOTE: translate 'driver_info' internally to 'management_configuration' + driver = wtypes.text + driver_info = {wtypes.text: wtypes.text} -class NodeIfaceController(rest.RestController): - """For GET /node/ifaces/.""" + # NOTE: translate 'extra' internally to 'meta_data' externally + extra = {wtypes.text: wtypes.text} - @wsme_pecan.wsexpose([Interface], unicode) - def get(self, node_id): - return [Interface.from_db_model(r) - for r in pecan.request.dbapi.get_ifaces_for_node(node_id)] + # NOTE: properties should use a class to enforce required properties + # current list: arch, cpus, disk, ram, image + properties = {wtypes.text: wtypes.text} + # NOTE: translate 'chassis_id' to a link to the chassis resource + # and accept a chassis uuid when creating a node. + chassis_id = int -class NodePowerController(rest.RestController): - """Initial mock of an API for /node//power.""" + # NOTE: also list / link to ports associated with this node - @wsme_pecan.wsexpose(unicode, unicode) - def get_one(self, node_id): - return pecan.request.rpcapi.get_node_power_state( - pecan.request.context, - node_id) + def __init__(self, **kwargs): + self.fields = node_obj.Node.fields.keys() + for k in self.fields: + setattr(self, k, kwargs.get(k)) class NodesController(rest.RestController): """REST controller for Nodes.""" + @wsme_pecan.wsexpose(Node, unicode) + def get_one(self, uuid): + """Retrieve information about the given node.""" + node = node_obj.Node.get_by_uuid(pecan.request.context, uuid) + return node + @wsme.validate(Node) - @wsme_pecan.wsexpose(Node, body=Node, status_code=201) + @wsme_pecan.wsexpose(Node, body=Node) def post(self, node): """Ceate a new node.""" try: - d = node.as_dict() - r = pecan.request.dbapi.create_node(d) + new_node = pecan.request.dbapi.create_node(node.as_dict()) except Exception as e: LOG.exception(e) raise wsme.exc.ClientSideError(_("Invalid data")) - return Node.from_db_model(r) + return new_node - @wsme_pecan.wsexpose() - def get_all(self): - """Retrieve a list of all nodes.""" - pass + @wsme.validate(Node) + @wsme_pecan.wsexpose(Node, unicode, body=Node) + def put(self, uuid, delta_node): + """Update an existing node.""" + node = node_obj.Node.get_by_uuid(pecan.request.context, uuid) + # NOTE: delta_node will be a full API Node instance, but only user- + # supplied fields will be set, so we extract those by converting + # the object to a dict, then scanning for non-None values, and + # only applying those changes to the Node object instance. + items = delta_node.as_dict().items() + for k, v in [(k, v) for (k, v) in items if v]: + node[k] = v - @wsme_pecan.wsexpose(Node, unicode) - def get_one(self, node_id): - """Retrieve information about the given node.""" - r = pecan.request.dbapi.get_node(node_id) - return Node.from_db_model(r) + # TODO(deva): catch exceptions here if node_obj refuses to save. + node.save() + + return node @wsme_pecan.wsexpose() def delete(self, node_id): """Delete a node.""" pecan.request.dbapi.destroy_node(node_id) - @wsme_pecan.wsexpose() - def put(self, node_id): - """Update a node.""" - pass - - ifaces = NodeIfaceController() - power = NodePowerController() - class Controller(object): """Version 1 API controller root.""" - # TODO(deva): _default and index - nodes = NodesController() - interfaces = InterfacesController() diff --git a/ironic/api/model/__init__.py b/ironic/api/model/__init__.py deleted file mode 100644 index 56425d0fce..0000000000 --- a/ironic/api/model/__init__.py +++ /dev/null @@ -1,16 +0,0 @@ -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - -# Copyright 2013 Hewlett-Packard Development Company, L.P. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. diff --git a/ironic/api/openstack/compute/contrib/baremetal_nodes.py b/ironic/api/openstack/compute/contrib/baremetal_nodes.py deleted file mode 100644 index f69db50f00..0000000000 --- a/ironic/api/openstack/compute/contrib/baremetal_nodes.py +++ /dev/null @@ -1,219 +0,0 @@ -# Copyright (c) 2013 NTT DOCOMO, INC. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -"""The bare-metal admin extension.""" - -import webob - -from nova.api.openstack import extensions -from nova.api.openstack import wsgi -from nova.api.openstack import xmlutil -from nova import exception -from nova.virt.baremetal import db - -authorize = extensions.extension_authorizer('compute', 'baremetal_nodes') - -node_fields = ['id', 'cpus', 'local_gb', 'memory_mb', 'pm_address', - 'pm_user', - 'service_host', 'terminal_port', 'instance_uuid', - ] - -interface_fields = ['id', 'address', 'datapath_id', 'port_no'] - - -def _node_dict(node_ref): - d = {} - for f in node_fields: - d[f] = node_ref.get(f) - return d - - -def _interface_dict(interface_ref): - d = {} - for f in interface_fields: - d[f] = interface_ref.get(f) - return d - - -def _make_node_elem(elem): - for f in node_fields: - elem.set(f) - - -def _make_interface_elem(elem): - for f in interface_fields: - elem.set(f) - - -class NodeTemplate(xmlutil.TemplateBuilder): - def construct(self): - node_elem = xmlutil.TemplateElement('node', selector='node') - _make_node_elem(node_elem) - ifs_elem = xmlutil.TemplateElement('interfaces') - if_elem = xmlutil.SubTemplateElement(ifs_elem, 'interface', - selector='interfaces') - _make_interface_elem(if_elem) - node_elem.append(ifs_elem) - return xmlutil.MasterTemplate(node_elem, 1) - - -class NodesTemplate(xmlutil.TemplateBuilder): - def construct(self): - root = xmlutil.TemplateElement('nodes') - node_elem = xmlutil.SubTemplateElement(root, 'node', selector='nodes') - _make_node_elem(node_elem) - ifs_elem = xmlutil.TemplateElement('interfaces') - if_elem = xmlutil.SubTemplateElement(ifs_elem, 'interface', - selector='interfaces') - _make_interface_elem(if_elem) - node_elem.append(ifs_elem) - return xmlutil.MasterTemplate(root, 1) - - -class InterfaceTemplate(xmlutil.TemplateBuilder): - def construct(self): - root = xmlutil.TemplateElement('interface', selector='interface') - _make_interface_elem(root) - return xmlutil.MasterTemplate(root, 1) - - -class BareMetalNodeController(wsgi.Controller): - """The Bare-Metal Node API controller for the OpenStack API.""" - - @wsgi.serializers(xml=NodesTemplate) - def index(self, req): - context = req.environ['nova.context'] - authorize(context) - nodes_from_db = db.bm_node_get_all(context) - nodes = [] - for node_from_db in nodes_from_db: - try: - ifs = db.bm_interface_get_all_by_bm_node_id( - context, node_from_db['id']) - except exception.NodeNotFound: - ifs = [] - node = _node_dict(node_from_db) - node['interfaces'] = [_interface_dict(i) for i in ifs] - nodes.append(node) - return {'nodes': nodes} - - @wsgi.serializers(xml=NodeTemplate) - def show(self, req, id): - context = req.environ['nova.context'] - authorize(context) - try: - node = db.bm_node_get(context, id) - except exception.NodeNotFound: - raise webob.exc.HTTPNotFound - try: - ifs = db.bm_interface_get_all_by_bm_node_id(context, id) - except exception.NodeNotFound: - ifs = [] - node = _node_dict(node) - node['interfaces'] = [_interface_dict(i) for i in ifs] - return {'node': node} - - @wsgi.serializers(xml=NodeTemplate) - def create(self, req, body): - context = req.environ['nova.context'] - authorize(context) - values = body['node'].copy() - prov_mac_address = values.pop('prov_mac_address', None) - node = db.bm_node_create(context, values) - node = _node_dict(node) - if prov_mac_address: - if_id = db.bm_interface_create(context, - bm_node_id=node['id'], - address=prov_mac_address, - datapath_id=None, - port_no=None) - if_ref = db.bm_interface_get(context, if_id) - node['interfaces'] = [_interface_dict(if_ref)] - else: - node['interfaces'] = [] - print node - return {'node': node} - - def delete(self, req, id): - context = req.environ['nova.context'] - authorize(context) - try: - db.bm_node_destroy(context, id) - except exception.NodeNotFound: - raise webob.exc.HTTPNotFound - return webob.Response(status_int=202) - - def _check_node_exists(self, context, node_id): - try: - db.bm_node_get(context, node_id) - except exception.NodeNotFound: - raise webob.exc.HTTPNotFound - - @wsgi.serializers(xml=InterfaceTemplate) - @wsgi.action('add_interface') - def _add_interface(self, req, id, body): - context = req.environ['nova.context'] - authorize(context) - self._check_node_exists(context, id) - body = body['add_interface'] - address = body['address'] - datapath_id = body.get('datapath_id') - port_no = body.get('port_no') - if_id = db.bm_interface_create(context, - bm_node_id=id, - address=address, - datapath_id=datapath_id, - port_no=port_no) - if_ref = db.bm_interface_get(context, if_id) - return {'interface': _interface_dict(if_ref)} - - @wsgi.response(202) - @wsgi.action('remove_interface') - def _remove_interface(self, req, id, body): - context = req.environ['nova.context'] - authorize(context) - self._check_node_exists(context, id) - body = body['remove_interface'] - if_id = body.get('id') - address = body.get('address') - if not if_id and not address: - raise webob.exc.HTTPBadRequest( - explanation=_("Must specify id or address")) - ifs = db.bm_interface_get_all_by_bm_node_id(context, id) - for i in ifs: - if if_id and if_id != i['id']: - continue - if address and address != i['address']: - continue - db.bm_interface_destroy(context, i['id']) - return webob.Response(status_int=202) - raise webob.exc.HTTPNotFound - - -class Baremetal_nodes(extensions.ExtensionDescriptor): - """Admin-only bare-metal node administration.""" - - name = "BareMetalNodes" - alias = "os-baremetal-nodes" - namespace = "http://docs.openstack.org/compute/ext/baremetal_nodes/api/v2" - updated = "2013-01-04T00:00:00+00:00" - - def get_resources(self): - resources = [] - res = extensions.ResourceExtension('os-baremetal-nodes', - BareMetalNodeController(), - member_actions={"action": "POST", }) - resources.append(res) - return resources diff --git a/ironic/db/models.py b/ironic/db/models.py deleted file mode 100644 index 6bda973ea5..0000000000 --- a/ironic/db/models.py +++ /dev/null @@ -1,70 +0,0 @@ -# -*- encoding: utf-8 -*- -# -# Copyright © 2013 New Dream Network, LLC (DreamHost) -# -# Author: Doug Hellmann -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -""" -Model classes for use above the storage layer. - -NOT YET IMPLEMENTED. -""" - - -class Model(object): - """Base class for API models.""" - - def __init__(self, **kwds): - self.fields = list(kwds) - for k, v in kwds.iteritems(): - setattr(self, k, v) - - def as_dict(self): - d = {} - for f in self.fields: - v = getattr(self, f) - if isinstance(v, Model): - v = v.as_dict() - elif isinstance(v, list) and v and isinstance(v[0], Model): - v = [sub.as_dict() for sub in v] - d[f] = v - return d - - def __eq__(self, other): - return self.as_dict() == other.as_dict() - - -class Node(Model): - """Representation of a bare metal node.""" - - def __init__(self, uuid, power_info, task_state, image_path, - instance_uuid, instance_name, extra): - Model.__init__(uuid=uuid, - power_info=power_info, - task_state=task_state, - image_path=image_path, - instance_uuid=instance_uuid, - instance_name=instance_name, - extra=extra, - ) - - -class Iface(Model): - """Representation of a network interface.""" - - def __init__(self, mac, node_id, extra): - Model.__init__(mac=mac, - node_id=node_id, - extra=extra, - ) diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index fc6f205a84..c41598d865 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -23,6 +23,7 @@ from oslo.config import cfg from sqlalchemy.orm.exc import NoResultFound from ironic.common import exception +from ironic.common import states from ironic.common import utils from ironic.db import api from ironic.db.sqlalchemy import models @@ -152,6 +153,18 @@ class Connection(api.Connection): @objects.objectify(objects.Node) def create_node(self, values): + # ensure defaults are present for new nodes + if not values.get('uuid'): + values['uuid'] = uuidutils.generate_uuid() + if not values.get('task_state'): + values['task_state'] = states.NOSTATE + if not values.get('properties'): + values['properties'] = '{}' + if not values.get('extra'): + values['extra'] = '{}' + if not values.get('driver_info'): + values['driver_info'] = '{}' + node = models.Node() node.update(values) node.save() diff --git a/ironic/objects/node.py b/ironic/objects/node.py index 25ddfa78dd..33fb8182c0 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -27,19 +27,22 @@ class Node(base.IronicObject): 'id': int, 'uuid': utils.str_or_none, + # NOTE: chassis_id should be read-only after node is created 'chassis_id': utils.int_or_none, + # NOTE: instance_uuid must be read-only when node is provisioned 'instance_uuid': utils.str_or_none, - # NOTE(deva): should driver_info be a nested_object_or_none, - # or does this bind the driver API too tightly? + # NOTE: driver should be read-only after node is created 'driver': utils.str_or_none, - 'driver_info': utils.str_or_none, + # NOTE: driver_info should probably be read-only when node + # is provisioned + 'driver_info': utils.dict_or_none, - 'properties': utils.str_or_none, + 'properties': utils.dict_or_none, 'reservation': utils.str_or_none, 'task_state': utils.str_or_none, 'task_start': utils.datetime_or_none, - 'extra': utils.str_or_none, + 'extra': utils.dict_or_none, } @staticmethod @@ -73,6 +76,10 @@ class Node(base.IronicObject): :param context: Security context """ + # TODO(deva): enforce safe limits on what fields may be changed + # depending on state. Eg., do not allow changing + # instance_uuid of an already-provisioned node. + # Raise exception if unsafe to change something. updates = {} changes = self.obj_what_changed() for field in changes: diff --git a/ironic/objects/utils.py b/ironic/objects/utils.py index 24af4ffc0d..89fdf6adf5 100644 --- a/ironic/objects/utils.py +++ b/ironic/objects/utils.py @@ -14,6 +14,7 @@ """Utility methods for objects""" +import ast import datetime import netaddr @@ -43,6 +44,19 @@ def str_or_none(val): return str(val) +def dict_or_none(val): + """Attempt to dictify a value, or None.""" + if val is None: + return {} + elif isinstance(val, str): + return dict(ast.literal_eval(val)) + else: + try: + return dict(val) + except ValueError: + return {} + + def ip_or_none(version): """Return a version-specific IP address validator.""" def validator(val, version=version): diff --git a/ironic/tests/objects/test_node.py b/ironic/tests/objects/test_node.py index 13b137bf58..ff5d11cb62 100644 --- a/ironic/tests/objects/test_node.py +++ b/ironic/tests/objects/test_node.py @@ -49,11 +49,11 @@ class TestNodeObject(base.DbTestCase): self.mox.StubOutWithMock(self.dbapi, 'update_node') self.dbapi.get_node(uuid).AndReturn(self.fake_node) - self.dbapi.update_node(uuid, {'properties': "new property"}) + self.dbapi.update_node(uuid, {'properties': {"fake": "property"}}) self.mox.ReplayAll() n = objects.Node.get_by_uuid(ctxt, uuid) - n.properties = "new property" + n.properties = {"fake": "property"} n.save() self.mox.VerifyAll() @@ -63,15 +63,15 @@ class TestNodeObject(base.DbTestCase): self.mox.StubOutWithMock(self.dbapi, 'get_node') self.dbapi.get_node(uuid).AndReturn( - dict(self.fake_node, properties="first")) + dict(self.fake_node, properties={"fake": "first"})) self.dbapi.get_node(uuid).AndReturn( - dict(self.fake_node, properties="second")) + dict(self.fake_node, properties={"fake": "second"})) self.mox.ReplayAll() n = objects.Node.get_by_uuid(ctxt, uuid) - self.assertEqual(n.properties, "first") + self.assertEqual(n.properties, {"fake": "first"}) n.refresh() - self.assertEqual(n.properties, "second") + self.assertEqual(n.properties, {"fake": "second"}) self.mox.VerifyAll() def test_objectify(self):