Merge pull request #12830 from vespa-engine/freva/fix-version

Create YumTester

7 files changed, 492 insertions, 269 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtlTester.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtlTester.java
index c7264c2fe4d..82906253f24 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtlTester.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtlTester.java
@@ -3,8 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.task.util.systemd;
 
 import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal;
 
-import java.util.HashSet;
-import java.util.Set;
+import java.util.function.Consumer;
 
 /**
  * A {@link SystemCtl} tester that simplifies testing interaction with systemd units.
@@ -13,8 +12,6 @@ import java.util.Set;
  */
 public class SystemCtlTester extends SystemCtl {
 
-    private final Set<String> runningUnits = new HashSet<>();
-
     private final TestTerminal terminal;
 
     public SystemCtlTester(TestTerminal terminal) {
@@ -22,93 +19,64 @@ public class SystemCtlTester extends SystemCtl {
         this.terminal = terminal;
     }
 
-    /** Create expectation for given unit */
-    public Expectation expect(String unit) {
-        return new Expectation(unit, this);
+    public Expectation expectServiceExists(String unit) {
+        return new Expectation(wantedReturn ->
+                expectCommand("systemctl list-unit-files " + unit + ".service 2>&1", 0, (wantedReturn ? 1 : 0) + " unit files listed."));
     }
 
-    private void startUnit(String unit) {
-        runningUnits.add(unit);
+    public Expectation expectIsActive(String unit) {
+        return new Expectation(wantedReturn -> {
+            expectCommand("systemctl --quiet is-active " + unit + ".service 2>&1", wantedReturn ? 0 : 1, "");
+        });
     }
 
-    private void expectCommand(String command, int exitCode, String output) {
-        terminal.expectCommand((useSudo() ? "sudo " : "") + command, exitCode, output);
-    }
+    public Expectation expectEnable(String unit) { return forChangeEnabledState(unit, true); }
+    public Expectation expectDisable(String unit) { return forChangeEnabledState(unit, false); }
+    public Expectation expectStart(String unit) { return forChangeRunningState(unit, true); }
+    public Expectation expectStop(String unit) { return forChangeRunningState(unit, false); }
 
-    public static class Expectation {
-
-        private final String unit;
-        private final SystemCtlTester systemCtl;
-
-        public Expectation(String unit, SystemCtlTester systemCtl) {
-            this.unit = unit;
-            this.systemCtl = systemCtl;
-        }
-
-        /** Create expectation for given unit */
-        public Expectation expect(String name) {
-            return systemCtl.expect(name);
-        }
-
-        /** Expect that this will be started */
-        public Expectation toStart() {
-            return toStart(true);
-        }
-
-        /** Expect that this is already started */
-        public Expectation isStarted() {
-            return toStart(false);
-        }
+    public SystemCtlTester expectRestart(String unit) {
+        expectCommand("systemctl restart " + unit + " 2>&1", 0, "");
+        return this;
+    }
 
-        /** Expect that given unit will be restarted */
-        public Expectation toRestart() {
-            systemCtl.expectCommand("systemctl restart " + unit + " 2>&1", 0, "");
-            systemCtl.startUnit(unit);
-            return this;
-        }
+    public SystemCtlTester expectDaemonReload() {
+        expectCommand("systemctl daemon-reload 2>&1", 0, "");
+        return this;
+    }
 
-        /** Expect that this will be stopped */
-        public Expectation toStop() {
-            systemCtl.expectCommand("systemctl stop " + unit + " 2>&1", 0, "");
-            systemCtl.runningUnits.remove(unit);
-            return this;
-        }
 
-        /** Expect query for state of this */
-        public Expectation toQueryState() {
-            systemCtl.expectCommand("systemctl --quiet is-active " + unit + ".service 2>&1",
-                                    systemCtl.runningUnits.contains(unit) ? 0 : 1, "");
-            return this;
-        }
+    private void expectCommand(String command, int exitCode, String output) {
+        terminal.expectCommand((useSudo() ? "sudo " : "") + command, exitCode, output);
+    }
 
-        /** Expect that this will be enabled */
-        public Expectation toEnable() {
-            return toEnable(true);
-        }
+    private Expectation forChangeEnabledState(String unit, boolean enable) {
+        return new Expectation(wantedReturn -> {
+            expectCommand("systemctl --quiet is-enabled " + unit + " 2>&1", enable != wantedReturn ? 0 : 1, "");
+            if (wantedReturn)
+                expectCommand("systemctl " + (enable ? "enable" : "disable") + " " + unit + " 2>&1", 0, "");
+        });
+    }
 
-        /** Expect that given unit is already enabled */
-        public Expectation isEnabled() {
-            return toEnable(false);
-        }
+    private Expectation forChangeRunningState(String unit, boolean start) {
+        return new Expectation(wantedReturn -> {
+            expectCommand("systemctl show " + unit + " 2>&1", 0, "ActiveState=" + (start != wantedReturn ? "active" : "inactive"));
+            if (wantedReturn)
+                expectCommand("systemctl " + (start ? "start" : "stop") + " " + unit + " 2>&1", 0, "");
+        });
+    }
 
-        private Expectation toStart(boolean start) {
-            systemCtl.expectCommand("systemctl show " + unit + " 2>&1", 0,
-                                    "ActiveState=" + (start ? "inactive" : "active"));
-            if (start) {
-                systemCtl.expectCommand("systemctl start " + unit + " 2>&1", 0, "");
-                systemCtl.startUnit(unit);
-            }
-            return this;
+    public class Expectation {
+        private final Consumer<Boolean> converger;
+        public Expectation(Consumer<Boolean> converger) {
+            this.converger = converger;
         }
 
-        private Expectation toEnable(boolean enable) {
-            systemCtl.expectCommand("systemctl --quiet is-enabled " + unit + " 2>&1", enable ? 1 : 0, "");
-            if (enable) {
-                systemCtl.expectCommand("systemctl enable " + unit + " 2>&1", 0, "");
-            }
-            return this;
+        /** Mock the return value of the converge(TaskContext) method for this operation (true iff system was modified) */
+        public SystemCtlTester andReturn(boolean value) {
+            converger.accept(value);
+            return SystemCtlTester.this;
         }
-
     }
 
 }
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/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java
new file mode 100644
index 00000000000..4d04e1199c5
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTester.java
@@ -0,0 +1,120 @@
+// 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.task.util.process.TestChildProcess2;
+import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ * A {@link Yum} tester that simplifies testing interaction with yum.
+ *
+ * @author freva
+ */
+public class YumTester extends Yum {
+
+    private final TestTerminal terminal;
+
+    public YumTester(TestTerminal terminal) {
+        super(terminal);
+        this.terminal = terminal;
+    }
+
+    public GenericYumCommandExpectation expectInstall(String... packages) {
+        return new GenericYumCommandExpectation("install", packages);
+    }
+
+    public GenericYumCommandExpectation expectUpdate(String... packages) {
+        return new GenericYumCommandExpectation("upgrade", packages);
+    }
+
+    public GenericYumCommandExpectation expectRemove(String... packages) {
+        return new GenericYumCommandExpectation("remove", packages);
+    }
+
+    public InstallFixedCommandExpectation expectInstallFixedVersion(String yumPackage) {
+        return new InstallFixedCommandExpectation(yumPackage);
+    }
+
+    public QueryInstalledExpectation expectQueryInstalled(String packageName) {
+        return new QueryInstalledExpectation(packageName);
+    }
+
+
+    public class GenericYumCommandExpectation {
+        private final String command;
+        protected final List<YumPackageName> packages;
+        private List<String> enableRepos = List.of();
+
+        private GenericYumCommandExpectation(String command, String... packages) {
+            this.command = command;
+            this.packages = Stream.of(packages).map(YumPackageName::fromString).collect(Collectors.toList());
+        }
+
+        public GenericYumCommandExpectation withEnableRepo(String... repo) {
+            this.enableRepos = List.of(repo);
+            return this;
+        }
+
+        /** Mock the return value of the converge(TaskContext) method for this operation (true iff system was modified) */
+        public YumTester andReturn(boolean value) {
+            if (value) return execute("Success");
+            switch (command) {
+                case "install": return execute("Nothing to do");
+                case "upgrade": return execute("No packages marked for update");
+                case "remove": return execute("No Packages marked for removal");
+                default: throw new IllegalArgumentException("Unknown command: " + command);
+            }
+        }
+
+        private YumTester execute(String output) {
+            StringBuilder cmd = new StringBuilder();
+            cmd.append("yum ").append(command).append(" --assumeyes");
+            enableRepos.forEach(repo -> cmd.append(" --enablerepo=").append(repo));
+            packages.forEach(pkg -> cmd.append(" ").append(pkg.toName()));
+            cmd.append(" 2>&1");
+
+            terminal.expectCommand(cmd.toString(), 0, output);
+            return YumTester.this;
+        }
+    }
+
+    public class InstallFixedCommandExpectation extends GenericYumCommandExpectation {
+        private InstallFixedCommandExpectation(String yumPackage) {
+            super("install", yumPackage);
+        }
+
+        @Override
+        public YumTester andReturn(boolean value) {
+            // Pretend package is already correctly version locked to simplify expectations
+            terminal.expectCommand("yum --quiet versionlock list 2>&1", 0, packages.get(0).toVersionLockName());
+            return super.andReturn(value);
+        }
+    }
+
+    public class QueryInstalledExpectation {
+        private final String packageName;
+
+        public QueryInstalledExpectation(String packageName) {
+            this.packageName = packageName;
+        }
+
+        /** Package name to return or null if package is not installed */
+        public YumTester andReturn(YumPackageName yumPackage) {
+            TestChildProcess2 process = new TestChildProcess2(
+                    yumPackage == null ? 1 : 0,
+                    yumPackage == null ? "not installed" : String.join("\n",
+                            yumPackage.getName(),
+                            yumPackage.getEpoch().orElse("(none)"),
+                            yumPackage.getVersion().orElseThrow(() -> new IllegalArgumentException("Version must be set")),
+                            yumPackage.getRelease().orElseThrow(() -> new IllegalArgumentException("Release must be set")),
+                            yumPackage.getArchitecture().orElse("(none)")));
+
+            terminal.expectCommand("rpm -q " + packageName + " --queryformat \"%{NAME}\\\\n%{EPOCH}\\\\n%{VERSION}\\\\n%{RELEASE}\\\\n%{ARCH}\" 2>&1", process);
+            return YumTester.this;
+        }
+    }
+
+}
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtlTesterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtlTesterTest.java
new file mode 100644
index 00000000000..1c3a6f6a412
--- /dev/null
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtlTesterTest.java
@@ -0,0 +1,52 @@
+// 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.systemd;
+
+import com.yahoo.vespa.hosted.node.admin.component.TestTaskContext;
+import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.function.Function;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author freva
+ */
+public class SystemCtlTesterTest {
+
+    private static final String unit = "my-unit";
+    private final TestTerminal terminal = new TestTerminal();
+    private final SystemCtlTester systemCtl = new SystemCtlTester(terminal);
+    private final TestTaskContext context = new TestTaskContext();
+
+    @Test
+    public void return_expectations() {
+        assertSystemCtlMethod(sct -> sct.expectEnable(unit), sc -> sc.enable(unit).converge(context));
+        assertSystemCtlMethod(sct -> sct.expectDisable(unit), sc -> sc.disable(unit).converge(context));
+        assertSystemCtlMethod(sct -> sct.expectStart(unit), sc -> sc.start(unit).converge(context));
+        assertSystemCtlMethod(sct -> sct.expectStop(unit), sc -> sc.stop(unit).converge(context));
+        assertSystemCtlMethod(sct -> sct.expectServiceExists(unit), sc -> sc.serviceExists(context, unit));
+        assertSystemCtlMethod(sct -> sct.expectIsActive(unit), sc -> sc.isActive(context, unit));
+    }
+
+    @Test
+    public void void_tests() {
+        systemCtl.expectRestart(unit);
+        systemCtl.restart(unit).converge(context);
+        terminal.verifyAllCommandsExecuted();
+
+        systemCtl.expectDaemonReload();
+        systemCtl.daemonReload(context);
+        terminal.verifyAllCommandsExecuted();
+    }
+
+    private void assertSystemCtlMethod(Function<SystemCtlTester, SystemCtlTester.Expectation> systemCtlTesterExpectationFunction,
+                                       Function<SystemCtl, Boolean> systemCtlFunction) {
+        List.of(true, false).forEach(wantedReturnValue -> {
+            systemCtlTesterExpectationFunction.apply(systemCtl).andReturn(wantedReturnValue);
+            assertEquals(wantedReturnValue, systemCtlFunction.apply(systemCtl));
+            terminal.verifyAllCommandsExecuted();
+        });
+    }
+}
\ 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 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());
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTesterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTesterTest.java
new file mode 100644
index 00000000000..ef380046b75
--- /dev/null
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTesterTest.java
@@ -0,0 +1,61 @@
+// 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.TestTaskContext;
+import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author freva
+ */
+public class YumTesterTest {
+
+    private static final String[] packages = {"pkg1", "pkg2"};
+    private static final String[] repos = {"repo1", "repo2"};
+    private static final YumPackageName minimalPackage = YumPackageName.fromString("my-pkg-1.13.1-0.el7");
+    private static final YumPackageName fullPackage = YumPackageName.fromString("2:my-pkg-1.13.1-0.el7.x86_64");
+
+    private final TestTerminal terminal = new TestTerminal();
+    private final YumTester yum = new YumTester(terminal);
+    private final TestTaskContext context = new TestTaskContext();
+
+    @Test
+    public void generic_yum_methods() {
+        assertYumMethod(yum -> yum.expectInstall(packages).withEnableRepo(repos),
+                yum -> yum.install(List.of(packages)).enableRepo(repos).converge(context));
+
+        assertYumMethod(yum -> yum.expectUpdate(packages).withEnableRepo(repos),
+                yum -> yum.upgrade(List.of(packages)).enableRepo(repos).converge(context));
+
+        assertYumMethod(yum -> yum.expectRemove(packages).withEnableRepo(repos),
+                yum -> yum.remove(List.of(packages)).enableRepo(repos).converge(context));
+
+        assertYumMethod(yum -> yum.expectInstallFixedVersion(minimalPackage.toName()).withEnableRepo(repos),
+                yum -> yum.installFixedVersion(minimalPackage).enableRepo(repos).converge(context));
+    }
+
+    @Test
+    public void expect_query_installed() {
+        Stream.of(minimalPackage, fullPackage, null).forEach(pkg -> {
+            yum.expectQueryInstalled(packages[0]).andReturn(pkg);
+            assertEquals(Optional.ofNullable(pkg), yum.queryInstalled(context, packages[0]));
+            terminal.verifyAllCommandsExecuted();
+        });
+    }
+
+    private void assertYumMethod(Function<YumTester, YumTester.GenericYumCommandExpectation> yumTesterExpectationFunction,
+                                       Function<Yum, Boolean> yumFunction) {
+        List.of(true, false).forEach(wantedReturnValue -> {
+            yumTesterExpectationFunction.apply(yum).andReturn(wantedReturnValue);
+            assertEquals(wantedReturnValue, yumFunction.apply(yum));
+            terminal.verifyAllCommandsExecuted();
+        });
+    }
+}
\ No newline at end of file