Merge "Additional changes to private flavors"

orm/services/flavor_manager/fms_rest/data/sql_alchemy/data_manager.py

@@ -111,3 +111,14 @@ class DataManager(object):
         results = datamanager.session.connection().execute(sql_query % (tenants_list, regions_list)).fetchall()
 
         return results
+
+    def get_valid_tenant_list(self, requested_tenants):
+        # validate if tenants in list exist in customer table
+
+        datamanager = DataManager()
+        # convert tenant and region lists to sql-friendly list format
+        tenants_list = str(tuple(requested_tenants)).rstrip(',)') + ')'
+        sql_query = '''select uuid from customer where uuid in %s'''
+        valid_tenants_list = datamanager.session.connection().execute(sql_query % (tenants_list)).fetchall()
+
+        return valid_tenants_list

orm/services/flavor_manager/fms_rest/data/sql_alchemy/db_models.py

@@ -346,6 +346,13 @@ class Flavor(Base, FMSBaseModel):
 
         return existing_region_names
 
+    def get_existing_tenant_ids(self):
+        existing_tenant_ids = []
+        for tenant in self.flavor_tenants:
+            existing_tenant_ids.append(tenant.tenant_id)
+
+        return existing_tenant_ids
+
 
 '''
 ' FlavorExtraSpec is a DataObject contains all fields defined in

orm/services/flavor_manager/fms_rest/logic/flavor_logic.py

@@ -14,26 +14,32 @@ from orm.common.orm_common.utils import utils
 from oslo_config import cfg
 import oslo_db
 
+
 LOG = get_logger(__name__)
 
 di = injector.get_di()
 
 
+def format_tenant_list(tenant_region_list):
+    # x[0] is the first element (tenant) in the tenant_region_list of tuples
+    # compile all the tenants and delete the NoneTypes from the tenant list
+    result_tenant_list = [x[0] for x in tenant_region_list if x[0]]
+
+    # clean up valid_tenants_list by removing duplicate items
+    valid_tenants_list = list(dict.fromkeys(result_tenant_list))
+
+    return valid_tenants_list
+
+
 def validate_tenants_regions_list(requested_tenants, requested_regions,
-                                  action, datamanager):
+                                  datamanager):
-    regions_list, tenants_list = [], []
+    valid_tenants_list, valid_regions_list = [], []
     results = datamanager.get_valid_tenant_region_list(requested_tenants,
                                                        requested_regions)
 
-    valid_tenants_list, valid_regions_list = [], []
-
     if results:
-        # the first element in the results tuple is the tenant prep
+        # first element in the results tuple is the tenant
-        # result_tenant_list from result_rows and remove NoneTypes from list
+        valid_tenants_list = format_tenant_list(results)
-        result_tenant_list = [x[0] for x in results]
-        result_tenant_list = filter(None, result_tenant_list)
-        # lastly clean up valid_tenants_list list by removing duplicate items
-        valid_tenants_list = list(dict.fromkeys(result_tenant_list))
 
         # second element in the results tuple is the region - compile the
         # region  data into valid_regions_list and eliminate duplicates
@@ -63,18 +69,23 @@ def create_flavor(flavor, flavor_uuid, transaction_id):
 
         # Execute the following logic only if at least one region AND one
         # tenant are provided in a private flavor:
-        #    Validate which tenants from the original tenants list to
+        #    identify which tenants from requested tenants list to be assigned
-        #    be assigned to the private flavor; if no valid tenants
+        #    to private flavor given the requested regions list; if no tenant
-        #    found, the valid_tenants_list will return empty list
+        #    identified, the valid_tenants_list will return empty list
-        if (flavor_regions and flavor.flavor.visibility == 'private' and
+        if flavor_regions and flavor.flavor.visibility == 'private':
-                flavor.flavor.tenants):
+            if flavor.flavor.tenants:
-            valid_tenants_list, valid_regions_list = \
+                valid_tenants_list, valid_regions_list = \
-                validate_tenants_regions_list(flavor.flavor.tenants,
+                    validate_tenants_regions_list(flavor.flavor.tenants,
-                                              flavor_regions,
+                                                  flavor_regions, datamanager)
-                                              'create', datamanager)
+
             # replace the original tenant list in the private flavor
             # with the valid_tenants_list
             flavor.flavor.tenants = valid_tenants_list
+        else:
+            # disallow tenant assignment if no flavor region assigned
+            if flavor.flavor.tenants:
+                raise ErrorStatus(
+                    400, 'Cannot add tenants with no flavor region assigned ')
 
         sql_flavor = flavor.to_db_model()
 
@@ -268,12 +279,17 @@ def add_regions(flavor_uuid, regions, transaction_id):
         # private flavor new logic
         # validate tenants assigned to the flavor
         for tenant in sql_flavor.flavor_tenants:
-            flvr_tenant_list.append(tenant.tenant_id)
+            flvr_tenant_list = sql_flavor.get_existing_tenant_ids()
+
+            # existing_region_names = db_flavor.get_existing_region_names()
+            # add existing flavor regions to flvr_region_list in order for
+            # the validate_tenants_regions_list to process correctly
+            flvr_region_list = sql_flavor.get_existing_region_names()
 
             valid_tenants_list, valid_regions_list = \
                 validate_tenants_regions_list(flvr_tenant_list,
-                                              flvr_region_list,
+                                              flvr_region_list, datamanager)
-                                              'create', datamanager)
+
         # update tenant list
         for tenant in flvr_tenant_list:
             if tenant not in valid_tenants_list:
@@ -289,7 +305,7 @@ def add_regions(flavor_uuid, regions, transaction_id):
         return ret
 
     except ErrorStatus as exp:
-        LOG.log_exception("FlavorLogic - Failed to update flavor", str(exp))
+        LOG.log_exception("FlavorLogic - Failed to add regions", str(exp))
         datamanager.rollback()
         raise exp
     except Exception as exp:
@@ -303,11 +319,52 @@ def add_regions(flavor_uuid, regions, transaction_id):
         datamanager.close()
 
 
+def delete_orphaned_tenants(sql_flavor, remaining_regions, datamanager):
+    # this function deletes all 'orphaned tenants' - i.e., tenants that
+    # are no longer associated with regions still assigned to the flavor
+
+    try:
+        # get existing tenants_list and orig_regions_list BEFORE remove_region
+        existing_tenants_list = sql_flavor.get_existing_tenant_ids()
+
+        valid_tenants_list, valid_regions_list = [], []
+        # use 'remaining_regions' to identify the valid tenants list
+        # for all regions assigned to flavor MINUS the deleted region
+        if remaining_regions and existing_tenants_list:
+            valid_tenants_list, valid_regions_list = \
+                validate_tenants_regions_list(existing_tenants_list,
+                                              remaining_regions,
+                                              datamanager)
+
+        # remove orphaned tenants identified
+        if valid_tenants_list:
+            for tenant in existing_tenants_list:
+                if tenant not in valid_tenants_list:
+                    sql_flavor.remove_tenant(tenant)
+        else:
+            if existing_tenants_list:
+                for tenant in existing_tenants_list:
+                    sql_flavor.remove_tenant(tenant)
+
+        datamanager.flush()  # get any exception from remove_tenant action
+        datamanager.commit()
+
+    except ErrorStatus as exp:
+        LOG.log_exception("FlavorLogic - Failed to remove tenant", str(exp))
+        datamanager.rollback()
+        raise exp
+
+    except Exception as exp:
+        LOG.log_exception(
+            "FlavorLogic - Failed to remove tenant - exception:", str(exp))
+        datamanager.rollback()
+        raise exp
+
+
 @di.dependsOn('data_manager')
 def delete_region(flavor_uuid, region_name, transaction_id, force_delete):
     DataManager = di.resolver.unpack(delete_region)
     datamanager = DataManager()
-
     try:
         flavor_rec = datamanager.get_record('flavor')
         sql_flavor = flavor_rec.get_flavor_by_id(flavor_uuid)
@@ -317,26 +374,33 @@ def delete_region(flavor_uuid, region_name, transaction_id, force_delete):
 
         existing_region_names = sql_flavor.get_existing_region_names()
         sql_flavor.remove_region(region_name)
+        remaining_regions = sql_flavor.get_existing_region_names()
 
         # get any exception created by previous actions against the database
         datamanager.flush()
+
         send_to_rds_if_needed(sql_flavor, existing_region_names, "put",
                               transaction_id)
+
         if force_delete:
             datamanager.commit()
         else:
             datamanager.rollback()
 
     except ErrorStatus as exp:
-        LOG.log_exception("FlavorLogic - Failed to update flavor", str(exp))
+        LOG.log_exception("FlavorLogic - Failed to delete region", str(exp))
         datamanager.rollback()
         raise exp
 
     except Exception as exp:
-        LOG.log_exception("FlavorLogic - Failed to delete region", str(exp))
+        LOG.log_exception(
+            "FlavorLogic - Failed to delete region - exception:", str(exp))
         datamanager.rollback()
         raise exp
 
+    else:
+        delete_orphaned_tenants(sql_flavor, remaining_regions, datamanager)
+
     finally:
         datamanager.close()
 
@@ -345,9 +409,6 @@ def delete_region(flavor_uuid, region_name, transaction_id, force_delete):
 def add_tenants(flavor_uuid, tenants, transaction_id):
     DataManager = di.resolver.unpack(add_tenants)
     datamanager = DataManager()
-
-    valid_tenants_list, valid_regions_list = [], []
-
     try:
         flavor_rec = datamanager.get_record('flavor')
         sql_flavor = flavor_rec.get_flavor_by_id(flavor_uuid)
@@ -358,32 +419,42 @@ def add_tenants(flavor_uuid, tenants, transaction_id):
         if sql_flavor.visibility == "public":
             raise ErrorStatus(405, 'Cannot add tenant to a public flavor')
 
-        existing_region_names = sql_flavor.get_existing_region_names()
+        existing_region_list = sql_flavor.get_existing_region_names()
-        existing_region_list = []
 
-        for x in existing_region_names:
+        # remove any empty strings in tenants list
-            existing_region_list.append(x)
+        while ('' in tenants.tenants):
+            tenants.tenants.remove('')
 
         if tenants.tenants:
-            valid_tenants_list, valid_regions_list = \
+            for tenant in tenants.tenants:
-                validate_tenants_regions_list(tenants.tenants,
+                if not isinstance(tenant, str):
-                                              existing_region_list,
+                    raise ValueError("tenant type must be a string type,"
-                                              'create', datamanager)
+                                     " got {} type".format(type(tenant)))
 
-        if not valid_tenants_list:
+            if existing_region_list:
+                # get lists of valid tenants/regions from CMS
+                valid_tenants_list, valid_regions_list = \
+                    validate_tenants_regions_list(tenants.tenants,
+                                                  existing_region_list,
+                                                  datamanager)
+            else:
+                # disallow tenant assignment if no flavor region assigned
+                raise ErrorStatus(
+                    400, 'Cannot add tenants with no flavor region assigned')
+
+        if not (tenants.tenants and valid_tenants_list):
             raise ValueError("At least one valid tenant must be provided")
 
         for tenant in valid_tenants_list:
-            if not isinstance(tenant, str):
-                raise ValueError("tenant type must be a string type,"
-                                 " got {} type".format(type(tenant)))
-
             db_tenant = FlavorTenant(tenant_id=tenant)
             sql_flavor.add_tenant(db_tenant)
+
+        send_to_rds_if_needed(
+            sql_flavor, existing_region_list, "put", transaction_id)
+
         # get any exception created by previous actions against the database
         datamanager.flush()
-        send_to_rds_if_needed(
+
-            sql_flavor, existing_region_names, "put", transaction_id)
         datamanager.commit()
 
         flavor = get_flavor_by_uuid(flavor_uuid)
@@ -391,7 +462,7 @@ def add_tenants(flavor_uuid, tenants, transaction_id):
         return ret
 
     except (ErrorStatus, ValueError) as exp:
-        LOG.log_exception("FlavorLogic - Failed to update flavor", str(exp))
+        LOG.log_exception("FlavorLogic - Failed to add tenants", str(exp))
         datamanager.rollback()
         raise
     except Exception as exp:
@@ -408,14 +479,13 @@ def add_tenants(flavor_uuid, tenants, transaction_id):
 def delete_tenant(flavor_uuid, tenant_id, transaction_id):
     DataManager = di.resolver.unpack(delete_tenant)
     datamanager = DataManager()
-
     try:
         flavor_rec = datamanager.get_record('flavor')
         sql_flavor = flavor_rec.get_flavor_by_id(flavor_uuid)
         if not sql_flavor:
             raise ErrorStatus(404,
                               'flavor id {0} not found'.format(flavor_uuid))
-        # if trying to delete the only one tenant then return value error
+
         if sql_flavor.visibility == "public":
             raise ValueError("{} is a public flavor, delete tenant"
                              " action is not relevant".format(flavor_uuid))
@@ -756,7 +826,6 @@ def get_flavor_by_uuid(flavor_uuid):
     datamanager = DataManager()
     try:
         flavor_record = datamanager.get_record('flavor')
-
         sql_flavor = flavor_record.get_flavor_by_id(flavor_uuid)
 
         if not sql_flavor:

orm/tests/unit/fms/test_flavor_logic.py

@@ -508,10 +508,11 @@ class TestFlavorLogic(FunctionalTest):
         ret_flavor = MagicMock()
         tenants = ['test_tenant']
         ret_flavor.flavor.tenants = tenants
+        ret_flavor.flavor.regions = [Region(name='test_region')]
         mock_gfbu.return_value = ret_flavor
         mock_val.return_value = ['test_tenant'], ['test_region']
         global error
-        error = 31
+        error = 3
         injector.override_injected_dependency(
             ('rds_proxy', get_rds_proxy_mock()))
 
@@ -546,7 +547,6 @@ class TestFlavorLogic(FunctionalTest):
                           TenantWrapper(tenants),
                           'transaction')
 
-        # test that no valid tenant found
         error = 31
         mock_val.return_value = [], ['test_region']
         self.assertRaises(ValueError,
@@ -554,6 +554,7 @@ class TestFlavorLogic(FunctionalTest):
                           TenantWrapper(tenants),
                           'transaction')
 
+        error = 3
         mock_val.return_value = ['test_tenant'], ['test_region']
         mock_strin.side_effect = exc.FlushError(
             'conflicts with persistent instance')