From 4b224156dd8d33dd75c97c25d55c3a8bc258e69a Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@verizonmedia.com>
Date: Fri, 15 Nov 2019 10:00:38 +0100
Subject: Stop services only for nodes that has no owner

---
 .../hosted/node/admin/nodeagent/NodeAgentImpl.java |  6 ++--
 .../node/admin/nodeagent/NodeAgentImplTest.java    | 41 +++++++++++++++-------
 2 files changed, 31 insertions(+), 16 deletions(-)

(limited to 'node-admin')

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index 6d3f2c32d12..f2aa186fe4d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -250,7 +250,7 @@ public class NodeAgentImpl implements NodeAgent {
     }
 
     private void stopServicesIfNeeded(NodeAgentContext context) {
-        if (hasStartedServices)
+        if (hasStartedServices && context.node().owner().isEmpty())
             stopServices(context);
     }
 
@@ -409,10 +409,8 @@ public class NodeAgentImpl implements NodeAgent {
             case reserved:
             case failed:
             case inactive:
-                removeContainerIfNeededUpdateContainerState(context, container);
-                updateNodeRepoWithCurrentAttributes(context);
-                break;
             case parked:
+                removeContainerIfNeededUpdateContainerState(context, container);
                 updateNodeRepoWithCurrentAttributes(context);
                 stopServicesIfNeeded(context);
                 break;
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
index 4a1c71fb847..63f980efd91 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.node.admin.nodeagent;
 
 import com.yahoo.component.Version;
+import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.flags.Flags;
@@ -655,29 +656,45 @@ public class NodeAgentImplTest {
         inOrder.verify(orchestrator).resume(hostName);
     }
 
+
+    // Tests that only containers without owners are stopped
     @Test
-    public void testStopContainerInParkedState() {
-        final NodeSpec node = nodeBuilder
+    public void testThatStopContainerDependsOnOwnerPresent() {
+        verifyThatContainerIsStopped(NodeState.parked, Optional.empty());
+        verifyThatContainerIsStopped(NodeState.parked, Optional.of(ApplicationId.defaultId()));
+        verifyThatContainerIsStopped(NodeState.failed, Optional.empty());
+        verifyThatContainerIsStopped(NodeState.failed, Optional.of(ApplicationId.defaultId()));
+        verifyThatContainerIsStopped(NodeState.inactive, Optional.of(ApplicationId.defaultId()));
+    }
+
+    private void verifyThatContainerIsStopped(NodeState nodeState, Optional<ApplicationId> owner) {
+        NodeSpec.Builder nodeBuilder = new NodeSpec.Builder()
+                .hostname(hostName)
+                .type(NodeType.tenant)
+                .flavor("docker")
                 .currentDockerImage(dockerImage)
                 .wantedDockerImage(dockerImage)
-                .state(NodeState.parked)
-                .currentVespaVersion(vespaVersion)
-                .build();
+                .state(nodeState);
+
+        owner.ifPresent(nodeBuilder::owner);
+        NodeSpec node = nodeBuilder.build();
 
         NodeAgentContext context = createContext(node);
-        NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, false);
+        NodeAgentImpl nodeAgent = makeNodeAgent(dockerImage, true);
 
         when(nodeRepository.getOptionalNode(eq(hostName))).thenReturn(Optional.of(node));
 
         nodeAgent.doConverge(context);
 
         verify(dockerOperations, never()).removeContainer(eq(context), any());
-        verify(dockerOperations, never()).createContainer(eq(context), any(), any());
-        verify(dockerOperations, times(1)).stopServices(eq(context));
-
-        nodeAgent.doConverge(context);
-        // Should not be called more than once, have already been stopped
-        verify(dockerOperations, times(1)).stopServices(eq(context));
+        if (owner.isPresent()) {
+            verify(dockerOperations, never()).stopServices(eq(context));
+        } else {
+            verify(dockerOperations, times(1)).stopServices(eq(context));
+            nodeAgent.doConverge(context);
+            // Should not be called more than once, have already been stopped
+            verify(dockerOperations, times(1)).stopServices(eq(context));
+        }
     }
 
     private NodeAgentImpl makeNodeAgent(DockerImage dockerImage, boolean isRunning) {
-- 
cgit v1.2.3