diff options
author | Valerij Fredriksen <valerijf@oath.com> | 2018-09-25 13:41:12 +0200 |
---|---|---|
committer | Valerij Fredriksen <valerijf@oath.com> | 2018-09-25 13:41:12 +0200 |
commit | d9fab8d7d6ca30f21e105a7aabdd054b3bd7058e (patch) | |
tree | 4999f5d9bc42b8c6b0b00479a38d7d116c4e8fb8 | |
parent | f63724bc13a248c49e20850be0df67ab8e301ffe (diff) |
Fetch containers to run when subsystem freeze expires
2 files changed, 23 insertions, 21 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java index 5ab73788299..02c144e3545 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImpl.java @@ -209,10 +209,11 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { // To avoid node agents stalling for too long, we'll force unfrozen ticks now. log.info("Timed out trying to freeze, will force unfreezed ticks"); nodeAdmin.setFrozen(false); + fetchContainersToRunFromNodeRepository(); } + } else if (currentState == RESUMED) { + fetchContainersToRunFromNodeRepository(); } - - fetchContainersToRunFromNodeRepository(); } private void setLastConvergenceException(RuntimeException exception) { @@ -277,20 +278,11 @@ public class NodeAdminStateUpdaterImpl implements NodeAdminStateUpdater { } private void fetchContainersToRunFromNodeRepository() { - synchronized (monitor) { - // Refresh containers to run even if we would like to suspend but have failed to do so yet, - // because it may take a long time to get permission to suspend. - if (currentState != RESUMED) { - log.info("Frozen, skipping fetching info from node repository"); - return; - } - - try { - final List<NodeSpec> containersToRun = nodeRepository.getNodes(dockerHostHostName); - nodeAdmin.refreshContainersToRun(containersToRun); - } catch (Exception e) { - log.log(LogLevel.WARNING, "Failed to update which containers should be running", e); - } + try { + final List<NodeSpec> containersToRun = nodeRepository.getNodes(dockerHostHostName); + nodeAdmin.refreshContainersToRun(containersToRun); + } catch (Exception e) { + log.log(LogLevel.WARNING, "Failed to update which containers should be running", e); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImplTest.java index cfdb576fb34..fd54773910d 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminStateUpdaterImplTest.java @@ -22,6 +22,7 @@ import java.util.stream.IntStream; import static com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdaterImpl.TRANSITION_EXCEPTION_MESSAGE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doNothing; @@ -172,19 +173,28 @@ public class NodeAdminStateUpdaterImplTest { // Let's start suspending, we are able to freeze the nodes, but orchestrator denies suspension when(nodeAdmin.subsystemFreezeDuration()).thenReturn(Duration.ofSeconds(1)); when(nodeAdmin.setFrozen(eq(true))).thenReturn(true); - doThrow(new RuntimeException(exceptionMsg)) - .when(orchestrator).suspend(eq(parentHostname)); + doThrow(new RuntimeException(exceptionMsg)).when(orchestrator).suspend(eq(parentHostname)); assertResumeStateError(NodeAdminStateUpdater.State.SUSPENDED_NODE_ADMIN, TRANSITION_EXCEPTION_MESSAGE); - tickAfter(0); + tickAfter(30); verify(nodeAdmin, times(1)).setFrozen(eq(true)); + tickAfter(30); + verify(nodeAdmin, times(2)).setFrozen(eq(true)); + verify(nodeAdmin, times(1)).setFrozen(eq(false)); // No new unfreezes during last 2 ticks + verify(nodeAdmin, times(1)).refreshContainersToRun(any()); assertResumeStateError(NodeAdminStateUpdater.State.SUSPENDED_NODE_ADMIN, exceptionMsg); + // Only resume and fetch containers when subsystem freeze duration expires + when(nodeAdmin.subsystemFreezeDuration()).thenReturn(Duration.ofHours(1)); + tickAfter(30); + verify(nodeAdmin, times(2)).setFrozen(eq(false)); + verify(nodeAdmin, times(2)).refreshContainersToRun(any()); + // We change our mind, want to remain resumed assertResumeStateError(NodeAdminStateUpdater.State.RESUMED, TRANSITION_EXCEPTION_MESSAGE); - tickAfter(0); + tickAfter(30); refresher.setResumeStateAndCheckIfResumed(NodeAdminStateUpdater.State.RESUMED); - verify(nodeAdmin, times(2)).setFrozen(eq(false)); // Make sure that we unfreeze! + verify(nodeAdmin, times(3)).setFrozen(eq(false)); // Make sure that we unfreeze! } @Test |