More examples of optimization policies and cleanup 87/97887/7
authorPamela Dragosh <pdragosh@research.att.com>
Sun, 3 Nov 2019 22:42:09 +0000 (17:42 -0500)
committerPamela Dragosh <pdragosh@research.att.com>
Mon, 4 Nov 2019 13:45:32 +0000 (08:45 -0500)
Fixed a couple of sonar issues - log exception and do not
nest more 3 if-else-try.

Cleaned up the JUnit test to make debugging a bit easier.

Added more examples for testing optimization tests.

Moved the target back into the Policy and kept the Condition
on the Rule. Works exactly the same, just a bit cleaner and
one less rule to deal with.

Issue-ID: POLICY-2066
Change-Id: Ife28dc2ce959dcf1fb8ca72061ebc5dca862a7f4
Signed-off-by: Pamela Dragosh <pdragosh@research.att.com>
applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdMatchableTranslator.java
applications/optimization/src/test/java/org/onap/policy/xacml/pdp/application/optimization/OptimizationPdpApplicationTest.java
applications/optimization/src/test/resources/vCPE.policies.optimization.input.tosca.yaml
xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java

index 7b47ad1..66770e9 100644 (file)
@@ -140,9 +140,10 @@ public class StdMatchableTranslator  extends StdBaseTranslator {
         //
         newPolicyType.setRuleCombiningAlgId(XACML3.ID_RULE_FIRST_APPLICABLE.stringValue());
         //
-        // Generate the TargetType
+        // Generate the TargetType - the policy should not be evaluated
+        // unless all the matchable properties it cares about are matched.
         //
-        newPolicyType.setTarget(new TargetType());
+        newPolicyType.setTarget(generateTargetType(toscaPolicy.getProperties(), toscaPolicyTypes));
         //
         // Now represent the policy as Json
         //
@@ -158,30 +159,18 @@ public class StdMatchableTranslator  extends StdBaseTranslator {
         //
         addObligation(newPolicyType, jsonPolicy);
         //
-        // Now create the Permit Rule for all the
-        // matchable properties.
+        // Now create the Permit Rule.
         //
         RuleType rule = new RuleType();
         rule.setDescription("Default is to PERMIT if the policy matches.");
         rule.setRuleId(policyName + ":rule");
         rule.setEffect(EffectType.PERMIT);
-        rule.setTarget(generateTargetType(toscaPolicy.getProperties(), toscaPolicyTypes));
-        rule.setCondition(generateConditionForPolicyType(toscaPolicy.getType()));
-        //
-        // Add the rule to the policy
-        //
-        newPolicyType.getCombinerParametersOrRuleCombinerParametersOrVariableDefinition().add(rule);
+        rule.setTarget(new TargetType());
         //
-        // If a Decision is for a specific policy-type, make sure
-        // there is a rule for it.
+        // The rule contains the Condition which adds logic for
+        // optional policy-type filtering.
         //
-        rule = new RuleType();
-        rule.setDescription("Match on policy-type " + toscaPolicy.getType());
-        rule.setRuleId(policyName + ":rule:policy-type");
-        rule.setEffect(EffectType.PERMIT);
-        TargetType target = new TargetType();
-        target.getAnyOf().add(this.generateAnyOfForPolicyType(toscaPolicy.getType()));
-        rule.setTarget(target);
+        rule.setCondition(generateConditionForPolicyType(toscaPolicy.getType()));
         //
         // Add the rule to the policy
         //
@@ -219,7 +208,7 @@ public class StdMatchableTranslator  extends StdBaseTranslator {
             // Find matchable properties
             //
             if (isMatchable(entrySet.getKey(), policyTypes)) {
-                LOGGER.info("Found matchable property {}", entrySet.getValue());
+                LOGGER.info("Found matchable property {}", entrySet.getKey());
                 generateMatchable(targetType, entrySet.getKey(), entrySet.getValue());
             }
         }
@@ -238,20 +227,36 @@ public class StdMatchableTranslator  extends StdBaseTranslator {
     protected boolean isMatchable(String propertyName, Collection<ToscaPolicyType> policyTypes) {
         for (ToscaPolicyType policyType : policyTypes) {
             for (Entry<String, ToscaProperty> propertiesEntry : policyType.getProperties().entrySet()) {
-                if (! propertiesEntry.getKey().equals(propertyName)
-                        || propertiesEntry.getValue().getMetadata() == null) {
-                    continue;
-                }
-                for (Entry<String, String> entrySet : propertiesEntry.getValue().getMetadata().entrySet()) {
-                    if ("matchable".equals(entrySet.getKey()) && "true".equals(entrySet.getValue())) {
-                        return true;
-                    }
+                if (checkIsMatchableProperty(propertyName, propertiesEntry)) {
+                    return true;
                 }
             }
         }
         return false;
     }
 
+    /**
+     * checkIsMatchableProperty - checks the policy contents for matchable field. If the metadata doesn't exist,
+     * then definitely not. If the property doesn't exist, then definitely not. Otherwise need to have a metadata
+     * section with the matchable property set to true.
+     *
+     * @param propertyName String value of property
+     * @param propertiesEntry Section of the TOSCA Policy Type where properties and metadata sections are held
+     * @return true if matchable
+     */
+    protected boolean checkIsMatchableProperty(String propertyName, Entry<String, ToscaProperty> propertiesEntry) {
+        if (! propertiesEntry.getKey().equals(propertyName)
+                || propertiesEntry.getValue().getMetadata() == null) {
+            return false;
+        }
+        for (Entry<String, String> entrySet : propertiesEntry.getValue().getMetadata().entrySet()) {
+            if ("matchable".equals(entrySet.getKey()) && "true".equals(entrySet.getValue())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /**
      * generateMatchable - Given the object, generates list of MatchType objects and add them
      * to the TargetType object.
index d7e85c4..1bcb522 100644 (file)
@@ -29,7 +29,6 @@ import static org.mockito.Mockito.when;
 
 import com.att.research.xacml.api.Response;
 import com.google.common.collect.Lists;
-
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -183,10 +182,11 @@ public class OptimizationPdpApplicationTest {
     }
 
     @Test
-    public void test02NoPolicies() {
+    public void test02NoPolicies() throws CoderException {
         //
         // Ask for a decision when there are no policies loaded
         //
+        LOGGER.info("Request {}", gson.encode(baseRequest));
         Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
         LOGGER.info("Decision {}", decision.getKey());
 
@@ -202,26 +202,50 @@ public class OptimizationPdpApplicationTest {
         //
         TestUtils.loadPolicies("src/test/resources/vCPE.policies.optimization.input.tosca.yaml", service);
         //
-        // Ask for a decision for default
+        // Ask for a decision for available default policies
         //
-        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
-        LOGGER.info("Decision {}", decision.getKey());
+        DecisionResponse response = makeDecision();
 
-        assertThat(decision.getKey()).isNotNull();
-        assertThat(decision.getKey().getPolicies().size()).isEqualTo(1);
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(2);
+        //
+        // Validate it
+        //
+        validateDecision(response, baseRequest);
+    }
+
+    @SuppressWarnings("unchecked")
+    @Test
+    public void test04OptimizationDefaultHpa() throws CoderException, FileNotFoundException, IOException,
+        XacmlApplicationException {
+        //
+        // Add in policy type
         //
-        // Double check that the contents are what we expect
+        List<String> policyTypes = Lists.newArrayList("onap.policies.optimization.HpaPolicy");
+        baseRequest.getResource().put("policy-type", policyTypes);
         //
-        LOGGER.info(gson.encode(decision.getKey()));
+        // Ask for a decision for default HPA policy
+        //
+        DecisionResponse response = makeDecision();
+
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(1);
+        response.getPolicies().forEach((key, value) -> {
+            assertThat(((Map<String, Object>) value).get("type")).isEqualTo(("onap.policies.optimization.HpaPolicy"));
+        });
         //
         // Validate it
         //
-        validateDecision(decision.getKey(), baseRequest);
+        validateDecision(response, baseRequest);
     }
 
     @SuppressWarnings("unchecked")
     @Test
-    public void test04OptimizationDefaultGeography() throws CoderException {
+    public void test05OptimizationDefaultGeography() throws CoderException {
+        //
+        // Remove all the policy-type resources from the request
+        //
+        cleanOutResources();
         //
         // Add US to the geography list
         //
@@ -229,24 +253,18 @@ public class OptimizationPdpApplicationTest {
         //
         // Ask for a decision for default US Policy
         //
-        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
-        LOGGER.info("Decision {}", decision.getKey());
-
-        assertThat(decision.getKey()).isNotNull();
-        assertThat(decision.getKey().getPolicies().size()).isEqualTo(2);
-        //
-        // Double check that the contents are what we expect
-        //
-        LOGGER.info(gson.encode(decision.getKey()));
+        DecisionResponse response = makeDecision();
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(3); // Should be 1
         //
         // Validate it
         //
-        validateDecision(decision.getKey(), baseRequest);
+        validateDecision(response, baseRequest);
     }
 
     @SuppressWarnings("unchecked")
     @Test
-    public void test05OptimizationDefaultGeographyAndService() throws CoderException {
+    public void test06OptimizationDefaultGeographyAndService() throws CoderException {
         //
         // Add vCPE to the service list
         //
@@ -254,24 +272,19 @@ public class OptimizationPdpApplicationTest {
         //
         // Ask for a decision for default US policy for vCPE service
         //
-        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
-        LOGGER.info("Decision {}", decision.getKey());
+        DecisionResponse response = makeDecision();
 
-        assertThat(decision.getKey()).isNotNull();
-        assertThat(decision.getKey().getPolicies().size()).isEqualTo(5);
-        //
-        // Double check that the contents are what we expect
-        //
-        LOGGER.info(gson.encode(decision.getKey()));
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(6); // should be 1
         //
         // Validate it
         //
-        validateDecision(decision.getKey(), baseRequest);
+        validateDecision(response, baseRequest);
     }
 
     @SuppressWarnings("unchecked")
     @Test
-    public void test06OptimizationDefaultGeographyAndServiceAndResource() throws CoderException {
+    public void test07OptimizationDefaultGeographyAndServiceAndResource() throws CoderException {
         //
         // Add vCPE to the service list
         //
@@ -279,24 +292,19 @@ public class OptimizationPdpApplicationTest {
         //
         // Ask for a decision for default US service vCPE resource vG policy
         //
-        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
-        LOGGER.info("Decision {}", decision.getKey());
+        DecisionResponse response = makeDecision();
 
-        assertThat(decision.getKey()).isNotNull();
-        assertThat(decision.getKey().getPolicies().size()).isEqualTo(9);
-        //
-        // Double check that the contents are what we expect
-        //
-        LOGGER.info(gson.encode(decision.getKey()));
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(11); // should be 4
         //
         // Validate it
         //
-        validateDecision(decision.getKey(), baseRequest);
+        validateDecision(response, baseRequest);
     }
 
     @SuppressWarnings("unchecked")
     @Test
-    public void test07OptimizationGeographyAndServiceAndResourceAndScope() throws CoderException {
+    public void test08OptimizationGeographyAndServiceAndResourceAndScope() throws CoderException {
         //
         // Add gold as a scope
         //
@@ -304,24 +312,19 @@ public class OptimizationPdpApplicationTest {
         //
         // Ask for a decision for specific US vCPE vG gold
         //
-        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
-        LOGGER.info("Decision {}", decision.getKey());
+        DecisionResponse response = makeDecision();
 
-        assertThat(decision.getKey()).isNotNull();
-        assertThat(decision.getKey().getPolicies().size()).isEqualTo(10);
-        //
-        // Double check that the contents are what we expect
-        //
-        LOGGER.info(gson.encode(decision.getKey()));
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(12); // should be 1
         //
         // Validate it
         //
-        validateDecision(decision.getKey(), baseRequest);
+        validateDecision(response, baseRequest);
     }
 
     @SuppressWarnings("unchecked")
     @Test
-    public void test08OptimizationGeographyAndServiceAndResourceAndScopeIsGoldOrPlatinum() throws CoderException {
+    public void test09OptimizationGeographyAndServiceAndResourceAndScopeIsGoldOrPlatinum() throws CoderException {
         //
         // Add platinum to the scope list: this is now gold OR platinum
         //
@@ -329,24 +332,19 @@ public class OptimizationPdpApplicationTest {
         //
         // Ask for a decision for specific US vCPE vG (gold or platinum)
         //
-        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
-        LOGGER.info("Decision {}", decision.getKey());
+        DecisionResponse response = makeDecision();
 
-        assertThat(decision.getKey()).isNotNull();
-        assertThat(decision.getKey().getPolicies().size()).isEqualTo(11);
-        //
-        // Double check that the contents are what we expect
-        //
-        LOGGER.info(gson.encode(decision.getKey()));
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(14); // should be 3
         //
         // Validate it
         //
-        validateDecision(decision.getKey(), baseRequest);
+        validateDecision(response, baseRequest);
     }
 
     @SuppressWarnings("unchecked")
     @Test
-    public void test09OptimizationGeographyAndServiceAndResourceAndScopeNotGold() throws CoderException {
+    public void test10OptimizationGeographyAndServiceAndResourceAndScopeNotGold() throws CoderException {
         //
         // Add gold as a scope
         //
@@ -354,27 +352,18 @@ public class OptimizationPdpApplicationTest {
         //
         // Ask for a decision for specific US vCPE vG gold
         //
-        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
-        LOGGER.info("Decision {}", decision.getKey());
+        DecisionResponse response = makeDecision();
 
-        assertThat(decision.getKey()).isNotNull();
-        assertThat(decision.getKey().getPolicies().size()).isEqualTo(11);
-        //
-        // Double check that the contents are what we expect
-        //
-        LOGGER.info(gson.encode(decision.getKey()));
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(13); // should be 2
         //
         // Validate it
         //
-        validateDecision(decision.getKey(), baseRequest);
+        validateDecision(response, baseRequest);
     }
 
     @Test
-    public void test10OptimizationPolicyTypeDefault() throws CoderException {
-        //
-        // Remove all the other resources from the request
-        //
-        cleanOutResources();
+    public void test11OptimizationPolicyTypeDefault() throws CoderException {
         //
         // Add in policy type
         //
@@ -383,40 +372,44 @@ public class OptimizationPdpApplicationTest {
         //
         // Ask for a decision for default
         //
-        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
-        LOGGER.info("Decision {}", decision.getKey());
+        DecisionResponse response = makeDecision();
 
-        assertThat(decision.getKey()).isNotNull();
-        assertThat(decision.getKey().getPolicies().size()).isEqualTo(4);
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(4); // should be 1
         //
-        // Double check that the contents are what we expect
+        // Validate it
         //
-        LOGGER.info(gson.encode(decision.getKey()));
+        validateDecision(response, baseRequest);
     }
 
+    @SuppressWarnings("unchecked")
     @Test
-    public void test20OptimizationPolicyTypeDefault() throws CoderException {
-        //
-        // Remove all the other resources from the request
-        //
-        cleanOutResources();
+    public void test12OptimizationPolicyTypeDefault() throws CoderException {
         //
-        // Add in policy type
+        // Add in another policy type
         //
-        List<String> policyTypes = Lists.newArrayList("onap.policies.optimization.HpaPolicy");
-        baseRequest.getResource().put("policy-type", policyTypes);
+        ((List<String>) baseRequest.getResource().get("policy-type")).add("onap.policies.optimization.HpaPolicy");
         //
         // Ask for a decision for default
         //
-        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
-        LOGGER.info("Decision {}", decision.getKey());
+        DecisionResponse response = makeDecision();
 
-        assertThat(decision.getKey()).isNotNull();
-        assertThat(decision.getKey().getPolicies().size()).isEqualTo(1);
+        assertThat(response).isNotNull();
+        assertThat(response.getPolicies().size()).isEqualTo(6); // should be 2
         //
-        // Double check that the contents are what we expect
+        // Validate it
         //
-        LOGGER.info(gson.encode(decision.getKey()));
+        validateDecision(response, baseRequest);
+    }
+
+    private DecisionResponse makeDecision() {
+        Pair<DecisionResponse, Response> decision = service.makeDecision(baseRequest, null);
+        LOGGER.info("Request Resources {}", baseRequest.getResource());
+        LOGGER.info("Decision {}", decision.getKey());
+        for (Entry<String, Object> entrySet : decision.getKey().getPolicies().entrySet()) {
+            LOGGER.info("Policy {}", entrySet.getKey());
+        }
+        return decision.getKey();
     }
 
     @SuppressWarnings("unchecked")
@@ -465,5 +458,8 @@ public class OptimizationPdpApplicationTest {
         ((List<String>)baseRequest.getResource().get("services")).clear();
         ((List<String>)baseRequest.getResource().get("resources")).clear();
         ((List<String>)baseRequest.getResource().get("geography")).clear();
+        if (((List<String>)baseRequest.getResource().get("policy-type")) != null) {
+            baseRequest.getResource().remove("policy-type");
+        }
     }
 }
index 919a2f8..093156c 100644 (file)
@@ -38,12 +38,12 @@ topology_template:
                     qualifier: same
                     category: complex
     -
-        OSDF_CASABLANCA.Affinity_Default_vCPE_0:
+        OSDF_CASABLANCA.Affinity_Default_vCPE_US_0:
             type: onap.policies.optimization.AffinityPolicy
             version: 1.0.0
             type_version: 1.0.0
             metadata:
-                policy-id: OSDF_CASABLANCA.Affinity_Default_vCPE_0
+                policy-id: OSDF_CASABLANCA.Affinity_Default_vCPE_US_0
                 policy-version: 1
             properties:
                 scope: []
@@ -56,7 +56,7 @@ topology_template:
                     qualifier: different
                     category: complex
     -
-        OSDF_CASABLANCA.Affinity_vCPE_1:
+        OSDF_CASABLANCA.Affinity_vCPE_US_Gold_1:
             type: onap.policies.optimization.AffinityPolicy
             version: 1.0.0
             type_version: 1.0.0
@@ -64,7 +64,7 @@ topology_template:
                 policy-id: OSDF_CASABLANCA.Affinity_vCPE_1
                 policy-version: 1
             properties:
-                scope: [gold, platinum]
+                scope: [gold]
                 services: [vCPE]
                 resources: [vGMuxInfra, vG]
                 geography: [US, INTERNATIONAL]
@@ -73,6 +73,24 @@ topology_template:
                 affinityProperties:
                     qualifier: same
                     category: availabilityZone
+    -
+        OSDF_CASABLANCA.Affinity_vCPE_US_Platinum_1:
+            type: onap.policies.optimization.AffinityPolicy
+            version: 1.0.0
+            type_version: 1.0.0
+            metadata:
+                policy-id: OSDF_CASABLANCA.Affinity_vCPE_1
+                policy-version: 1
+            properties:
+                scope: [platinum]
+                services: [vCPE]
+                resources: [vGMuxInfra, vG]
+                geography: [US, INTERNATIONAL]
+                identity: affinity_vCPE
+                applicableResources: any
+                affinityProperties:
+                    qualifier: different
+                    category: availabilityZone
     -
         OSDF_CASABLANCA.Capacity_vG_1:
             type: onap.policies.optimization.Vim_fit
@@ -91,6 +109,24 @@ topology_template:
                 capacityProperty:
                    controller: multicloud
                    request: "{\"vCPU\": 10, \"Memory\": {\"quantity\": {\"get_param\": \"REQUIRED_MEM\"}, \"unit\": \"GB\"}, \"Storage\": {\"quantity\": {\"get_param\": \"REQUIRED_DISK\"}, \"unit\": \"GB\"}}"
+    -
+        OSDF_CASABLANCA.Capacity_vG_2:
+            type: onap.policies.optimization.Vim_fit
+            version: 1.0.0
+            type_version: 1.0.0
+            metadata:
+                policy-id: OSDF_CASABLANCA.Capacity_vG_2
+                policy-version: 1
+            properties:
+                scope: []
+                services: [vCPE]
+                resources: [vG]
+                geography: [US, INTERNATIONAL]
+                identity: capacity_vG
+                applicableResources: any
+                capacityProperty:
+                   controller: multicloud
+                   request: "{\"vCPU\": 15, \"Memory\": {\"quantity\": {\"get_param\": \"REQUIRED_MEM\"}, \"unit\": \"MB\"}, \"Storage\": {\"quantity\": {\"get_param\": \"REQUIRED_DISK\"}, \"unit\": \"GB\"}}"
     -
         OSDF_CASABLANCA.Distance_vG_1:
             type: onap.policies.optimization.DistancePolicy
@@ -112,6 +148,54 @@ topology_template:
                         value: 1500
                         operator: "<"
                         unit: km
+    -
+        OSDF_CASABLANCA.hpa_policy_Default:
+            type: onap.policies.optimization.HpaPolicy
+            version: 1.0.0
+            type_version: 1.0.0
+            metadata:
+                policy-id: OSDF_CASABLANCA.hpa_policy_Default
+                policy-version: 1
+            properties:
+                scope: []
+                services: []
+                resources: []
+                geography: []
+                identity: hpa-vG
+                flavorFeatures:
+                    -
+                         id: vg_1
+                         type: vnfc
+                         directives:
+                             -    type: flavor_directives
+                                  attributes:
+                                      -    attribute_name: flavor_label_vm_01
+                                           attribute_value: ""
+                         flavorProperties:
+                             -
+                                  hpa-feature: basicCapabilities
+                                  mandatory: True
+                                  architecture: generic
+                                  directives: []
+                                  hpa-feature-attributes:
+                                      -    hpa-attribute-key: numVirtualCpu
+                                           hpa-attribute-value: 8
+                                           operator: ['>=']
+                                           unit: ""
+                                      -    hpa-attribute-key: virtualMemSize
+                                           hpa-attribute-value: 6
+                                           operator: ['<=']
+                                           unit: ""
+                             -
+                                  hpa-feature: ovsDpdk
+                                  mandatory: False
+                                  architecture: generic
+                                  directives: []
+                                  hpa-feature-attributes:
+                                      -    hpa-attribute-key: dataProcessingAccelerationLibrary
+                                           hpa-attribute-value: ovsDpdk_version
+                                           operator: [=]
+                                           unit: ""
     -
         OSDF_CASABLANCA.hpa_policy_vG_1:
             type: onap.policies.optimization.HpaPolicy
@@ -122,7 +206,7 @@ topology_template:
                 policy-version: 1
             properties:
                 scope: []
-                services: [vCPE]
+                services: [vCPE, vOtherService]
                 resources: [vG]
                 geography: []
                 identity: hpa-vG
index 16bea1d..70f9ebc 100644 (file)
@@ -69,8 +69,7 @@ public class TestUtils {
         try {
             serviceTemplate = yamlCoder.decode(policyYaml, ToscaServiceTemplate.class);
         } catch (CoderException e) {
-            throw new XacmlApplicationException("Failed to decode policy from resource file "
-                + e.getLocalizedMessage());
+            throw new XacmlApplicationException("Failed to decode policy from resource file", e);
         }
         //
         // Make sure all the fields are setup properly