Log output of nodectl subcommands

5 files changed, 49 insertions, 27 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java
index ed1c035f543..89e13294a71 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperations.java
@@ -45,20 +45,21 @@ public interface ContainerOperations {
     CommandResult executeCommandInNetworkNamespace(NodeAgentContext context, String... command);
 
 
-    /** Resume node. Resuming a node means that it is ready to take on traffic. */
-    void resumeNode(NodeAgentContext context);
+    /** Resume node. Resuming a node means that it is ready to take on traffic.
+     * @return*/
+    String resumeNode(NodeAgentContext context);
 
     /**
-     * Suspend node. Suspending a node means the node should be taken temporarly offline,
+     * Suspend node and return output. Suspending a node means the node should be taken temporarly offline,
      * such that maintenance of the node can be done (upgrading, rebooting, etc).
      */
-    void suspendNode(NodeAgentContext context);
+    String suspendNode(NodeAgentContext context);
 
-    void restartVespa(NodeAgentContext context);
+    String restartVespa(NodeAgentContext context);
 
-    void startServices(NodeAgentContext context);
+    String startServices(NodeAgentContext context);
 
-    void stopServices(NodeAgentContext context);
+    String stopServices(NodeAgentContext context);
 
     Optional<ContainerStats> getContainerStats(NodeAgentContext context);
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java
index 9e12720aa05..4f57a03786b 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/ContainerOperationsImpl.java
@@ -243,28 +243,28 @@ public class ContainerOperationsImpl implements ContainerOperations {
     }
 
     @Override
-    public void resumeNode(NodeAgentContext context) {
-        executeNodeCtlInContainer(context, "resume");
+    public String resumeNode(NodeAgentContext context) {
+        return executeNodeCtlInContainer(context, "resume").getOutput();
     }
 
     @Override
-    public void suspendNode(NodeAgentContext context) {
-        executeNodeCtlInContainer(context, "suspend");
+    public String suspendNode(NodeAgentContext context) {
+        return executeNodeCtlInContainer(context, "suspend").getOutput();
     }
 
     @Override
-    public void restartVespa(NodeAgentContext context) {
-        executeNodeCtlInContainer(context, "restart-vespa");
+    public String restartVespa(NodeAgentContext context) {
+        return executeNodeCtlInContainer(context, "restart-vespa").getOutput();
     }
 
     @Override
-    public void startServices(NodeAgentContext context) {
-        executeNodeCtlInContainer(context, "start");
+    public String startServices(NodeAgentContext context) {
+        return executeNodeCtlInContainer(context, "start").getOutput();
     }
 
     @Override
-    public void stopServices(NodeAgentContext context) {
-        executeNodeCtlInContainer(context, "stop");
+    public String stopServices(NodeAgentContext context) {
+        return executeNodeCtlInContainer(context, "stop").getOutput();
     }
 
     ProcessResult executeNodeCtlInContainer(NodeAgentContext context, String program) {
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 efc94d11033..74ba19a72c5 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
@@ -166,16 +166,22 @@ public class NodeAgentImpl implements NodeAgent {
 
     void startServicesIfNeeded(NodeAgentContext context) {
         if (!hasStartedServices) {
-            context.log(logger, "Starting services");
-            containerOperations.startServices(context);
+            context.log(logger, "Invoking vespa-nodectl to start services");
+            String output = containerOperations.startServices(context);
+            if (!output.isBlank()) {
+                context.log(logger, "Start services output: " + output);
+            }
             hasStartedServices = true;
         }
     }
 
     void resumeNodeIfNeeded(NodeAgentContext context) {
         if (!hasResumedNode) {
-            context.log(logger, Level.FINE, "Starting optional node program resume command");
-            containerOperations.resumeNode(context);
+            context.log(logger, "Invoking vespa-nodectl to resume services");
+            String output = containerOperations.resumeNode(context);
+            if (!output.isBlank()) {
+                context.log(logger, "Resume services output: " + output);
+            }
             hasResumedNode = true;
         }
     }
@@ -243,10 +249,13 @@ public class NodeAgentImpl implements NodeAgent {
             }
 
             shouldRestartServices(context, existingContainer.get()).ifPresent(restartReason -> {
-                context.log(logger, "Will restart services: " + restartReason);
+                context.log(logger, "Invoking vespa-nodectl to restart services: " + restartReason);
                 orchestratorSuspendNode(context);
 
-                containerOperations.restartVespa(context);
+                String output = containerOperations.restartVespa(context);
+                if (!output.isBlank()) {
+                    context.log(logger, "Restart services output: " + output);
+                }
                 currentRestartGeneration = context.node().wantedRestartGeneration();
             });
         }
@@ -290,11 +299,14 @@ public class NodeAgentImpl implements NodeAgent {
     }
 
     public void suspend(NodeAgentContext context) {
-        context.log(logger, "Suspending services on node");
         if (containerState == ABSENT) return;
         try {
             hasResumedNode = false;
-            containerOperations.suspendNode(context);
+            context.log(logger, "Invoking vespa-nodectl to suspend services");
+            String output = containerOperations.suspendNode(context);
+            if (!output.isBlank()) {
+                context.log(logger, "Suspend services output: " + output);
+            }
         } catch (ContainerNotFoundException e) {
             containerState = ABSENT;
         } catch (RuntimeException e) {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java
index 7aeaa37b4af..a856c9d9b8d 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/ContainerEngineMock.java
@@ -92,7 +92,7 @@ public class ContainerEngineMock implements ContainerEngine {
 
     @Override
     public ProcessResult executeInContainerAsUser(ContainerName containerName, String user, OptionalLong timeout, String... args) {
-        return new ProcessResult(0, null, "");
+        return new ProcessResult(0, "", "");
     }
 
     @Override
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 9cedf7777eb..9475e3720c2 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
@@ -27,6 +27,7 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer;
 import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer;
 import com.yahoo.vespa.hosted.node.admin.maintenance.identity.CredentialsMaintainer;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException;
+import org.junit.Before;
 import org.junit.Test;
 import org.mockito.InOrder;
 
@@ -71,6 +72,14 @@ public class NodeAgentImplTest {
     private final InMemoryFlagSource flagSource = new InMemoryFlagSource();
     private final ManualClock clock = new ManualClock(Instant.now());
 
+    @Before
+    public void setUp() {
+        when(containerOperations.suspendNode(any())).thenReturn("");
+        when(containerOperations.resumeNode(any())).thenReturn("");
+        when(containerOperations.restartVespa(any())).thenReturn("");
+        when(containerOperations.startServices(any())).thenReturn("");
+        when(containerOperations.stopServices(any())).thenReturn("");
+    }
 
     @Test
     public void upToDateContainerIsUntouched() {
@@ -515,7 +524,7 @@ public class NodeAgentImplTest {
 
         final InOrder inOrder = inOrder(orchestrator, containerOperations, nodeRepository);
         doThrow(new RuntimeException("Failed 1st time"))
-                .doNothing()
+                .doReturn("")
                 .when(containerOperations).resumeNode(eq(context));
 
         // 1st try