Parse vespa version

* Make sure that we handle newlines, irregular versions etc.

2 files changed, 49 insertions, 1 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerImpl.java
index b481a778789..98941ed6fbb 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerImpl.java
@@ -51,6 +51,8 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -64,6 +66,7 @@ public class DockerImpl implements Docker {
     private static final int SECONDS_TO_WAIT_BEFORE_KILLING = 10;
     private static final String FRAMEWORK_CONTAINER_PREFIX = "/";
     static final String[] COMMAND_GET_VESPA_VERSION = new String[]{"vespa-nodectl", "vespa-version"};
+    private static final Pattern VESPA_VERSION_PATTERN = Pattern.compile("^(\\d+\\.\\d+\\S*)", Pattern.MULTILINE);
 
     private static final String LABEL_NAME_MANAGEDBY = "com.yahoo.vespa.managedby";
     private static final String LABEL_VALUE_MANAGEDBY = "node-admin";
@@ -258,8 +261,16 @@ public class DockerImpl implements Docker {
             throw new RuntimeException("Container " + containerName.asString() + ": Command "
                     + Arrays.toString(COMMAND_GET_VESPA_VERSION) + " failed: " + result);
         }
+        return parseVespaVersion(result.getOutput())
+                .orElseThrow(() -> new RuntimeException(
+                        "Container " + containerName.asString() + ": Failed to parse vespa version from "
+                                + result.getOutput()));
+    }
 
-        return result.getOutput();
+    // Returns empty if vespa version cannot be parsed.
+    static Optional<String> parseVespaVersion(final String rawVespaVersion) {
+        final Matcher matcher = VESPA_VERSION_PATTERN.matcher(rawVespaVersion);
+        return matcher.find() ? Optional.of(matcher.group(1)) : Optional.empty();
     }
 
     @Override
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerImplTest.java
index 9066019b2a2..cc297d8eef7 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerImplTest.java
@@ -17,6 +17,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CountDownLatch;
@@ -85,6 +86,42 @@ public class DockerImplTest {
     }
 
     @Test
+    public void vespaVersionIsParsed() {
+        assertThat(DockerImpl.parseVespaVersion("5.119.53"), is(Optional.of("5.119.53")));
+    }
+
+    @Test
+    public void vespaVersionIsParsedWithTrailingNewline() {
+        assertThat(DockerImpl.parseVespaVersion("5.119.53\n"), is(Optional.of("5.119.53")));
+    }
+
+    @Test
+    public void vespaVersionIsParsedWithIrregularVersionScheme() {
+        assertThat(DockerImpl.parseVespaVersion("7.2"), is(Optional.of("7.2")));
+        assertThat(DockerImpl.parseVespaVersion("8.0-beta"), is(Optional.of("8.0-beta")));
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void vespaVersionIsNotParsedFromNull() {
+        assertThat(DockerImpl.parseVespaVersion(null), is(Optional.empty()));
+    }
+
+    @Test
+    public void vespaVersionIsNotParsedFromEmptyString() {
+        assertThat(DockerImpl.parseVespaVersion(""), is(Optional.empty()));
+    }
+
+    @Test
+    public void vespaVersionIsNotParsedFromUnexpectedContent() {
+        assertThat(DockerImpl.parseVespaVersion("foo"), is(Optional.empty()));
+        assertThat(DockerImpl.parseVespaVersion("119"), is(Optional.empty()));
+        assertThat(DockerImpl.parseVespaVersion("vespa-5.119.53"), is(Optional.empty()));
+        assertThat(DockerImpl.parseVespaVersion("vespa- 5.119.53"), is(Optional.empty()));
+        assertThat(DockerImpl.parseVespaVersion("vespa-"), is(Optional.empty()));
+        assertThat(DockerImpl.parseVespaVersion("No such command 'vespanodectl'"), is(Optional.empty()));
+    }
+
+    @Test
     public void testExecuteCompletes() throws Exception {
         final DockerClient dockerClient = mock(DockerClient.class);
         final String containerId = "container-id";