Commands that dont need root to execute now execute as "yahoo"

5 files changed, 30 insertions, 35 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
index 52b5bede912..e1e5f955e6f 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java
@@ -5,6 +5,7 @@ import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
+import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
 import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec;
 
 import java.util.List;
@@ -25,7 +26,9 @@ public interface DockerOperations {
 
     void removeContainer(ContainerNodeSpec nodeSpec, Container existingContainer);
 
-    void executeCommandInContainer(ContainerName containerName, String[] command);
+    ProcessResult executeCommandInContainer(ContainerName containerName, String[] command);
+
+    ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, String[] command);
 
     void executeCommandInNetworkNamespace(ContainerName containerName, String[] command);
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index 45c9f97d0d8..905f0e436a8 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
@@ -141,20 +141,6 @@ public class DockerOperationsImpl implements DockerOperations {
         return docker.getContainer(hostname);
     }
 
-    /**
-     * Executes a program and returns its result, or if it doesn't exist, return a result
-     * as-if the program executed with exit status 0 and no output.
-     */
-    Optional<ProcessResult> executeOptionalProgramInContainer(ContainerName containerName, String... args) {
-        assert args.length > 0;
-        String[] nodeProgramExistsCommand = programExistsCommand(args[0]);
-        if (!docker.executeInContainer(containerName, nodeProgramExistsCommand).isSuccess()) {
-            return Optional.empty();
-        }
-
-        return Optional.of(docker.executeInContainer(containerName, args));
-    }
-
     String[] programExistsCommand(String programPath) {
         return new String[]{ "/usr/bin/env", "test", "-x", programPath };
     }
@@ -168,23 +154,15 @@ public class DockerOperationsImpl implements DockerOperations {
      */
     @Override
     public void trySuspendNode(ContainerName containerName) {
-        PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
-        Optional<ProcessResult> result;
-
         try {
             // TODO: Change to waiting w/o timeout (need separate thread that we can stop).
-            result = executeOptionalProgramInContainer(containerName, SUSPEND_NODE_COMMAND);
+            executeCommandInContainer(containerName, SUSPEND_NODE_COMMAND);
         } catch (RuntimeException e) {
+            PrefixLogger logger = PrefixLogger.getNodeAgentLogger(DockerOperationsImpl.class, containerName);
             // It's bad to continue as-if nothing happened, but on the other hand if we do not proceed to
             // remove container, we will not be able to upgrade to fix any problems in the suspend logic!
             logger.warning("Failed trying to suspend container " + containerName.asString() + "  with "
                    + Arrays.toString(SUSPEND_NODE_COMMAND), e);
-            return;
-        }
-
-        if (result.isPresent() && !result.get().isSuccess()) {
-            logger.warning("The suspend program " + Arrays.toString(SUSPEND_NODE_COMMAND)
-                    + " failed: " + result.get().getOutput() + " for container " + containerName.asString());
         }
     }
 
@@ -242,7 +220,7 @@ public class DockerOperationsImpl implements DockerOperations {
             }
 
             DIRECTORIES_TO_MOUNT.entrySet().stream().filter(Map.Entry::getValue).forEach(entry ->
-                    docker.executeInContainer(nodeSpec.containerName, "sudo", "chmod", "-R", "a+w", entry.getKey()));
+                    docker.executeInContainerAsRoot(nodeSpec.containerName, "chmod", "-R", "a+w", entry.getKey()));
         } catch (IOException e) {
             throw new RuntimeException("Failed to create container " + nodeSpec.containerName.asString(), e);
         }
@@ -289,13 +267,19 @@ public class DockerOperationsImpl implements DockerOperations {
     }
 
     @Override
-    public void executeCommandInContainer(ContainerName containerName, String[] command) {
-        Optional<ProcessResult> result = executeOptionalProgramInContainer(containerName, command);
+    public ProcessResult executeCommandInContainer(ContainerName containerName, String[] command) {
+        ProcessResult result = docker.executeInContainer(containerName, command);
 
-        if (result.isPresent() && !result.get().isSuccess()) {
-            throw new RuntimeException("Container " + containerName.asString()
-                                               + ": command " + Arrays.toString(command) + " failed: " + result.get());
+        if (result.isSuccess()) {
+            throw new RuntimeException("Container " + containerName.asString() +
+                    ": command " + Arrays.toString(command) + " failed: " + result);
         }
+        return result;
+    }
+
+    @Override
+    public ProcessResult executeCommandInContainerAsRoot(ContainerName containerName, String[] command) {
+        return docker.executeInContainerAsRoot(containerName, command);
     }
 
     @Override
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 7f1ce37c1d2..0c3f0f4a139 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
@@ -579,7 +579,7 @@ public class NodeAgentImpl implements NodeAgent {
         try {
             scheduleMaker.writeTo(yamasAgentFolder);
             final String[] restartYamasAgent = new String[] {"service" , "yamas-agent", "restart"};
-            dockerOperations.executeCommandInContainer(nodeSpec.containerName, restartYamasAgent);
+            dockerOperations.executeCommandInContainerAsRoot(nodeSpec.containerName, restartYamasAgent);
         } catch (IOException e) {
             throw new RuntimeException("Failed to write secret-agent schedules for " + nodeSpec.containerName, e);
         }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
index 58095c14a9b..ae4ea905882 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java
@@ -98,12 +98,12 @@ public class ComponentsProviderImpl implements ComponentsProvider {
 
     private void setCorePattern(Docker docker) {
         final String[] sysctlCorePattern = {"sysctl", "-w", "kernel.core_pattern=/home/y/var/crash/%e.core.%p"};
-        docker.executeInContainer(NODE_ADMIN_CONTAINER_NAME, sysctlCorePattern);
+        docker.executeInContainerAsRoot(NODE_ADMIN_CONTAINER_NAME, sysctlCorePattern);
     }
 
     private void initializeNodeAgentSecretAgent(Docker docker) {
         final Path yamasAgentFolder = Paths.get("/etc/yamas-agent/");
-        docker.executeInContainer(NODE_ADMIN_CONTAINER_NAME, "sudo", "chmod", "a+w", yamasAgentFolder.toString());
+        docker.executeInContainerAsRoot(NODE_ADMIN_CONTAINER_NAME, "chmod", "a+w", yamasAgentFolder.toString());
 
         Path nodeAdminCheckPath = Paths.get("/usr/bin/curl");
         SecretAgentScheduleMaker scheduleMaker = new SecretAgentScheduleMaker("node-admin", 60, nodeAdminCheckPath,
@@ -111,7 +111,7 @@ public class ComponentsProviderImpl implements ComponentsProvider {
 
         try {
             scheduleMaker.writeTo(yamasAgentFolder);
-            docker.executeInContainer(NODE_ADMIN_CONTAINER_NAME, "service", "yamas-agent", "restart");
+            docker.executeInContainerAsRoot(NODE_ADMIN_CONTAINER_NAME, "service", "yamas-agent", "restart");
         } catch (IOException e) {
             throw new RuntimeException("Failed to write secret-agent schedules for node-admin", e);
         }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
index fa4b235066b..f7b139b5e8f 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
@@ -156,6 +156,14 @@ public class DockerMock implements Docker {
         return new ProcessResult(0, null, "");
     }
 
+    @Override
+    public ProcessResult executeInContainerAsRoot(ContainerName containerName, String... args) {
+        synchronized (monitor) {
+            callOrderVerifier.add("executeInContainer as root with " + containerName + ", args: " + Arrays.toString(args));
+        }
+        return new ProcessResult(0, null, "");
+    }
+
 
     public static class StartContainerCommandMock implements CreateContainerCommand {
         @Override