Nova epoxy upgrade batch 2

nova/api/openstack/common.py

@@ -242,6 +242,16 @@ def get_limit_and_marker(request):
     return limit, marker
 
 
+def get_limit_and_offset(request):
+    """Get limit and offset parameters from request."""
+    params = get_pagination_params(request)
+    limit = CONF.api.max_limit
+    limit = min(limit, params.get('limit', limit))
+    offset = params.get('offset', None)
+
+    return limit, offset
+
+
 def get_id_from_href(href):
     """Return the id or uuid portion of a url.
 

nova/api/openstack/compute/routes.py

@@ -684,6 +684,7 @@ def _create_controller(main_controller, action_controller_list):
     }),
     ('/os-server-groups/{id}', {
         'GET': [server_groups_controller, 'show'],
+        'PUT': [server_groups_controller, 'update'],
         'DELETE': [server_groups_controller, 'delete']
     }),
     ('/os-services', {

nova/api/openstack/compute/schemas/server_groups.py

@@ -99,3 +99,18 @@
     'properties': {},
     'additionalProperties': True,
 }
+
+update = {
+    'type': 'object',
+    'properties': {
+        'add_members': {
+            'type': 'array',
+            'items': parameter_types.server_id,
+        },
+        'remove_members': {
+            'type': 'array',
+            'items': parameter_types.server_id,
+        }
+    },
+    'additionalProperties': False
+}

nova/api/openstack/compute/server_groups.py

@@ -16,6 +16,7 @@
 """The Server Group API Extension."""
 
 import collections
+import itertools
 
 from oslo_log import log as logging
 import webob
@@ -26,6 +27,7 @@
 from nova.api.openstack.compute.schemas import server_groups as schema
 from nova.api.openstack import wsgi
 from nova.api import validation
+from nova.compute import api as compute
 import nova.conf
 from nova import context as nova_context
 import nova.exception
@@ -40,20 +42,29 @@
 CONF = nova.conf.CONF
 
 
-def _get_not_deleted(context, uuids):
+GROUP_POLICY_OBJ_MICROVERSION = "2.64"
+
+
+def _get_not_deleted(context, uuids, not_deleted_inst=None):
+    if not_deleted_inst:
+        # short-cut if we already pre-built a list of not deleted instances to
+        # be more efficient
+        return {u: not_deleted_inst[u] for u in uuids
+                if u in not_deleted_inst}
+
     mappings = objects.InstanceMappingList.get_by_instance_uuids(
         context, uuids)
     inst_by_cell = collections.defaultdict(list)
     cell_mappings = {}
-    found_inst_uuids = []
+    found_inst = {}
 
     # Get a master list of cell mappings, and a list of instance
     # uuids organized by cell
     for im in mappings:
         if not im.cell_mapping:
             # Not scheduled yet, so just throw it in the final list
             # and move on
-            found_inst_uuids.append(im.instance_uuid)
+            found_inst[im.instance_uuid] = None
             continue
         if im.cell_mapping.uuid not in cell_mappings:
             cell_mappings[im.cell_mapping.uuid] = im.cell_mapping
@@ -67,11 +78,11 @@ def _get_not_deleted(context, uuids):
                   {'cell': cell_mapping.identity, 'num': len(inst_uuids)})
         filters = {'uuid': inst_uuids, 'deleted': False}
         with nova_context.target_cell(context, cell_mapping) as ctx:
-            found_inst_uuids.extend([
-                inst.uuid for inst in objects.InstanceList.get_by_filters(
-                    ctx, filters=filters)])
+            instances = objects.InstanceList.get_by_filters(
+                            ctx, filters=filters)
+            found_inst.update({inst.uuid: inst.host for inst in instances})
 
-    return found_inst_uuids
+    return found_inst
 
 
 def _should_enable_custom_max_server_rules(context, rules):
@@ -86,7 +97,20 @@ def _should_enable_custom_max_server_rules(context, rules):
 class ServerGroupController(wsgi.Controller):
     """The Server group API controller for the OpenStack API."""
 
-    def _format_server_group(self, context, group, req):
+    def __init__(self, **kwargs):
+        super(ServerGroupController, self).__init__(**kwargs)
+        self.compute_api = compute.API()
+
+    def _format_server_group(self, context, group, req,
+                             not_deleted_inst=None):
+        """Format ServerGroup according to API version.
+
+        Displays only not-deleted members.
+
+        :param:not_deleted_inst: Pre-built dict of instance-uuid: host for
+                                 multiple server-groups that are found to be
+                                 not deleted.
+        """
         # the id field has its value as the uuid of the server group
         # There is no 'uuid' key in server_group seen by clients.
         # In addition, clients see policies as a ["policy-name"] list;
@@ -105,7 +129,8 @@ def _format_server_group(self, context, group, req):
         members = []
         if group.members:
             # Display the instances that are not deleted.
-            members = _get_not_deleted(context, group.members)
+            members = list(_get_not_deleted(context, group.members,
+                           not_deleted_inst))
         server_group['members'] = members
         # Add project id information to the response data for
         # API version v2.13
@@ -149,6 +174,7 @@ def delete(self, req, id):
     def index(self, req):
         """Returns a list of server groups."""
         context = req.environ['nova.context']
+        limit, offset = common.get_limit_and_offset(req)
         project_id = context.project_id
         # NOTE(gmann): Using context's project_id as target here so
         # that when we remove the default target from policy class,
@@ -166,13 +192,19 @@ def index(self, req):
             # Until then, let's keep the old behaviour.
             context.can(sg_policies.POLICY_ROOT % 'index:all_projects',
                         target={'project_id': project_id})
-            sgs = objects.InstanceGroupList.get_all(context)
+            sgs = objects.InstanceGroupList.get_all(context,
+                                                    limit=limit,
+                                                    offset=offset)
         else:
             sgs = objects.InstanceGroupList.get_by_project_id(
-                    context, project_id)
-        limited_list = common.limited(sgs.objects, req)
-        result = [self._format_server_group(context, group, req)
-                  for group in limited_list]
+                    context, project_id, limit=limit, offset=offset)
+
+        members = list(itertools.chain.from_iterable(sg.members
+                                                     for sg in sgs
+                                                     if sg.members))
+        not_deleted = _get_not_deleted(context, members)
+        result = [self._format_server_group(context, group, req, not_deleted)
+                  for group in sgs]
         return {'server_groups': result}
 
     @wsgi.Controller.api_version("2.1")
@@ -248,3 +280,140 @@ def create(self, req, body):
                 raise exc.HTTPForbidden(explanation=msg)
 
         return {'server_group': self._format_server_group(context, sg, req)}
+
+    @wsgi.Controller.api_version("2.64")
+    @validation.schema(schema.update)
+    @wsgi.expected_errors((400, 404))
+    def update(self, req, id, body):
+        """Update a server-group's members
+
+        Striving for idempotency, we accept already removed or already
+        contained members.
+
+        We always remove first and then check if we can add the requested
+        members. That way, removing an instance for a host and adding another
+        one works in one request.
+
+        We do all requested changes or no change.
+        """
+        context = req.environ['nova.context']
+        project_id = context.project_id
+        context.can(sg_policies.POLICY_ROOT % 'update',
+                    target={'project_id': project_id})
+        try:
+            sg = objects.InstanceGroup.get_by_uuid(context, id)
+        except nova.exception.InstanceGroupNotFound as e:
+            raise webob.exc.HTTPNotFound(explanation=e.format_message())
+
+        members_to_remove = set(body.get('remove_members', []))
+        members_to_add = set(body.get('add_members', []))
+        LOG.info('Called update for server-group %s with add_members: %s and '
+                 'remove_members %s',
+                 id, ', '.join(members_to_add), ', '.join(members_to_remove))
+
+        overlap = members_to_remove & members_to_add
+        if overlap:
+            msg = ('Parameters "add_members" and "remove_members" are '
+                  'overlapping in {}'.format(', '.join(overlap)))
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        if not members_to_remove and not members_to_add:
+            LOG.info("No update requested.")
+            formatted_sg = self._format_server_group(context, sg, req)
+            return {'server_group': formatted_sg}
+
+        # don't do work if it's not necessary. we might be able to get a fast
+        # way out if this request is already fulfilled
+        members_to_remove = members_to_remove & set(sg.members)
+        members_to_add = members_to_add - set(sg.members)
+
+        if not members_to_remove and not members_to_add:
+            LOG.info("State already satisfied.")
+            formatted_sg = self._format_server_group(context, sg, req)
+            return {'server_group': formatted_sg}
+
+        # retrieve all the instances to add, failing if one doesn't exist,
+        # because we need to check the hosts against the policy and adding
+        # non-existent instances doesn't make sense
+        members_to_search = members_to_add | members_to_remove
+        found_instances_hosts = _get_not_deleted(context, members_to_search)
+        missing_uuids = members_to_add - set(found_instances_hosts)
+        if missing_uuids:
+            msg = ("One or more members in add_members cannot be found: {}"
+                   .format(', '.join(missing_uuids)))
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        # check if (some of) the VMs are already members of another
+        # instance_group. We cannot support this as they might contradict.
+        found_server_groups = \
+            objects.InstanceGroupList.get_by_instance_uuids(context,
+                                                            members_to_add)
+        other_server_groups = [_x.uuid for _x in found_server_groups
+                               if _x.uuid != id]
+        if other_server_groups:
+            msg = ("One or more members in add_members is already assigned "
+                   "to another server group. Server groups: {}"
+                   .format(', '.join(other_server_groups)))
+            raise exc.HTTPBadRequest(explanation=msg)
+
+        # check if the policy is still valid with these changes
+        if sg.policy in ('affinity', 'anti-affinity'):
+            current_members_hosts = _get_not_deleted(context, sg.members)
+            current_hosts = set(h for u, h in current_members_hosts.items()
+                                if u not in members_to_remove)
+            if sg.policy == 'affinity':
+                outliers = [u for u, h in found_instances_hosts.items()
+                            if h and h not in current_hosts]
+            elif sg.policy == 'anti-affinity':
+                outliers = [u for u, h in found_instances_hosts.items()
+                            if h and h in current_hosts]
+            else:
+                outliers = None
+                LOG.warning('server-group update check not implemented for '
+                            'policy %s', sg.policy)
+            if outliers:
+                LOG.info('Update of server-group %s with policy %s aborted: '
+                         'policy violation by %s',
+                         id, sg.policy, ', '.join(outliers))
+                msg = ("Adding instance(s) {} would violate policy '{}'."
+                       .format(', '.join(outliers), sg.policy))
+                raise exc.HTTPBadRequest(explanation=msg)
+
+        # update the server group and save it
+        if members_to_remove:
+            objects.InstanceGroup.remove_members(context, sg.id,
+                                                 members_to_remove, sg.uuid)
+        if members_to_add:
+            try:
+                objects.InstanceGroup.add_members(context, id, members_to_add)
+            except Exception:
+                LOG.exception('Failed to add members.')
+                if members_to_remove:
+                    LOG.info('Trying to add removed members again after '
+                             'error.')
+                    objects.InstanceGroup.add_members(context, id,
+                                                      members_to_remove)
+                raise
+
+        LOG.info("Changed server-group %s in DB.", id)
+
+        # refresh InstanceGroup object, because we changed it directly in the
+        # DB.
+        sg.refresh()
+
+        # update the request-specs of the updated members
+        for member_uuid in found_instances_hosts:
+            request_spec = \
+                objects.RequestSpec.get_by_instance_uuid(context, member_uuid)
+            if member_uuid in members_to_add:
+                request_spec.instance_group = sg
+            else:
+                request_spec.instance_group = None
+            request_spec.save()
+
+        # tell the compute hosts about the update, so they can sync if
+        # necessary
+        hosts_to_update = set(h for h in found_instances_hosts.values() if h)
+        self.compute_api.sync_server_group(context, hosts_to_update, id)
+
+        return {'server_group': self._format_server_group(context, sg, req)}