From c671b6ae8b9ffbfa5de5279c029d95bff0521d3a Mon Sep 17 00:00:00 2001 From: Michael Vorburger Date: Mon, 2 Jul 2018 20:47:42 +0200 Subject: [PATCH] make security group rule ID mandatory in YANG model To simplify the dependency check - it is not really the role of the new areAllDependenciesAvailable() method to enforce required properties programmatically - doing that declaratively in the YANG model is the ODL way. This lets us re-activate a test which covers this scenario which we had to temporarily comment out in the previous to previous commit for "add dependency check for security rule's group update(), not just add()"; we need the previous commit re. "propagate datastore exceptions all the way to northbound" for YANG mandatory constraints to actually have any effect (because before that, we basically just stupidly ignored ANY problem when commiting the transaction!). Changing this also reveals a subtle mistake in an existing test: Modifying an existing entity without required fields is not actually working / supported (never was; it's not that this break it) - an MD SAL put() is does *NOT* apply a delta, but replaces the existing object. A few Neutron Northbound classes override the updateDelta() method of AbstractNeutronNorthbound with custom logic which seems to attempt to "compensate" for this be doing e.g. delta.setID(uuid); etc. but this is implemented inconsistently in existing code (e.g. many including the NeutronSecurityRulesNorthbound do not do this). Many more fields in the model which in truth are mandatory should ideally be so declared in follow-up changes imitating this one now. JIRA: NEUTRON-158 Change-Id: I077906a682225ac87caf1c4c933f01bdf40c1353 Signed-off-by: Michael Vorburger --- .../neutron/e2etest/NeutronSecurityRuleTests.java | 4 ++-- model/src/main/yang/neutron-secgroups.yang | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java index ac5989a90..d5e937c76 100644 --- a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java +++ b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java @@ -150,7 +150,7 @@ public class NeutronSecurityRuleTests { public static void runTests(String base) { NeutronSecurityRuleTests securityRuleTester = new NeutronSecurityRuleTests(base); - // TODO securityRuleTester.singleton_sr_without_groupid_create_test(500); + securityRuleTester.singleton_sr_without_groupid_create_test(500); securityRuleTester.singleton_sr_create_test(HttpUtils.HTTP_MISSING_DEPENDENCY); // NEUTRON-158 securityRuleTester.singleton_sr_modify_test(404, true); // cannot modify a SR that has not been created new NeutronSecurityGroupTests(base).singleton_sg_create(TEST_SECURITY_GROUP_ID); @@ -158,7 +158,7 @@ public class NeutronSecurityRuleTests { securityRuleTester.singleton_sr_get_with_one_query_item_test(createJsonString); securityRuleTester.multiple_sr_create_test(); securityRuleTester.singleton_sr_modify_test(200, true); - securityRuleTester.singleton_sr_modify_test(200, false); + securityRuleTester.singleton_sr_modify_test(500, false); // Sic! Partial delta updates are not possible. securityRuleTester.sr_element_get_test(); securityRuleTester.sr_element_get_with_query_test(); securityRuleTester.securityRule_collection_get_test(); diff --git a/model/src/main/yang/neutron-secgroups.yang b/model/src/main/yang/neutron-secgroups.yang index 609b6fc53..132d814dc 100644 --- a/model/src/main/yang/neutron-secgroups.yang +++ b/model/src/main/yang/neutron-secgroups.yang @@ -45,6 +45,7 @@ module neutron-secgroups { } leaf security-group-id { type yang:uuid; + mandatory true; description "The security group ID to associate with this security group rule."; } leaf remote-group-id { -- 2.36.6