Use YumPackageName internally

 - Make install, remove, etc also work with YumPackageName.
 - adds hashCode and equals

4 files changed, 353 insertions, 120 deletions

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 226135a1617..cb23f053086 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
@@ -12,6 +12,7 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 /**
  * @author hakonhall
@@ -31,27 +32,41 @@ public class Yum {
         this.terminal = terminal;
     }
 
-    /**
-     * @param packages A list of packages, each package being of the form name-1.2.3-1.el7.noarch
-     */
-    public GenericYumCommand install(String... packages) {
+    public GenericYumCommand install(YumPackageName... packages) {
         return newYumCommand("install", packages, INSTALL_NOOP_PATTERN);
     }
 
-    /**
-     * @param packages A list of packages, each package being of the form name-1.2.3-1.el7.noarch,
-     *                 if no packages are given, will upgrade all installed packages
-     */
-    public GenericYumCommand upgrade(String... packages) {
+    public GenericYumCommand install(String package1, String... packages) {
+        return install(toYumPackageNameArray(package1, packages));
+    }
+
+    public GenericYumCommand upgrade(YumPackageName... packages) {
         return newYumCommand("upgrade", packages, UPGRADE_NOOP_PATTERN);
     }
 
-    public GenericYumCommand remove(String... packages) {
+    public GenericYumCommand upgrade(String package1, String... packages) {
+        return upgrade(toYumPackageNameArray(package1, packages));
+    }
+
+    public GenericYumCommand remove(YumPackageName... packages) {
         return newYumCommand("remove", packages, REMOVE_NOOP_PATTERN);
     }
 
+    public GenericYumCommand remove(String package1, String... packages) {
+        return remove(toYumPackageNameArray(package1, packages));
+    }
+
+    static YumPackageName[] toYumPackageNameArray(String package1, String... packages) {
+        YumPackageName[] array = new YumPackageName[1 + packages.length];
+        array[0] = YumPackageName.fromString(package1);
+        for (int i = 0; i < packages.length; ++i) {
+            array[1 + i] = YumPackageName.fromString(packages[i]);
+        }
+        return array;
+    }
+
     private GenericYumCommand newYumCommand(String yumCommand,
-                                            String[] packages,
+                                            YumPackageName[] packages,
                                             Pattern noopPattern) {
         return new GenericYumCommand(
                 terminal,
@@ -63,7 +78,7 @@ public class Yum {
     public static class GenericYumCommand {
         private final Terminal terminal;
         private final String yumCommand;
-        private final List<String> packages;
+        private final List<YumPackageName> packages;
         private final Pattern commandOutputNoopPattern;
 
         private Optional<String> enabledRepo = Optional.empty();
@@ -71,7 +86,7 @@ public class Yum {
 
         private GenericYumCommand(Terminal terminal,
                                   String yumCommand,
-                                  List<String> packages,
+                                  List<YumPackageName> packages,
                                   Pattern commandOutputNoopPattern) {
             this.terminal = terminal;
             this.yumCommand = yumCommand;
@@ -93,11 +108,12 @@ public class Yum {
          * Ensure the version of the installs are locked.
          *
          * <p>WARNING: In order to simplify the user interface of {@link #lockVersion()},
-         * the package name specified in the command, e.g. {@link #install(String...)}, MUST be of
+         * the package name specified in the command, e.g. {@link #install(String, String...)}, MUST be of
          * a simple format, see {@link YumPackageName#fromString(String)}.
          */
         public GenericYumCommand lockVersion() {
-            packages.forEach(YumPackageName::fromString); // to throw any parse error here instead of later
+            // Verify each package has sufficient info to form a proper version lock name.
+            packages.forEach(YumPackageName::toVersionLockName);
             lockVersion = true;
             return this;
         }
@@ -109,11 +125,9 @@ public class Yum {
             if (lockVersion) {
                 // Remove all locks for other version
 
-                packages.stream()
-                        .map(YumPackageName::fromString)
-                        .forEach(packageName -> {
+                packages.forEach(packageName -> {
                             packageNamesToLock.add(packageName.getName());
-                            fullPackageNamesToLock.add(packageName.toVersionLock());
+                            fullPackageNamesToLock.add(packageName.toVersionLockName());
                         });
 
                 terminal.newCommandLine(context)
@@ -128,7 +142,7 @@ public class Yum {
                             if (packageNamesToLock.contains(packageName.getName())) {
                                 // If existing lock doesn't exactly match the full package name,
                                 // it means it's locked to another version and we must remove that lock.
-                                String versionLockName = packageName.toVersionLock();
+                                String versionLockName = packageName.toVersionLockName();
                                 if (!fullPackageNamesToLock.remove(versionLockName)) {
                                     terminal.newCommandLine(context)
                                             .add("yum", "versionlock", "delete", versionLockName)
@@ -141,7 +155,7 @@ public class Yum {
             CommandLine commandLine = terminal.newCommandLine(context);
             commandLine.add("yum", yumCommand, "--assumeyes");
             enabledRepo.ifPresent(repo -> commandLine.add("--enablerepo=" + repo));
-            commandLine.add(packages);
+            commandLine.add(packages.stream().map(YumPackageName::toName).collect(Collectors.toList()));
 
             // There's no way to figure out whether a yum command would have been a no-op.
             // Therefore, run the command and parse the output to decide.
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageName.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageName.java
index d6565a67979..d894af9d378 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageName.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageName.java
@@ -1,18 +1,47 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.task.util.yum;
 
+import java.util.Arrays;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 /**
  * YUM package name.
  *
+ * <p>From yum(8): YUM package names are used with install, update, remove, list, info etc
+ * with any of the following as well as globs of any of the following, with any of the
+ * following as well as globs of any of the following:
+ *
+ * <ol>
+ *     <li>name
+ *     <li>name.arch
+ *     <li>name-ver
+ *     <li>name-ver-rel
+ *     <li>name-ver-rel.arch
+ *     <li>name-epoch:ver-rel.arch
+ *     <li>epoch:name-ver-rel.arch
+ * </ol>
+ *
+ * <p>However this specification is terribly ambiguous. This class allows constructing
+ * a package name from its components, which is beneficial because with certain YUM
+ * commands that needs to canonicalize names (e.g. versionlock).
+ *
  * @author hakonhall
  */
 public class YumPackageName {
-    private static final Pattern ARCHITECTURE_PATTERN = Pattern.compile("\\.(noarch|x86_64|i686|i386|\\*)$");
-    private static final Pattern NAME_VER_REL_PATTERN = Pattern.compile("^(.+)-([^-]*[0-9][^-]*)-([^-]*[0-9][^-]*)$");
+    private enum Architecture { noarch, x86_64, i386, i586, i686 }
+
+    private static final String ARCHITECTURES_OR =
+            Arrays.stream(Architecture.values()).map(Architecture::name).collect(Collectors.joining("|"));
+    private static final Pattern ARCHITECTURE_PATTERN = Pattern.compile("\\.(" + ARCHITECTURES_OR + "|\\*)$");
+    private static final Pattern EPOCH_PATTERN = Pattern.compile("^((.+)-)?([0-9]+)$");
+    private static final Pattern NAME_VER_REL_PATTERN = Pattern.compile("^((.+)-)?" +
+            "([a-z0-9._]*[0-9][a-z0-9._]*)-" + // ver contains at least one digit
+            "([a-z0-9._]*[0-9][a-z0-9._]*)$"); // rel contains at least one digit
+    private static final Pattern NAME_PATTERN = Pattern.compile("^[a-z0-9._-]+$");
 
     public final Optional<String> epoch;
     public final String name;
@@ -20,6 +49,34 @@ public class YumPackageName {
     public final Optional<String> release;
     public final Optional<String> architecture;
 
+    public static class Builder {
+        private Optional<String> epoch = Optional.empty();
+        private String name;
+        private Optional<String> version = Optional.empty();
+        private Optional<String> release = Optional.empty();
+        private Optional<String> architecture = Optional.empty();
+
+        public Builder(String name) {
+            this.name = name;
+        }
+
+        public Builder(YumPackageName packageName) {
+            epoch = packageName.epoch;
+            name = packageName.name;
+            version = packageName.version;
+            release = packageName.release;
+            architecture = packageName.architecture;
+        }
+
+        public Builder setEpoch(String epoch) { this.epoch = Optional.of(epoch); return this; }
+        public Builder setName(String name) { this.name = name; return this; }
+        public Builder setRelease(String version) { this.version = Optional.of(version); return this; }
+        public Builder setVersion(String release) { this.release = Optional.of(release); return this; }
+        public Builder setArchitecture(String architecture) { this.architecture = Optional.of(architecture); return this; }
+
+        public YumPackageName build() { return new YumPackageName(epoch, name, version, release, architecture); }
+    }
+
     /** @see Builder */
     private YumPackageName(Optional<String> epoch,
                            String name,
@@ -36,39 +93,54 @@ public class YumPackageName {
     /**
      * Parse the string specification of a YUM package.
      *
-     * <p>According to yum(8) a package can be specified using a variety of different
-     * and ambiguous formats. We'll use a subset:
+     * <p>The following formats are supported:
      *
-     * <ul>
-     *     <li>spec MUST be of the form name-ver-rel, name-ver-rel.arch, or epoch:name-ver-rel.arch.
-     *     <li>If specified, arch should be one of "noarch", "i686", "x86_64", or "*". The wildcard
-     *     is equivalent to not specifying arch.
-     *     <li>rel cannot end in something that would be mistaken for the '.arch' suffix.
-     *     <li>ver and rel are assumed to not contain any '-' to uniquely identify name,
-     *     and must contain a digit.
-     * </ul>
+     * <ol>
+     *     <li>name
+     *     <li>name.arch
+     *     <li>name-ver-rel
+     *     <li>name-ver-rel.arch
+     *     <li>name-epoch:ver-rel.arch
+     *     <li>epoch:name-ver-rel.arch
+     * </ol>
      *
-     * @param spec A package name of the form epoch:name-ver-rel.arch, name-ver-rel.arch, or name-ver-rel.
-     * @return The package with that name.
      * @throws IllegalArgumentException if spec does not specify a package name.
+     * @see #parseString(String)
      */
-    public static YumPackageName fromString(String spec) {
-        return parseString(spec).orElseThrow(() -> new IllegalArgumentException("Failed to decode the YUM package spec '" + spec + "'"));
-    }
-
-    /** See {@link #fromString(String)}. */
-    public static Optional<YumPackageName> parseString(String spec) {
+    public static YumPackageName fromString(final String packageSpec) {
+        String spec = packageSpec;
         Optional<String> epoch = Optional.empty();
+        String name = null;
+
+        //       packageSpec                spec
+        //  name                       name
+        //  name.arch                  name.arch
+        //  name-ver-rel               name-ver-rel
+        //  name-ver-rel.arch          name-ver-rel.arch
+        //  name-epoch:ver-rel.arch    name-epoch:ver-rel.arch
+        //  epoch:name-ver-rel.arch    epoch:name-ver-rel.arch
+
         int epochColon = spec.indexOf(':');
         if (epochColon >= 0) {
-            epoch = Optional.of(spec.substring(0, epochColon));
-            if (!epoch.get().chars().allMatch(Character::isDigit)) {
-                throw new IllegalArgumentException("Epoch is not a number: " + epoch.get());
+            Matcher epochMatcher = EPOCH_PATTERN.matcher(spec.substring(0, epochColon));
+            if (!epochMatcher.find()) {
+                throw new IllegalArgumentException("Unexpected epoch format: " + packageSpec);
             }
 
+            name = epochMatcher.group(2);
+            epoch = Optional.of(epochMatcher.group(3));
+
             spec = spec.substring(epochColon + 1);
         }
 
+        //       packageSpec                spec
+        //  name                       name
+        //  name.arch                  name.arch
+        //  name-ver-rel               name-ver-rel
+        //  name-ver-rel.arch          name-ver-rel.arch
+        //  name-epoch:ver-rel.arch    ver-rel.arch (non-null name)
+        //  epoch:name-ver-rel.arch    name-ver-rel.arch
+
         Optional<String> architecture = Optional.empty();
         Matcher architectureMatcher = ARCHITECTURE_PATTERN.matcher(spec);
         if (architectureMatcher.find()) {
@@ -76,18 +148,60 @@ public class YumPackageName {
             spec = spec.substring(0, architectureMatcher.start());
         }
 
+        //       packageSpec                spec
+        //  name                       name
+        //  name.arch                  name
+        //  name-ver-rel               name-ver-rel
+        //  name-ver-rel.arch          name-ver-rel
+        //  name-epoch:ver-rel.arch    ver-rel (non-null name)
+        //  epoch:name-ver-rel.arch    name-ver-rel
 
+        Optional<String> version = Optional.empty();
+        Optional<String> release = Optional.empty();
         Matcher matcher = NAME_VER_REL_PATTERN.matcher(spec);
         if (matcher.find()) {
-            return Optional.of(new YumPackageName(
-                    epoch,
-                    matcher.group(1),
-                    Optional.of(matcher.group(2)),
-                    Optional.of(matcher.group(3)),
-                    architecture));
+            // spec format one of:
+            //  1. name-ver-rel
+            //  2. ver-rel
+
+            spec = matcher.group(2);
+            if (spec == null) {
+                if (name == null) {
+                    throw new IllegalArgumentException("No package name was found: " + packageSpec);
+                }
+                spec = name; // makes spec hold the package name in all cases below.
+            } else if (name != null) {
+                throw new IllegalArgumentException("Ambiguous package names were found for " +
+                        packageSpec + ": '" + name + "' and '" + spec + "'");
+            }
+
+            version = Optional.of(matcher.group(3));
+            release = Optional.of(matcher.group(4));
         }
 
-        return Optional.empty();
+        //       packageSpec                spec
+        //  name                       name
+        //  name.arch                  name
+        //  name-ver-rel               name
+        //  name-ver-rel.arch          name
+        //  name-epoch:ver-rel.arch    name
+        //  epoch:name-ver-rel.arch    name
+
+        if (!NAME_PATTERN.matcher(spec).find()) {
+            throw new IllegalArgumentException("Bad package name in " + packageSpec + ": '" + spec + "'");
+        }
+        name = spec;
+
+        return new YumPackageName(epoch, name, version, release, architecture);
+    }
+
+    /** See {@link #fromString(String)}. */
+    public static Optional<YumPackageName> parseString(final String packageSpec) {
+        try {
+            return Optional.of(fromString(packageSpec));
+        } catch (IllegalArgumentException e) {
+            return Optional.empty();
+        }
     }
 
     public Optional<String> getEpoch() { return epoch; }
@@ -96,53 +210,46 @@ public class YumPackageName {
     public Optional<String> getRelease() { return release; }
     public Optional<String> getArchitecture() { return architecture; }
 
+    /** Return package name, omitting components that are not specified. */
+    public String toName() {
+        StringBuilder builder = new StringBuilder();
+        epoch.ifPresent(ep -> builder.append(ep).append(':'));
+        builder.append(name);
+        version.ifPresent(ver -> builder.append('-').append(ver));
+        release.ifPresent(rel -> builder.append('-').append(rel));
+        architecture.ifPresent(arch -> builder.append('.').append(arch));
+        return builder.toString();
+    }
+
     /**
-     * Return the full name of the package in the format epoch:name-ver-rel.arch, which can
-     * be used with e.g. the YUM install and versionlock commands.
+     * The package name output by 'yum versionlock list'. Can also be used with 'add' and 'delete'.
      *
-     * <p>The package MUST have both version and release. Absent epoch defaults to "0".
-     * Absent arch defaults to "*".
+     * @throws IllegalStateException if any field required for the version lock spec is missing
      */
-    public String toFullName() {
+    public String toVersionLockName() {
         return String.format("%s:%s-%s-%s.%s",
-                epoch.orElse("0"),
-                name,
-                version.orElseThrow(() -> new IllegalStateException("Version is missing for YUM package " + name)),
-                release.orElseThrow(() -> new IllegalStateException("Release is missing for YUM package " + name)),
-                architecture.orElse("*"));
-    }
-
-    /** The package name output by 'yum versionlock list'. Can also be used with 'add' and 'delete'. */
-    public String toVersionLock() {
-        return String.format("%s:%s-%s-%s.%s",
-                epoch.orElse("0"),
+                epoch.orElseThrow(() -> new IllegalStateException("Epoch is missing for YUM package " + name)),
                 name,
                 version.orElseThrow(() -> new IllegalStateException("Version is missing for YUM package " + name)),
                 release.orElseThrow(() -> new IllegalStateException("Release is missing for YUM package " + name)),
                 "*");
     }
 
-    public static class Builder {
-        private Optional<String> epoch;
-        private String name;
-        private Optional<String> version;
-        private Optional<String> release;
-        private Optional<String> architecture;
-
-        public Builder(YumPackageName aPackage) {
-            epoch = aPackage.epoch;
-            name = aPackage.name;
-            version = aPackage.version;
-            release = aPackage.release;
-            architecture = aPackage.architecture;
-        }
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        YumPackageName that = (YumPackageName) o;
+        return Objects.equals(epoch, that.epoch) &&
+                Objects.equals(name, that.name) &&
+                Objects.equals(version, that.version) &&
+                Objects.equals(release, that.release) &&
+                Objects.equals(architecture, that.architecture);
+    }
 
-        public Builder setEpoch(String epoch) { this.epoch = Optional.of(epoch); return this; }
-        public Builder setName(String name) { this.name = name; return this; }
-        public Builder setRelease(String version) { this.version = Optional.of(version); return this; }
-        public Builder setVersion(String release) { this.release = Optional.of(release); return this; }
-        public Builder setArchitecture(String architecture) { this.architecture = Optional.of(architecture); return this; }
+    @Override
+    public int hashCode() {
 
-        public YumPackageName build() { return new YumPackageName(epoch, name, version, release, architecture); }
+        return Objects.hash(epoch, name, version, release, architecture);
     }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageNameTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageNameTest.java
index 911a5b64b33..2e1ef4c0a61 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageNameTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumPackageNameTest.java
@@ -3,49 +3,124 @@ package com.yahoo.vespa.hosted.node.admin.task.util.yum;
 
 import org.junit.Test;
 
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.containsStringIgnoringCase;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 
 public class YumPackageNameTest {
 
     @Test
-    public void parsePackageName() {
-        YumPackageName packageName = YumPackageName.fromString("docker-engine-selinux-1.12.6-1.el7");
-        assertFalse(packageName.getEpoch().isPresent());
-        assertEquals("docker-engine-selinux", packageName.getName());
-        assertEquals("1.12.6", packageName.getVersion().get());
-        assertEquals("1.el7", packageName.getRelease().get());
-        assertFalse(packageName.getArchitecture().isPresent());
-        assertEquals("0:docker-engine-selinux-1.12.6-1.el7.*", packageName.toFullName());
+    public void testAllValidFormats() {
+        // name
+        verifyPackageName(
+                "docker-engine-selinux",
+                null,
+                "docker-engine-selinux",
+                null,
+                null,
+                null,
+                "docker-engine-selinux",
+                null);
+
+        // name.arch
+        verifyPackageName(
+                "docker-engine-selinux.x86_64",
+                null,
+                "docker-engine-selinux",
+                null,
+                null,
+                "x86_64",
+                "docker-engine-selinux.x86_64",
+                null);
+
+        // name-ver-rel
+        verifyPackageName("docker-engine-selinux-1.12.6-1.el7",
+                null,
+                "docker-engine-selinux",
+                "1.12.6",
+                "1.el7",
+                null,
+                "docker-engine-selinux-1.12.6-1.el7",
+                null);
+
+        // name-ver-rel.arch
+        verifyPackageName("docker-engine-selinux-1.12.6-1.el7.x86_64",
+                null,
+                "docker-engine-selinux",
+                "1.12.6",
+                "1.el7",
+                "x86_64",
+                "docker-engine-selinux-1.12.6-1.el7.x86_64",
+                null);
+
+        // name-epoch:ver-rel.arch
+        verifyPackageName(
+                "docker-2:1.12.6-71.git3e8e77d.el7.centos.1.x86_64",
+                "2",
+                "docker",
+                "1.12.6",
+                "71.git3e8e77d.el7.centos.1",
+                "x86_64",
+                "2:docker-1.12.6-71.git3e8e77d.el7.centos.1.x86_64",
+                "2:docker-1.12.6-71.git3e8e77d.el7.centos.1.*");
+
+        // epoch:name-ver-rel.arch
+        verifyPackageName(
+                "2:docker-1.12.6-71.git3e8e77d.el7.centos.1.x86_64",
+                "2",
+                "docker",
+                "1.12.6",
+                "71.git3e8e77d.el7.centos.1",
+                "x86_64",
+                "2:docker-1.12.6-71.git3e8e77d.el7.centos.1.x86_64",
+                "2:docker-1.12.6-71.git3e8e77d.el7.centos.1.*");
     }
 
-    @Test
-    public void parsePackageNameWithArchitecture() {
-        YumPackageName packageName = YumPackageName.fromString("docker-engine-selinux-1.12.6-1.el7.x86_64");
-        assertFalse(packageName.getEpoch().isPresent());
-        assertEquals("docker-engine-selinux", packageName.getName());
-        assertEquals("1.12.6", packageName.getVersion().get());
-        assertEquals("1.el7", packageName.getRelease().get());
-        assertEquals("x86_64", packageName.getArchitecture().get());
-        assertEquals("0:docker-engine-selinux-1.12.6-1.el7.x86_64", packageName.toFullName());
-        assertEquals("0:docker-engine-selinux-1.12.6-1.el7.*", packageName.toVersionLock());
+    private void verifyPackageName(String packageName,
+                                   String epoch,
+                                   String name,
+                                   String version,
+                                   String release,
+                                   String architecture,
+                                   String toName,
+                                   String toVersionName) {
+        YumPackageName yumPackageName = YumPackageName.fromString(packageName);
+        verifyValue(epoch, yumPackageName.getEpoch());
+        verifyValue(name, Optional.of(yumPackageName.getName()));
+        verifyValue(version, yumPackageName.getVersion());
+        verifyValue(release, yumPackageName.getRelease());
+        verifyValue(architecture, yumPackageName.getArchitecture());
+        verifyValue(toName, Optional.of(yumPackageName.toName()));
+
+        if (toVersionName == null) {
+            try {
+                yumPackageName.toVersionLockName();
+                fail();
+            } catch (IllegalStateException e) {
+                assertThat(e.getMessage(), containsStringIgnoringCase("epoch is missing"));
+            }
+        } else {
+            assertEquals(toVersionName, yumPackageName.toVersionLockName());
+        }
     }
 
-    @Test
-    public void parsePackageNameWithEpochAndArchitecture() {
-        YumPackageName packageName = YumPackageName.fromString("1:docker-engine-selinux-1.12.6-1.el7.x86_64");
-        assertEquals("1", packageName.getEpoch().get());
-        assertEquals("docker-engine-selinux", packageName.getName());
-        assertEquals("1.12.6", packageName.getVersion().get());
-        assertEquals("1.el7", packageName.getRelease().get());
-        assertEquals("x86_64", packageName.getArchitecture().get());
-        assertEquals("1:docker-engine-selinux-1.12.6-1.el7.x86_64", packageName.toFullName());
-        assertEquals("1:docker-engine-selinux-1.12.6-1.el7.*", packageName.toVersionLock());
+    private void verifyValue(String value, Optional<String> actual) {
+        if (value == null) {
+            assertFalse(actual.isPresent());
+        } else {
+            assertEquals(value, actual.get());
+        }
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void failParsingOfPackageName() {
-        YumPackageName.fromString("docker-engine-selinux");
+    @Test
+    public void testArchitectures() {
+        assertEquals("x86_64", YumPackageName.fromString("docker.x86_64").getArchitecture().get());
+        assertEquals("i686", YumPackageName.fromString("docker.i686").getArchitecture().get());
+        assertEquals("noarch", YumPackageName.fromString("docker.noarch").getArchitecture().get());
     }
 
     @Test
@@ -55,8 +130,13 @@ public class YumPackageNameTest {
         assertEquals("1.el7.i486", packageName.getRelease().get());
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void failParsingOfPackageNameWithEpochAndArchitecture() {
-        YumPackageName.fromString("epoch:docker-engine-selinux-1.12.6-1.el7.x86_64");
+        try {
+            YumPackageName.fromString("epoch:docker-engine-selinux-1.12.6-1.el7.x86_64");
+            fail();
+        } catch (IllegalArgumentException e) {
+            assertThat(e.getMessage(), containsStringIgnoringCase("epoch"));
+        }
     }
 }
\ No newline at end of file
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java
index f2a2306263a..a635dd6a44d 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java
@@ -4,12 +4,15 @@ package com.yahoo.vespa.hosted.node.admin.task.util.yum;
 import com.yahoo.vespa.hosted.node.admin.component.TaskContext;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.ChildProcessFailureException;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal;
+import org.hamcrest.CoreMatchers;
 import org.junit.After;
 import org.junit.Test;
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
@@ -25,6 +28,25 @@ public class YumTest {
     }
 
     @Test
+    public void testArrayConversion() {
+        YumPackageName[] expected = new YumPackageName[] { new YumPackageName.Builder("1").build() };
+        assertArrayEquals(expected, Yum.toYumPackageNameArray("1"));
+
+        YumPackageName[] expected2 = new YumPackageName[] {
+                new YumPackageName.Builder("1").build(),
+                new YumPackageName.Builder("2").build()
+        };
+        assertArrayEquals(expected2, Yum.toYumPackageNameArray("1", "2"));
+
+        YumPackageName[] expected3 = new YumPackageName[] {
+                new YumPackageName.Builder("1").build(),
+                new YumPackageName.Builder("2").build(),
+                new YumPackageName.Builder("3").build()
+        };
+        assertArrayEquals(expected3, Yum.toYumPackageNameArray("1", "2", "3"));
+    }
+
+    @Test
     public void testAlreadyInstalled() {
         terminal.expectCommand(
                 "yum install --assumeyes --enablerepo=repo-name package-1 package-2 2>&1",
@@ -145,6 +167,16 @@ public class YumTest {
                 .converge(taskContext));
     }
 
+    @Test
+    public void testBadPackageNameWithLock() {
+        try {
+            yum.install("package-1-0.10-654.el7").lockVersion();
+            fail();
+        } catch (IllegalStateException e) {
+            assertThat(e.getMessage(), CoreMatchers.containsStringIgnoringCase("epoch is missing"));
+        }
+    }
+
     @Test(expected = ChildProcessFailureException.class)
     public void testFailedInstall() {
         terminal.expectCommand(