diff options
5 files changed, 75 insertions, 70 deletions
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 bc9d3df79..5f36bcc20 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 @@ -60,6 +60,12 @@ public class ControlLoopEventManager implements LockCallback, Serializable { private static final String GENERIC_VNF_IS_CLOSED_LOOP_DISABLED = "generic-vnf.is-closed-loop-disabled"; private static final String VSERVER_IS_CLOSED_LOOP_DISABLED = "vserver.is-closed-loop-disabled"; + /** + * Additional time, in seconds, to add to a "lock" request. This ensures that the lock + * won't expire right before an operation completes. + */ + private static final int ADDITIONAL_LOCK_SEC = 60; + private static final Logger logger = LoggerFactory.getLogger(ControlLoopEventManager.class); private static final long serialVersionUID = -1216568161322872641L; @@ -447,14 +453,17 @@ public class ControlLoopEventManager implements LockCallback, Serializable { // TODO: Make sure the current lock is for the same target. // Currently, it should be. But in the future it may not. // - return new LockResult<>(GuardResult.LOCK_ACQUIRED, this.targetLock); + GuardResult result = PolicyGuard.lockTarget(targetLock, + this.currentOperation.getOperationTimeout() + ADDITIONAL_LOCK_SEC); + return new LockResult<>(result, this.targetLock); } else { // // Ask the Guard // LockResult<GuardResult, TargetLock> lockResult = PolicyGuard.lockTarget(this.currentOperation.policy.getTarget().getType(), - this.currentOperation.getTargetEntity(), this.onset.getRequestId(), this); + this.currentOperation.getTargetEntity(), this.onset.getRequestId(), this, + this.currentOperation.getOperationTimeout() + ADDITIONAL_LOCK_SEC); // // Was it acquired? // diff --git a/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManagerTest.java b/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManagerTest.java index 182791d22..1af4a312d 100644 --- a/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManagerTest.java +++ b/controlloop/common/eventmanager/src/test/java/org/onap/policy/controlloop/eventmanager/ControlLoopEventManagerTest.java @@ -755,9 +755,11 @@ public class ControlLoopEventManagerTest { LockResult<GuardResult, TargetLock> lockLock = manager.lockCurrentOperation(); assertNotNull(lockLock); + assertEquals(GuardResult.LOCK_ACQUIRED, lockLock.getA()); LockResult<GuardResult, TargetLock> lockLockAgain = manager.lockCurrentOperation(); assertNotNull(lockLockAgain); + assertEquals(GuardResult.LOCK_ACQUIRED, lockLockAgain.getA()); assertEquals(lockLock.getB(), lockLockAgain.getB()); assertEquals(lockLock.getB(), manager.unlockCurrentOperation()); diff --git a/controlloop/common/guard/src/main/java/org/onap/policy/guard/PolicyGuard.java b/controlloop/common/guard/src/main/java/org/onap/policy/guard/PolicyGuard.java index 66a25ea63..64d8a2f74 100644 --- a/controlloop/common/guard/src/main/java/org/onap/policy/guard/PolicyGuard.java +++ b/controlloop/common/guard/src/main/java/org/onap/policy/guard/PolicyGuard.java @@ -2,7 +2,7 @@ * ============LICENSE_START======================================================= * guard * ================================================================================ - * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. + * Copyright (C) 2017-2018 AT&T Intellectual Property. All rights reserved. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,8 +21,6 @@ package org.onap.policy.guard; import java.util.UUID; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; import org.onap.policy.controlloop.policy.TargetType; import org.onap.policy.drools.core.lock.PolicyResourceLockManager; import org.onap.policy.guard.impl.PNFTargetLock; @@ -87,15 +85,16 @@ public class PolicyGuard { * @param targetInstance the target instance * @param requestID the request Id * @param callback the LockCallback + * @param holdSec maximum number of seconds to hold the lock * @return the LockResult * @throws IllegalArgumentException if an argument is null */ public static LockResult<GuardResult, TargetLock> lockTarget(TargetType targetType, String targetInstance, - UUID requestID, LockCallback callback) { + UUID requestID, LockCallback callback, int holdSec) { String owner = makeOwner(targetType, requestID); - GuardResult guardResult = managerLockTarget(targetInstance, owner); + GuardResult guardResult = managerLockTarget(targetInstance, owner, holdSec); if(guardResult != GuardResult.LOCK_ACQUIRED) { return LockResult.createLockResult(guardResult, null); } @@ -133,31 +132,31 @@ public class PolicyGuard { logger.debug("Locked {}", lock); return LockResult.createLockResult(GuardResult.LOCK_ACQUIRED, lock); } + + /** + * Extends a lock on a target. + * @param lock current lock + * @param holdSec maximum number of seconds to hold the lock + * @return the result: acquired or denied + */ + public static GuardResult lockTarget(TargetLock lock, int holdSec) { + String owner = makeOwner(lock.getTargetType(), lock.getRequestID()); + GuardResult result = managerLockTarget(lock.getTargetInstance(), owner, holdSec); + + logger.debug("Lock {} extend {}", lock, result); + return result; + } /** * Asks the manager to lock the given target. * @param targetInstance * @param owner - * @return the result: acquired, denied, or exception + * @param holdSec maximum number of seconds to hold the lock + * @return the result: acquired or denied */ - private static GuardResult managerLockTarget(String targetInstance, String owner) { - try { - Future<Boolean> result = factory.getManager().lock(targetInstance, owner, null); - return(result.get() ? GuardResult.LOCK_ACQUIRED : GuardResult.LOCK_DENIED); - - } catch(IllegalStateException e) { - logger.warn("{} attempted to re-lock {}", owner, targetInstance); - return GuardResult.LOCK_DENIED; - - } catch (InterruptedException e) { - logger.error("exception getting lock for {}", targetInstance, e); - Thread.currentThread().interrupt(); - return GuardResult.LOCK_EXCEPTION; - - } catch (ExecutionException e) { - logger.error("exception getting lock for {}", targetInstance, e); - return GuardResult.LOCK_EXCEPTION; - } + private static GuardResult managerLockTarget(String targetInstance, String owner, int holdSec) { + boolean result = factory.getManager().lock(targetInstance, owner, holdSec); + return(result ? GuardResult.LOCK_ACQUIRED : GuardResult.LOCK_DENIED); } /** diff --git a/controlloop/common/guard/src/test/java/org/onap/policy/guard/PolicyGuardTest.java b/controlloop/common/guard/src/test/java/org/onap/policy/guard/PolicyGuardTest.java index b073fe5a9..9cd34b70a 100644 --- a/controlloop/common/guard/src/test/java/org/onap/policy/guard/PolicyGuardTest.java +++ b/controlloop/common/guard/src/test/java/org/onap/policy/guard/PolicyGuardTest.java @@ -25,13 +25,12 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doThrow; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.UUID; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -46,6 +45,7 @@ import org.onap.policy.guard.impl.VNFTargetLock; public class PolicyGuardTest { private static final String INSTANCENAME = "targetInstance"; + private static final int LOCK_SEC = 10; private static Factory saveFactory; @@ -131,7 +131,7 @@ public class PolicyGuardTest { // Test isLocked before and after lock added assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); - LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); + LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC); assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); assertEquals(GuardResult.LOCK_ACQUIRED, result.getA()); @@ -155,7 +155,7 @@ public class PolicyGuardTest { // Test isLocked before and after lock added assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); - LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); + LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC); assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); assertEquals(GuardResult.LOCK_ACQUIRED, result.getA()); @@ -180,7 +180,7 @@ public class PolicyGuardTest { // Test isLocked before and after lock added assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); - LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); + LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC); assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); assertEquals(GuardResult.LOCK_ACQUIRED, result.getA()); @@ -204,7 +204,7 @@ public class PolicyGuardTest { // Test isLocked before and after lock added assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); - LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); + LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC); assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); assertEquals(GuardResult.LOCK_EXCEPTION, result.getA()); @@ -221,18 +221,12 @@ public class PolicyGuardTest { // Test isLocked before and after lock added assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); - LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); + LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC); assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); assertEquals(GuardResult.LOCK_ACQUIRED, result.getA()); assertEquals(VMTargetLock.class, result.getB().getClass()); - result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); - assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); - - assertEquals(GuardResult.LOCK_DENIED, result.getA()); - assertNull(result.getB()); - // Test isLocked after lock removed PolicyGuard.unlockTarget(new DummyTargetLock(type, uuid)); assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); @@ -248,13 +242,16 @@ public class PolicyGuardTest { // Test isLocked before and after lock added assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); - LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); + LockResult<GuardResult, TargetLock> result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC); assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); assertEquals(GuardResult.LOCK_ACQUIRED, result.getA()); assertEquals(VMTargetLock.class, result.getB().getClass()); - result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); + UUID uuid2 = UUID.randomUUID(); + result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid2, dlcb, LOCK_SEC); + assertFalse(PolicyGuard.isLocked(type, INSTANCENAME, uuid2)); + assertTrue(PolicyGuard.isLocked(type, INSTANCENAME, uuid)); assertEquals(GuardResult.LOCK_DENIED, result.getA()); @@ -283,47 +280,41 @@ public class PolicyGuardTest { public void testManagerLockTarget() throws Exception { TargetType type = TargetType.VM; - @SuppressWarnings("unchecked") - Future<Boolean> fut = mock(Future.class); - mgr = mock(PolicyResourceLockManager.class); - when(mgr.lock(any(), any(), any())).thenReturn(fut); LockResult<GuardResult, TargetLock> result; // acquired - when(fut.get()).thenReturn(true); - result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); + when(mgr.lock(anyString(), anyString(), anyInt())).thenReturn(true); + result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC); + verify(mgr).lock(INSTANCENAME, type.toString()+":"+uuid.toString(), LOCK_SEC); assertEquals(GuardResult.LOCK_ACQUIRED, result.getA()); assertEquals(VMTargetLock.class, result.getB().getClass()); // denied - when(fut.get()).thenReturn(false); - result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); + when(mgr.lock(anyString(), anyString(), anyInt())).thenReturn(false); + result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC+2); + verify(mgr).lock(INSTANCENAME, type.toString()+":"+uuid.toString(), LOCK_SEC+2); assertEquals(GuardResult.LOCK_DENIED, result.getA()); assertNull(result.getB()); + } - // illegal state exception - doThrow(new IllegalStateException("state expected")).when(fut).get(); - result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); - assertEquals(GuardResult.LOCK_DENIED, result.getA()); - assertNull(result.getB()); - assertFalse(Thread.currentThread().isInterrupted()); + @Test + public void testManagerLockTargetTargetLockInt() throws Exception { + TargetType type = TargetType.VM; + DummyTargetLock lock = new DummyTargetLock(type, uuid); + + mgr = mock(PolicyResourceLockManager.class); - // interrupted exception - doThrow(new InterruptedException("interrupt expected")).when(fut).get(); - result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); - assertEquals(GuardResult.LOCK_EXCEPTION, result.getA()); - assertNull(result.getB()); - // verify interrupted & reset interrupt status - assertTrue(Thread.interrupted()); + // acquired + when(mgr.lock(anyString(), anyString(), anyInt())).thenReturn(true); + assertEquals(GuardResult.LOCK_ACQUIRED, PolicyGuard.lockTarget(lock, LOCK_SEC)); + verify(mgr).lock(INSTANCENAME, type.toString()+":"+uuid.toString(), LOCK_SEC); - // exec exception - doThrow(new ExecutionException(new IllegalArgumentException("exec expected"))).when(fut).get(); - result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb); - assertEquals(GuardResult.LOCK_EXCEPTION, result.getA()); - assertNull(result.getB()); - assertFalse(Thread.currentThread().isInterrupted()); + // denied + when(mgr.lock(anyString(), anyString(), anyInt())).thenReturn(false); + assertEquals(GuardResult.LOCK_DENIED, PolicyGuard.lockTarget(lock, LOCK_SEC+1)); + verify(mgr).lock(INSTANCENAME, type.toString()+":"+uuid.toString(), LOCK_SEC+1); } @Test(expected = IllegalArgumentException.class) diff --git a/controlloop/templates/archetype-cl-amsterdam/src/main/resources/archetype-resources/src/main/resources/__closedLoopControlName__.drl b/controlloop/templates/archetype-cl-amsterdam/src/main/resources/archetype-resources/src/main/resources/__closedLoopControlName__.drl index b1773f556..1274d6c16 100644 --- a/controlloop/templates/archetype-cl-amsterdam/src/main/resources/archetype-resources/src/main/resources/__closedLoopControlName__.drl +++ b/controlloop/templates/archetype-cl-amsterdam/src/main/resources/archetype-resources/src/main/resources/__closedLoopControlName__.drl @@ -449,6 +449,10 @@ rule "${policyName}.EVENT.MANAGER" retract($event); retract($manager); retract($clTimer); + + if(result.getB() != null) { + retract(result.getB()); + } } logger.info("{}: {}: starting operation={}", $params.getClosedLoopControlName(), drools.getRule().getName(), |