diff options
author | Håkon Hallingstad <hakon@oath.com> | 2018-04-25 09:27:27 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@oath.com> | 2018-04-25 09:27:27 +0200 |
commit | 0764bef8ed946308b227e4d6617b7d1b7bd87a74 (patch) | |
tree | 94293e68006a5e41e0fdac5ff03479e7189b5604 /node-admin | |
parent | af63325a5e8dba303caa87c8fca869a10037fb7f (diff) |
Fix NPE
The problem was the null DockerImage's DockerImage::asString() method was
called in NodeAttributes::toString.
Diffstat (limited to 'node-admin')
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("")))); } |