Fix Source/Target Lock

The locking of the target is now taken care of when
the operation manager is initialized. This allows us
to keep the template flow the same and to ensure that
for vFW we will not lock the source if it is not intended
to be the target. NOTE: This will have to be revised in
the future if policy chaining will support having
different targets specified for each policy in the chain.

Additional work was done for hardening the APPC model code
to make sure exceptions are caught and we gracefully end
processing the event with memory clean up and a final failure
notification sent.

Issue-Id: POLICY-367
Change-Id: Ic796d95eb5400067744492f810dd8069ba6241b3
Signed-off-by: Daniel Cruz <dc443y@att.com>
Signed-off-by: daniel <dc443y@att.com>

9 files changed, 76 insertions, 105 deletions

diff --git a/controlloop/common/actors/actor.appc/pom.xml b/controlloop/common/actors/actor.appc/pom.xml
index cc5f3b604..3036f52b4 100644
--- a/controlloop/common/actors/actor.appc/pom.xml
+++ b/controlloop/common/actors/actor.appc/pom.xml
@@ -18,18 +18,6 @@
 		</dependency>
 		<dependency>
 			<groupId>org.onap.policy.drools-applications</groupId>
-			<artifactId>actor.appclcm</artifactId>
-			<version>1.1.0-SNAPSHOT</version>
-			<scope>provided</scope>
-		</dependency>
-		<dependency>
-			<groupId>org.onap.policy.drools-applications</groupId>
-			<artifactId>aai</artifactId>
-			<version>1.1.0-SNAPSHOT</version>
-			<scope>provided</scope>
-		</dependency>
-		<dependency>
-			<groupId>org.onap.policy.drools-applications</groupId>
 			<artifactId>appc</artifactId>
 			<version>1.1.0-SNAPSHOT</version>
 			<scope>provided</scope>
diff --git a/controlloop/common/actors/actor.appc/src/main/java/org/onap/policy/controlloop/actor/appc/APPCActorServiceProvider.java b/controlloop/common/actors/actor.appc/src/main/java/org/onap/policy/controlloop/actor/appc/APPCActorServiceProvider.java
index 44411ca72..d525c4c85 100644
--- a/controlloop/common/actors/actor.appc/src/main/java/org/onap/policy/controlloop/actor/appc/APPCActorServiceProvider.java
+++ b/controlloop/common/actors/actor.appc/src/main/java/org/onap/policy/controlloop/actor/appc/APPCActorServiceProvider.java
@@ -24,9 +24,6 @@ import java.util.Collections;
 import java.util.List;
 
 import org.onap.policy.controlloop.VirtualControlLoopEvent;
-import org.onap.policy.controlloop.actor.appclcm.AppcLcmActorServiceProvider;
-import org.onap.policy.aai.AAIGETVnfResponse;
-import org.onap.policy.aai.util.AAIException;
 import org.onap.policy.appc.CommonHeader;
 import org.onap.policy.appc.Request;
 import org.onap.policy.controlloop.ControlLoopOperation;
@@ -91,7 +88,7 @@ public class APPCActorServiceProvider implements Actor {
 	 * @throws AAIException 
 	 */
 	public static Request constructRequest(VirtualControlLoopEvent onset, ControlLoopOperation operation,
-	                Policy policy, AAIGETVnfResponse vnfResponse) throws AAIException {
+	                Policy policy, String targetVnf) {
 		/*
 		 * Construct an APPC request
 		 */
@@ -101,24 +98,6 @@ public class APPCActorServiceProvider implements Actor {
 		request.CommonHeader.SubRequestID = operation.subRequestId;
 		request.Action = policy.getRecipe().substring(0, 1).toUpperCase() 
                         + policy.getRecipe().substring(1);
-		
-		/*
-		 * The target vnf-id may not be the same as the source vnf-id
-		 * specified in the yaml, the target vnf-id is retrieved by
-		 * a named query to A&AI.
-		 */
-		String sourceVnf = onset.AAI.get("generic-vnf.vnf-id");
-		if (sourceVnf == null) {
-		    /*
-		     * Lets see if the vnf-name is provided
-		     */
-		    sourceVnf = vnfResponse.vnfID;
-		    if (sourceVnf == null) {
-		        throw new AAIException("No vnf-id found");
-		    }
-		}
-		String targetVnf = AppcLcmActorServiceProvider.vnfNamedQuery(
-                    policy.getTarget().getResourceID(), sourceVnf);
 	
 		/*
 		 * For now Policy generates the PG Streams as a demo, in the
diff --git a/controlloop/common/actors/actor.appc/src/test/java/org/onap/policy/controlloop/actor/appc/AppcServiceProviderTest.java b/controlloop/common/actors/actor.appc/src/test/java/org/onap/policy/controlloop/actor/appc/AppcServiceProviderTest.java
index 5a107f814..31a041d4b 100644
--- a/controlloop/common/actors/actor.appc/src/test/java/org/onap/policy/controlloop/actor/appc/AppcServiceProviderTest.java
+++ b/controlloop/common/actors/actor.appc/src/test/java/org/onap/policy/controlloop/actor/appc/AppcServiceProviderTest.java
@@ -29,8 +29,6 @@ import java.util.UUID;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
-import org.onap.policy.aai.AAIGETVnfResponse;
-import org.onap.policy.aai.util.AAIException;
 import org.onap.policy.appc.Request;
 import org.onap.policy.appc.Response;
 import org.onap.policy.appc.ResponseCode;
@@ -55,7 +53,6 @@ public class AppcServiceProviderTest {
     private static VirtualControlLoopEvent onsetEvent;
     private static ControlLoopOperation operation;
     private static Policy policy;
-    private static AAIGETVnfResponse aaiResponse;
 
     static {
         /* 
@@ -94,10 +91,6 @@ public class AppcServiceProviderTest {
         policy.setRetry(2);
         policy.setTimeout(300);
         
-        /* Construct a mock A&AI response */
-        aaiResponse = new AAIGETVnfResponse();
-        aaiResponse.vnfID = "vnf01";
-        
         /* Set environment properties */
         PolicyEngine.manager.setEnvironmentProperty("aai.url", "http://localhost:6666");
         PolicyEngine.manager.setEnvironmentProperty("aai.username", "AAI");
@@ -123,12 +116,7 @@ public class AppcServiceProviderTest {
     public void constructModifyConfigRequestTest() {
         
         Request appcRequest = null;
-		try {
-			appcRequest = APPCActorServiceProvider.constructRequest(onsetEvent, operation, policy, aaiResponse);
-		} catch (AAIException e) {
-			logger.warn(e.toString());
-			fail("no vnfid found");
-		}
+        appcRequest = APPCActorServiceProvider.constructRequest(onsetEvent, operation, policy, "vnf01");
         
         /* The service provider must return a non null APPC request */
         assertNotNull(appcRequest);
diff --git a/controlloop/common/actors/actor.appclcm/src/main/java/org/onap/policy/controlloop/actor/appclcm/AppcLcmActorServiceProvider.java b/controlloop/common/actors/actor.appclcm/src/main/java/org/onap/policy/controlloop/actor/appclcm/AppcLcmActorServiceProvider.java
index 1c7ee9089..ed59aafc5 100644
--- a/controlloop/common/actors/actor.appclcm/src/main/java/org/onap/policy/controlloop/actor/appclcm/AppcLcmActorServiceProvider.java
+++ b/controlloop/common/actors/actor.appclcm/src/main/java/org/onap/policy/controlloop/actor/appclcm/AppcLcmActorServiceProvider.java
@@ -33,7 +33,6 @@ import java.util.UUID;
 
 import org.onap.policy.aai.AAINQInstanceFilters;
 import org.onap.policy.aai.AAINQInventoryResponseItem;
-import org.onap.policy.aai.AAIGETVnfResponse;
 import org.onap.policy.aai.AAIManager;
 import org.onap.policy.aai.AAINQNamedQuery;
 import org.onap.policy.aai.AAINQQueryParameters;
@@ -58,9 +57,6 @@ import org.slf4j.LoggerFactory;
 public class AppcLcmActorServiceProvider implements Actor {
     
     private static final Logger logger = LoggerFactory.getLogger(AppcLcmActorServiceProvider.class);
-    
-    /* The source vnf-id provided from the DCAE onset */
-    private static final String DCAE_VNF_ID = "generic-vnf.vnf-id";
 
     /* To be used in future releases to restart a single vm */
     private static final String APPC_VM_ID = "vm-id";
@@ -204,7 +200,7 @@ public class AppcLcmActorServiceProvider implements Actor {
      * @throws AAIException 
      */
     public static LCMRequestWrapper constructRequest(VirtualControlLoopEvent onset, 
-                ControlLoopOperation operation, Policy policy, AAIGETVnfResponse vnfResponse) throws AAIException {
+                ControlLoopOperation operation, Policy policy, String targetVnf) throws AAIException {
         
         /* Construct an APPC request using LCM Model */
         
@@ -235,14 +231,7 @@ public class AppcLcmActorServiceProvider implements Actor {
          * a vnf-id.
          */
         HashMap<String, String> requestActionIdentifiers = new HashMap<>();
-        String vnfId = onset.AAI.get(DCAE_VNF_ID);
-        if (vnfId == null) {
-            vnfId = vnfResponse.vnfID;
-            if (vnfId == null) {
-                throw new AAIException("No vnf-id found");
-            }
-        }
-        requestActionIdentifiers.put("vnf-id", vnfId);
+        requestActionIdentifiers.put("vnf-id", targetVnf);
         
         appcRequest.setActionIdentifiers(requestActionIdentifiers);
         
diff --git a/controlloop/common/actors/actor.appclcm/src/test/java/org/onap/policy/controlloop/actor/appclcm/AppcLcmServiceProviderTest.java b/controlloop/common/actors/actor.appclcm/src/test/java/org/onap/policy/controlloop/actor/appclcm/AppcLcmServiceProviderTest.java
index 7c0f1882a..d446968ef 100644
--- a/controlloop/common/actors/actor.appclcm/src/test/java/org/onap/policy/controlloop/actor/appclcm/AppcLcmServiceProviderTest.java
+++ b/controlloop/common/actors/actor.appclcm/src/test/java/org/onap/policy/controlloop/actor/appclcm/AppcLcmServiceProviderTest.java
@@ -30,7 +30,6 @@ import java.util.UUID;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
-import org.onap.policy.aai.AAIGETVnfResponse;
 import org.onap.policy.aai.util.AAIException;
 import org.onap.policy.appclcm.LCMCommonHeader;
 import org.onap.policy.appclcm.LCMRequest;
@@ -58,7 +57,6 @@ public class AppcLcmServiceProviderTest {
     private static VirtualControlLoopEvent onsetEvent;
     private static ControlLoopOperation operation;
     private static Policy policy;
-    private static AAIGETVnfResponse aaiResponse;
     private static LCMRequestWrapper dmaapRequest;
     private static LCMResponseWrapper dmaapResponse;
 
@@ -97,10 +95,6 @@ public class AppcLcmServiceProviderTest {
         policy.setPayload(null);
         policy.setRetry(2);
         policy.setTimeout(300);
-        
-        /* Construct a mock A&AI response */
-        aaiResponse = new AAIGETVnfResponse();
-        aaiResponse.vnfID = "vnf01";
 
         /* A sample DMAAP request wrapper. */
         dmaapRequest = new LCMRequestWrapper();
@@ -171,7 +165,7 @@ public class AppcLcmServiceProviderTest {
         
         LCMRequestWrapper dmaapRequest = null;
         try {
-            dmaapRequest = AppcLcmActorServiceProvider.constructRequest(onsetEvent, operation, policy, aaiResponse);
+            dmaapRequest = AppcLcmActorServiceProvider.constructRequest(onsetEvent, operation, policy, "vnf01");
         } catch (AAIException e) {
             logger.warn(e.toString());
             fail("no vnfid found");
@@ -315,12 +309,12 @@ public class AppcLcmServiceProviderTest {
         String resourceId = "82194af1-3c2c-485a-8f44-420e22a9eaa4";
         String targetVnfId = null;
         try {
-            targetVnfId = AppcLcmActorServiceProvider.vnfNamedQuery(resourceId, aaiResponse.vnfID);
+            targetVnfId = AppcLcmActorServiceProvider.vnfNamedQuery(resourceId, "vnf01");
         } catch (AAIException e) {
             logger.warn(e.toString());
             fail("no vnf-id found");
         }
         assertNotNull(targetVnfId);
-        assertEquals(targetVnfId, aaiResponse.vnfID);
+        assertEquals(targetVnfId, "vnf01");
     }
 }
diff --git a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager.java b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager.java
index 91db147c8..2a5a3d0a9 100644
--- a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager.java
+++ b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManager.java
@@ -31,6 +31,7 @@ import java.util.UUID;
 import org.onap.policy.aai.AAIGETVnfResponse;
 import org.onap.policy.aai.AAIGETVserverResponse;
 import org.onap.policy.aai.AAIManager;
+import org.onap.policy.aai.util.AAIException;
 import org.onap.policy.controlloop.ControlLoopEventStatus;
 import org.onap.policy.controlloop.ControlLoopException;
 import org.onap.policy.controlloop.ControlLoopNotificationType;
@@ -289,7 +290,7 @@ public class ControlLoopEventManager implements LockCallback, Serializable {
 		return notification;
 	}
 		
-	public ControlLoopOperationManager	processControlLoop() throws ControlLoopException {
+	public ControlLoopOperationManager	processControlLoop() throws ControlLoopException, AAIException {
 		//
 		// Check if they activated us
 		//
@@ -397,7 +398,7 @@ public class ControlLoopEventManager implements LockCallback, Serializable {
 			//
 			LockResult<GuardResult, TargetLock> lockResult = PolicyGuard.lockTarget(
 																		this.currentOperation.policy.getTarget().getType(), 
-																		this.getTargetInstance(this.currentOperation.policy),
+																		this.currentOperation.getTargetEntity(),
 																		this.onset.requestID,
 																		this);
 			//
@@ -694,32 +695,6 @@ public class ControlLoopEventManager implements LockCallback, Serializable {
 		return false;
 	}
 
-	public String getTargetInstance(Policy policy) {
-		if (policy.getTarget() != null) {
-			if (policy.getTarget().getType() != null) {
-				switch(policy.getTarget().getType()) {
-				case PNF:
-					break;
-				case VM:
-				case VNF:
-					if (this.onset.target.equalsIgnoreCase("vserver.vserver-name")) {
-						return this.onset.AAI.get("vserver.vserver-name");
-					}
-					else if (this.onset.target.equalsIgnoreCase("generic-vnf.vnf-id")) {
-					    return this.onset.AAI.get("generic-vnf.vnf-id");
-					}
-					else if (this.onset.target.equalsIgnoreCase("generic-vnf.vnf-name")) {
-					    return this.onset.AAI.get("generic-vnf.vnf-name");
-					}
-					break;
-				default:
-					break;
-				}
-			}
-		}
-		return null;
-	}
-
 	@Override
 	public String toString() {
 		return "ControlLoopEventManager [closedLoopControlName=" + closedLoopControlName + ", requestID=" + requestID
diff --git a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManager.java b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManager.java
index eeb724ad2..1c46b9991 100644
--- a/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManager.java
+++ b/controlloop/common/eventmanager/src/main/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManager.java
@@ -84,6 +84,7 @@ public class ControlLoopOperationManager implements Serializable {
 	private LinkedList<Operation> operationHistory = new LinkedList<Operation>();
 	private PolicyResult policyResult = null;
 	private ControlLoopEventManager eventManager = null;
+	private String targetEntity;
 
 	public ControlLoopEventManager getEventManager() {
 		return eventManager;
@@ -93,6 +94,9 @@ public class ControlLoopOperationManager implements Serializable {
 		this.eventManager = eventManager;
 	}
 
+	public String getTargetEntity() {
+	    return this.targetEntity;
+	}
 
 	//
 	// Internal class used for tracking
@@ -123,21 +127,72 @@ public class ControlLoopOperationManager implements Serializable {
 		this.guardApprovalStatus = guardApprovalStatus;
 	}
 
-
-	public ControlLoopOperationManager(ControlLoopEvent onset, Policy policy, ControlLoopEventManager em) throws ControlLoopException {
+	public String getTarget(Policy policy) throws ControlLoopException, AAIException {
+        if (policy.getTarget() != null) {
+            if (policy.getTarget().getType() != null) {
+                switch(policy.getTarget().getType()) {
+                case PNF:
+                    break;
+                case VM:
+                case VNF:
+                    VirtualControlLoopEvent virtualOnset = (VirtualControlLoopEvent) this.onset;
+                    if (this.onset.target.equalsIgnoreCase("vserver.vserver-name")) {
+                        return virtualOnset.AAI.get("vserver.vserver-name");
+                    }
+                    else if (this.onset.target.equalsIgnoreCase("generic-vnf.vnf-id")) {
+                        return virtualOnset.AAI.get("generic-vnf.vnf-id");
+                    }
+                    else if (this.onset.target.equalsIgnoreCase("generic-vnf.vnf-name")) {
+                        /*
+                         * If the vnf-name was retrieved from the onset then the vnf-id
+                         * must be obtained from the event manager's A&AI GET query
+                         */
+                        String vnfId = this.eventManager.getVnfResponse().vnfID;
+                        if (vnfId == null) {
+                            throw new AAIException("No vnf-id found");
+                        }
+                        return vnfId;
+                    }
+                    break;
+                default:
+                    throw new ControlLoopException("The target type is not supported");
+                }
+            }
+            else {
+                throw new ControlLoopException("The target type is null");
+            }
+        }
+        else {
+            throw new ControlLoopException("The target is null");
+        }
+        return null;
+    }
+	
+	public ControlLoopOperationManager(ControlLoopEvent onset, Policy policy, ControlLoopEventManager em) throws ControlLoopException, AAIException {
 		this.onset = onset;
 		this.policy = policy;
 		this.guardApprovalStatus = "NONE";
 		this.eventManager = em;
-
+		this.targetEntity = getTarget(policy);
+		
 		//
 		// Let's make a sanity check
 		//
 		switch (policy.getActor()) {
 		case "APPC":
+		    if ("ModifyConfig".equalsIgnoreCase(policy.getRecipe())) {
+		        /*
+                 * The target vnf-id may not be the same as the source vnf-id
+                 * specified in the yaml, the target vnf-id is retrieved by
+                 * a named query to A&AI.
+                 */
+		        String targetVnf = AppcLcmActorServiceProvider.vnfNamedQuery(
+		                    policy.getTarget().getResourceID(), this.targetEntity);
+		        this.targetEntity = targetVnf;
+		    }
 			break;
 		case "SO":
-			break;
+		    break;
 		case "VFC":
 			break;
 		default:
@@ -210,11 +265,11 @@ public class ControlLoopOperationManager implements Serializable {
 		    if ("ModifyConfig".equalsIgnoreCase(policy.getRecipe())) {
 
 	            this.operationRequest = APPCActorServiceProvider.constructRequest((VirtualControlLoopEvent) onset, 
-	                                    operation.operation, this.policy, eventManager.getVnfResponse());
+	                                    operation.operation, this.policy, this.targetEntity);
 		    }
 		    else {
 		        this.operationRequest = AppcLcmActorServiceProvider.constructRequest((VirtualControlLoopEvent) onset, 
-		                                operation.operation, this.policy, eventManager.getVnfResponse());
+		                                operation.operation, this.policy, this.targetEntity);
 		    }
 			//
 			// Save the operation
@@ -593,7 +648,7 @@ public class ControlLoopOperationManager implements Serializable {
 		newEntry.requestId = this.onset.requestID.toString();
 		newEntry.actor = this.currentOperation.operation.actor;
 		newEntry.operation = this.currentOperation.operation.operation;
-		newEntry.target = this.eventManager.getTargetInstance(this.policy);
+		newEntry.target = this.targetEntity;
 		newEntry.starttime = Timestamp.from(this.currentOperation.operation.start);
 		newEntry.subrequestId = this.currentOperation.operation.subRequestId;
 		newEntry.endtime = new Timestamp(this.currentOperation.operation.end.toEpochMilli());
diff --git a/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManagerTest.java b/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManagerTest.java
index caa17ff38..d41d8bf57 100644
--- a/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManagerTest.java
+++ b/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopOperationManagerTest.java
@@ -40,6 +40,7 @@ import org.onap.policy.appclcm.LCMResponseWrapper;
 import org.onap.policy.controlloop.ControlLoopEventStatus;
 import org.onap.policy.controlloop.VirtualControlLoopEvent;
 import org.onap.policy.controlloop.ControlLoopException;
+import org.onap.policy.controlloop.ControlLoopTargetType;
 import org.onap.policy.controlloop.Util;
 import org.onap.policy.controlloop.policy.ControlLoopPolicy;
 import org.onap.policy.controlloop.policy.PolicyResult;
@@ -56,6 +57,7 @@ public class ControlLoopOperationManagerTest {
 		onset = new VirtualControlLoopEvent();
 		onset.requestID = UUID.randomUUID();
 		onset.target = "generic-vnf.vnf-name";
+		onset.target_type = ControlLoopTargetType.VNF;
 		onset.closedLoopAlarmStart = Instant.now();
 		onset.AAI = new HashMap<>();
 		onset.AAI.put("generic-vnf.vnf-name", "testTriggerSource");
diff --git a/controlloop/common/eventmanager/src/test/resources/test.yaml b/controlloop/common/eventmanager/src/test/resources/test.yaml
index e15c8f0e1..7a9435ecc 100644
--- a/controlloop/common/eventmanager/src/test/resources/test.yaml
+++ b/controlloop/common/eventmanager/src/test/resources/test.yaml
@@ -23,7 +23,8 @@ policies:
     description:
     actor: APPC
     recipe: Restart
-    target: VM
+    target:
+      type: VNF
     retry: 1
     timeout: 20
     success: final_success