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 889bac683..35b4329eb 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py @@ -50,10 +50,7 @@ class ValidationManager(object): self.sfcd = driver.obj def validate(self, repair=False): - # REVISIT: Replace print calls throughout this module with an - # output stream that can be sent to stdout/stderr and/or an - # output file? - print("Validating deployment, repair: %s" % repair) + self.output("Validating deployment, repair: %s" % repair) self.result = api.VALIDATION_PASSED self.repair = repair @@ -109,21 +106,27 @@ class ValidationManager(object): # Commit or rollback transaction. if self.result is api.VALIDATION_REPAIRED: - print("Committing repairs") + self.output("Committing repairs") self.actual_session.commit() else: - if self.repair and self.result is api.VALIDATION_FAILED: - print("Rolling back attempted repairs") + if (self.repair and + self.result is api.VALIDATION_FAILED_UNREPAIRABLE): + self.output("Rolling back attempted repairs") self.actual_session.rollback() # Bind unbound ports outside transaction. - if self.repair and self.result is not api.VALIDATION_FAILED: - print("Binding unbound ports") + if (self.repair and + self.result is not api.VALIDATION_FAILED_UNREPAIRABLE): + self.output("Binding unbound ports") self.md.bind_unbound_ports(self) - print("Validation result: %s" % self.result) + self.output("Validation result: %s" % self.result) return self.result + def output(self, msg): + LOG.info(msg) + print(msg) + def register_aim_resource_class(self, resource_class): if resource_class not in self._expected_aim_resources: self._expected_aim_resources[resource_class] = {} @@ -139,7 +142,7 @@ class ValidationManager(object): del expected_resources[key] return elif not replace and key in expected_resources: - print("resource %s already expected" % resource) + self.output("resource %s already expected" % resource) raise InternalValidationError() for attr_name, attr_type in resource.other_attributes.items(): attr_type_type = attr_type['type'] @@ -199,16 +202,19 @@ class ValidationManager(object): return expected_instances.values() def should_repair(self, problem, action='Repairing'): - if self.repair and self.result is not api.VALIDATION_FAILED: - self.result = api.VALIDATION_REPAIRED - print("%s %s" % (action, problem)) + if self.result is not api.VALIDATION_FAILED_UNREPAIRABLE: + self.output("%s %s" % (action, problem)) + self.result = (api.VALIDATION_REPAIRED if self.repair + else api.VALIDATION_FAILED_REPAIRABLE) return True - else: - self.validation_failed(problem) def validation_failed(self, reason): - print("Failed due to %s" % reason) - self.result = api.VALIDATION_FAILED + # REVISIT: Do we need drivers to be able to specify repairable + # vs unrepairable? If so, add a keyword arg and make sure + # VALIDATION_FAILED_REPAIRABLE does not overwrite + # VALIDATION_FAILED_UNREPAIRABLE. + self.output("Failed UNREPAIRABLE due to %s" % reason) + self.result = api.VALIDATION_FAILED_UNREPAIRABLE def _validate_aim_resources(self): for resource_class in self._expected_aim_resources.keys(): diff --git a/gbpservice/neutron/services/grouppolicy/group_policy_driver_api.py b/gbpservice/neutron/services/grouppolicy/group_policy_driver_api.py index 45e58bea8..ee4574737 100644 --- a/gbpservice/neutron/services/grouppolicy/group_policy_driver_api.py +++ b/gbpservice/neutron/services/grouppolicy/group_policy_driver_api.py @@ -20,7 +20,8 @@ from gbpservice.common import utils VALIDATION_PASSED = "passed" VALIDATION_REPAIRED = "repaired" -VALIDATION_FAILED = "failed" +VALIDATION_FAILED_REPAIRABLE = "failed repairable" +VALIDATION_FAILED_UNREPAIRABLE = "failed unrepairable" @six.add_metaclass(abc.ABCMeta) @@ -1395,9 +1396,11 @@ class PolicyDriver(object): :param repair: Repair invalid state if True. - Called from validation tool to validate policy driver's persistent - state. Returns VALIDATION_PASSED, VALIDATION_REPAIRED, or - VALIDATION_FAILED. + 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. """ return VALIDATION_PASSED diff --git a/gbpservice/neutron/services/grouppolicy/policy_driver_manager.py b/gbpservice/neutron/services/grouppolicy/policy_driver_manager.py index 874d2915f..cc74ec9bd 100644 --- a/gbpservice/neutron/services/grouppolicy/policy_driver_manager.py +++ b/gbpservice/neutron/services/grouppolicy/policy_driver_manager.py @@ -499,9 +499,10 @@ 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: - result = this_result - elif (this_result == api.VALIDATION_REPAIRED and - result != api.VALIDATION_FAILED): + 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)): 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 0ce6487ed..7af8b48ec 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py @@ -46,7 +46,8 @@ class AimValidationTestMixin(object): def _validate_repair_validate(self): # Validate should fail. - self.assertEqual(api.VALIDATION_FAILED, self.av_mgr.validate()) + self.assertEqual( + api.VALIDATION_FAILED_REPAIRABLE, self.av_mgr.validate()) # Repair. self.assertEqual( @@ -58,7 +59,8 @@ class AimValidationTestMixin(object): def _validate_unrepairable(self): # Repair should fail. self.assertEqual( - api.VALIDATION_FAILED, self.av_mgr.validate(repair=True)) + api.VALIDATION_FAILED_UNREPAIRABLE, + self.av_mgr.validate(repair=True)) def _test_aim_resource(self, resource, unexpected_attr_name='name', unexpected_attr_value='unexpected', diff --git a/gbpservice/tools/validate/cli.py b/gbpservice/tools/validate/cli.py index 903883bcf..80e462873 100644 --- a/gbpservice/tools/validate/cli.py +++ b/gbpservice/tools/validate/cli.py @@ -53,6 +53,7 @@ def main(): sys.exit("GBP service plugin not configured.") result = gbp_plugin.validate_state(cfg.CONF.repair) - if result == api.VALIDATION_FAILED: - sys.exit("Validation failed") + if result in [api.VALIDATION_FAILED_REPAIRABLE, + api.VALIDATION_FAILED_UNREPAIRABLE]: + sys.exit(result) return 0