aboutsummaryrefslogtreecommitdiffstats
path: root/main/src
diff options
context:
space:
mode:
authorJim Hahn <jrh3@att.com>2019-11-07 16:37:18 -0500
committerJim Hahn <jrh3@att.com>2019-11-07 16:37:18 -0500
commitfc556cec79885034897d8647b957125c6fc95b5e (patch)
tree2449f43613257780e2012d23a024abd139d1b4b6 /main/src
parenta3bf3134c01d979cebc94f5b2c915cfa400a9a72 (diff)
PAP should not deactivate PDPs
Modified the code so that it does not send a PASSIVE request to PDPs when they are fail, whether due to an inability to deploy a policy or due to a timeout. However, it still removes the PDP from the PDP Group, if a timeout occurs (but not due to a deployment failure). This review does not include any changes to undeploy failed policies; that will come in a later review. Issue-ID: POLICY-2155 Signed-off-by: Jim Hahn <jrh3@att.com> Change-Id: I15821d299bc3261478fd7fbb9ee62ea4a90123a4
Diffstat (limited to 'main/src')
-rw-r--r--main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java77
-rw-r--r--main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java130
2 files changed, 105 insertions, 102 deletions
diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java b/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
index 6f763204..49ded962 100644
--- a/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
@@ -21,7 +21,6 @@
package org.onap.policy.pap.main.comm;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -326,27 +325,36 @@ public class PdpModifyRequestMap {
private class SingletonListener implements RequestListener {
private final PdpRequests requests;
private final Request request;
+ private final String pdpName;
public SingletonListener(PdpRequests requests, Request request) {
this.requests = requests;
this.request = request;
+ this.pdpName = requests.getPdpName();
}
@Override
- public void failure(String pdpName, String reason) {
- if (requests.getPdpName().equals(pdpName)) {
- disablePdp(requests);
- }
+ public void failure(String responsePdpName, String reason) {
+ requestCompleted(responsePdpName);
}
@Override
- public void success(String pdpName) {
- if (requests.getPdpName().equals(pdpName)) {
- if (pdp2requests.get(requests.getPdpName()) == requests) {
- startNextRequest(requests, request);
+ public void success(String responsePdpName) {
+ requestCompleted(responsePdpName);
+ }
+
+ /**
+ * Handles a request completion, starting the next request, if there is one.
+ *
+ * @param responsePdpName name of the PDP provided in the response
+ */
+ private void requestCompleted(String responsePdpName) {
+ if (pdpName.equals(responsePdpName)) {
+ if (pdp2requests.get(pdpName) == requests) {
+ startNextRequest(request);
} else {
- logger.info("discard old requests for {}", pdpName);
+ logger.info("discard old requests for {}", responsePdpName);
requests.stopPublishing();
}
}
@@ -354,65 +362,42 @@ public class PdpModifyRequestMap {
@Override
public void retryCountExhausted() {
- disablePdp(requests);
+ removePdp();
}
/**
* Starts the next request associated with a PDP.
*
- * @param requests current set of requests
* @param request the request that just completed
*/
- private void startNextRequest(PdpRequests requests, Request request) {
+ private void startNextRequest(Request request) {
if (!requests.startNextRequest(request)) {
- pdp2requests.remove(requests.getPdpName(), requests);
+ pdp2requests.remove(pdpName, requests);
}
}
/**
- * Disables a PDP by removing it from its subgroup and then sending it a PASSIVE
- * request.
- *
- * @param requests the requests associated with the PDP to be disabled
+ * Removes a PDP from its subgroup.
*/
- private void disablePdp(PdpRequests requests) {
-
- policyNotifier.removePdp(requests.getPdpName());
+ private void removePdp() {
+ requests.stopPublishing();
// remove the requests from the map
- if (!pdp2requests.remove(requests.getPdpName(), requests)) {
- // don't have the info we need to disable it
- logger.warn("no requests with which to disable {}", requests.getPdpName());
+ if (!pdp2requests.remove(pdpName, requests)) {
+ // wasn't in the map - the requests must be old
+ logger.warn("discarding old requests for {}", pdpName);
return;
}
- logger.warn("disabling {}", requests.getPdpName());
+ logger.warn("removing {}", pdpName);
- requests.stopPublishing();
+ policyNotifier.removePdp(pdpName);
// remove the PDP from all groups
- boolean removed = false;
try {
- removed = removeFromGroups(requests.getPdpName());
+ removeFromGroups(pdpName);
} catch (PfModelException e) {
- logger.info("unable to remove PDP {} from subgroup", requests.getPdpName(), e);
- }
-
- // send the state change
- PdpStateChange change = new PdpStateChange();
- change.setName(requests.getPdpName());
- change.setState(PdpState.PASSIVE);
-
- if (removed) {
- // send an update, too
- PdpUpdate update = new PdpUpdate();
- update.setName(requests.getPdpName());
- update.setPolicies(Collections.emptyList());
-
- addRequest(update, change);
-
- } else {
- addRequest(change);
+ logger.info("unable to remove PDP {} from subgroup", pdpName, e);
}
}
}
diff --git a/main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java b/main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java
index e90566eb..74f8b392 100644
--- a/main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java
+++ b/main/src/test/java/org/onap/policy/pap/main/comm/PdpModifyRequestMapTest.java
@@ -24,7 +24,6 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
@@ -335,7 +334,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
}
@Test
- public void testDisablePdp() throws Exception {
+ public void testRemovePdp() throws Exception {
map.addRequest(update);
// put the PDP in a group
@@ -344,8 +343,8 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
- // indicate failure
- invokeFailureHandler(1);
+ // indicate retries exhausted
+ invokeLastRetryHandler(1);
// should have stopped publishing
verify(requests).stopPublishing();
@@ -353,66 +352,58 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
// should have generated a notification
verify(notifier).removePdp(PDP1);
- // should have published a state-change
- PdpMessage msg2 = getSingletons(3).get(1).getMessage();
- assertNotNull(msg2);
- assertTrue(msg2 instanceof PdpStateChange);
-
- change = (PdpStateChange) msg2;
- assertEquals(PDP1, change.getName());
- assertEquals(PdpState.PASSIVE, change.getState());
-
- // should have published a new update
- msg2 = getSingletons(3).get(2).getMessage();
- assertNotNull(msg2);
- assertTrue(msg2 instanceof PdpUpdate);
-
- // update should have null group & subgroup
- update = (PdpUpdate) msg2;
- assertEquals(PDP1, update.getName());
- assertNull(update.getPdpGroup());
- assertNull(update.getPdpSubgroup());
+ // should have removed from the group
+ List<PdpGroup> groups = getGroupUpdates();
+ assertEquals(1, groups.size());
+ assertSame(group, groups.get(0));
+ assertEquals(0, group.getPdpSubgroups().get(0).getCurrentInstanceCount());
}
@Test
- public void testDisablePdp_NotInGroup() {
+ public void testRemovePdp_NotInGroup() throws PfModelException {
map.addRequest(update);
- // indicate failure
- invokeFailureHandler(1);
+ // indicate retries exhausted
+ invokeLastRetryHandler(1);
// should have stopped publishing
verify(requests).stopPublishing();
- // should have published a new state-change
- PdpMessage msg2 = getSingletons(2).get(1).getMessage();
- assertNotNull(msg2);
- assertTrue(msg2 instanceof PdpStateChange);
-
- change = (PdpStateChange) msg2;
- assertEquals(PDP1, change.getName());
- assertEquals(PdpState.PASSIVE, change.getState());
+ // should not have done any updates
+ verify(dao, never()).updatePdpGroups(any());
}
@Test
- public void testDisablePdp_AlreadyRemoved() {
+ public void testRemovePdp_AlreadyRemovedFromMap() throws PfModelException {
map.addRequest(change);
map.stopPublishing(PDP1);
- invokeFailureHandler(1);
+ // put the PDP in a group
+ PdpGroup group = makeGroup(MY_GROUP);
+ group.setPdpSubgroups(Arrays.asList(makeSubGroup(MY_SUBGROUP, PDP1)));
- // should not have stopped publishing a second time
- verify(requests, times(1)).stopPublishing();
+ when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
+
+ invokeLastRetryHandler(1);
+
+ // should have stopped publishing a second time
+ verify(requests, times(2)).stopPublishing();
+
+ // should NOT have done any updates
+ verify(dao, never()).updatePdpGroups(any());
}
@Test
- public void testDisablePdp_NoGroup() {
+ public void testRemovePdp_NoGroup() throws PfModelException {
map.addRequest(change);
- invokeFailureHandler(1);
+ invokeLastRetryHandler(1);
// should not have stopped publishing
verify(requests).stopPublishing();
+
+ // should not have done any updates
+ verify(dao, never()).updatePdpGroups(any());
}
@Test
@@ -425,7 +416,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
- invokeFailureHandler(1);
+ invokeLastRetryHandler(1);
// verify that the PDP was removed from the subgroup
List<PdpGroup> groups = getGroupUpdates();
@@ -445,7 +436,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
when(dao.getFilteredPdpGroups(any())).thenThrow(new PfModelException(Status.BAD_REQUEST, "expected exception"));
- invokeFailureHandler(1);
+ invokeLastRetryHandler(1);
// should still stop publishing
verify(requests).stopPublishing();
@@ -459,7 +450,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
public void testRemoveFromGroup_NoGroups() throws Exception {
map.addRequest(change);
- invokeFailureHandler(1);
+ invokeLastRetryHandler(1);
verify(dao, never()).updatePdpGroups(any());
}
@@ -473,7 +464,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
- invokeFailureHandler(1);
+ invokeLastRetryHandler(1);
verify(dao, never()).updatePdpGroups(any());
}
@@ -487,7 +478,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
- invokeFailureHandler(1);
+ invokeLastRetryHandler(1);
// verify that the PDP was removed from the subgroup
List<PdpGroup> groups = getGroupUpdates();
@@ -519,22 +510,29 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
// invoke the method
invokeFailureHandler(1);
- verify(requests).stopPublishing();
+ verify(requests, never()).stopPublishing();
+
+ // requests should have been removed from the map so this should allocate another
+ map.addRequest(update);
+ assertEquals(2, map.nalloc);
}
@Test
- public void testSingletonListenerFailure_WrongPdpName() throws Exception {
+ public void testSingletonListenerSuccess() throws Exception {
map.addRequest(change);
- // invoke the method - has wrong PDP name
- when(requests.getPdpName()).thenReturn(DIFFERENT);
- invokeFailureHandler(1);
+ // invoke the method
+ invokeSuccessHandler(1);
verify(requests, never()).stopPublishing();
+
+ // requests should have been removed from the map so this should allocate another
+ map.addRequest(update);
+ assertEquals(2, map.nalloc);
}
@Test
- public void testSingletonListenerSuccess_LastRequest() throws Exception {
+ public void testRequestCompleted_LastRequest() throws Exception {
map.addRequest(change);
// invoke the method
@@ -548,11 +546,19 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
}
@Test
- public void testSingletonListenerSuccess_NameMismatch() throws Exception {
+ public void testRequestCompleted_NameMismatch() throws Exception {
+ // use a different name
+ when(requests.getPdpName()).thenReturn(DIFFERENT);
+
map.addRequest(change);
- // invoke the method - with a different name
- when(requests.getPdpName()).thenReturn(DIFFERENT);
+ // put the PDP in a group
+ PdpGroup group = makeGroup(MY_GROUP);
+ group.setPdpSubgroups(Arrays.asList(makeSubGroup(MY_SUBGROUP, PDP1, DIFFERENT)));
+
+ when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
+
+ // invoke the method - with a different name (i.e., PDP1 instead of DIFFERENT)
invokeSuccessHandler(1);
verify(requests, never()).stopPublishing();
@@ -560,10 +566,13 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
// no effect on the map
map.addRequest(update);
assertEquals(1, map.nalloc);
+
+ // no updates
+ verify(dao, never()).updatePdpGroups(any());
}
@Test
- public void testSingletonListenerSuccess_AlreadyStopped() throws Exception {
+ public void testRequestCompleted_AlreadyStopped() throws Exception {
map.addRequest(change);
map.stopPublishing(PDP1);
@@ -584,7 +593,7 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
map.addRequest(change);
// invoke the method
- getListener(getSingletons(1).get(0)).retryCountExhausted();
+ invokeLastRetryHandler(1);
verify(requests).stopPublishing();
}
@@ -609,6 +618,15 @@ public class PdpModifyRequestMapTest extends CommonRequestBase {
}
/**
+ * Invokes the first request's listener.retryCountExhausted() method.
+ *
+ * @param count expected number of requests
+ */
+ private void invokeLastRetryHandler(int count) {
+ getListener(getSingletons(count).get(0)).retryCountExhausted();
+ }
+
+ /**
* Gets the name of the PDPs contained within a subgroup.
*
* @param subgroup subgroup of interest