From 4bc66f1e8dd82cdcc4b2afd0a9f9ab532310cf69 Mon Sep 17 00:00:00 2001 From: Robert Kukura Date: Wed, 24 Oct 2018 08:18:01 -0400 Subject: [PATCH] [AIM] Validation tool updates monitored resources The validation tool now updates monitored resources. This is needed because repair/migration may update these pre-existing resources, such as by adding provided/consumed contracts. Since these start out as copies of the existing resources (fixed for common Tenant), only the required changes should be applied. An existing UT now clears contracts on a pre-existing ExternalNetwork to test that these are properly updated. Also, when creating common Tenant in AIM, set its display name to an empty string instead of 'CommonTenant'. Change-Id: Iaa8ca7e62aa89860d244a388167f3212cf79dc9c --- .../drivers/apic_aim/mechanism_driver.py | 8 ++++++-- .../drivers/cisco/apic/aim_mapping.py | 5 ++--- .../drivers/cisco/apic/aim_validation.py | 17 +++++++---------- .../tests/unit/plugins/ml2plus/test_apic_aim.py | 6 +++--- .../services/grouppolicy/test_aim_validation.py | 7 ++++++- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py index 611521875..4517f51c5 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py @@ -2893,8 +2893,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, def _ensure_common_tenant(self, aim_ctx): attrs = aim_resource.Tenant( - name=COMMON_TENANT_NAME, monitored=True, - display_name=aim_utils.sanitize_display_name('CommonTenant')) + name=COMMON_TENANT_NAME, monitored=True, display_name='') tenant = self.aim.get(aim_ctx, attrs) if not tenant: LOG.info("Creating common tenant") @@ -4056,6 +4055,11 @@ class ApicMechanismDriver(api_plus.MechanismDriver, mgr.register_aim_resource_class(aim_resource.VMMDomain) mgr.register_aim_resource_class(aim_resource.VRF) + # Copy common Tenant from actual to expected AIM store. + for tenant in mgr.aim_mgr.find( + mgr.actual_aim_ctx, aim_resource.Tenant, name=COMMON_TENANT_NAME): + mgr.aim_mgr.create(mgr.expected_aim_ctx, tenant) + # Copy AIM resources that are managed via aimctl from actual # to expected AIM stores. for resource_class in [aim_infra.HostDomainMappingV2, diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py index 450a67359..7aee86207 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py @@ -2450,9 +2450,8 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin): def _aim_tenant_name(self, session, tenant_id, aim_resource_class=None, gbp_resource=None, gbp_obj=None): - attrs = aim_resource.Tenant(name=md.COMMON_TENANT_NAME, - display_name=aim_utils.sanitize_display_name( - 'CommonTenant')) + attrs = aim_resource.Tenant( + name=md.COMMON_TENANT_NAME, display_name='') tenant = aim_mgr.get(aim_ctx, attrs) if not tenant: tenant = aim_mgr.create(aim_ctx, attrs) diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py index f4f02b662..889bac683 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py @@ -234,16 +234,13 @@ class ValidationManager(object): if not getattr(actual_resource, 'monitored', True): self._handle_unexpected_aim_resource(actual_resource) else: - # Some infra resources do not have the monitored - # attribute, but are treated as if they are monitored. - if getattr(expected_resource, 'monitored', True): - # REVISIT: Make sure actual resource is monitored, but - # ignore other differences. - pass - else: - if not expected_resource.user_equal(actual_resource): - self._handle_incorrect_aim_resource( - expected_resource, actual_resource) + # Update both monitored and unmonitored + # resources. Monitored resources should have started out + # as copies of actual resources, but may have had changes + # made, such as additional contracts provided or consumed. + if not expected_resource.user_equal(actual_resource): + self._handle_incorrect_aim_resource( + expected_resource, actual_resource) def _handle_unexpected_aim_resource(self, actual_resource): if self.should_repair( diff --git a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py index 2e2f0a72b..d13eaa681 100644 --- a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py +++ b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py @@ -643,7 +643,7 @@ class TestAimMapping(ApicAimTestCase): tenant_aname = vrf.tenant_name vrf_tenant_dname = None else: - vrf_tenant_dname = 'CommonTenant' + vrf_tenant_dname = '' elif scope: scope = self._actual_scopes.get(scope['id'], scope) vrf_aname = self.name_mapper.address_scope(None, scope['id']) @@ -667,7 +667,7 @@ class TestAimMapping(ApicAimTestCase): vrf_aname = self.driver.apic_system_id + '_UnroutedVRF' vrf_dname = 'CommonUnroutedVRF' vrf_tenant_aname = 'common' - vrf_tenant_dname = 'CommonTenant' + vrf_tenant_dname = '' if net[SVI]: ext_net = aim_resource.ExternalNetwork.from_dn( @@ -933,7 +933,7 @@ class TestAimMapping(ApicAimTestCase): # Check common Tenant. tenant = self._get_tenant('common') self.assertEqual('common', tenant.name) - self.assertEqual('CommonTenant', tenant.display_name) + self.assertEqual('', tenant.display_name) self.assertEqual('', tenant.descr) # Check unrouted VRF. diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py index 840cb003d..0ce6487ed 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py @@ -490,13 +490,18 @@ class TestNeutronMapping(AimValidationTestCase): net = self._test_external_network(vrf_name='v1') net_id = net['id'] - # Delete network extension data to test migration use case. + # Delete network extension data and clear ExternalNetwork + # contracts to test migration use case. (self.db_session.query(ext_db.NetworkExtensionDb). filter_by(network_id=net_id). delete()) (self.db_session.query(ext_db.NetworkExtensionCidrDb). filter_by(network_id=net_id). delete()) + self.aim_mgr.update( + self.aim_ctx, ext_net, + provided_contract_names=[], + consumed_contract_names=[]) # Test without DN for migration. self._validate_unrepairable()