Split yum command implementations to separate class

3 files changed, 214 insertions, 192 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 4fde82f6fd6..27054f6ed01 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
@@ -2,33 +2,28 @@
 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.CommandLine;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Function;
-import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
 
+import static com.yahoo.vespa.hosted.node.admin.task.util.yum.YumCommand.GenericYumCommand;
+import static com.yahoo.vespa.hosted.node.admin.task.util.yum.YumCommand.InstallFixedYumCommand;
+
 /**
  * @author hakonhall
  */
 public class Yum {
     // Note: "(?dm)" makes newline be \n (only), and enables multiline mode where ^$ match lines with find()
-    private static final Pattern CHECKING_FOR_UPDATE_PATTERN =
-            Pattern.compile("(?dm)^Package matching [^ ]+ already installed\\. Checking for update\\.$");
-    private static final Pattern NOTHING_TO_DO_PATTERN = Pattern.compile("(?dm)^Nothing to do$");
-    private static final Pattern INSTALL_NOOP_PATTERN = NOTHING_TO_DO_PATTERN;
+    private static final Pattern INSTALL_NOOP_PATTERN = Pattern.compile("(?dm)^Nothing to do$");
     private static final Pattern UPGRADE_NOOP_PATTERN = Pattern.compile("(?dm)^No packages marked for update$");
     private static final Pattern REMOVE_NOOP_PATTERN = Pattern.compile("(?dm)^No Packages marked for removal$");
-    private static final Pattern UNKNOWN_PACKAGE_PATTERN = Pattern.compile(
-            "(?dm)^No package ([^ ]+) available\\.$");
 
 
     // WARNING: These must be in the same order as the supplier below
@@ -64,90 +59,13 @@ public class Yum {
         return Optional.of(builder.build());
     }
 
-    /**
-     * Lock and install, or if necessary downgrade, a package to a given version.
-     *
-     * @return false only if the package was already locked and installed at the given version (no-op)
-     */
-    public boolean installFixedVersion(TaskContext context, YumPackageName yumPackage, String... repos) {
-        String targetVersionLockName = yumPackage.toVersionLockName();
-
-        boolean alreadyLocked = terminal
-                .newCommandLine(context)
-                .add("yum", "--quiet", "versionlock", "list")
-                .executeSilently()
-                .getOutputLinesStream()
-                .map(YumPackageName::parseString)
-                .filter(Optional::isPresent) // removes garbage first lines, even with --quiet
-                .map(Optional::get)
-                .anyMatch(packageName -> {
-                    // Ignore lines for other packages
-                    if (packageName.getName().equals(yumPackage.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.toVersionLockName();
-                        if (versionLockName.equals(targetVersionLockName)) {
-                            return true;
-                        } else {
-                            terminal.newCommandLine(context)
-                                    .add("yum", "versionlock", "delete", versionLockName)
-                                    .execute();
-                        }
-                    }
-
-                    return false;
-                });
-
-        boolean modified = false;
-
-        if (!alreadyLocked) {
-            CommandLine commandLine = terminal.newCommandLine(context).add("yum", "versionlock", "add");
-            // If the targetVersionLockName refers to a package in a by-default-disabled repo,
-            // we must enable the repo unless targetVersionLockName is already installed.
-            // The other versionlock commands (list, delete) does not require --enablerepo.
-            for (String repo : repos) commandLine.add("--enablerepo=" + repo);
-            commandLine.add(targetVersionLockName).execute();
-            modified = true;
-        }
-
-        // The following 3 things may happen with yum install:
-        //  1. The package is installed or upgraded to the target version, in case we'd return
-        //     true from converge()
-        //  2. The package is already installed at target version, in case
-        //     "Nothing to do" is printed in the last line and we may return false from converge()
-        //  3. The package is already installed but at a later version than the target version,
-        //     in case the last 2 lines of the output is:
-        //       - "Package matching yakl-client-0.10-654.el7.x86_64 already installed. Checking for update."
-        //       - "Nothing to do"
-        //     And in case we need to downgrade and return true from converge()
-
-        var installCommand = terminal.newCommandLine(context).add("yum", "install");
-        for (String repo : repos) installCommand.add("--enablerepo=" + repo);
-        installCommand.add("--assumeyes", yumPackage.toName());
-        String output = installCommand.executeSilently().getUntrimmedOutput();
-
-        if (NOTHING_TO_DO_PATTERN.matcher(output).find()) {
-            if (CHECKING_FOR_UPDATE_PATTERN.matcher(output).find()) {
-                // case 3.
-                var upgradeCommand = terminal.newCommandLine(context).add("yum", "downgrade", "--assumeyes");
-                for (String repo : repos) upgradeCommand.add("--enablerepo=" + repo);
-                upgradeCommand.add(yumPackage.toName()).execute();
-                modified = true;
-            } else {
-                // case 2.
-            }
-        } else {
-            // case 1.
-            installCommand.recordSilentExecutionAsSystemModification();
-            modified = true;
-        }
-
-        return modified;
+    /** Lock and install, or if necessary downgrade, a package to a given version. */
+    public InstallFixedYumCommand installFixedVersion(YumPackageName yumPackage) {
+        return new InstallFixedYumCommand(terminal, yumPackage);
     }
 
-
     public GenericYumCommand install(YumPackageName... packages) {
-        return newYumCommand("install", packages, INSTALL_NOOP_PATTERN);
+        return new GenericYumCommand(terminal, "install", List.of(packages), INSTALL_NOOP_PATTERN);
     }
 
     public GenericYumCommand install(String package1, String... packages) {
@@ -160,7 +78,7 @@ public class Yum {
 
 
     public GenericYumCommand upgrade(YumPackageName... packages) {
-        return newYumCommand("upgrade", packages, UPGRADE_NOOP_PATTERN);
+        return new GenericYumCommand(terminal, "upgrade", List.of(packages), UPGRADE_NOOP_PATTERN);
     }
 
     public GenericYumCommand upgrade(String package1, String... packages) {
@@ -173,7 +91,7 @@ public class Yum {
 
 
     public GenericYumCommand remove(YumPackageName... packages) {
-        return newYumCommand("remove", packages, REMOVE_NOOP_PATTERN);
+        return new GenericYumCommand(terminal, "remove", List.of(packages), REMOVE_NOOP_PATTERN);
     }
 
     public GenericYumCommand remove(String package1, String... packages) {
@@ -192,66 +110,4 @@ public class Yum {
         }
         return array;
     }
-
-
-    private GenericYumCommand newYumCommand(String yumCommand, YumPackageName[] packages, Pattern noopPattern) {
-        return new GenericYumCommand(terminal, yumCommand, List.of(packages), noopPattern);
-    }
-
-    public static class GenericYumCommand {
-        private final Terminal terminal;
-        private final String yumCommand;
-        private final List<YumPackageName> packages;
-        private final Pattern commandOutputNoopPattern;
-
-        private final List<String> enabledRepo = new ArrayList<>();
-
-        private GenericYumCommand(Terminal terminal,
-                                  String yumCommand,
-                                  List<YumPackageName> packages,
-                                  Pattern commandOutputNoopPattern) {
-            this.terminal = terminal;
-            this.yumCommand = yumCommand;
-            this.packages = packages;
-            this.commandOutputNoopPattern = commandOutputNoopPattern;
-
-            if (packages.isEmpty() && ! "upgrade".equals(yumCommand)) {
-                throw new IllegalArgumentException("No packages specified");
-            }
-        }
-
-        public GenericYumCommand enableRepos(String... repos) {
-            enabledRepo.addAll(List.of(repos));
-            return this;
-        }
-
-        public boolean converge(TaskContext context) {
-            CommandLine commandLine = terminal.newCommandLine(context);
-            commandLine.add("yum", yumCommand, "--assumeyes");
-            enabledRepo.forEach(repo -> commandLine.add("--enablerepo=" + repo));
-            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.
-            boolean modifiedSystem = commandLine
-                    .executeSilently()
-                    .mapOutput(this::mapOutput);
-
-            if (modifiedSystem) {
-                commandLine.recordSilentExecutionAsSystemModification();
-            }
-
-            return modifiedSystem;
-        }
-
-        private boolean mapOutput(String output) {
-            Matcher unknownPackageMatcher = UNKNOWN_PACKAGE_PATTERN.matcher(output);
-            if (unknownPackageMatcher.find()) {
-                throw new IllegalArgumentException("Unknown package: " + unknownPackageMatcher.group(1));
-            }
-
-            return !commandOutputNoopPattern.matcher(output).find();
-        }
-    }
-
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java
new file mode 100644
index 00000000000..12029c4fe7e
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumCommand.java
@@ -0,0 +1,187 @@
+// Copyright 2020 Oath Inc. 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 com.yahoo.vespa.hosted.node.admin.component.TaskContext;
+import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandLine;
+import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal;
+
+import java.util.List;
+import java.util.Optional;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * @author freva
+ */
+public abstract class YumCommand<T extends YumCommand<T>> {
+
+    private List<String> enabledRepos = List.of();
+
+    /** Enables the given repos for this command */
+    public T enableRepo(String... repo) {
+        enabledRepos = List.of(repo);
+        return getThis();
+    }
+
+    protected abstract T getThis(); // Hack to get around unchecked cast warning
+
+    protected void addParametersToCommandLine(CommandLine commandLine) {
+        commandLine.add("--assumeyes");
+        enabledRepos.forEach(repo -> commandLine.add("--enablerepo=" + repo));
+    }
+
+    public abstract boolean converge(TaskContext context);
+
+
+    public static class GenericYumCommand extends YumCommand<GenericYumCommand> {
+        private static final Pattern UNKNOWN_PACKAGE_PATTERN = Pattern.compile("(?dm)^No package ([^ ]+) available\\.$");
+
+        private final Terminal terminal;
+        private final String yumCommand;
+        private final Pattern commandOutputNoopPattern;
+        private final List<YumPackageName> packages;
+
+        GenericYumCommand(Terminal terminal, String yumCommand, List<YumPackageName> packages, Pattern commandOutputNoopPattern) {
+            this.terminal = terminal;
+            this.yumCommand = yumCommand;
+            this.packages = packages;
+            this.commandOutputNoopPattern = commandOutputNoopPattern;
+
+            if (packages.isEmpty() && ! "upgrade".equals(yumCommand)) {
+                throw new IllegalArgumentException("No packages specified");
+            }
+        }
+
+        @Override
+        public boolean converge(TaskContext context) {
+            if (packages.isEmpty() && ! "upgrade".equals(yumCommand)) {
+                throw new IllegalArgumentException("No packages specified");
+            }
+
+            CommandLine commandLine = terminal.newCommandLine(context);
+            commandLine.add("yum", yumCommand);
+            addParametersToCommandLine(commandLine);
+            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.
+            boolean modifiedSystem = commandLine
+                    .executeSilently()
+                    .mapOutput(this::mapOutput);
+
+            if (modifiedSystem) {
+                commandLine.recordSilentExecutionAsSystemModification();
+            }
+
+            return modifiedSystem;
+        }
+
+        private boolean mapOutput(String output) {
+            Matcher unknownPackageMatcher = UNKNOWN_PACKAGE_PATTERN.matcher(output);
+            if (unknownPackageMatcher.find()) {
+                throw new IllegalArgumentException("Unknown package: " + unknownPackageMatcher.group(1));
+            }
+
+            return !commandOutputNoopPattern.matcher(output).find();
+        }
+
+        protected GenericYumCommand getThis() { return this; }
+    }
+
+
+    public static class InstallFixedYumCommand extends YumCommand<InstallFixedYumCommand> {
+        // Note: "(?dm)" makes newline be \n (only), and enables multiline mode where ^$ match lines with find()
+        private static final Pattern CHECKING_FOR_UPDATE_PATTERN =
+                Pattern.compile("(?dm)^Package matching [^ ]+ already installed\\. Checking for update\\.$");
+        private static final Pattern NOTHING_TO_DO_PATTERN = Pattern.compile("(?dm)^Nothing to do$");
+
+        private final Terminal terminal;
+        private final YumPackageName yumPackage;
+
+        InstallFixedYumCommand(Terminal terminal, YumPackageName yumPackage) {
+            this.terminal = terminal;
+            this.yumPackage = yumPackage;
+        }
+
+        @Override
+        public boolean converge(TaskContext context) {
+            String targetVersionLockName = yumPackage.toVersionLockName();
+
+            boolean alreadyLocked = terminal
+                    .newCommandLine(context)
+                    .add("yum", "--quiet", "versionlock", "list")
+                    .executeSilently()
+                    .getOutputLinesStream()
+                    .map(YumPackageName::parseString)
+                    .filter(Optional::isPresent) // removes garbage first lines, even with --quiet
+                    .map(Optional::get)
+                    .anyMatch(packageName -> {
+                        // Ignore lines for other packages
+                        if (packageName.getName().equals(yumPackage.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.toVersionLockName();
+                            if (versionLockName.equals(targetVersionLockName)) {
+                                return true;
+                            } else {
+                                terminal.newCommandLine(context)
+                                        .add("yum", "versionlock", "delete", versionLockName)
+                                        .execute();
+                            }
+                        }
+
+                        return false;
+                    });
+
+            boolean modified = false;
+
+            if (!alreadyLocked) {
+                CommandLine commandLine = terminal.newCommandLine(context).add("yum", "versionlock", "add");
+                // If the targetVersionLockName refers to a package in a by-default-disabled repo,
+                // we must enable the repo unless targetVersionLockName is already installed.
+                // The other versionlock commands (list, delete) does not require --enablerepo.
+                addParametersToCommandLine(commandLine);
+                commandLine.add(targetVersionLockName).execute();
+                modified = true;
+            }
+
+            // The following 3 things may happen with yum install:
+            //  1. The package is installed or upgraded to the target version, in case we'd return
+            //     true from converge()
+            //  2. The package is already installed at target version, in case
+            //     "Nothing to do" is printed in the last line and we may return false from converge()
+            //  3. The package is already installed but at a later version than the target version,
+            //     in case the last 2 lines of the output is:
+            //       - "Package matching yakl-client-0.10-654.el7.x86_64 already installed. Checking for update."
+            //       - "Nothing to do"
+            //     And in case we need to downgrade and return true from converge()
+
+            var installCommand = terminal.newCommandLine(context).add("yum", "install");
+            addParametersToCommandLine(installCommand);
+            installCommand.add(yumPackage.toName());
+
+            String output = installCommand.executeSilently().getUntrimmedOutput();
+
+            if (NOTHING_TO_DO_PATTERN.matcher(output).find()) {
+                if (CHECKING_FOR_UPDATE_PATTERN.matcher(output).find()) {
+                    // case 3.
+                    var upgradeCommand = terminal.newCommandLine(context).add("yum", "downgrade");
+                    addParametersToCommandLine(upgradeCommand);
+                    upgradeCommand.add(yumPackage.toName()).execute();
+                    modified = true;
+                } else {
+                    // case 2.
+                }
+            } else {
+                // case 1.
+                installCommand.recordSilentExecutionAsSystemModification();
+                modified = true;
+            }
+
+            return modified;
+        }
+
+        protected InstallFixedYumCommand getThis() { return this; }
+    }
+}
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 f4034b38495..a0887e74e47 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
@@ -9,7 +9,6 @@ import org.junit.Test;
 
 import java.util.Optional;
 
-import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -74,25 +73,6 @@ 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=repo1 --enablerepo=repo2 package-1 package-2 2>&1",
@@ -101,7 +81,7 @@ public class YumTest {
 
         assertFalse(yum
                 .install("package-1", "package-2")
-                .enableRepos("repo1", "repo2")
+                .enableRepo("repo1", "repo2")
                 .converge(taskContext));
     }
 
@@ -150,7 +130,7 @@ public class YumTest {
 
         assertTrue(yum
                 .install("package-1", "package-2")
-                .enableRepos("repo-name")
+                .enableRepo("repo-name")
                 .converge(taskContext));
     }
 
@@ -160,14 +140,13 @@ public class YumTest {
                 0,
                 "Repository chef_rpms-release is listed more than once in the configuration\n" +
                         "0:chef-12.21.1-1.el7.*\n");
-        terminal.expectCommand("yum versionlock add \"0:package-1-0.10-654.el7.*\" 2>&1");
+        terminal.expectCommand("yum versionlock add --assumeyes \"0:package-1-0.10-654.el7.*\" 2>&1");
         terminal.expectCommand(
                 "yum install --assumeyes 0:package-1-0.10-654.el7.x86_64 2>&1",
                 0,
                 "installing");
 
-        assertTrue(yum.installFixedVersion(taskContext,
-                YumPackageName.fromString("0:package-1-0.10-654.el7.x86_64")));
+        assertTrue(yum.installFixedVersion(YumPackageName.fromString("0:package-1-0.10-654.el7.x86_64")).converge(taskContext));
     }
 
     @Test
@@ -180,18 +159,18 @@ public class YumTest {
 
         terminal.expectCommand("yum versionlock delete \"0:package-1-0.1-8.el7.*\" 2>&1");
 
-        terminal.expectCommand("yum versionlock add --enablerepo=somerepo \"0:package-1-0.10-654.el7.*\" 2>&1");
+        terminal.expectCommand("yum versionlock add --assumeyes --enablerepo=somerepo \"0:package-1-0.10-654.el7.*\" 2>&1");
 
         terminal.expectCommand(
-                "yum install --enablerepo=somerepo --assumeyes 0:package-1-0.10-654.el7 2>&1",
+                "yum install --assumeyes --enablerepo=somerepo 0:package-1-0.10-654.el7 2>&1",
                 0,
                 "Nothing to do\n");
 
 
-        assertTrue(yum.installFixedVersion(
-                taskContext,
-                YumPackageName.fromString("0:package-1-0.10-654.el7"),
-                "somerepo"));
+        assertTrue(yum
+                .installFixedVersion(YumPackageName.fromString("0:package-1-0.10-654.el7"))
+                .enableRepo("somerepo")
+                .converge(taskContext));
     }
 
     @Test
@@ -206,7 +185,7 @@ public class YumTest {
                 0,
                 "Nothing to do\n");
 
-        assertFalse(yum.installFixedVersion(taskContext, YumPackageName.fromString("0:package-1-0.10-654.el7")));
+        assertFalse(yum.installFixedVersion(YumPackageName.fromString("0:package-1-0.10-654.el7")).converge(taskContext));
     }
 
     @Test
@@ -225,7 +204,7 @@ public class YumTest {
 
         terminal.expectCommand("yum downgrade --assumeyes 0:package-1-0.10-654.el7 2>&1");
 
-        assertTrue(yum.installFixedVersion(taskContext, YumPackageName.fromString("0:package-1-0.10-654.el7")));
+        assertTrue(yum.installFixedVersion(YumPackageName.fromString("0:package-1-0.10-654.el7")).converge(taskContext));
     }
 
     @Test(expected = ChildProcessFailureException.class)
@@ -235,8 +214,9 @@ public class YumTest {
                 1,
                 "error");
 
-        yum.install("package-1", "package-2")
-                .enableRepos("repo-name")
+        yum
+                .install("package-1", "package-2")
+                .enableRepo("repo-name")
                 .converge(taskContext);
         fail();
     }
@@ -252,10 +232,9 @@ public class YumTest {
                         "No package package-2 available.\n" +
                         "Nothing to do\n");
 
-        Yum.GenericYumCommand install = yum.install("package-1", "package-2", "package-3");
-
+        var command = yum.install("package-1", "package-2", "package-3");
         try {
-            install.converge(taskContext);
+            command.converge(taskContext);
             fail();
         } catch (Exception e) {
             assertNotNull(e.getCause());