diff --git a/devstack/lib/apic_aim b/devstack/lib/apic_aim index 83af1c7a2..5d91562c9 100644 --- a/devstack/lib/apic_aim +++ b/devstack/lib/apic_aim @@ -1,9 +1,10 @@ function install_apic_aim { echo_summary "Installing apic_aim" + install_acitoolkit + install_apicapi install_aim install_opflex - install_apicapi } function configure_apic_aim { @@ -42,6 +43,20 @@ function configure_apic_aim { init_aim } +function install_acitoolkit { + git_clone $ACITOOLKIT_REPO $ACITOOLKIT_DIR $ACITOOLKIT_BRANCH + touch $ACITOOLKIT_DIR/setup.cfg + setup_develop $ACITOOLKIT_DIR +} + +function install_apicapi { + git_clone $APICAPI_REPO $APICAPI_DIR $APICAPI_BRANCH + mv $APICAPI_DIR/test-requirements.txt $APICAPI_DIR/_test-requirements.txt + touch $APICAPI_DIR/setup.cfg + setup_develop $APICAPI_DIR + mv $APICAPI_DIR/_test-requirements.txt $APICAPI_DIR/test-requirements.txt +} + function install_aim { git_clone $AIM_REPO $AIM_DIR $AIM_BRANCH mv $AIM_DIR/test-requirements.txt $AIM_DIR/_test-requirements.txt diff --git a/devstack/lib/group-based-policy b/devstack/lib/group-based-policy index 9a54ab835..01e38f77f 100755 --- a/devstack/lib/group-based-policy +++ b/devstack/lib/group-based-policy @@ -28,6 +28,8 @@ OPFLEX_REPO=http://github.com/noironetworks/python-opflex-agent.git OPFLEX_DIR=$DEST/opflexagent APICAPI_REPO=http://github.com/noironetworks/apicapi.git APICAPI_DIR=$DEST/apicapi +ACITOOLKIT_REPO=http://github.com/noironetworks/acitoolkit.git +ACITOOLKIT_DIR=$DEST/acitoolkit # Save trace setting XTRACE=$(set +o | grep xtrace) @@ -85,14 +87,6 @@ function install_gbpui { mv $GBPUI_DIR/_test-requirements.txt $GBPUI_DIR/test-requirements.txt } -function install_apicapi { - git_clone $APICAPI_REPO $APICAPI_DIR $APICAPI_BRANCH - mv $APICAPI_DIR/test-requirements.txt $APICAPI_DIR/_test-requirements.txt - touch $APICAPI_DIR/setup.cfg - setup_develop $APICAPI_DIR - mv $APICAPI_DIR/_test-requirements.txt $APICAPI_DIR/test-requirements.txt -} - # Restore xtrace $XTRACE diff --git a/devstack/settings b/devstack/settings index 9a5e45faa..b188f07f7 100755 --- a/devstack/settings +++ b/devstack/settings @@ -33,6 +33,7 @@ GBPHEAT_BRANCH=${GBPHEAT_BRANCH:-master} AIM_BRANCH=${AIM_BRANCH:-master} OPFLEX_BRANCH=${OPFLEX_BRANCH:-master} APICAPI_BRANCH=${APICAPI_BRANCH:-master} +ACITOOLKIT_BRANCH=${ACITOOLKIT_BRANCH:-noiro-lite} # Enable necessary services, including group-policy (and disable others) disable_service n-net 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 3959538db..a3070ac8b 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py @@ -4712,7 +4712,6 @@ class ApicMechanismDriver(api_plus.MechanismDriver, bd = None epg = None vrf = None - ext_net = None if net_db.external: bd, epg, vrf = self._validate_external_network( mgr, net_db, ext_net_routers, router_dbs, router_vrfs, @@ -4753,11 +4752,19 @@ class ApicMechanismDriver(api_plus.MechanismDriver, # EPG? pass - # Expect NetworkMapping record if applicable. - if bd or epg or vrf or ext_net: + # Expect NetworkMapping record if applicable. Note that + # when validation of SVI networks is implemented, there + # will also be an ext_net resource, and when this is used, + # the bd and epg are not used. + if bd and epg and vrf: self._add_network_mapping( - mgr.expected_session, net_db.id, bd, epg, vrf, ext_net, + mgr.expected_session, net_db.id, bd, epg, vrf, update_network=False) + elif bd or epg or vrf: + mgr.validation_failed( + "Missing resource(s) needed to create expected " + "NetworkMapping for network %s - bd: %s, epg: %s, " + "vrf: %s" % (net_db.id, bd, epg, vrf)) def _get_router_ext_contracts(self, mgr): # Get external contracts for routers. @@ -5079,6 +5086,12 @@ class ApicMechanismDriver(api_plus.MechanismDriver, cidrs = sorted(self.get_external_cidrs_by_ext_net_dn( mgr.actual_session, ext_net.dn, lock_update=False)) ns.update_external_cidrs(mgr.expected_aim_ctx, ext_net, cidrs) + + # Get the L3Outside resources, and check that there is an + # external VRF. + bd = None + epg = None + vrf = None for resource in ns.get_l3outside_resources( mgr.expected_aim_ctx, l3out): if isinstance(resource, aim_resource.BridgeDomain): @@ -5087,6 +5100,9 @@ class ApicMechanismDriver(api_plus.MechanismDriver, epg = resource elif isinstance(resource, aim_resource.VRF): vrf = resource + if not vrf: + mgr.validation_failed( + "missing external VRF for external network %s" % net_db.id) for subnet_db in net_db.subnets: if subnet_db.gateway_ip: 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 e48e0fae7..8c8df056d 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py @@ -123,8 +123,10 @@ class ValidationManager(object): except RollbackTransaction: pass except Exception as exc: - self.output("Validation failed with exception: %s" % exc) - return api.VALIDATION_FAILED_UNREPAIRABLE + self.output("Validation failed with exception: %s - see log for " + "details" % exc) + LOG.exception(exc) + return api.VALIDATION_FAILED_WITH_EXCEPTION # Bind unbound ports outside transaction. if (self.repair and diff --git a/gbpservice/neutron/services/grouppolicy/group_policy_driver_api.py b/gbpservice/neutron/services/grouppolicy/group_policy_driver_api.py index e33f037bd..8d365c4b3 100644 --- a/gbpservice/neutron/services/grouppolicy/group_policy_driver_api.py +++ b/gbpservice/neutron/services/grouppolicy/group_policy_driver_api.py @@ -21,8 +21,18 @@ from gbpservice.common import utils VALIDATION_PASSED = "passed" VALIDATION_REPAIRED = "repaired" VALIDATION_FAILED_REPAIRABLE = "failed repairable" -VALIDATION_FAILED_UNREPAIRABLE = "failed unrepairable" VALIDATION_FAILED_BINDING_PORTS = "failed binding ports" +VALIDATION_FAILED_UNREPAIRABLE = "failed unrepairable" +VALIDATION_FAILED_WITH_EXCEPTION = "failed with exception" + +VALIDATION_RESULT_PRECEDENCE = [ + VALIDATION_PASSED, + VALIDATION_REPAIRED, + VALIDATION_FAILED_REPAIRABLE, + VALIDATION_FAILED_BINDING_PORTS, + VALIDATION_FAILED_UNREPAIRABLE, + VALIDATION_FAILED_WITH_EXCEPTION, +] @six.add_metaclass(abc.ABCMeta) @@ -1400,8 +1410,10 @@ class PolicyDriver(object): Called from validation tool to validate policy driver's persistent state. Returns VALIDATION_PASSED, VALIDATION_REPAIRED (only if repair == True), - VALIDATION_FAILED_REPAIRABLE (only if repair == False) or - VALIDATION_FAILED_UNREPAIRABLE. + VALIDATION_FAILED_REPAIRABLE (only if repair == False), + VALIDATION_FAILED_UNREPAIRABLE, + VALIDATION_FAILED_WITH_EXCEPTION, or + VALIDATION_FAILED_BINDING_PORTS. """ return VALIDATION_PASSED diff --git a/gbpservice/neutron/services/grouppolicy/policy_driver_manager.py b/gbpservice/neutron/services/grouppolicy/policy_driver_manager.py index cc74ec9bd..85d6464a4 100644 --- a/gbpservice/neutron/services/grouppolicy/policy_driver_manager.py +++ b/gbpservice/neutron/services/grouppolicy/policy_driver_manager.py @@ -499,10 +499,12 @@ class PolicyDriverManager(stevedore.named.NamedExtensionManager): result = api.VALIDATION_PASSED for driver in self.ordered_policy_drivers: this_result = driver.obj.validate_state(repair) - if (this_result == api.VALIDATION_FAILED_UNREPAIRABLE or - (this_result == api.VALIDATION_FAILED_REPAIRABLE and - result != api.VALIDATION_FAILED_UNREPAIRABLE) or - (this_result == api.VALIDATION_REPAIRED and - result == api.VALIDATION_PASSED)): + if this_result not in api.VALIDATION_RESULT_PRECEDENCE: + LOG.error("Policy driver %s validate_state returned " + "unrecognized result: %s" % + (driver.name, this_result)) + this_result = api.VALIDATION_FAILED_WITH_EXCEPTION + if (api.VALIDATION_RESULT_PRECEDENCE.index(this_result) > + api.VALIDATION_RESULT_PRECEDENCE.index(result)): result = this_result return result 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 e0771c21e..dff36eeca 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py @@ -580,6 +580,42 @@ class TestNeutronMapping(AimValidationTestCase): self._validate_repair_validate() self._test_network_attrs(net) + # Delete pre-existing AIM VRF and test. + self.aim_mgr.delete(self.aim_ctx, vrf) + self._validate_unrepairable() + + # Replace pre-existing AIM VRF and test. + self.aim_mgr.create(self.aim_ctx, vrf) + self._validate() + + # REVISIT: Missing AIM L3Outsides, ExternalNetworks, and + # ExternalSubnets that were supposed to be pre-existing all + # get silently created by the NAT strategy code, so these are + # considered repairable for now, but the repair is likely to + # result in referencing the wrong VRF, the resource no longer + # being considered monitored, and other configuration + # errors. Consider adding validation code to check that the + # ExternalNetwork identified by + # NetworkExtensionDb.external_network_dn, along with its + # parent L3Outside, actually exist before calling the NAT + # strategy code, and failing with + # VALIDATION_FAILED_UNREPAIRABLE if they don't exist. The + # mechanism driver should probably also check that they exist + # when the network is created, and fail if they don't, but + # that might break existing use cases. + + # Delete pre-existing AIM L3Outside and test. + self.aim_mgr.delete(self.aim_ctx, l3out) + self._validate_repair_validate() + + # Delete pre-existing AIM ExternalNetwork and test. + self.aim_mgr.delete(self.aim_ctx, ext_net) + self._validate_repair_validate() + + # Delete pre-existing AIM ExternalSubnet and test. + self.aim_mgr.delete(self.aim_ctx, ext_sn) + self._validate_repair_validate() + def test_svi_network(self): # REVISIT: Test validation of actual mapping once implemented. diff --git a/gbpservice/tests/contrib/devstack/local-aim.conf b/gbpservice/tests/contrib/devstack/local-aim.conf index a39042112..a17a29a47 100644 --- a/gbpservice/tests/contrib/devstack/local-aim.conf +++ b/gbpservice/tests/contrib/devstack/local-aim.conf @@ -20,3 +20,4 @@ ENABLE_APIC_AIM_GATE=True AIM_BRANCH=master OPFLEX_BRANCH=master APICAPI_BRANCH=master +ACITOOLKIT_BRANCH=noiro-lite diff --git a/gbpservice/tools/validate/cli.py b/gbpservice/tools/validate/cli.py index 80e462873..3846bd706 100644 --- a/gbpservice/tools/validate/cli.py +++ b/gbpservice/tools/validate/cli.py @@ -54,6 +54,7 @@ def main(): result = gbp_plugin.validate_state(cfg.CONF.repair) if result in [api.VALIDATION_FAILED_REPAIRABLE, - api.VALIDATION_FAILED_UNREPAIRABLE]: + api.VALIDATION_FAILED_UNREPAIRABLE, + api.VALIDATION_FAILED_WITH_EXCEPTION]: sys.exit(result) return 0 diff --git a/test-requirements.txt b/test-requirements.txt index 5a3b5e413..794b9d6b0 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -44,7 +44,9 @@ pyOpenSSL>=16.2.0 # Apache-2.0 python-heatclient python-keystoneclient -# REVISIT: Until co-gating and/or stable branches are implemented for the -# aci-integration-module repo, it may be necessary to pin to a working -# commit. +# REVISIT: Until co-gating and/or stable branches are implemented for +# the aci-integration-module repo, it may be necessary to pin to a +# working commit. Also, specific branches in indirect dependencies +# seem to be ignored, so we list them here too. +-e git+https://github.com/noironetworks/acitoolkit.git@noiro-lite#egg=acitoolkit -e git+https://github.com/noironetworks/aci-integration-module.git@master#egg=aci-integration-module