From: Jim Hahn Date: Mon, 16 Jul 2018 21:04:55 +0000 (-0400) Subject: Use refresh() instead of lock() in guard X-Git-Tag: 1.3.0~49 X-Git-Url: https://gerrit.onap.org/r/gitweb?a=commitdiff_plain;h=f892e8b13ea87c1d7fb1c890f64ff00fc6e2de5f;p=policy%2Fdrools-applications.git Use refresh() instead of lock() in guard This is the second step of separating the lock "refresh" operation from the original "lock" operation. Modified PolicyGuard to use refresh() to extend a lock. Also removed the host name from the owner String, as it will no be longer needed (once the final step is done). Change-Id: I3a498e6f81d9dba1bbdfd6f7388087355192bcf5 Issue-ID: POLICY-872 Signed-off-by: Jim Hahn --- 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 73baf205d..e9ff01072 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 @@ -23,7 +23,6 @@ package org.onap.policy.guard; import java.util.UUID; import org.onap.policy.controlloop.policy.TargetType; import org.onap.policy.drools.core.lock.PolicyResourceLockManager; -import org.onap.policy.drools.utils.NetworkUtil; import org.onap.policy.guard.impl.PNFTargetLock; import org.onap.policy.guard.impl.VMTargetLock; import org.onap.policy.guard.impl.VNFTargetLock; @@ -94,10 +93,10 @@ public class PolicyGuard { UUID requestID, LockCallback callback, int holdSec) { String owner = makeOwner(targetType, requestID); - - GuardResult guardResult = managerLockTarget(targetInstance, owner, holdSec); - if(guardResult != GuardResult.LOCK_ACQUIRED) { - return LockResult.createLockResult(guardResult, null); + + boolean result = factory.getManager().lock(targetInstance, owner, holdSec); + if(!result) { + return LockResult.createLockResult(GuardResult.LOCK_DENIED, null); } TargetLock lock = null; @@ -142,21 +141,10 @@ public class PolicyGuard { */ public static GuardResult lockTarget(TargetLock lock, int holdSec) { String owner = makeOwner(lock.getTargetType(), lock.getRequestID()); - GuardResult result = managerLockTarget(lock.getTargetInstance(), owner, holdSec); + + boolean result = factory.getManager().refresh(lock.getTargetInstance(), owner, holdSec); logger.debug("Lock {} extend {}", lock, result); - return result; - } - - /** - * Asks the manager to lock the given target. - * @param targetInstance - * @param owner - * @param holdSec maximum number of seconds to hold the lock - * @return the result: acquired or denied - */ - 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); } @@ -210,7 +198,7 @@ public class PolicyGuard { throw new IllegalArgumentException("null requestID for lock type " + targetType); } - return factory.getHostname() + ":" + targetType + ":" + requestID; + return targetType + ":" + requestID; } /** @@ -224,12 +212,5 @@ public class PolicyGuard { public PolicyResourceLockManager getManager() { return PolicyResourceLockManager.getInstance(); } - - /** - * @return the current host name - */ - public String getHostname() { - return NetworkUtil.getHostname(); - } } } 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 e24706989..dd1ede49c 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 @@ -43,7 +43,6 @@ import org.onap.policy.guard.impl.VMTargetLock; import org.onap.policy.guard.impl.VNFTargetLock; public class PolicyGuardTest { - private static final String HOSTNAME = "my.host"; private static final String INSTANCENAME = "targetInstance"; private static final int LOCK_SEC = 10; @@ -117,7 +116,6 @@ public class PolicyGuardTest { factory = mock(Factory.class); when(factory.getManager()).thenReturn(mgr); - when(factory.getHostname()).thenReturn(HOSTNAME); uuid = UUID.randomUUID(); dlcb = new DummyLockCallback(); @@ -277,42 +275,48 @@ public class PolicyGuardTest { } @Test - public void testManagerLockTarget() throws Exception { + public void testLockTargetTargetTypeStringUUIDLockCallbackInt() throws Exception { TargetType type = TargetType.VM; LockResult result; // acquired result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC); - verify(mgr).lock(INSTANCENAME, HOSTNAME+":"+type+":"+uuid, LOCK_SEC); + verify(mgr).lock(INSTANCENAME, type+":"+uuid, LOCK_SEC); assertEquals(GuardResult.LOCK_ACQUIRED, result.getA()); assertEquals(VMTargetLock.class, result.getB().getClass()); - // diff host name - denied - when(factory.getHostname()).thenReturn(HOSTNAME+"x"); - result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC+10); - verify(mgr).lock(INSTANCENAME, HOSTNAME+"x:"+type+":"+uuid, LOCK_SEC+10); + // diff owner - denied + UUID uuid2 = UUID.randomUUID(); + result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid2, dlcb, LOCK_SEC+10); + verify(mgr).lock(INSTANCENAME, type+":"+uuid2, LOCK_SEC+10); assertEquals(GuardResult.LOCK_DENIED, result.getA()); assertNull(result.getB()); } @Test - public void testManagerLockTargetTargetLockInt() throws Exception { + public void testLockTargetTargetLockInt() throws Exception { TargetType type = TargetType.VM; - DummyTargetLock lock = new DummyTargetLock(type, uuid); + + LockResult result; // acquired - assertEquals(GuardResult.LOCK_ACQUIRED, PolicyGuard.lockTarget(lock, LOCK_SEC)); - verify(mgr).lock(INSTANCENAME, HOSTNAME+":"+type+":"+uuid, LOCK_SEC); + result = PolicyGuard.lockTarget(type, INSTANCENAME, uuid, dlcb, LOCK_SEC); + verify(mgr).lock(INSTANCENAME, type+":"+uuid, LOCK_SEC); + assertEquals(GuardResult.LOCK_ACQUIRED, result.getA()); + assertEquals(VMTargetLock.class, result.getB().getClass()); - // same host name - re-acquired + TargetLock lock = result.getB(); + + // refresh - re-acquired assertEquals(GuardResult.LOCK_ACQUIRED, PolicyGuard.lockTarget(lock, LOCK_SEC+1)); - verify(mgr).lock(INSTANCENAME, HOSTNAME+":"+type+":"+uuid, LOCK_SEC+1); + verify(mgr).refresh(INSTANCENAME, type+":"+uuid, LOCK_SEC+1); + + // unlock + PolicyGuard.unlockTarget(lock); - // diff host name - denied - when(factory.getHostname()).thenReturn(HOSTNAME+"_"); + // refresh - denied, as we no longer own the lock assertEquals(GuardResult.LOCK_DENIED, PolicyGuard.lockTarget(lock, LOCK_SEC+2)); - verify(mgr).lock(INSTANCENAME, HOSTNAME+"_:"+type+":"+uuid, LOCK_SEC+2); } @Test(expected = IllegalArgumentException.class)