Merge pull request #6858 from vespa-engine/freva/node-admin-fixes-2

NodeAdmin fixes 2

8 files changed, 51 insertions, 69 deletions

diff --git a/docker-api/pom.xml b/docker-api/pom.xml
index 64410c32f06..74e463ef157 100644
--- a/docker-api/pom.xml
+++ b/docker-api/pom.xml
@@ -18,16 +18,20 @@
     <name>${project.artifactId}</name>
 
     <dependencies>
+        <!-- Provided -->
         <dependency>
             <groupId>com.yahoo.vespa</groupId>
             <artifactId>container-dev</artifactId>
             <version>${project.version}</version>
             <scope>provided</scope>
         </dependency>
+
+        <!-- Compile -->
         <dependency>
             <groupId>com.github.docker-java</groupId>
             <artifactId>docker-java</artifactId>
             <version>3.0.13</version>
+            <scope>compile</scope>
             <exclusions>
                 <exclusion>
                     <groupId>org.slf4j</groupId>
@@ -92,6 +96,7 @@
         <dependency>
             <groupId>net.jpountz.lz4</groupId>
             <artifactId>lz4</artifactId>
+            <scope>compile</scope>
         </dependency>
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
@@ -100,6 +105,7 @@
             docker-java so the dependency is declared closer to the root of maven and
             more likely be the version that is finally being used. -->
             <version>4.4.1</version>
+            <scope>compile</scope>
         </dependency>
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
@@ -108,7 +114,10 @@
             docker-java so the dependency is declared closer to the root of maven and
             more likely be the version that is finally being used. -->
             <version>4.5</version>
+            <scope>compile</scope>
         </dependency>
+
+        <!-- Test -->
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
@@ -121,7 +130,6 @@
         </dependency>
     </dependencies>
 
-
     <build>
         <plugins>
             <plugin>
diff --git a/node-admin/pom.xml b/node-admin/pom.xml
index 7daeacec463..64958554f53 100644
--- a/node-admin/pom.xml
+++ b/node-admin/pom.xml
@@ -18,6 +18,7 @@
     <name>${project.artifactId}</name>
 
     <dependencies>
+        <!-- Provided -->
         <dependency>
             <groupId>com.yahoo.vespa</groupId>
             <artifactId>docker-api</artifactId>
@@ -32,49 +33,55 @@
         </dependency>
         <dependency>
             <groupId>com.yahoo.vespa</groupId>
-            <artifactId>node-repository</artifactId>
+            <artifactId>defaults</artifactId>
             <version>${project.version}</version>
+            <scope>provided</scope>
         </dependency>
         <dependency>
             <groupId>com.yahoo.vespa</groupId>
-            <artifactId>defaults</artifactId>
+            <artifactId>container-dev</artifactId>
             <version>${project.version}</version>
             <scope>provided</scope>
         </dependency>
         <dependency>
             <groupId>com.yahoo.vespa</groupId>
-            <artifactId>container-dev</artifactId>
+            <artifactId>vespa-athenz</artifactId>
             <version>${project.version}</version>
             <scope>provided</scope>
         </dependency>
+
+        <!-- Compile -->
         <dependency>
-            <groupId>net.jpountz.lz4</groupId>
-            <artifactId>lz4</artifactId>
+            <groupId>com.yahoo.vespa</groupId>
+            <artifactId>orchestrator-restapi</artifactId>
+            <version>${project.version}</version>
+            <scope>compile</scope>
+        </dependency>
+        <dependency>
+            <groupId>com.yahoo.vespa</groupId>
+            <artifactId>node-repository</artifactId>
+            <version>${project.version}</version>
             <scope>compile</scope>
         </dependency>
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpcore</artifactId>
             <version>4.4.1</version>
+            <scope>compile</scope>
         </dependency>
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpclient</artifactId>
             <version>4.5</version>
-        </dependency>
-        <dependency>
-            <groupId>com.yahoo.vespa</groupId>
-            <artifactId>orchestrator-restapi</artifactId>
-            <version>${project.version}</version>
             <scope>compile</scope>
         </dependency>
         <dependency>
-            <groupId>com.yahoo.vespa</groupId>
-            <artifactId>vespa-athenz</artifactId>
-            <version>${project.version}</version>
-            <scope>provided</scope>
+            <groupId>org.apache.velocity</groupId>
+            <artifactId>velocity</artifactId>
+            <scope>compile</scope>
         </dependency>
 
+        <!-- Test -->
         <dependency>
             <groupId>org.hamcrest</groupId>
             <artifactId>hamcrest-junit</artifactId>
@@ -82,6 +89,11 @@
             <scope>test</scope>
         </dependency>
         <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
             <scope>test</scope>
@@ -89,24 +101,24 @@
         <dependency>
             <groupId>com.yahoo.vespa</groupId>
             <artifactId>application</artifactId>
-            <scope>test</scope>
             <version>${project.version}</version>
+            <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>com.yahoo.vespa</groupId>
-            <artifactId>orchestrator</artifactId>
+            <artifactId>application-model</artifactId>
             <version>${project.version}</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>com.yahoo.vespa</groupId>
-            <artifactId>service-monitor</artifactId>
+            <artifactId>orchestrator</artifactId>
             <version>${project.version}</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>com.yahoo.vespa</groupId>
-            <artifactId>application-model</artifactId>
+            <artifactId>service-monitor</artifactId>
             <version>${project.version}</version>
             <scope>test</scope>
         </dependency>
@@ -116,16 +128,6 @@
             <version>${project.version}</version>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.mockito</groupId>
-            <artifactId>mockito-core</artifactId>
-            <scope>test</scope>
-        </dependency>
-        <dependency>
-            <groupId>org.apache.velocity</groupId>
-            <artifactId>velocity</artifactId>
-            <scope>compile</scope>
-        </dependency>
     </dependencies>
     <build>
         <plugins>
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
index f5f0fa5a3f1..7036f6852fe 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java
@@ -641,6 +641,7 @@ public class NodeSpec {
 
         public Builder updateFromNodeAttributes(NodeAttributes attributes) {
             attributes.getDockerImage().ifPresent(this::currentDockerImage);
+            attributes.getCurrentOsVersion().ifPresent(this::currentOsVersion);
             attributes.getHardwareDivergence().ifPresent(this::hardwareDivergence);
             attributes.getRebootGeneration().ifPresent(this::currentRebootGeneration);
             attributes.getRestartGeneration().ifPresent(this::currentRestartGeneration);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java
index a0657c3d34c..16992bcb13a 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdmin.java
@@ -6,7 +6,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import java.time.Duration;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 /**
  * NodeAdmin manages the life cycle of NodeAgents.
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java
index 96e1461bc32..ba8a2e55587 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java
@@ -157,8 +157,8 @@ public class NodeAdminImpl implements NodeAdmin {
         Map<String, Object> debug = new LinkedHashMap<>();
         debug.put("isFrozen", isFrozen);
 
-        List<Map<String, Object>> nodeAgentDebugs = nodeAgentsByHostname.entrySet().stream()
-                .map(node -> node.getValue().debugInfo()).collect(Collectors.toList());
+        List<Map<String, Object>> nodeAgentDebugs = nodeAgentsByHostname.values().stream()
+                .map(NodeAgent::debugInfo).collect(Collectors.toList());
         debug.put("NodeAgents", nodeAgentDebugs);
         return debug;
     }
@@ -171,7 +171,7 @@ public class NodeAdminImpl implements NodeAdmin {
             } catch (Throwable e) {
                 logger.warning("Metric fetcher scheduler failed", e);
             }
-        }, 0, 55, TimeUnit.SECONDS);
+        }, 10, 55, TimeUnit.SECONDS);
 
         int delay = 120; // WARNING: Reducing this will increase the load on config servers.
         aclScheduler.scheduleWithFixedDelay(() -> {
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 5f2093c4719..7c84150009e 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
@@ -26,14 +26,11 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.identity.AthenzCredentialsM
 import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
 import com.yahoo.vespa.hosted.provision.Node;
 
-import java.text.SimpleDateFormat;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Date;
 import java.util.LinkedHashMap;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -80,9 +77,6 @@ public class NodeAgentImpl implements NodeAgent {
     private final Duration timeBetweenEachConverge;
     private final AthenzCredentialsMaintainer athenzCredentialsMaintainer;
 
-    private final SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
-    private final LinkedList<String> debugMessages = new LinkedList<>();
-
     private int numberOfUnhandledException = 0;
     private Instant lastConverge;
 
@@ -155,7 +149,7 @@ public class NodeAgentImpl implements NodeAgent {
         synchronized (monitor) {
             if (wantFrozen != frozen) {
                 wantFrozen = frozen;
-                addDebugMessage(wantFrozen ? "Freezing" : "Unfreezing");
+                logger.debug(wantFrozen ? "Freezing" : "Unfreezing");
                 signalWorkToBeDone();
             }
 
@@ -163,17 +157,6 @@ public class NodeAgentImpl implements NodeAgent {
         }
     }
 
-    private void addDebugMessage(String message) {
-        synchronized (debugMessages) {
-            while (debugMessages.size() > 1000) {
-                debugMessages.pop();
-            }
-
-            logger.debug(message);
-            debugMessages.add("[" + sdf.format(new Date()) + "] " + message);
-        }
-    }
-
     @Override
     public Map<String, Object> debugInfo() {
         Map<String, Object> debug = new LinkedHashMap<>();
@@ -182,18 +165,13 @@ public class NodeAgentImpl implements NodeAgent {
         debug.put("wantFrozen", wantFrozen);
         debug.put("terminated", terminated);
         debug.put("workToDoNow", workToDoNow);
-        synchronized (debugMessages) {
-            debug.put("history", new LinkedList<>(debugMessages));
-        }
         debug.put("nodeRepoState", lastNode.getState().name());
         return debug;
     }
 
     @Override
     public void start() {
-        String message = "Starting with interval " + timeBetweenEachConverge.toMillis() + " ms";
-        logger.info(message);
-        addDebugMessage(message);
+        logger.info("Starting with interval " + timeBetweenEachConverge.toMillis() + " ms");
 
         loopThread.start();
 
@@ -213,7 +191,6 @@ public class NodeAgentImpl implements NodeAgent {
 
     @Override
     public void stop() {
-        addDebugMessage("Stopping");
         filebeatRestarter.shutdown();
         if (!terminated.compareAndSet(false, true)) {
             throw new RuntimeException("Can not re-stop a node agent.");
@@ -240,7 +217,7 @@ public class NodeAgentImpl implements NodeAgent {
             currentFilebeatRestarter = Optional.of(filebeatRestarter.scheduleWithFixedDelay(
                     () -> serviceRestarter.accept("filebeat"), 1, 1, TimeUnit.DAYS));
 
-            addDebugMessage("Starting optional node program resume command");
+            logger.debug("Starting optional node program resume command");
             dockerOperations.resumeNode(containerName);
             resumeScriptRun = true;
         }
@@ -266,8 +243,6 @@ public class NodeAgentImpl implements NodeAgent {
         if (!currentAttributes.equals(wantedAttributes)) {
             logger.info("Publishing new set of attributes to node repo: "
                     + currentAttributes + " -> " + wantedAttributes);
-            addDebugMessage("Publishing new set of attributes to node repo: {" +
-                    currentAttributes + "} -> {" + wantedAttributes + "}");
             nodeRepository.updateNodeAttributes(hostname, wantedAttributes);
         }
     }
@@ -386,7 +361,7 @@ public class NodeAgentImpl implements NodeAgent {
         synchronized (monitor) {
             if (!workToDoNow) {
                 workToDoNow = true;
-                addDebugMessage("Signaling work to be done");
+                logger.debug("Signaling work to be done");
                 monitor.notifyAll();
             }
         }
@@ -421,21 +396,19 @@ public class NodeAgentImpl implements NodeAgent {
         boolean converged = false;
 
         if (isFrozenCopy) {
-            addDebugMessage("tick: isFrozen");
+            logger.debug("tick: isFrozen");
         } else {
             try {
                 converge();
                 converged = true;
             } catch (OrchestratorException e) {
                 logger.info(e.getMessage());
-                addDebugMessage(e.getMessage());
             } catch (DockerException e) {
                 numberOfUnhandledException++;
                 logger.error("Caught a DockerException, resetting containerState to " + containerState, e);
             } catch (Exception e) {
                 numberOfUnhandledException++;
                 logger.error("Unhandled exception, ignoring.", e);
-                addDebugMessage(e.getMessage());
             }
         }
 
@@ -462,7 +435,7 @@ public class NodeAgentImpl implements NodeAgent {
                 storageMaintainer.writeMetricsConfig(containerName, node);
             }
 
-            addDebugMessage("Loading new node spec: " + node.toString());
+            logger.debug("Loading new node spec: " + node.toString());
             lastNode = node;
         }
 
@@ -484,7 +457,7 @@ public class NodeAgentImpl implements NodeAgent {
 
                 scheduleDownLoadIfNeeded(node);
                 if (isDownloadingImage()) {
-                    addDebugMessage("Waiting for image to download " + imageBeingDownloaded.asString());
+                    logger.debug("Waiting for image to download " + imageBeingDownloaded.asString());
                     return;
                 }
                 container = removeContainerIfNeededUpdateContainerState(node, container);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/Yum.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/Yum.java
index f41711c1594..3a9c49a0f2d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/Yum.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/Yum.java
@@ -163,7 +163,7 @@ public class Yum {
         private final List<YumPackageName> packages;
         private final Pattern commandOutputNoopPattern;
 
-        private List<String> enabledRepo = new ArrayList<>();
+        private final List<String> enabledRepo = new ArrayList<>();
 
         private GenericYumCommand(Terminal terminal,
                                   String yumCommand,
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
index 603ad3ebccf..d0e4377ffc5 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
@@ -14,7 +14,6 @@ import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl;
 import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer;
 import com.yahoo.vespa.hosted.node.admin.maintenance.identity.AthenzCredentialsMaintainer;
-import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdmin;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminImpl;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.NodeAdminStateUpdaterImpl;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgent;