Merge "Update the API and database models"

valet/api/common/i18n.py

@@ -17,4 +17,5 @@
 
 import gettext
 
+# TODO(jdandrea): Use oslo_i18n.TranslatorFactory
 _ = gettext.gettext

valet/api/common/ostro_helper.py

@@ -13,34 +13,39 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Ostro helper library."""
+"""Ostro helper library"""
 
+import copy
 import json
+import time
+import uuid
 
 from pecan import conf
-import time
 
-import uuid
 from valet.api.common.i18n import _
+from valet.api.common import validation
 from valet.api.db.models.music.groups import Group
 from valet.api.db.models.music.ostro import PlacementRequest
 from valet.api.db.models.music.ostro import PlacementResult
 from valet.api.db.models import Query
 from valet.api import LOG
 
+SERVER = 'OS::Nova::Server'
 SERVICEABLE_RESOURCES = [
-    'OS::Nova::Server'
+    SERVER,
 ]
-GROUP_ASSIGNMENT = 'ATT::Valet::GroupAssignment'
+METADATA = 'metadata'
-GROUP_TYPE = 'group_type'
+GROUP_ASSIGNMENT = 'OS::Valet::GroupAssignment'
-GROUP_NAME = 'group_name'
+GROUP_ID = 'group'
-AFFINITY = 'affinity'
+_GROUP_TYPES = (
-DIVERSITY = 'diversity'
+    AFFINITY, DIVERSITY, EXCLUSIVITY,
-EXCLUSIVITY = 'exclusivity'
+) = (
+    'affinity', 'diversity', 'exclusivity',
+)
 
 
 def _log(text, title="Ostro"):
-    """Log helper."""
+    """Log helper"""
     log_text = "%s: %s" % (title, text)
     LOG.debug(log_text)
 
@@ -49,6 +54,7 @@ class Ostro(object):
     """Ostro optimization engine helper class."""
 
     args = None
+    asynchronous = False
     request = None
     response = None
     error_uri = None
@@ -60,71 +66,47 @@ class Ostro(object):
     # Interval in seconds to poll for placement.
     interval = None
 
+    # valet-engine response types
+    _STATUS = (
+        STATUS_OK, STATUS_ERROR,
+    ) = (
+        'ok', 'error',
+    )
+
     @classmethod
-    def _build_error(cls, message):
+    def _build_error(cls, message=None):
-        """Build an Ostro-style error message."""
+        """Build an Ostro-style error response"""
         if not message:
             message = _("Unknown error")
-        error = {
+        return cls._build_response(cls.STATUS_ERROR, message)
+
+    @classmethod
+    def _build_ok(cls, message):
+        """Build an Ostro-style ok response"""
+        if not message:
+            message = _("Unknown message")
+        return cls._build_response(cls.STATUS_OK, message)
+
+    @classmethod
+    def _build_response(cls, status=None, message=None):
+        """Build an Ostro-style response"""
+        if status not in (cls._STATUS):
+            status = cls.STATUS_ERROR
+        if not message:
+            message = _("Unknown")
+        response = {
             'status': {
-                'type': 'error',
+                'type': status,
                 'message': message,
             }
         }
-        return error
+        return response
-
-    @classmethod
-    def _build_uuid_map(cls, resources):
-        """Build a dict mapping names to UUIDs."""
-        mapping = {}
-        for key in resources.iterkeys():
-            if 'name' in resources[key]:
-                name = resources[key]['name']
-                mapping[name] = key
-        return mapping
-
-    @classmethod
-    def _sanitize_resources(cls, resources):
-        """Ensure lowercase keys at the top level of each resource."""
-        for res in resources.itervalues():
-            for key in list(res.keys()):
-                if not key.islower():
-                    res[key.lower()] = res.pop(key)
-        return resources
 
     def __init__(self):
-        """Initializer."""
+        """Initializer"""
         self.tries = conf.music.get('tries', 1000)
         self.interval = conf.music.get('interval', 0.1)
 
-    def _map_names_to_uuids(self, mapping, data):
-        """Map resource names to their UUID equivalents."""
-        if isinstance(data, dict):
-            for key in data.iterkeys():
-                if key != 'name':
-                    data[key] = self._map_names_to_uuids(mapping, data[key])
-        elif isinstance(data, list):
-            for key, value in enumerate(data):
-                data[key] = self._map_names_to_uuids(mapping, value)
-        elif isinstance(data, basestring) and data in mapping:
-            return mapping[data]
-        return data
-
-    def _prepare_resources(self, resources):
-        """Pre-digest resource data for use by Ostro.
-
-        Maps Heat resource names to Orchestration UUIDs.
-        Ensures exclusivity groups exist and have tenant_id as a member.
-        """
-        mapping = self._build_uuid_map(resources)
-        ostro_resources = self._map_names_to_uuids(mapping, resources)
-        self._sanitize_resources(ostro_resources)
-
-        verify_error = self._verify_groups(ostro_resources, self.tenant_id)
-        if isinstance(verify_error, dict):
-            return verify_error
-        return {'resources': ostro_resources}
-
     # TODO(JD): This really belongs in valet-engine once it exists.
     def _send(self, stack_id, request):
         """Send request."""
@@ -132,9 +114,16 @@ class Ostro(object):
         PlacementRequest(stack_id=stack_id, request=request)
         result_query = Query(PlacementResult)
 
-        for __ in range(self.tries, 0, -1):  # pylint: disable=W0612
+        if self.asynchronous:
+            message = _("Asynchronous request sent")
+            LOG.info(_("{} for stack_id = {}").format(message, stack_id))
+            response = self._build_ok(message)
+            return json.dumps(response)
+
+        for __ in range(self.tries, 0, -1):
             # Take a breather in between checks.
-            # TODO(JD): This is a blocking operation at the moment.
+            # FIXME(jdandrea): This is blocking. Use futurist...
+            # or oslo.message. Hint hint. :)
             time.sleep(self.interval)
 
             result = result_query.filter_by(stack_id=stack_id).first()
@@ -144,117 +133,126 @@ class Ostro(object):
                 return placement
 
         self.error_uri = '/errors/server_error'
-        message = "Timed out waiting for a response."
+        message = _("Timed out waiting for a response")
-
+        LOG.error(_("{} for stack_id = {}").format(message, stack_id))
-        LOG.error(message + " for stack_id = " + stack_id)
-
         response = self._build_error(message)
         return json.dumps(response)
 
-    def _verify_groups(self, resources, tenant_id):
+    def _resolve_group(self, group_id):
-        """Verify group settings.
+        """Resolve a group by ID or name"""
-
+        if validation.is_valid_uuid4(group_id):
-        Returns an error status dict if the group type is invalid, if a
+            group = Group.query.filter_by(id=group_id).first()
-        group name is used when the type is affinity or diversity, if a
-        nonexistant exclusivity group is found, or if the tenant
-        is not a group member. Returns None if ok.
-        """
-        message = None
-        for res in resources.itervalues():
-            res_type = res.get('type')
-            if res_type == GROUP_ASSIGNMENT:
-                properties = res.get('properties')
-                group_type = properties.get(GROUP_TYPE, '').lower()
-                group_name = properties.get(GROUP_NAME, '').lower()
-                if group_type == AFFINITY or \
-                   group_type == DIVERSITY:
-                    if group_name:
-                        self.error_uri = '/errors/conflict'
-                        message = _("%s must not be used when"
-                                    " {0} is '{1}'.").format(GROUP_NAME,
-                                                             GROUP_TYPE,
-                                                             group_type)
-                        break
-                elif group_type == EXCLUSIVITY:
-                    message = self._verify_exclusivity(group_name, tenant_id)
         else:
+            group = Group.query.filter_by(name=group_id).first()
+        if not group:
+            self.error_uri = '/errors/not_found'
+            message = _("Group '{}' not found").format(group_id)
+            return (None, message)
+
+        if not group.name or not group.type or not group.level:
             self.error_uri = '/errors/invalid'
-                    message = _("{0} '{1}' is invalid.").format(GROUP_TYPE,
+            message = _("Group name, type, and level "
-                                                                group_type)
+                        "must all be specified.")
-                    break
+            return (None, message)
+
+        if group.type not in _GROUP_TYPES:
+            self.error_uri = '/errors/invalid'
+            message = _("Unknown group type '{}'.").format(
+                group.type)
+            return (None, message)
+        elif (len(group.members) > 0 and
+              self.tenant_id not in group.members):
+            self.error_uri = '/errors/conflict'
+            message = _("ID {} not a member of "
+                        "group {} ({})").format(
+                self.tenant_id, group.name, group.id)
+            return (None, message)
+
+        return (group, None)
+
+    def _prepare_resources(self, resources):
+        """Pre-digests resource data for use by Ostro.
+
+        Maps Heat resource names to Orchestration UUIDs.
+        Removes opaque metadata from resources.
+        Ensures group assignments refer to valid groups.
+        Ensures groups have tenant_id as a member.
+        """
+
+        # We're going to mess with the resources, so make a copy.
+        res_copy = copy.deepcopy(resources)
+        groups = {}
+        message = None
+
+        for res in res_copy.itervalues():
+            if METADATA in res:
+                # Discard valet-api-specific metadata.
+                res.pop(METADATA)
+            res_type = res.get('type')
+
+            # If OS::Nova::Server has valet metadata, use it
+            # to propagate group assignments to the engine.
+            if res_type == SERVER:
+                properties = res.get('properties')
+                metadata = properties.get(METADATA, {})
+                valet_metadata = metadata.get('valet', {})
+                group_assignments = valet_metadata.get('groups', [])
+
+                # Resolve all the groups and normalize the IDs.
+                normalized_ids = []
+                for group_id in group_assignments:
+                    (group, message) = self._resolve_group(group_id)
                     if message:
                         return self._build_error(message)
 
-    def _verify_exclusivity(self, group_name, tenant_id):
+                    # Normalize each group id
-        return_message = None
+                    normalized_ids.append(group.id)
-        if not group_name:
-            self.error_uri = '/errors/invalid'
-            return _("%s must be used when {0} is '{1}'.").format(GROUP_NAME,
-                                                                  GROUP_TYPE,
-                                                                  EXCLUSIVITY)
 
-        group = Group.query.filter_by(name=group_name).first()
+                    groups[group.id] = {
-
+                        "name": group.name,
-        if not group:
+                        "type": group.type,
-            self.error_uri = '/errors/not_found'
+                        "level": group.level,
-            return_message = "%s '%s' not found" % (GROUP_NAME, group_name)
-        elif group and tenant_id not in group.members:
-            self.error_uri = '/errors/conflict'
-            return_message = _("Tenant ID %s not a member of "
-                               "{0} '{1}' ({2})").format(self.tenant_id,
-                                                         GROUP_NAME,
-                                                         group.name,
-                                                         group.id)
-        return return_message
-
-    def build_request(self, **kwargs):
-        """Build an Ostro request.
-
-        If False is returned then the response attribute contains
-        status as to the error.
-        """
-        # TODO(JD): Refactor this into create and update methods?
-        self.args = kwargs.get('args')
-        self.tenant_id = kwargs.get('tenant_id')
-        self.response = None
-        self.error_uri = None
-
-        resources = self.args['resources']
-        if 'resources_update' in self.args:
-            action = 'update'
-            resources_update = self.args['resources_update']
-        else:
-            action = 'create'
-            resources_update = None
-
-        # If we get any status in the response, it's an error. Bail.
-        self.response = self._prepare_resources(resources)
-        if 'status' in self.response:
-            return False
-
-        self.request = {
-            "action": action,
-            "resources": self.response['resources'],
-            "stack_id": self.args['stack_id'],
                     }
 
-        # Only add locations if we have it (no need for an empty object)
+                # Update all the IDs with normalized values if we have 'em.
-        locations = self.args.get('locations')
+                if normalized_ids and valet_metadata:
-        if locations:
+                    valet_metadata['groups'] = normalized_ids
-            self.request['locations'] = locations
 
-        if resources_update:
+            # OS::Valet::GroupAssignment has been pre-empted.
-            # If we get any status in the response, it's an error. Bail.
+            # We're opting to leave the existing/working logic as-is.
-            self.response = self._prepare_resources(resources_update)
+            # Propagate group assignment resources to the engine.
-            if 'status' in self.response:
+            if res_type == GROUP_ASSIGNMENT:
-                return False
+                properties = res.get('properties')
-            self.request['resources_update'] = self.response['resources']
+                group_id = properties.get(GROUP_ID)
+                if not group_id:
+                    self.error_uri = '/errors/invalid'
+                    message = _("Property 'group' must be specified.")
+                    break
 
-        return True
+                (group, message) = self._resolve_group(group_id)
+                if message:
+                    return self._build_error(message)
+
+                # Normalize the group id
+                properties[GROUP_ID] = group.id
+
+                groups[group.id] = {
+                    "name": group.name,
+                    "type": group.type,
+                    "level": group.level,
+                }
+
+        if message:
+            return self._build_error(message)
+        prepared_resources = {
+            "resources": res_copy,
+            "groups": groups,
+        }
+        return prepared_resources
 
     def is_request_serviceable(self):
-        """Return true if request has at least one serviceable resource."""
+        """Returns true if request has at least one serviceable resources."""
-        # TODO(JD): Ostro should return no placements vs throw an error.
+        # TODO(jdandrea): Ostro should return no placements vs throw an error.
         resources = self.request.get('resources', {})
         for res in resources.itervalues():
             res_type = res.get('type')
@@ -262,6 +260,53 @@ class Ostro(object):
                 return True
         return False
 
+    # FIXME(jdandrea): Change name to create_or_update
+    def build_request(self, **kwargs):
+        """Create or update a set of placements.
+
+        If False is returned, response attribute contains error info.
+        """
+
+        self.args = kwargs.get('args')
+        self.tenant_id = kwargs.get('tenant_id')
+        self.response = None
+        self.error_uri = None
+
+        request = {
+            "action": kwargs.get('action', 'create'),
+            "stack_id": self.args.get('stack_id'),
+            "tenant_id": self.tenant_id,
+            "groups": {},  # Start with an empty dict to aid updates
+        }
+
+        # If we're updating, original_resources arg will have original info.
+        # Get this info first.
+        original_resources = self.args.get('original_resources')
+        if original_resources:
+            self.response = self._prepare_resources(original_resources)
+            if 'status' in self.response:
+                return False
+            request['original_resources'] = self.response['resources']
+            if 'groups' in self.response:
+                request['groups'] = self.response['groups']
+
+        # resources arg must always have new/updated info.
+        resources = self.args.get('resources')
+        self.response = self._prepare_resources(resources)
+        if 'status' in self.response:
+            return False
+        request['resources'] = self.response['resources']
+        if 'groups' in self.response:
+            # Update groups dict with new/updated group info.
+            request['groups'].update(self.response['groups'])
+
+        locations = self.args.get('locations')
+        if locations:
+            request['locations'] = locations
+
+        self.request = request
+        return True
+
     def ping(self):
         """Send a ping request and obtain a response."""
         stack_id = str(uuid.uuid4())
@@ -282,10 +327,24 @@ class Ostro(object):
             "action": "replan",
             "stack_id": self.args['stack_id'],
             "locations": self.args['locations'],
+            "resource_id": self.args['resource_id'],
             "orchestration_id": self.args['orchestration_id'],
             "exclusions": self.args['exclusions'],
         }
 
+    def identify(self, **kwargs):
+        """Identify a placement for an existing resource."""
+        self.args = kwargs.get('args')
+        self.response = None
+        self.error_uri = None
+        self.asynchronous = True
+        self.request = {
+            "action": "identify",
+            "stack_id": self.args['stack_id'],
+            "orchestration_id": self.args['orchestration_id'],
+            "resource_id": self.args['uuid'],
+        }
+
     def migrate(self, **kwargs):
         """Replan the placement for an existing resource."""
         self.args = kwargs.get('args')
@@ -294,6 +353,7 @@ class Ostro(object):
         self.request = {
             "action": "migrate",
             "stack_id": self.args['stack_id'],
+            "tenant_id": self.args['tenant_id'],
             "excluded_hosts": self.args['excluded_hosts'],
             "orchestration_id": self.args['orchestration_id'],
         }

valet/api/common/validation.py

@@ -0,0 +1,28 @@
+#
+# Copyright (c) 2014-2017 AT&T Intellectual Property
+#
+# 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.
+
+"""Common Validation Helpers"""
+
+import uuid
+
+
+def is_valid_uuid4(uuid_string):
+    """Ensure uuid_string is v4 compliant."""
+
+    try:
+        val = uuid.UUID(uuid_string, version=4)
+    except ValueError:
+        return False
+    return str(val) == uuid_string or val.hex == uuid_string

valet/api/db/models/music/groups.py

@@ -12,20 +12,23 @@
 # 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.
+
+"""Group Model"""
+
 import simplejson
 
 from valet.api.db.models.music import Base
 
 
 class Group(Base):
-    """Group model."""
+    """Group model"""
-
     __tablename__ = 'groups'
 
-    id = None  # pylint: disable=C0103
+    id = None
     name = None
     description = None
-    type = None  # pylint: disable=W0622
+    type = None
+    level = None
     members = None
 
     @classmethod
@@ -36,6 +39,7 @@ class Group(Base):
             'name': 'text',
             'description': 'text',
             'type': 'text',
+            'level': 'text',
             'members': 'text',
             'PRIMARY KEY': '(id)',
         }
@@ -43,48 +47,52 @@ class Group(Base):
 
     @classmethod
     def pk_name(cls):
-        """Primary key name."""
+        """Primary key name"""
         return 'id'
 
     def pk_value(self):
-        """Primary key value."""
+        """Primary key value"""
         return self.id
 
     def values(self):
-        """Values."""
+        """Values"""
-        # TODO(UNKNOWN): Support lists in Music
+        # TODO(JD): Support lists in Music
         # Lists aren't directly supported in Music, so we have to
         # convert to/from json on the way out/in.
         return {
             'name': self.name,
             'description': self.description,
             'type': self.type,
+            'level': self.level,
             'members': simplejson.dumps(self.members),
         }
 
-    def __init__(self, name, description, type, members, _insert=True):
+    def __init__(self, name, description, type, level, members, _insert=True):
-        """Initializer."""
+        """Initializer"""
         super(Group, self).__init__()
         self.name = name
         self.description = description or ""
         self.type = type
+        self.level = level
         if _insert:
-            self.members = []  # members ignored at init time
+            self.members = members
             self.insert()
         else:
             # TODO(UNKNOWN): Support lists in Music
             self.members = simplejson.loads(members)
 
     def __repr__(self):
-        """Object representation."""
+        """Object representation"""
-        return '<Group %r>' % self.name
+        return '<Group {} (type={}, level={})>'.format(
+            self.name, self.type, self.level)
 
     def __json__(self):
-        """JSON representation."""
+        """JSON representation"""
         json_ = {}
         json_['id'] = self.id
         json_['name'] = self.name
         json_['description'] = self.description
         json_['type'] = self.type
+        json_['level'] = self.level
         json_['members'] = self.members
         return json_

valet/api/db/models/music/placements.py

@@ -15,6 +15,8 @@
 
 """Placement Model."""
 
+import json
+
 from valet.api.db.models.music import Base
 from valet.api.db.models.music import Query
 
@@ -29,6 +31,7 @@ class Placement(Base):
     orchestration_id = None
     resource_id = None
     location = None
+    metadata = None
     plan_id = None
     plan = None
 
@@ -43,6 +46,7 @@ class Placement(Base):
             'location': 'text',
             'reserved': 'boolean',
             'plan_id': 'text',
+            'metadata': 'text',
             'PRIMARY KEY': '(id)',
         }
         return schema
@@ -64,12 +68,14 @@ class Placement(Base):
             'resource_id': self.resource_id,
             'location': self.location,
             'reserved': self.reserved,
+            'metadata': json.dumps(self.metadata),
             'plan_id': self.plan_id,
         }
 
     def __init__(self, name, orchestration_id, resource_id=None, plan=None,
-                 plan_id=None, location=None, reserved=False, _insert=True):
+                 plan_id=None, location=None, reserved=False, metadata=None,
-        """Initializer."""
+                 _insert=True):
+        """Initializer"""
         super(Placement, self).__init__()
         self.name = name
         self.orchestration_id = orchestration_id
@@ -81,7 +87,10 @@ class Placement(Base):
         self.location = location
         self.reserved = reserved
         if _insert:
+            self.metadata = metadata
             self.insert()
+        else:
+            self.metadata = json.loads(metadata or "{}")
 
     def __repr__(self):
         """Object representation."""
@@ -96,5 +105,6 @@ class Placement(Base):
         json_['resource_id'] = self.resource_id
         json_['location'] = self.location
         json_['reserved'] = self.reserved
+        json_['metadata'] = self.metadata
         json_['plan_id'] = self.plan.id
         return json_

valet/api/v1/controllers/__init__.py

@@ -13,34 +13,61 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Controllers Package."""
+"""Controllers Package"""
 
-from notario.decorators import instance_of
-from notario import ensure
 from os import path
+import string
+import uuid
+
+from notario.exceptions import Invalid
+from notario.utils import forced_leaf_validator
 from pecan import redirect
 from pecan import request
-import string
 
 from valet import api
 from valet.api.common.i18n import _
 from valet.api.db.models.music.placements import Placement
 
+# Supported valet-engine query types
+QUERY_TYPES = (
+    'group_vms',
+    'invalid_placements'
+)
+
+#
+# Notario Helpers
+#
+
 
 def valid_group_name(value):
     """Validator for group name type."""
-    if (not value or
+    valid_chars = set(string.letters + string.digits + "-._~")
-            not set(value) <= set(string.letters + string.digits + "-._~")):
+    if not value or not set(value) <= valid_chars:
         api.LOG.error("group name is not valid")
         api.LOG.error("group name must contain only uppercase and lowercase "
                       "letters, decimal digits, hyphens, periods, "
-                      "underscores, "" and tildes [RFC 3986, Section 2.3]")
+                      "underscores, and tildes [RFC 3986, Section 2.3]")
 
 
-@instance_of((list, dict))
+# There is a bug in Notario that prevents basic checks for a list/dict
-def valid_plan_resources(value):
+# (without recursion/depth). Instead, we borrow a hack used in the Ceph
-    """Validator for plan resources."""
+# installer, which it turns out also isn't quite correct. Some of the
-    ensure(len(value) > 0)
+# code has been removed. Source: https://github.com/ceph/ceph-installer ...
+# /blob/master/ceph_installer/schemas.py#L15-L31 (devices_object())
+@forced_leaf_validator
+def list_or_dict(value, *args):
+    """Validator - Value must be of type list or dict"""
+    error_msg = 'not of type list or dict'
+    if isinstance(value, dict):
+        return
+    try:
+        assert isinstance(value, list)
+    except AssertionError:
+        if args:
+            # What does 'dict type' and 'value' mean in this context?
+            raise Invalid(
+                'dict type', pair='value', msg=None, reason=error_msg, *args)
+        raise
 
 
 def valid_plan_update_action(value):
@@ -53,13 +80,14 @@ def valid_plan_update_action(value):
 
 
 def set_placements(plan, resources, placements):
-    """Set placements."""
+    """Set placements"""
-    for uuid in placements.iterkeys():
+    for uuid_key in placements.iterkeys():
-        name = resources[uuid]['name']
+        name = resources[uuid_key]['name']
-        properties = placements[uuid]['properties']
+        properties = placements[uuid_key]['properties']
         location = properties['host']
-        Placement(name, uuid, plan=plan, location=location)
+        metadata = resources[uuid_key].get('metadata', {})
-
+        Placement(name, uuid_key, plan=plan,
+                  location=location, metadata=metadata)
     return plan
 
 
@@ -70,41 +98,75 @@ def reserve_placement(placement, resource_id=None, reserve=True, update=True):
     the data store (if the update will be made later).
     """
     if placement:
-        api.LOG.info(_('%(rsrv)s placement of %(orch_id)s in %(loc)s.'),
+        msg = _('%(rsrv)s placement of %(orch_id)s in %(loc)s.')
-                     {'rsrv': _("Reserving") if reserve else _("Unreserving"),
+        args = {
+            'rsrv': _("Reserving") if reserve else _("Unreserving"),
             'orch_id': placement.orchestration_id,
-                      'loc': placement.location})
+            'loc': placement.location,
+        }
+        api.LOG.info(msg, args)
         placement.reserved = reserve
         if resource_id:
-            msg = _('Associating resource id %(res_id)s with orchestration '
+            msg = _('Associating resource id %(res_id)s with '
-                    'id %(orch_id)s.')
+                    'orchestration id %(orch_id)s.')
-            api.LOG.info(msg, {'res_id': resource_id,
+            args = {
-                               'orch_id': placement.orchestration_id})
+                'res_id': resource_id,
+                'orch_id': placement.orchestration_id,
+            }
+            api.LOG.info(msg, args)
             placement.resource_id = resource_id
         if update:
             placement.update()
 
 
-def update_placements(placements, reserve_id=None, unlock_all=False):
+def engine_query_args(query_type=None, parameters={}):
+    """Make a general query of valet-engine."""
+    if query_type not in QUERY_TYPES:
+        return {}
+    transaction_id = str(uuid.uuid4())
+    args = {
+        "stack_id": transaction_id,
+    }
+    if query_type:
+        args['type'] = query_type
+    args['parameters'] = parameters
+    ostro_kwargs = {
+        "args": args,
+    }
+    return ostro_kwargs
+
+
+def update_placements(placements, plan=None, resources=None,
+                      reserve_id=None, unlock_all=False):
     """Update placements. Optionally reserve one placement."""
-    for uuid in placements.iterkeys():
+    new_placements = {}
-        placement = Placement.query.filter_by(  # pylint: disable=E1101
+    for uuid_key in placements.iterkeys():
-            orchestration_id=uuid).first()
+        placement = Placement.query.filter_by(
+            orchestration_id=uuid_key).first()
         if placement:
-            properties = placements[uuid]['properties']
+            # Don't use plan or resources for upates (metadata stays as-is).
+            properties = placements[uuid_key]['properties']
             location = properties['host']
             if placement.location != location:
-                msg = _('Changing placement of %(orch_id)s from %(old_loc)s '
+                msg = _('Changing placement of %(orch_id)s from '
-                        'to %(new_loc)s.')
+                        '%(old_loc)s to %(new_loc)s.')
-                api.LOG.info(msg, {'orch_id': placement.orchestration_id,
+                args = {
+                    'orch_id': placement.orchestration_id,
                     'old_loc': placement.location,
-                                   'new_loc': location})
+                    'new_loc': location,
+                }
+                api.LOG.info(msg, args)
                 placement.location = location
             if unlock_all:
                 reserve_placement(placement, reserve=False, update=False)
             elif reserve_id and placement.orchestration_id == reserve_id:
                 reserve_placement(placement, reserve=True, update=False)
             placement.update()
+        else:
+            new_placements[uuid_key] = placements[uuid_key]
+
+    if new_placements and plan and resources:
+        set_placements(plan, resources, new_placements)
     return
 
 
@@ -113,7 +175,7 @@ def update_placements(placements, reserve_id=None, unlock_all=False):
 #
 
 def error(url, msg=None, **kwargs):
-    """Error handler."""
+    """Error handler"""
     if msg:
         request.context['error_message'] = msg
     if kwargs:

valet/api/v1/controllers/groups.py

@@ -13,7 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Groups."""
+"""Groups"""
 
 from notario import decorators
 from notario.validators import types
@@ -28,38 +28,31 @@ from valet.api.common.compute import nova_client
 from valet.api.common.i18n import _
 from valet.api.common.ostro_helper import Ostro
 from valet.api.db.models.music.groups import Group
+from valet.api.v1.controllers import engine_query_args
 from valet.api.v1.controllers import error
 from valet.api.v1.controllers import valid_group_name
 
-
 GROUPS_SCHEMA = (
     (decorators.optional('description'), types.string),
+    ('level', types.string),
     ('name', valid_group_name),
-    ('type', types.string)
+    ('type', types.string),
 )
 
+# Schemas with one field MUST NOT get trailing commas, kthx.
 UPDATE_GROUPS_SCHEMA = (
-    (decorators.optional('description'), types.string)
+    (decorators.optional('description'), types.string))
-)
-
 MEMBERS_SCHEMA = (
-    ('members', types.array)
+    ('members', types.array))
-)
-
-# pylint: disable=R0201
 
 
 def server_list_for_group(group):
-    """Return a list of VMs associated with a member/group."""
+    """Returns a list of VMs associated with a member/group."""
-    args = {
+    parameters = {
-        "type": "group_vms",
-        "parameters": {
         "group_name": group.name,
-        },
-    }
-    ostro_kwargs = {
-        "args": args,
     }
+    ostro_kwargs = engine_query_args(query_type="group_vms",
+                                     parameters=parameters)
     ostro = Ostro()
     ostro.query(**ostro_kwargs)
     ostro.send()
@@ -74,7 +67,7 @@ def server_list_for_group(group):
 
 
 def tenant_servers_in_group(tenant_id, group):
-    """Return a list of servers the current tenant has in group_name."""
+    """Returns a list of servers the current tenant has in group_name"""
     servers = []
     server_list = server_list_for_group(group)
     nova = nova_client()
@@ -97,17 +90,16 @@ def no_tenant_servers_in_group(tenant_id, group):
     """
     server_list = tenant_servers_in_group(tenant_id, group)
     if server_list:
-        error('/errors/conflict', _('Tenant Member {0} has servers in group '
+        msg = _('Tenant Member {0} has servers in group "{1}": {2}')
-                                    '"{1}": {2}').format(tenant_id,
+        error('/errors/conflict',
-                                                         group.name,
+              msg.format(tenant_id, group.name, server_list))
-                                                         server_list))
 
 
 class MembersItemController(object):
-    """Member Item Controller /v1/groups/{group_id}/members/{member_id}."""
+    """Members Item Controller /v1/groups/{group_id}/members/{member_id}"""
 
     def __init__(self, member_id):
-        """Initialize group member."""
+        """Initialize group member"""
         group = request.context['group']
         if member_id not in group.members:
             error('/errors/not_found', _('Member not found in group'))
@@ -115,30 +107,30 @@ class MembersItemController(object):
 
     @classmethod
     def allow(cls):
-        """Allowed methods."""
+        """Allowed methods"""
         return 'GET,DELETE'
 
     @expose(generic=True, template='json')
     def index(self):
-        """Catch all for unallowed methods."""
+        """Catch all for unallowed methods"""
         message = _('The %s method is not allowed.') % request.method
         kwargs = {'allow': self.allow()}
         error('/errors/not_allowed', message, **kwargs)
 
     @index.when(method='OPTIONS', template='json')
     def index_options(self):
-        """Index Options."""
+        """Options"""
         response.headers['Allow'] = self.allow()
         response.status = 204
 
     @index.when(method='GET', template='json')
     def index_get(self):
-        """Verify group member."""
+        """Verify group member"""
         response.status = 204
 
     @index.when(method='DELETE', template='json')
     def index_delete(self):
-        """Delete group member."""
+        """Delete group member"""
         group = request.context['group']
         member_id = request.context['member_id']
 
@@ -151,38 +143,39 @@ class MembersItemController(object):
 
 
 class MembersController(object):
-    """Members Controller /v1/groups/{group_id}/members."""
+    """Members Controller /v1/groups/{group_id}/members"""
 
     @classmethod
     def allow(cls):
-        """Allowed methods."""
+        """Allowed methods"""
         return 'PUT,DELETE'
 
     @expose(generic=True, template='json')
     def index(self):
-        """Catchall for unallowed methods."""
+        """Catchall for unallowed methods"""
         message = _('The %s method is not allowed.') % request.method
         kwargs = {'allow': self.allow()}
         error('/errors/not_allowed', message, **kwargs)
 
     @index.when(method='OPTIONS', template='json')
     def index_options(self):
-        """Index Options."""
+        """Options"""
         response.headers['Allow'] = self.allow()
         response.status = 204
 
     @index.when(method='PUT', template='json')
     @validate(MEMBERS_SCHEMA, '/errors/schema')
     def index_put(self, **kwargs):
-        """Add one or more members to a group."""
+        """Add one or more members to a group"""
-        new_members = kwargs.get('members', None)
+        new_members = kwargs.get('members', [])
 
         if not conf.identity.engine.is_tenant_list_valid(new_members):
-            error('/errors/conflict', _('Member list contains '
+            error('/errors/conflict',
-                                        'invalid tenant IDs'))
+                  _('Member list contains invalid tenant IDs'))
 
         group = request.context['group']
-        group.members = list(set(group.members + new_members))
+        member_list = group.members or []
+        group.members = list(set(member_list + new_members))
         group.update()
         response.status = 201
 
@@ -192,7 +185,7 @@ class MembersController(object):
 
     @index.when(method='DELETE', template='json')
     def index_delete(self):
-        """Delete all group members."""
+        """Delete all group members"""
         group = request.context['group']
 
         # Can't delete a member if it has associated VMs.
@@ -205,51 +198,52 @@ class MembersController(object):
 
     @expose()
     def _lookup(self, member_id, *remainder):
-        """Pecan subcontroller routing callback."""
+        """Pecan subcontroller routing callback"""
         return MembersItemController(member_id), remainder
 
 
 class GroupsItemController(object):
-    """Group Item Controller /v1/groups/{group_id}."""
+    """Groups Item Controller /v1/groups/{group_id}"""
 
     members = MembersController()
 
     def __init__(self, group_id):
-        """Initialize group."""
+        """Initialize group"""
-        # pylint:disable=E1101
         group = Group.query.filter_by(id=group_id).first()
+        if not group:
+            group = Group.query.filter_by(name=group_id).first()
             if not group:
                 error('/errors/not_found', _('Group not found'))
         request.context['group'] = group
 
     @classmethod
     def allow(cls):
-        """Allowed methods."""
+        """Allowed methods"""
         return 'GET,PUT,DELETE'
 
     @expose(generic=True, template='json')
     def index(self):
-        """Catchall for unallowed methods."""
+        """Catchall for unallowed methods"""
         message = _('The %s method is not allowed.') % request.method
         kwargs = {'allow': self.allow()}
         error('/errors/not_allowed', message, **kwargs)
 
     @index.when(method='OPTIONS', template='json')
     def index_options(self):
-        """Index Options."""
+        """Options"""
         response.headers['Allow'] = self.allow()
         response.status = 204
 
     @index.when(method='GET', template='json')
     def index_get(self):
-        """Display a group."""
+        """Display a group"""
         return {"group": request.context['group']}
 
     @index.when(method='PUT', template='json')
     @validate(UPDATE_GROUPS_SCHEMA, '/errors/schema')
     def index_put(self, **kwargs):
-        """Update a group."""
+        """Update a group"""
-        # Name and type are immutable.
+        # Name, type, and level are immutable.
         # Group Members are updated in MembersController.
         group = request.context['group']
         group.description = kwargs.get('description', group.description)
@@ -262,42 +256,44 @@ class GroupsItemController(object):
 
     @index.when(method='DELETE', template='json')
     def index_delete(self):
-        """Delete a group."""
+        """Delete a group"""
         group = request.context['group']
+        # tenant_id = request.context['tenant_id']
         if isinstance(group.members, list) and len(group.members) > 0:
-            error('/errors/conflict',
+            message = _('Unable to delete a Group with members.')
-                  _('Unable to delete a Group with members.'))
+            error('/errors/conflict', message)
+
         group.delete()
         response.status = 204
 
 
 class GroupsController(object):
-    """Group Controller /v1/groups."""
+    """Groups Controller /v1/groups"""
 
     @classmethod
     def allow(cls):
-        """Allowed methods."""
+        """Allowed methods"""
         return 'GET,POST'
 
     @expose(generic=True, template='json')
     def index(self):
-        """Catch all for unallowed methods."""
+        """Catch all for unallowed methods"""
         message = _('The %s method is not allowed.') % request.method
         kwargs = {'allow': self.allow()}
         error('/errors/not_allowed', message, **kwargs)
 
     @index.when(method='OPTIONS', template='json')
     def index_options(self):
-        """Index Options."""
+        """Options"""
         response.headers['Allow'] = self.allow()
         response.status = 204
 
     @index.when(method='GET', template='json')
     def index_get(self):
-        """List groups."""
+        """List groups"""
         try:
             groups_array = []
-            for group in Group.query.all():  # pylint: disable=E1101
+            for group in Group.query.all():
                 groups_array.append(group)
         except Exception:
             import traceback
@@ -308,14 +304,21 @@ class GroupsController(object):
     @index.when(method='POST', template='json')
     @validate(GROUPS_SCHEMA, '/errors/schema')
     def index_post(self, **kwargs):
-        """Create a group."""
+        """Create a group"""
         group_name = kwargs.get('name', None)
         description = kwargs.get('description', None)
         group_type = kwargs.get('type', None)
+        group_level = kwargs.get('level', None)
         members = []  # Use /v1/groups/members endpoint to add members
 
+        group = Group.query.filter_by(name=group_name).first()
+        if group:
+            message = _("A group named {} already exists")
+            error('/errors/invalid', message.format(group_name))
+
         try:
-            group = Group(group_name, description, group_type, members)
+            group = Group(group_name, description, group_type,
+                          group_level, members)
             if group:
                 response.status = 201
 
@@ -327,5 +330,5 @@ class GroupsController(object):
 
     @expose()
     def _lookup(self, group_id, *remainder):
-        """Pecan subcontroller routing callback."""
+        """Pecan subcontroller routing callback"""
         return GroupsItemController(group_id), remainder

valet/api/v1/controllers/placements.py

@@ -12,6 +12,11 @@
 # 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.
+
+"""Placements"""
+
+import json
+
 from pecan import expose
 from pecan import request
 from pecan import response
@@ -26,40 +31,35 @@ from valet.api.v1.controllers import reserve_placement
 from valet.api.v1.controllers import update_placements
 
 
-# pylint: disable=R0201
-
-
 class PlacementsItemController(object):
-    """Placements Item Controller /v1/placements/{placement_id}."""
+    """Placements Item Controller /v1/placements/{placement_id}"""
 
     def __init__(self, uuid4):
         """Initializer."""
         self.uuid = uuid4
         self.placement = Placement.query.filter_by(id=self.uuid).first()
-        # pylint: disable=E1101
         if not self.placement:
             self.placement = Placement.query.filter_by(
                 orchestration_id=self.uuid).first()
-            # disable=E1101
             if not self.placement:
                 error('/errors/not_found', _('Placement not found'))
         request.context['placement_id'] = self.placement.id
 
     @classmethod
     def allow(cls):
-        """Allowed methods."""
+        """Allowed methods"""
         return 'GET,POST,DELETE'
 
     @expose(generic=True, template='json')
     def index(self):
-        """Catchall for unallowed methods."""
+        """Catchall for unallowed methods"""
         message = _('The %s method is not allowed.') % request.method
         kwargs = {'allow': self.allow()}
         error('/errors/not_allowed', message, **kwargs)
 
     @index.when(method='OPTIONS', template='json')
     def index_options(self):
-        """Index Options."""
+        """Options"""
         response.headers['Allow'] = self.allow()
         response.status = 204
 
@@ -75,37 +75,72 @@ class PlacementsItemController(object):
     def index_post(self, **kwargs):
         """Reserve a placement. This and other placements may be replanned.
 
-        Once reserved, the location effectively becomes immutable.
+        Once reserved, the location effectively becomes immutable unless
+        a replan is forced (due to a resource replacement, for example).
         """
         res_id = kwargs.get('resource_id')
-        msg = _('Placement reservation request for resource id %(res_id)s, '
+        api.LOG.info(_('Placement reservation request for resource \
-                'orchestration id %(orch_id)s.')
+                        id %(res_id)s, orchestration id %(orch_id)s.'),
-        api.LOG.info(msg, {'res_id': res_id,
+                     {'res_id': res_id,
                       'orch_id': self.placement.orchestration_id})
+
+        actions = ('reserve', 'replan')
+        action = kwargs.get('action', 'reserve')
+        if action not in actions:
+            message = _('Invalid action: {}. Must be one of {}')
+            error('/errors/invalid', message.format(action, actions))
+
         locations = kwargs.get('locations', [])
         locations_str = ', '.join(locations)
         api.LOG.info(_('Candidate locations: %s'), locations_str)
-        if self.placement.location in locations:
+        if action == 'reserve' and self.placement.location in locations:
             # Ostro's placement is in the list of candidates. Good!
-            # Reserve it. Remember the resource id too.
+            # But first, we have to pass the engine's identify test.
+
+            plan = Plan.query.filter_by(id=self.placement.plan_id).first()
+
+            args = {
+                "stack_id": plan.stack_id,
+                "orchestration_id": self.placement.orchestration_id,
+                "uuid": res_id,
+            }
+            ostro_kwargs = {"args": args, }
+            ostro = Ostro()
+            ostro.identify(**ostro_kwargs)
+            ostro.send()
+
+            status_type = ostro.response['status']['type']
+            if status_type != 'ok':
+                message = ostro.response['status']['message']
+                error(ostro.error_uri, _('Ostro error: %s') % message)
+
+            # We're in the clear. Reserve it. Remember the resource id too.
             kwargs = {'resource_id': res_id}
             reserve_placement(self.placement, **kwargs)
             response.status = 201
         else:
+            if action == 'reserve':
                 # Ostro's placement is NOT in the list of candidates.
                 # Time for Plan B.
-            msg = _('Placement of resource id %(res_id)s, orchestration id '
+                api.LOG.info(_('Placement of resource id %(res_id)s, \
-                    '%(orch_id)s in %(loc)s not allowed. Replanning.')
+                                orchestration id %(orch_id)s in %(loc)s \
-            api.LOG.info(msg, {'res_id': res_id,
+                                not allowed. Replanning.'),
+                             {'res_id': res_id,
                               'orch_id': self.placement.orchestration_id,
                               'loc': self.placement.location})
+            else:
+                # A replan was expressly requested (action == 'replan')
+                api.LOG.info(_('Replanning resource id %(res_id)s, \
+                                orchestration id %(orch_id)s.'),
+                             {'res_id': res_id,
+                              'orch_id': self.placement.orchestration_id})
 
             # Unreserve the placement. Remember the resource id too.
             kwargs = {'resource_id': res_id, 'reserve': False}
             reserve_placement(self.placement, **kwargs)
 
             # Find all the reserved placements for the related plan.
-            reserved = Placement.query.filter_by(  # pylint: disable=E1101
+            reserved = Placement.query.filter_by(
                 plan_id=self.placement.plan_id, reserved=True)
 
             # Keep this placement's orchestration ID handy.
@@ -125,11 +160,11 @@ class PlacementsItemController(object):
             # One of those will be the original placement
             # we are trying to reserve.
             plan = Plan.query.filter_by(id=self.placement.plan_id).first()
-            # pylint: disable=E1101
 
             args = {
                 "stack_id": plan.stack_id,
                 "locations": locations,
+                "resource_id": res_id,
                 "orchestration_id": orchestration_id,
                 "exclusions": exclusions,
             }
@@ -148,49 +183,97 @@ class PlacementsItemController(object):
             update_placements(placements, reserve_id=orchestration_id)
             response.status = 201
 
-        placement = Placement.query.filter_by(  # pylint: disable=E1101
+        placement = Placement.query.filter_by(
             orchestration_id=self.placement.orchestration_id).first()
         return {"placement": placement}
 
     @index.when(method='DELETE', template='json')
     def index_delete(self):
-        """Delete a Placement."""
+        """Delete a Placement"""
         orch_id = self.placement.orchestration_id
         self.placement.delete()
-        api.LOG.info(_('Placement with orchestration id %s deleted.'), orch_id)
+        api.LOG.info(_('Placement with orchestration id %s deleted.'),
+                     orch_id)
         response.status = 204
 
 
 class PlacementsController(object):
-    """Placements Controller /v1/placements."""
+    """Placements Controller /v1/placements"""
 
     @classmethod
     def allow(cls):
-        """Allowed methods."""
+        """Allowed methods"""
         return 'GET'
 
     @expose(generic=True, template='json')
     def index(self):
-        """Catchall for unallowed methods."""
+        """Catchall for unallowed methods"""
         message = _('The %s method is not allowed.') % request.method
         kwargs = {'allow': self.allow()}
         error('/errors/not_allowed', message, **kwargs)
 
     @index.when(method='OPTIONS', template='json')
     def index_options(self):
-        """Index Options."""
+        """Options"""
         response.headers['Allow'] = self.allow()
         response.status = 204
 
     @index.when(method='GET', template='json')
-    def index_get(self):
+    def index_get(self, **kwargs):
         """Get placements."""
         placements_array = []
-        for placement in Placement.query.all():  # pylint: disable=E1101
+        for placement in Placement.query.all():
+            # If there are query string args, look for them in two places,
+            # and in this order:
+            #
+            # 1. The main placement object, only for these reserved
+            #    keys: id, orchestration_id, plan_id, resource_id,
+            #          location, name, reserved.
+            # 2. The metadata.
+            #
+            # Support only exact matches for now. AND, not OR.
+            #
+            # Start by presuming we have a match, and look for fail cases.
+            # If search fails, no error, just don't append that placement.
+            # This also ends up appending if there are no kwargs (good).
+            append = True
+            for key, value in kwargs.iteritems():
+                # We don't allow the same key multiple times, so no lists,
+                # only strings. Don't even allow NoneType.
+                if not isinstance(value, basestring):
+                    append = False
+                    break
+
+                # Try loading as if it were json. If we can't, that's ok.
+                try:
+                    # Using json_value to prevent side-effects.
+                    json_value = json.loads(value)
+                    value = json_value
+                except (TypeError, ValueError):
+                    pass
+
+                # 1. If the key is one of our reserved keys ...
+                if key in ('id', 'orchestration_id', 'plan_id',
+                           'resource_id', 'location', 'name',
+                           'reserved') and hasattr(placement, key):
+                    # ... and the value does not match in the main object,
+                    # don't append it, and don't go on to check metadata.
+                    if value != getattr(placement, key):
+                        append = False
+                        break
+                # 2. Otherwise, if the key is not in the metadata or
+                #    the value does not match, don't append it.
+                elif (key not in placement.metadata or
+                      value != placement.metadata.get(key)):
+                    append = False
+                    break
+
+            if append:
                 placements_array.append(placement)
+
         return {"placements": placements_array}
 
     @expose()
     def _lookup(self, uuid4, *remainder):
-        """Pecan subcontroller routing callback."""
+        """Pecan subcontroller routing callback"""
         return PlacementsItemController(uuid4), remainder

valet/api/v1/controllers/plans.py

@@ -13,7 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Plans."""
+"""Plans"""
 
 from notario import decorators
 from notario.validators import types
@@ -28,41 +28,37 @@ from valet.api.db.models.music.placements import Placement
 from valet.api.db.models.music.plans import Plan
 from valet.api import LOG
 from valet.api.v1.controllers import error
+from valet.api.v1.controllers import list_or_dict
 from valet.api.v1.controllers import set_placements
 from valet.api.v1.controllers import update_placements
 from valet.api.v1.controllers import valid_plan_update_action
 
-
 CREATE_SCHEMA = (
     (decorators.optional('locations'), types.array),
     ('plan_name', types.string),
     ('resources', types.dictionary),
     ('stack_id', types.string),
-    (decorators.optional('timeout'), types.string)
+    (decorators.optional('timeout'), types.string))
-)
 
 UPDATE_SCHEMA = (
     ('action', valid_plan_update_action),
     (decorators.optional('excluded_hosts'), types.array),
+    (decorators.optional('original_resources'), types.dictionary),
     (decorators.optional('plan_name'), types.string),
-    # FIXME: resources needs to work against valid_plan_resources
+    ('resources', list_or_dict),  # list: migrate, dict: update
-    ('resources', types.array),
+    (decorators.optional('timeout'), types.string))
-    (decorators.optional('timeout'), types.string)
-)
 
 
 class PlansItemController(object):
-    """Plan Item Controller /v1/plans/{plan_id}."""
+    """Plans Item Controller /v1/plans/{plan_id}"""
 
     def __init__(self, uuid4):
         """Initializer."""
         self.uuid = uuid4
         self.plan = Plan.query.filter_by(id=self.uuid).first()
-        # pylint: disable=E1101
 
         if not self.plan:
             self.plan = Plan.query.filter_by(stack_id=self.uuid).first()
-            # pylint: disable=E1101
 
             if not self.plan:
                 error('/errors/not_found', _('Plan not found'))
@@ -70,32 +66,33 @@ class PlansItemController(object):
 
     @classmethod
     def allow(cls):
-        """Allowed methods."""
+        """Allowed methods"""
         return 'GET,PUT,DELETE'
 
     @expose(generic=True, template='json')
     def index(self):
-        """Catchall for unallowed methods."""
+        """Catchall for unallowed methods"""
         message = _('The %s method is not allowed.') % request.method
         kwargs = {'allow': self.allow()}
         error('/errors/not_allowed', message, **kwargs)
 
     @index.when(method='OPTIONS', template='json')
     def index_options(self):
-        """Index Options."""
+        """Options"""
         response.headers['Allow'] = self.allow()
         response.status = 204
 
     @index.when(method='GET', template='json')
     def index_get(self):
-        """Get plan."""
+        """Get plan"""
         return {"plan": self.plan}
 
     @index.when(method='PUT', template='json')
     @validate(UPDATE_SCHEMA, '/errors/schema')
     def index_put(self, **kwargs):
-        """Update a Plan."""
+        """Update a Plan"""
-        action = kwargs.get('action')
+        ostro = Ostro()
+        action = kwargs.get('action', 'update')
         if action == 'migrate':
             # Replan the placement of an existing resource.
             excluded_hosts = kwargs.get('excluded_hosts', [])
@@ -109,28 +106,26 @@ class PlansItemController(object):
             # We either got a resource or orchestration id.
             the_id = resources[0]
             placement = Placement.query.filter_by(resource_id=the_id).first()
-            # pylint: disable=E1101
             if not placement:
                 placement = Placement.query.filter_by(
-                    orchestration_id=the_id).first()  # pylint: disable=E1101
+                    orchestration_id=the_id).first()
                 if not placement:
-                    error('/errors/invalid',
+                    msg = _('Unknown resource or orchestration id: %s')
-                          _('Unknown resource or '
+                    error('/errors/invalid', msg.format(the_id))
-                            'orchestration id: %s') % the_id)
-
-            LOG.info(_('Migration request for resource id {0}, '
-                       'orchestration id {1}.').format(
-                placement.resource_id, placement.orchestration_id))
 
+            msg = _('Migration request for resource id {0}, '
+                    'orchestration id {1}.')
+            LOG.info(msg.format(placement.resource_id,
+                                placement.orchestration_id))
             args = {
                 "stack_id": self.plan.stack_id,
+                "tenant_id": request.context['tenant_id'],
                 "excluded_hosts": excluded_hosts,
                 "orchestration_id": placement.orchestration_id,
             }
             ostro_kwargs = {
                 "args": args,
             }
-            ostro = Ostro()
             ostro.migrate(**ostro_kwargs)
             ostro.send()
 
@@ -146,57 +141,61 @@ class PlansItemController(object):
             # Flush so that the DB is current.
             self.plan.flush()
             self.plan = Plan.query.filter_by(
-                stack_id=self.plan.stack_id).first()  # pylint: disable=E1101
+                stack_id=self.plan.stack_id).first()
             LOG.info(_('Plan with stack id %s updated.'), self.plan.stack_id)
             return {"plan": self.plan}
+        elif action == 'update':
+            # Update an existing plan.
+            resources = kwargs.get('resources', [])
 
-        # TODO(JD): Throw unimplemented error?
+            if not isinstance(resources, dict):
+                error('/errors/invalid', _('resources must be a dictionary.'))
 
-        # pylint: disable=W0612
+            ostro_kwargs = {
-        '''
+                'action': 'update',
-        # FIXME: This is broken. Save for Valet 1.1
-        # New placements are not being seen in the response, so
-        # set_placements is currently failing as a result.
-        ostro = Ostro()
-        args = request.json
-
-        kwargs = {
                 'tenant_id': request.context['tenant_id'],
-            'args': args
+                'args': kwargs,
             }
 
+            # stack_id comes from the plan at update-time
+            ostro_kwargs['args']['stack_id'] = self.plan.stack_id
+
             # Prepare the request. If request prep fails,
             # an error message will be in the response.
             # Though the Ostro helper reports the error,
             # we cite it as a Valet error.
-        if not ostro.build_request(**kwargs):
+            if not ostro.build_request(**ostro_kwargs):
                 message = ostro.response['status']['message']
                 error(ostro.error_uri, _('Valet error: %s') % message)
 
+            # If there are no serviceable resources, bail. Not an error.
+            # Treat it as if an "empty plan" was created.
+            # FIXME: Ostro should likely handle this and not error out.
+            if not ostro.is_request_serviceable():
+                LOG.info(_('Plan has no serviceable resources. Skipping.'))
+                response.status = 201
+                return {"plan": self.plan}
+
             ostro.send()
             status_type = ostro.response['status']['type']
             if status_type != 'ok':
                 message = ostro.response['status']['message']
                 error(ostro.error_uri, _('Ostro error: %s') % message)
 
-        # TODO(JD): Keep. See if we will eventually need these for Ostro.
+            resources = ostro.request['resources']
-        #plan_name = args['plan_name']
-        #stack_id = args['stack_id']
-        resources = ostro.request['resources_update']
             placements = ostro.response['resources']
 
-        set_placements(self.plan, resources, placements)
+            update_placements(placements, plan=self.plan, resources=resources)
             response.status = 201
 
             # Flush so that the DB is current.
             self.plan.flush()
-        return self.plan
+            LOG.info(_('Plan with stack id %s updated.'), self.plan.stack_id)
-        '''
+            return {"plan": self.plan}
-        # pylint: enable=W0612
 
     @index.when(method='DELETE', template='json')
     def index_delete(self):
-        """Delete a Plan."""
+        """Delete a Plan"""
         for placement in self.plan.placements():
             placement.delete()
         stack_id = self.plan.stack_id
@@ -206,42 +205,43 @@ class PlansItemController(object):
 
 
 class PlansController(object):
-    """Plans Controller /v1/plans."""
+    """Plans Controller /v1/plans"""
 
     @classmethod
     def allow(cls):
-        """Allowed methods."""
+        """Allowed methods"""
         return 'GET,POST'
 
     @expose(generic=True, template='json')
     def index(self):
-        """Catchall for unallowed methods."""
+        """Catchall for unallowed methods"""
         message = _('The %s method is not allowed.') % request.method
         kwargs = {'allow': self.allow()}
         error('/errors/not_allowed', message, **kwargs)
 
     @index.when(method='OPTIONS', template='json')
     def index_options(self):
-        """Index Options."""
+        """Options"""
         response.headers['Allow'] = self.allow()
         response.status = 204
 
     @index.when(method='GET', template='json')
     def index_get(self):
-        """Get all the plans."""
+        """Get all the plans"""
         plans_array = []
-        for plan in Plan.query.all():  # pylint: disable=E1101
+        for plan in Plan.query.all():
             plans_array.append(plan)
         return {"plans": plans_array}
 
     @index.when(method='POST', template='json')
     @validate(CREATE_SCHEMA, '/errors/schema')
     def index_post(self):
-        """Create a Plan."""
+        """Create a Plan"""
         ostro = Ostro()
         args = request.json
 
         kwargs = {
+            'action': 'create',
             'tenant_id': request.context['tenant_id'],
             'args': args,
         }
@@ -287,5 +287,5 @@ class PlansController(object):
 
     @expose()
     def _lookup(self, uuid4, *remainder):
-        """Pecan subcontroller routing callback."""
+        """Pecan subcontroller routing callback"""
         return PlansItemController(uuid4), remainder

valet/api/v1/controllers/resources.py

@@ -0,0 +1,89 @@
+#
+# Copyright 2014-2017 AT&T Intellectual Property
+#
+# 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.
+
+"""Resources"""
+
+from pecan import expose
+from pecan import request
+from pecan import response
+from valet.api.common.i18n import _
+from valet.api.common.ostro_helper import Ostro
+from valet.api.v1.controllers import engine_query_args
+from valet.api.v1.controllers import error
+
+
+class ResourcesController(object):
+    """Status Controller /v1/resources"""
+
+    def _invalid_placements(self):
+        """Returns a dict of VMs with invalid placements."""
+
+        # TODO(gjung): Support checks on individual placements as well
+        ostro_kwargs = engine_query_args(query_type="invalid_placements")
+        ostro = Ostro()
+        ostro.query(**ostro_kwargs)
+        ostro.send()
+
+        status_type = ostro.response['status']['type']
+        if status_type != 'ok':
+            message = ostro.response['status']['message']
+            error(ostro.error_uri, _('Ostro error: %s') % message)
+
+        resources = ostro.response['resources']
+        return resources or {}
+
+    def _resource_status(self):
+        """Get resource status."""
+
+        # All we do at the moment is check for invalid placements.
+        # This structure will evolve in the future. The only kind of
+        # resource type we'll see at the moment are servers.
+        invalid = self._invalid_placements()
+        resources = {}
+        for resource_id, info in invalid.items():
+            resources[resource_id] = {
+                "type": "OS::Nova::Server",
+                "status": "error",
+                "message": info.get('status'),
+            }
+        response = {
+            "resources": resources,
+        }
+        return response
+
+    @classmethod
+    def allow(cls):
+        """Allowed methods"""
+        return 'GET'
+
+    @expose(generic=True, template='json')
+    def index(self):
+        """Catchall for unallowed methods"""
+        message = _('The %s method is not allowed.') % request.method
+        kwargs = {'allow': self.allow()}
+        error('/errors/not_allowed', message, **kwargs)
+
+    @index.when(method='OPTIONS', template='json')
+    def index_options(self):
+        """Options"""
+        response.headers['Allow'] = self.allow()
+        response.status = 204
+
+    @index.when(method='GET', template='json')
+    def index_get(self):
+        """Get Valet resource status"""
+        _response = self._resource_status()
+        response.status = 200
+        return _response

valet/api/v1/controllers/v1.py

@@ -24,6 +24,7 @@ from valet.api.v1.controllers import error
 from valet.api.v1.controllers.groups import GroupsController
 from valet.api.v1.controllers.placements import PlacementsController
 from valet.api.v1.controllers.plans import PlansController
+from valet.api.v1.controllers.resources import ResourcesController
 from valet.api.v1.controllers.status import StatusController
 
 
@@ -33,6 +34,7 @@ class V1Controller(SecureController):
     groups = GroupsController()
     placements = PlacementsController()
     plans = PlansController()
+    resources = ResourcesController()
     status = StatusController()
 
     # Update this whenever a new endpoint is made.

valet/tests/unit/api/common/test_ostro_helper.py

@@ -16,170 +16,291 @@
 """Test Ostro Helper."""
 
 import mock
-import valet.api.common.ostro_helper as helper
+
-from valet.api.common.ostro_helper import Ostro
+from valet.api.common import ostro_helper
-from valet.tests.unit.api.v1.api_base import ApiBase
+from valet.api.db.models import music as models
+from valet.tests.unit.api.v1 import api_base
+from valet.tests.unit import fakes
 
 
-class TestOstroHelper(ApiBase):
+class TestOstroHelper(api_base.ApiBase):
-    """Test Ostro (Engine) Helper Class."""
-
     def setUp(self):
         """Setup Test Ostro and call init Ostro."""
         super(TestOstroHelper, self).setUp()
 
-        self.ostro = self.init_Ostro()
+        self.engine = self.init_engine()
+        self.groups = []
 
-    @mock.patch.object(helper, 'conf')
+        kwargs = {
-    def init_Ostro(self, mock_conf):
+            'description': 'test',
-        """Init Engine(Ostro) and return."""
+            'members': ['test_tenant_id'],
-        mock_conf.ostro = {}
+        }
-        mock_conf.ostro["tries"] = 10
+        for group_type in ('affinity', 'diversity', 'exclusivity'):
-        mock_conf.ostro["interval"] = 1
+            kwargs['type'] = group_type
+            for group_level in ('host', 'rack'):
+                # Build names like host_affinity, rack_diversity, etc.
+                kwargs['name'] = "{}_{}".format(group_level, group_type)
+                kwargs['level'] = group_level
+                group = models.groups.Group(**kwargs)
+                self.groups.append(group)
 
-        return Ostro()
+    @mock.patch.object(ostro_helper, 'conf')
+    def init_engine(self, mock_conf):
+        mock_conf.music = {}
+        mock_conf.music["tries"] = 10
+        mock_conf.music["interval"] = 1
 
-    def test_build_request(self):
+        return ostro_helper.Ostro()
-        """Test Build Request in Engine API using many different kwargs."""
-        kwargs = {'tenant_id': 'test_tenant_id',
-                  'args': {'stack_id': 'test_stack_id',
-                           'plan_name': 'test_plan_name',
-                           'resources': {'test_resource': {
-                               'Type': 'ATT::Valet::GroupAssignment',
-                                       'Properties': {
-                                           'resources': ['my-instance-1',
-                                                         'my-instance-2'],
-                                           'group_type': 'affinity',
-                                           'level': 'host'},
-                                       'name': 'test-affinity-group3'}}}}
-        self.validate_test(self.ostro.build_request(**kwargs))
 
-        kwargs = {'tenant_id': 'test_tenant_id',
+    def build_request_kwargs(self):
-                  'args': {'stack_id': 'test_stack_id',
+        """Boilerplate for the build_request tests"""
-                           'plan_name': 'test_plan_name',
+        # TODO(jdandrea): Sample Data should be co-located elsewhere
-                           'resources': {'test_resource': {
+        base_kwargs = {
-                               'Type': 'ATT::Valet::GroupAssignment',
+            'tenant_id': 'test_tenant_id',
-                                       'Properties': {
+            'args': {
-                                           'resources': ['my-instance-1',
+                'stack_id': 'test_stack_id',
-                                                         'my-instance-2'],
-                                           'group_type': 'affinity',
-                                           'group_name': "test_group_name",
-                                           'level': 'host'},
-                                       'name': 'test-affinity-group3'}}}}
-        self.validate_test(not self.ostro.build_request(**kwargs))
-        self.validate_test("conflict" in self.ostro.error_uri)
-
-        kwargs = {'tenant_id': 'test_tenant_id',
-                  'args': {'stack_id': 'test_stack_id',
-                           'plan_name': 'test_plan_name',
-                           'resources': {'test_resource': {
-                               'Type': 'ATT::Valet::GroupAssignment',
-                                       'Properties': {
-                                           'resources': ['my-instance-1',
-                                                         'my-instance-2'],
-                                           'group_type': 'exclusivity',
-                                           'level': 'host'},
-                                       'name': 'test-affinity-group3'}}}}
-        self.validate_test(not self.ostro.build_request(**kwargs))
-        self.validate_test("invalid" in self.ostro.error_uri)
-
-        kwargs = {'tenant_id': 'test_tenant_id',
-                  'args': {'stack_id': 'test_stack_id',
-                           'plan_name': 'test_plan_name',
-                           'resources': {'test_resource': {
-                               'Type': 'ATT::Valet::GroupAssignment',
-                                       'Properties': {
-                                           'resources': ['my-instance-1',
-                                                         'my-instance-2'],
-                                           'group_type': 'exclusivity',
-                                           'group_name': "test_group_name",
-                                           'level': 'host'},
-                                       'name': 'test-affinity-group3'}}}}
-        self.validate_test(not self.ostro.build_request(**kwargs))
-        self.validate_test("not_found" in self.ostro.error_uri)
-
-        kwargs = {'tenant_id': 'test_tenant_id',
-                  'args': {'stack_id': 'test_stack_id',
                 'plan_name': 'test_plan_name',
                 'timeout': '60 sec',
                 'resources': {
-                               'ca039d18-1976-4e13-b083-edb12b806e25': {
+                    "test_server": {
-                                   'Type': 'ATT::Valet::GroupAssignment',
+                        'type': 'OS::Nova::Server',
-                                           'Properties': {
+                        'properties': {
-                                               'resources': ['my-instance-1',
+                            'key_name': 'ssh_key',
-                                                             'my-instance-2'],
+                            'image': 'ubuntu_server',
-                                               'group_type': 'non_type',
+                            'name': 'my_server',
-                                               'group_name': "test_group_name",
+                            'flavor': 'm1.small',
-                                               'level': 'host'},
+                            'metadata': {
-                                           'name': 'test-affinity-group3'}}}}
+                                'valet': {
-        self.validate_test(not self.ostro.build_request(**kwargs))
+                                    'groups': [
-        self.validate_test("invalid" in self.ostro.error_uri)
+                                        'host_affinity'
+                                    ]
+                                }
+                            },
+                            'networks': [
+                                {
+                                    'network': 'private'
+                                }
+                            ]
+                        },
+                        'name': 'my_instance',
+                    },
+                    'test_group_assignment': {
+                        'type': 'OS::Valet::GroupAssignment',
+                        'properties': {
+                            'group': 'host_affinity',
+                            'resources': ['my-instance-1', 'my-instance-2'],
+                        },
+                        'name': 'test_name',
+                    }
+                }
+            }
+        }
+        return base_kwargs
 
-    @mock.patch.object(helper, 'uuid')
+    # TODO(jdandrea): Turn these build_request tests into scenarios?
+
+    # The next five build_request methods exercise OS::Nova::Server metadata
+
+    @mock.patch.object(models.Results, 'first')
+    def test_build_request_host_affinity_using_metadata(self, mock_results):
+        mock_results.return_value = fakes.group(type="affinity")
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_server'][
+            'properties']['metadata']['valet']['groups'][0] = "host_affinity"
+        request = self.engine.build_request(**kwargs)
+        self.assertTrue(request)
+
+    @mock.patch.object(models.Results, 'first')
+    def test_build_request_host_diversity_using_metadata(self, mock_results):
+        mock_results.return_value = fakes.group(type="diversity")
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_server'][
+            'properties']['metadata']['valet']['groups'][0] = \
+            "host_diversity"
+        request = self.engine.build_request(**kwargs)
+        self.assertTrue(request)
+
+    @mock.patch.object(models.Results, 'first')
+    def test_build_request_host_exclusivity_using_metadata(self, mock_results):
+        mock_results.return_value = \
+            fakes.group(name="host_exclusivity", type="exclusivity")
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_server'][
+            'properties']['metadata']['valet']['groups'][0] = \
+            "host_exclusivity"
+        request = self.engine.build_request(**kwargs)
+        self.assertTrue(request)
+
+    @mock.patch.object(models.Results, 'first')
+    def test_build_request_host_exclusivity_wrong_tenant_using_metadata(
+            self, mock_results):
+        mock_results.return_value = \
+            fakes.group(name="host_exclusivity", type="exclusivity")
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_server'][
+            'properties']['metadata']['valet']['groups'][0] = \
+            "host_exclusivity"
+        kwargs['tenant_id'] = "bogus_tenant"
+        request = self.engine.build_request(**kwargs)
+        self.assertFalse(request)
+        self.assertIn('conflict', self.engine.error_uri)
+
+    def test_build_request_nonexistant_group_using_metadata(self):
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_server'][
+            'properties']['metadata']['valet']['groups'][0] = "bogus_name"
+        request = self.engine.build_request(**kwargs)
+        self.assertFalse(request)
+        self.assertIn('not_found', self.engine.error_uri)
+
+    # The next five build_request methods exercise OS::Valet::GroupAssignment
+
+    @mock.patch.object(models.Results, 'first')
+    def test_build_request_host_affinity(self, mock_results):
+        mock_results.return_value = fakes.group(type="affinity")
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_group_assignment'][
+            'properties']['group'] = "host_affinity"
+        request = self.engine.build_request(**kwargs)
+        self.assertTrue(request)
+
+    @mock.patch.object(models.Results, 'first')
+    def test_build_request_host_diversity(self, mock_results):
+        mock_results.return_value = fakes.group(type="diversity")
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_group_assignment'][
+            'properties']['group'] = "host_diversity"
+        request = self.engine.build_request(**kwargs)
+        self.assertTrue(request)
+
+    @mock.patch.object(models.Results, 'first')
+    def test_build_request_host_exclusivity(self, mock_results):
+        mock_results.return_value = \
+            fakes.group(name="host_exclusivity", type="exclusivity")
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_group_assignment'][
+            'properties']['group'] = "host_exclusivity"
+        request = self.engine.build_request(**kwargs)
+        self.assertTrue(request)
+
+    @mock.patch.object(models.Results, 'first')
+    def test_build_request_host_exclusivity_wrong_tenant(self, mock_results):
+        mock_results.return_value = \
+            fakes.group(name="host_exclusivity", type="exclusivity")
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_group_assignment'][
+            'properties']['group'] = "host_exclusivity"
+        kwargs['tenant_id'] = "bogus_tenant"
+        request = self.engine.build_request(**kwargs)
+        self.assertFalse(request)
+        self.assertIn('conflict', self.engine.error_uri)
+
+    def test_build_request_nonexistant_group(self):
+        kwargs = self.build_request_kwargs()
+        kwargs['args']['resources']['test_group_assignment'][
+            'properties']['group'] = "bogus_name"
+        request = self.engine.build_request(**kwargs)
+        self.assertFalse(request)
+        self.assertIn('not_found', self.engine.error_uri)
+
+    @mock.patch.object(ostro_helper, 'uuid')
     def test_ping(self, mock_uuid):
         """Validate engine ping by checking engine request equality."""
         mock_uuid.uuid4.return_value = "test_stack_id"
-        self.ostro.ping()
+        self.engine.ping()
 
-        self.validate_test(self.ostro.request['stack_id'] == "test_stack_id")
+        self.assertTrue(self.engine.request['stack_id'] == "test_stack_id")
 
     def test_is_request_serviceable(self):
-        """Validate if engine request serviceable."""
+        self.engine.request = {
-        self.ostro.request = {
+            'resources': {
-            'resources': {"bla": {'type': "OS::Nova::Server"}}}
+                "bla": {
-        self.validate_test(self.ostro.is_request_serviceable())
+                    'type': "OS::Nova::Server",
+                }
+            }
+        }
+        self.assertTrue(self.engine.is_request_serviceable())
 
-        self.ostro.request = {}
+        self.engine.request = {}
-        self.validate_test(not self.ostro.is_request_serviceable())
+        self.assertFalse(self.engine.is_request_serviceable())
 
     def test_replan(self):
-        """Validate engine replan."""
+        kwargs = {
-        kwargs = {'args': {'stack_id': 'test_stack_id',
+            'args': {
+                'stack_id': 'test_stack_id',
                 'locations': 'test_locations',
                 'orchestration_id': 'test_orchestration_id',
-                           'exclusions': 'test_exclusions'}}
+                'exclusions': 'test_exclusions',
-        self.ostro.replan(**kwargs)
+                'resource_id': 'test_resource_id',
+            }
+        }
+        self.engine.replan(**kwargs)
 
-        self.validate_test(self.ostro.request['stack_id'] == "test_stack_id")
+        self.assertTrue(self.engine.request['stack_id'] == "test_stack_id")
-        self.validate_test(self.ostro.request['locations'] == "test_locations")
+        self.assertTrue(self.engine.request['locations'] == "test_locations")
+        self.assertTrue(
+            self.engine.request['orchestration_id'] ==
+            "test_orchestration_id")
+        self.assertTrue(
+            self.engine.request['exclusions'] == "test_exclusions")
 
-        self.validate_test(
+    def test_identify(self):
-            self.ostro.request['orchestration_id'] == "test_orchestration_id")
+        kwargs = {
-
+            'args': {
-        self.validate_test(
+                'stack_id': 'test_stack_id',
-            self.ostro.request['exclusions'] == "test_exclusions")
+                'orchestration_id': 'test_orchestration_id',
+                'uuid': 'test_uuid',
+            }
+        }
+        self.engine.identify(**kwargs)
+        self.assertEqual(self.engine.request['stack_id'], "test_stack_id")
+        self.assertEqual(self.engine.request['orchestration_id'],
+                         "test_orchestration_id")
+        self.assertEqual(self.engine.request['resource_id'], "test_uuid")
+        self.assertTrue(self.engine.asynchronous)
 
     def test_migrate(self):
-        """Validate engine migrate."""
+        kwargs = {
-        kwargs = {'args': {'stack_id': 'test_stack_id',
+            'args': {
+                'stack_id': 'test_stack_id',
+                'tenant_id': 'test_tenant_id',
                 'excluded_hosts': 'test_excluded_hosts',
-                           'orchestration_id': 'test_orchestration_id'}}
+                'orchestration_id': 'test_orchestration_id',
-        self.ostro.migrate(**kwargs)
+            }
+        }
+        self.engine.migrate(**kwargs)
 
-        self.validate_test(self.ostro.request['stack_id'] == "test_stack_id")
+        self.assertTrue(self.engine.request['stack_id'] == "test_stack_id")
+        self.assertTrue(
+            self.engine.request['excluded_hosts'] == "test_excluded_hosts")
+        self.assertTrue(
+            self.engine.request['orchestration_id'] ==
+            "test_orchestration_id")
 
-        self.validate_test(
+    @mock.patch.object(ostro_helper, 'uuid')
-            self.ostro.request['excluded_hosts'] == "test_excluded_hosts")
-
-        self.validate_test(
-            self.ostro.request['orchestration_id'] == "test_orchestration_id")
-
-    @mock.patch.object(helper, 'uuid')
     def test_query(self, mock_uuid):
         """Validate test query by validating several engine requests."""
         mock_uuid.uuid4.return_value = "test_stack_id"
-        kwargs = {'args': {'type': 'test_type',
+        kwargs = {
-                           'parameters': 'test_parameters'}}
+            'args': {
-        self.ostro.query(**kwargs)
+                'type': 'test_type',
+                'parameters': 'test_parameters',
+            }
+        }
+        self.engine.query(**kwargs)
 
-        self.validate_test(self.ostro.request['stack_id'] == "test_stack_id")
+        self.assertTrue(self.engine.request['stack_id'] == "test_stack_id")
-        self.validate_test(self.ostro.request['type'] == "test_type")
+        self.assertTrue(self.engine.request['type'] == "test_type")
+        self.assertTrue(
+            self.engine.request['parameters'] == "test_parameters")
 
-        self.validate_test(
+    @mock.patch.object(ostro_helper, '_log')
-            self.ostro.request['parameters'] == "test_parameters")
+    @mock.patch.object(ostro_helper.Ostro, '_send')
-
+    @mock.patch.object(models.ostro, 'PlacementRequest')
-    def test_send(self):
+    @mock.patch.object(models, 'Query')
-        """Validate test send by checking engine server error."""
+    def test_send(self, mock_query, mock_request, mock_send, mock_logger):
-        self.ostro.args = {'stack_id': 'test_stack_id'}
+        mock_send.return_value = '{"status":{"type":"ok"}}'
-        self.ostro.send()
+        self.engine.args = {'stack_id': 'test_stack_id'}
-        self.validate_test("server_error" in self.ostro.error_uri)
+        self.engine.request = {}
+        self.engine.send()
+        self.assertIsNone(self.engine.error_uri)

valet/tests/unit/api/common/test_validation.py

@@ -0,0 +1,44 @@
+#
+# Copyright (c) 2014-2017 AT&T Intellectual Property
+#
+# 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.
+
+"""Common Validation Helpers"""
+
+from valet.api.common import validation
+from valet.tests.unit.api.v1 import api_base
+
+
+class TestValidation(api_base.ApiBase):
+    """Test Harness"""
+
+    uuid = '731056cc-c802-4797-a32b-17eaced354fa'
+
+    def setUp(self):
+        """Initializer"""
+        super(TestValidation, self).setUp()
+
+    def test_is_valid_uuid4(self):
+        """Test with a valid UUID"""
+        valid = validation.is_valid_uuid4(self.uuid)
+        self.assertTrue(valid)
+
+    def test_is_valid_uuid4_no_hyphens(self):
+        """Test with a valid UUID, no hyphens"""
+        valid = validation.is_valid_uuid4(self.uuid.replace('-', ''))
+        self.assertTrue(valid)
+
+    def test_is_invalid_uuid4(self):
+        """Test with an invalid UUID"""
+        valid = validation.is_valid_uuid4("not_a_uuid")
+        self.assertFalse(valid)

valet/tests/unit/api/db/test_groups.py

@@ -35,7 +35,8 @@ class TestGroups(ApiBase):
         """Init a test group object and return."""
         mock_insert.return_value = None
         members = ["me", "you"]
-        return Group("test_name", "test_description", "test_type", members)
+        return Group("test_name", "test_description", "test_type",
+                     "test_level", members)
 
     def test__repr__(self):
         """Validate test name in group repr."""

valet/tests/unit/api/db/test_placements.py

@@ -35,11 +35,9 @@ class TestPlacement(ApiBase):
     def init_Placement(self, mock_insert):
         """Return init test placement object for class init."""
         mock_insert.return_value = None
-        return Placement("test_name",
+        plan = Plan("plan_name", "stack_id", _insert=False)
-                         "test_orchestration_id",
+        return Placement("test_name", "test_orchestration_id",
-                         plan=Plan("plan_name", "stack_id", _insert=False),
+                         plan=plan, location="test_location", _insert=False)
-                         location="test_location",
-                         _insert=False)
 
     def test__repr__(self):
         """Test name from placement repr."""

valet/tests/unit/api/v1/api_base.py

@@ -16,18 +16,60 @@
 """Api Base."""
 
 import mock
+
 import pecan
+
 from valet.tests.base import Base
+# from valet.tests import db
 
 
 class ApiBase(Base):
     """Api Base Test Class, calls valet tests base."""
 
+    # FIXME(jdandrea): No camel-case! Use __init__().
     def setUp(self):
         """Setup api base and mock pecan identity/music/state."""
         super(ApiBase, self).setUp()
         pecan.conf.identity = mock.MagicMock()
         pecan.conf.music = mock.MagicMock()
+
+        """
+        # pecan.conf.music.keyspace = \
+        #     mock.PropertyMock(return_value="valet")
+
+        # Set up the mock Music API
+        # TODO(jdandrea): In all honesty, instead of
+        # using a mock object here, it may be better
+        # to mock out only the surface that is being
+        # crossed during a given test. We're most of
+        # the way there. We may end up dumbing down
+        # what the mock object does (vs. having it
+        # do simplified in-memory storage).
+        keyspace = 'valet'
+        engine = db.MusicAPIWithOldMethodNames()
+
+        # FIXME(jdandrea): pecan.conf.music used to be
+        # a MagicMock, however it does not appear possible
+        # to setattr() on a MagicMock (not one that can be
+        # retrieved via obj.get('key') at least). That means
+        # keys/values that were magically handled before are
+        # no longer being handled now. We may end up filling
+        # in the rest of the expected music conf settings
+        # with individual mock object values if necessary.
+        pecan.conf.music = {
+            'keyspace': keyspace,
+            'engine': engine,
+        }
+
+        # Create a keyspace and various tables (no schema needed)
+        pecan.conf.music.engine.keyspace_create(keyspace)
+        for table in ('plans', 'placements', 'groups',
+                      'placement_requests', 'placement_results',
+                      'query'):
+            pecan.conf.music.engine.table_create(
+                keyspace, table, schema=mock.MagicMock())
+        """
+
         self.response = None
         pecan.core.state = mock.MagicMock()
 

valet/tests/unit/api/v1/test_groups.py

@@ -12,13 +12,14 @@
 # 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.
+
 import mock
 import pecan
 
 from valet.api.db.models.music.groups import Group
 from valet.api.db.models.music import Query
 from valet.api.db.models.music import Results
-import valet.api.v1.controllers.groups as groups
+from valet.api.v1.controllers import groups
 from valet.api.v1.controllers.groups import GroupsController
 from valet.api.v1.controllers.groups import GroupsItemController
 from valet.api.v1.controllers.groups import MembersController
@@ -51,10 +52,9 @@ class TestGroups(ApiBase):
     def init_GroupsItemController(self, mock_filter, mock_request):
         """Called by Setup, return GroupsItemController object with id."""
         mock_request.context = {}
-        mock_filter.return_value = Results([Group("test_name",
+        mock_filter.return_value = Results(
-                                                  "test_description",
+            [Group("test_name", "test_description", "test_type",
-                                                  "test_type",
+                   "test_level", None)])
-                                                  None)])
         contrler = GroupsItemController("group_id")
 
         self.validate_test("test_name" == groups.request.context['group'].name)
@@ -69,11 +69,8 @@ class TestGroups(ApiBase):
     @mock.patch.object(groups, 'error', ApiBase.mock_error)
     @mock.patch.object(groups, 'request')
     def init_MembersItemController(self, mock_request):
-        """Called by Setup, return MembersItemController with demo members."""
+        grp = Group("test_member_item_name", "test_description",
-        grp = Group("test_member_item_name",
+                    "test_type", "test_level", None)
-                    "test_description",
-                    "test_type",
-                    None)
         grp.members = ["demo members"]
         mock_request.context = {'group': grp}
 
@@ -126,10 +123,9 @@ class TestGroups(ApiBase):
         """Test members_controller index_put method, check status/tenant_id."""
         pecan.conf.identity.engine.is_tenant_list_valid.return_value = True
 
-        mock_request.context = {'group': Group("test_name",
+        mock_request.context = {'group': Group(
-                                               "test_description",
+                                "test_name", "test_description",
-                                               "test_type",
+                                "test_type", "test_level", None)}
-                                               None)}
         r = self.members_controller.index_put(members=[self.tenant_id])
 
         self.validate_test(groups.response.status == 201)
@@ -143,10 +139,9 @@ class TestGroups(ApiBase):
         """Test members_controller index_put method with invalid tenants."""
         pecan.conf.identity.engine.is_tenant_list_valid.return_value = False
 
-        mock_request.context = {'group': Group("test_name",
+        mock_request.context = {'group': Group(
-                                               "test_description",
+                                "test_name", "test_description",
-                                               "test_type",
+                                "test_type", "test_level", None)}
-                                               None)}
         self.members_controller.index_put(members=[self.tenant_id])
 
         self.validate_test("Member list contains invalid tenant IDs" in
@@ -169,8 +164,8 @@ class TestGroups(ApiBase):
     @mock.patch.object(groups, 'request')
     def test_index_delete_member_item_controller(self, mock_request,
                                                  mock_func):
-        """Members_item_controller index_delete, check status and members."""
+        grp = Group("test_name", "test_description",
-        grp = Group("test_name", "test_description", "test_type", None)
+                    "test_type", "test_level", None)
         grp.members = ["demo members"]
 
         mock_request.context = {'group': grp, 'member_id': "demo members"}
@@ -184,11 +179,10 @@ class TestGroups(ApiBase):
     @mock.patch.object(groups, 'error', ApiBase.mock_error)
     @mock.patch.object(groups, 'tenant_servers_in_group')
     @mock.patch.object(groups, 'request')
-    def test_index_delete_member_item_controller_unhappy(self,
+    def test_index_delete_member_item_controller_unhappy(self, mock_request,
-                                                         mock_request,
                                                          mock_func):
-        """Members_item_controller index_delete, check member not found."""
+        grp = Group("test_name", "test_description",
-        grp = Group("test_name", "test_description", "test_type", None)
+                    "test_type", "test_level", None)
         grp.members = ["demo members"]
 
         mock_request.context = {'group': grp, 'member_id': "demo members"}
@@ -213,21 +207,18 @@ class TestGroups(ApiBase):
 
     @mock.patch.object(groups, 'request')
     def test_index_put_groups_item_controller(self, mock_request):
-        """Test index_put for item_controller, check status and description."""
+        mock_request.context = {'group': Group(
-        mock_request.context = {'group': Group("test_name",
+                                "test_name", "test_description",
-                                               "test_description",
+                                "test_type", "test_level", None)}
-                                               "test_type",
-                                               None)}
         r = self.groups_item_controller.index_put(
             description="new description")
 
         self.validate_test(groups.response.status == 201)
         self.validate_test(r.description == "new description")
 
-        mock_request.context = {'group': Group("test_name",
+        mock_request.context = {'group': Group(
-                                               "test_description",
+                                "test_name", "test_description",
-                                               "test_type",
+                                "test_type", "test_level", None)}
-                                               None)}
         r = self.groups_item_controller.index_put()
 
         self.validate_test(groups.response.status == 201)
@@ -235,11 +226,9 @@ class TestGroups(ApiBase):
 
     @mock.patch.object(groups, 'request')
     def test_index_delete_groups_item_controller(self, mock_request):
-        """Test groups_item_controller index_delete works, check response."""
+        mock_request.context = {'group': Group(
-        mock_request.context = {'group': Group("test_name",
+                                "test_name", "test_description",
-                                               "test_description",
+                                "test_type", "test_level", None)}
-                                               "test_type",
-                                               None)}
         self.groups_item_controller.index_delete()
 
         self.validate_test(groups.response.status == 204)
@@ -247,8 +236,8 @@ class TestGroups(ApiBase):
     @mock.patch.object(groups, 'error', ApiBase.mock_error)
     @mock.patch.object(groups, 'request')
     def test_index_delete_groups_item_controller_unhappy(self, mock_request):
-        """Test to check that you can't delete a group with members."""
+        grp = Group("test_name", "test_description",
-        grp = Group("test_name", "test_description", "test_type", None)
+                    "test_type", "test_level", None)
         grp.members = ["demo members"]
         mock_request.context = {'group': grp}
         self.groups_item_controller.index_delete()
@@ -265,10 +254,9 @@ class TestGroups(ApiBase):
         mock_all.return_value = all_groups
         response = self.groups_controller.index_get()
 
-        mock_request.context = {'group': Group("test_name",
+        mock_request.context = {'group': Group(
-                                               "test_description",
+                                "test_name", "test_description",
-                                               "test_type",
+                                "test_type", "test_level", None)}
-                                               None)}
         item_controller_response = self.groups_item_controller.index_get()
 
         self.members_item_controller.index_get()
@@ -281,22 +269,20 @@ class TestGroups(ApiBase):
         self.validate_test(all_groups == response["groups"])
 
     def test_index_post(self):
-        """Test group_controller index_post, check status and name."""
         group = self.groups_controller.index_post(
-            name="testgroup",
+            name="testgroup", description="test description",
-            description="test description",
+            type="testtype", level="test_evel")
-            type="testtype")
 
         self.validate_test(groups.response.status == 201)
         self.validate_test(group.name == "testgroup")
 
     @mock.patch.object(groups, 'error', ApiBase.mock_error)
-    def test_index_post_unhappy(self):
+    @mock.patch.object(groups.Group, '__init__')
-        """Test groups_controller index_post with error."""
+    def test_index_post_unhappy(self, mock_group_init):
-        pecan.conf.music = None
+        mock_group_init.return_value = Exception()
-        self.groups_controller.index_post(name="testgroup",
+        self.groups_controller.index_post(
-                                          description="test description",
+            name="testgroup", description="test description",
-                                          type="testtype")
+            type="testtype", level="test_level")
 
         self.validate_test("Unable to create Group" in TestGroups.response)
 

valet/tests/unit/api/v1/test_placements.py

@@ -14,16 +14,38 @@
 # limitations under the License.
 import mock
 
-from valet.api.db.models.music.placements import Placement
+from valet.api.common import ostro_helper
 from valet.api.db.models.music.plans import Plan
 from valet.api.db.models.music import Query
 from valet.api.db.models.music import Results
 import valet.api.v1.controllers.placements as placements
+from valet.api.v1.controllers.placements import Placement
 from valet.api.v1.controllers.placements import PlacementsController
 from valet.api.v1.controllers.placements import PlacementsItemController
 from valet.tests.unit.api.v1.api_base import ApiBase
 
 
+def fake_filter_by(self, **kwargs):
+    """Fake filter for Music queries.
+
+    FIXME(jdandrea): Find a way to get rid of this. It's here
+    in order to get some of the tests working, but there ought
+    to be a better way that doesn't introduce more surface area.
+    """
+    if 'id' in kwargs:
+        return Results([Plan("plan_name", "stack_id", _insert=False)])
+    elif 'plan_id' in kwargs:
+        # FIXME(jdandrea) this is duplicated in
+        # init_PlacementsItemController (and there shouldn't be a
+        # separate init; that pattern blurs/confuses things IMO)
+        return Results([
+            Placement("placement_name", "test_orchestration_id",
+                      plan=Plan("plan_name", "stack_id", _insert=False),
+                      location="test_location", _insert=False)])
+    else:
+        return Results([])
+
+
 class TestPlacements(ApiBase):
     """Unit tests for valet.api.v1.controllers.placements."""
 
@@ -103,27 +125,45 @@ class TestPlacements(ApiBase):
         self.validate_test("plan_name" in response['placement'].plan.name)
         self.validate_test("stack_id" in response['placement'].plan.stack_id)
 
-    @mock.patch.object(placements, 'error', ApiBase.mock_error)
+    @mock.patch.object(ostro_helper, '_log')
-    @mock.patch.object(Query, 'filter_by', mock.MagicMock)
+    @mock.patch.object(ostro_helper.Ostro, '_send')
-    @mock.patch.object(placements, 'update_placements')
+    @mock.patch.object(Query, 'filter_by')
-    def test_index_post(self, mock_plcment):
+    def test_index_post_with_locations(self, mock_filter,
-        """Test index_post for placements, validate from response status."""
+                                       mock_send, mock_logging):
         kwargs = {'resource_id': "resource_id", 'locations': ["test_location"]}
+        mock_filter.return_value = Results([
+            Plan("plan_name", "stack_id", _insert=False)])
+        mock_send.return_value = '{"status":{"type":"ok"}}'
         self.placements_item_controller.index_post(**kwargs)
         self.validate_test(placements.response.status == 201)
 
-        with mock.patch('valet.api.v1.controllers.placements.Ostro') \
+    @mock.patch('valet.api.db.models.music.Query.filter_by',
-                as mock_ostro:
+                fake_filter_by)
+    @mock.patch.object(placements, 'error', ApiBase.mock_error)
+    @mock.patch.object(ostro_helper, '_log')
+    @mock.patch.object(ostro_helper.Ostro, '_send')
+    def test_index_post_with_engine_error(self, mock_send, mock_logging):
         kwargs = {'resource_id': "resource_id", 'locations': [""]}
+        mock_send.return_value = \
+            '{"status":{"type":"error","message":"error"},' \
+            '"resources":{"iterkeys":[]}}'
         self.placements_item_controller.index_post(**kwargs)
         self.validate_test("Ostro error:" in ApiBase.response)
 
-            mock_plcment.return_value = None
+    @mock.patch('valet.api.db.models.music.Query.filter_by',
+                fake_filter_by)
+    @mock.patch.object(ostro_helper, '_log')
+    @mock.patch.object(ostro_helper.Ostro, '_send')
+    @mock.patch.object(placements, 'update_placements')
+    def test_index_post_with_placement_update(self, mock_update,
+                                              mock_send, mock_logging):
+        kwargs = {'resource_id': "resource_id", 'locations': [""]}
+        mock_update.return_value = None
 
-            status_type = mock.MagicMock()
+        # FIXME(jdandrea): Why was "iterkeys" used here as a resource??
-            status_type.response = {"status": {"type": "ok"},
+        # That's a Python iterator reference, not a reasonable resource key.
-                                    "resources": {"iterkeys": []}}
+        mock_send.return_value = \
-            mock_ostro.return_value = status_type
+            '{"status":{"type":"ok"},"resources":{"iterkeys":[]}}'
 
         self.placements_item_controller.index_post(**kwargs)
         self.validate_test(placements.response.status == 201)

valet/tests/unit/fakes.py

@@ -0,0 +1,27 @@
+#
+# Copyright 2014-2017 AT&T Intellectual Property
+#
+# 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.
+
+import uuid
+
+from valet.api.db.models import music as models
+
+
+def group(name="mock_group", description="mock group", type="affinity",
+          level="host", members='["test_tenant_id"]'):
+    """Boilerplate for creating a group"""
+    group = models.groups.Group(name=name, description=description, type=type,
+                                level=level, members=members, _insert=False)
+    group.id = str(uuid.uuid4())
+    return group