Fix NPE

The problem was the null DockerImage's DockerImage::asString() method was
called in NodeAttributes::toString.

7 files changed, 36 insertions, 45 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java
index 8fbd9368318..be54df31c50 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeAttributes.java
@@ -4,50 +4,51 @@ package com.yahoo.vespa.hosted.node.admin.configserver.noderepository;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
 
 import java.util.Objects;
+import java.util.Optional;
 
 public class NodeAttributes {
 
-    private Long restartGeneration = null;
-    private Long rebootGeneration = null;
-    private DockerImage dockerImage = null;
-    private String hardwareDivergence = null;
+    private Optional<Long> restartGeneration = Optional.empty();
+    private Optional<Long> rebootGeneration = Optional.empty();
+    private Optional<DockerImage> dockerImage = Optional.empty();
+    private Optional<String> hardwareDivergence = Optional.empty();
 
     public NodeAttributes() { }
 
-    public NodeAttributes withRestartGeneration(Long restartGeneration) {
+    public NodeAttributes withRestartGeneration(Optional<Long> restartGeneration) {
         this.restartGeneration = restartGeneration;
         return this;
     }
 
-    public NodeAttributes withRebootGeneration(Long rebootGeneration) {
-        this.rebootGeneration = rebootGeneration;
+    public NodeAttributes withRebootGeneration(long rebootGeneration) {
+        this.rebootGeneration = Optional.of(rebootGeneration);
         return this;
     }
 
     public NodeAttributes withDockerImage(DockerImage dockerImage) {
-        this.dockerImage = dockerImage;
+        this.dockerImage = Optional.of(dockerImage);
         return this;
     }
 
     public NodeAttributes withHardwareDivergence(String hardwareDivergence) {
-        this.hardwareDivergence = hardwareDivergence;
+        this.hardwareDivergence = Optional.of(hardwareDivergence);
         return this;
     }
 
 
-    public Long getRestartGeneration() {
+    public Optional<Long> getRestartGeneration() {
         return restartGeneration;
     }
 
-    public Long getRebootGeneration() {
+    public Optional<Long> getRebootGeneration() {
         return rebootGeneration;
     }
 
-    public DockerImage getDockerImage() {
+    public Optional<DockerImage> getDockerImage() {
         return dockerImage;
     }
 
-    public String getHardwareDivergence() {
+    public Optional<String> getHardwareDivergence() {
         return hardwareDivergence;
     }
 
@@ -72,10 +73,10 @@ public class NodeAttributes {
     @Override
     public String toString() {
         return "NodeAttributes{" +
-                "restartGeneration=" + restartGeneration +
-                ", rebootGeneration=" + rebootGeneration +
-                ", dockerImage=" + dockerImage.asString() +
-                ", hardwareDivergence='" + hardwareDivergence + '\'' +
+                "restartGeneration=" + restartGeneration.map(String::valueOf).orElse("") +
+                ", rebootGeneration=" + rebootGeneration.map(String::valueOf).orElse("") +
+                ", dockerImage=" + dockerImage.map(DockerImage::asString).orElse("") +
+                ", hardwareDivergence='" + hardwareDivergence.orElse(null) + "'" +
                 '}';
     }
 }
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 a290f843663..7c15bef32b4 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
@@ -571,22 +571,10 @@ public class NodeSpec {
         }
 
         public Builder updateFromNodeAttributes(NodeAttributes attributes) {
-            if (attributes.getDockerImage() != null) {
-                currentDockerImage = Optional.of(attributes.getDockerImage());
-            }
-
-            if (attributes.getHardwareDivergence() != null) {
-                hardwareDivergence = Optional.of(attributes.getHardwareDivergence());
-            }
-
-            if (attributes.getRebootGeneration() != null) {
-                currentRebootGeneration = attributes.getRebootGeneration();
-            }
-
-            if (attributes.getRestartGeneration() != null) {
-                currentRestartGeneration = Optional.of(attributes.getRestartGeneration());
-            }
-
+            attributes.getDockerImage().ifPresent(this::currentDockerImage);
+            attributes.getHardwareDivergence().ifPresent(this::hardwareDivergence);
+            attributes.getRebootGeneration().ifPresent(this::currentRebootGeneration);
+            attributes.getRestartGeneration().ifPresent(this::currentRestartGeneration);
             return this;
         }
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
index 0d534a31659..8c34002f5d0 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java
@@ -205,10 +205,10 @@ public class RealNodeRepository implements NodeRepository {
 
     private static NodeRepositoryNode nodeRepositoryNodeFromNodeAttributes(NodeAttributes nodeAttributes) {
         NodeRepositoryNode node = new NodeRepositoryNode();
-        node.currentDockerImage = Optional.ofNullable(nodeAttributes.getDockerImage()).map(DockerImage::asString).orElse(null);
-        node.currentRestartGeneration = nodeAttributes.getRestartGeneration();
-        node.currentRebootGeneration = nodeAttributes.getRebootGeneration();
-        node.hardwareDivergence = nodeAttributes.getHardwareDivergence();
+        node.currentDockerImage = nodeAttributes.getDockerImage().map(DockerImage::asString).orElse(null);
+        node.currentRestartGeneration = nodeAttributes.getRestartGeneration().orElse(null);
+        node.currentRebootGeneration = nodeAttributes.getRebootGeneration().orElse(null);
+        node.hardwareDivergence = nodeAttributes.getHardwareDivergence().orElse(null);
         return node;
     }
 }
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 8dcb17da5f5..d5c81c09133 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
@@ -245,12 +245,12 @@ public class NodeAgentImpl implements NodeAgent {
 
     private void updateNodeRepoWithCurrentAttributes(final NodeSpec node) {
         final NodeAttributes currentNodeAttributes = new NodeAttributes()
-                .withRestartGeneration(node.getCurrentRestartGeneration().orElse(null))
+                .withRestartGeneration(node.getCurrentRestartGeneration())
                 .withRebootGeneration(node.getCurrentRebootGeneration())
                 .withDockerImage(node.getCurrentDockerImage().orElse(new DockerImage("")));
 
         final NodeAttributes wantedNodeAttributes = new NodeAttributes()
-                .withRestartGeneration(node.getWantedRestartGeneration().orElse(null))
+                .withRestartGeneration(node.getWantedRestartGeneration())
                 // update reboot gen with wanted gen if set, we ignore reboot for Docker nodes but
                 // want the two to be equal in node repo
                 .withRebootGeneration(node.getWantedRebootGeneration())
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java
index 62a4b51e9d0..2759883ca66 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java
@@ -138,7 +138,7 @@ public class RealNodeRepositoryTest {
         nodeRepositoryApi.updateNodeAttributes(
                 hostname,
                 new NodeAttributes()
-                        .withRestartGeneration(1L)
+                        .withRestartGeneration(Optional.of(1L))
                         .withDockerImage(new DockerImage("image-1:6.2.3")));
     }
 
@@ -148,7 +148,7 @@ public class RealNodeRepositoryTest {
         nodeRepositoryApi.updateNodeAttributes(
                 hostname,
                 new NodeAttributes()
-                        .withRestartGeneration(1L)
+                        .withRestartGeneration(Optional.of(1L))
                         .withDockerImage(new DockerImage("image-1")));
     }
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/CallOrderVerifier.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/CallOrderVerifier.java
index 0464289101b..df351d4cc2e 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/CallOrderVerifier.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/CallOrderVerifier.java
@@ -78,6 +78,8 @@ public class CallOrderVerifier {
             for (String functionCall : functionCalls) {
                 int temp = indexOf(callOrder.listIterator(pos), functionCall);
                 if (temp == -1) {
+                    System.out.println("Function call '" + functionCall +
+                            "' never made at position " + pos);
                     return false;
                 }
                 pos += temp;
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 4ba030455f5..a90afe767af 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
@@ -161,14 +161,14 @@ public class NodeAgentImplTest {
 
     @Test
     public void absentContainerCausesStart() throws Exception {
-        final long restartGeneration = 1;
+        final Optional<Long> restartGeneration = Optional.of(1L);
         final long rebootGeneration = 0;
         final NodeSpec node = nodeBuilder
                 .wantedDockerImage(dockerImage)
                 .state(Node.State.active)
                 .wantedVespaVersion(vespaVersion)
-                .wantedRestartGeneration(restartGeneration)
-                .currentRestartGeneration(restartGeneration)
+                .wantedRestartGeneration(restartGeneration.get())
+                .currentRestartGeneration(restartGeneration.get())
                 .wantedRebootGeneration(rebootGeneration)
                 .build();
 
@@ -430,7 +430,7 @@ public class NodeAgentImplTest {
         // current Docker image and vespa version should be cleared
         verify(nodeRepository, times(1)).updateNodeAttributes(
                 any(String.class), eq(new NodeAttributes()
-                        .withRestartGeneration(wantedRestartGeneration.orElse(null))
+                        .withRestartGeneration(wantedRestartGeneration)
                         .withRebootGeneration(0L)
                         .withDockerImage(new DockerImage(""))));
     }