Merge pull request #5511 from vespa-engine/hakonhall/fail-convergence-on-unknown-yum-packages

Fail convergence on unknown yum packages

5 files changed, 49 insertions, 15 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandResult.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandResult.java
index e0e84242f78..34cc49770aa 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandResult.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandResult.java
@@ -55,7 +55,7 @@ public class CommandResult {
      * Map this CommandResult to an instance of type R.
      *
      * If a RuntimeException is thrown by the mapper, it is wrapped in an
-     * UnexpectedOutputException2 that includes a snippet of the output in the message.
+     * UnexpectedOutputException that includes a snippet of the output in the message.
      *
      * This method is intended to be used as part of the verification of the output.
      */
@@ -63,19 +63,19 @@ public class CommandResult {
         try {
             return mapper.apply(this);
         } catch (RuntimeException e) {
-            throw new UnexpectedOutputException2(e, "Failed to map output", commandLine.toString(), output);
+            throw new UnexpectedOutputException(e, "Failed to map output", commandLine.toString(), output);
         }
     }
 
     /**
      * Map the output to an instance of type R according to mapper, wrapping any
-     * RuntimeException in UnexpectedOutputException2 w/output snippet. See map() for details.
+     * RuntimeException in UnexpectedOutputException w/output snippet. See map() for details.
      */
     public <R> R mapOutput(Function<String, R> mapper) { return map(result -> mapper.apply(result.getOutput())); }
 
     /**
      * Map each output line to an instance of type R according to mapper, wrapping any
-     * RuntimeException in UnexpectedOutputException2 w/output snippet. See map() for details.
+     * RuntimeException in UnexpectedOutputException w/output snippet. See map() for details.
      */
     public <R> List<R> mapEachLine(Function<String, R> mapper) {
         return map(result -> result.getOutputLinesStream().map(mapper).collect(Collectors.toList()));
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/UnexpectedOutputException2.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/UnexpectedOutputException.java
index 82fae1aa70e..c7f572fa041 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/UnexpectedOutputException2.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/UnexpectedOutputException.java
@@ -6,21 +6,21 @@ package com.yahoo.vespa.hosted.node.admin.task.util.process;
  * @author hakonhall
  */
 @SuppressWarnings("serial")
-public class UnexpectedOutputException2 extends ChildProcessException {
+public class UnexpectedOutputException extends ChildProcessException {
     /**
      * @param problem Problem description, e.g. "Output is not of the form ^NAME=VALUE$"
      */
-    public UnexpectedOutputException2(String problem, String commandLine, String possiblyHugeOutput) {
+    public UnexpectedOutputException(String problem, String commandLine, String possiblyHugeOutput) {
         super("output was not of the expected format: " + problem, commandLine, possiblyHugeOutput);
     }
 
     /**
      * @param problem Problem description, e.g. "Output is not of the form ^NAME=VALUE$"
      */
-    public UnexpectedOutputException2(RuntimeException cause,
-                                      String problem,
-                                      String commandLine,
-                                      String possiblyHugeOutput) {
+    public UnexpectedOutputException(RuntimeException cause,
+                                     String problem,
+                                     String commandLine,
+                                     String possiblyHugeOutput) {
         super(cause, "output was not of the expected format: " + problem, commandLine, possiblyHugeOutput);
     }
 }
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 26a88e39e14..a9d6d779f37 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
@@ -8,7 +8,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
-import java.util.logging.Logger;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 /**
@@ -20,6 +20,9 @@ public class Yum {
     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\\.$");
+
     private final TaskContext taskContext;
     private final Terminal terminal;
 
@@ -55,8 +58,6 @@ public class Yum {
     }
 
     public static class GenericYumCommand {
-        private static Logger logger = Logger.getLogger(GenericYumCommand.class.getName());
-
         private final TaskContext taskContext;
         private final Terminal terminal;
         private final String yumCommand;
@@ -96,7 +97,7 @@ public class Yum {
             // Therefore, run the command and parse the output to decide.
             boolean modifiedSystem = commandLine
                     .executeSilently()
-                    .mapOutput(output -> !commandOutputNoopPattern.matcher(output).find());
+                    .mapOutput(this::mapOutput);
 
             if (modifiedSystem) {
                 commandLine.recordSilentExecutionAsSystemModification();
@@ -104,5 +105,14 @@ public class Yum {
 
             return modifiedSystem;
         }
+
+        public 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/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java
index 75e33db97d8..5bc45d7540e 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/CommandLineTest.java
@@ -124,7 +124,7 @@ public class CommandLineTest {
         try {
             result.mapOutput(output -> { throw exception; });
             fail();
-        } catch (UnexpectedOutputException2 e) {
+        } catch (UnexpectedOutputException e) {
             assertEquals("Command 'program 2>&1' output was not of the expected format: " +
                     "Failed to map output: stdout/stderr: 'output'", e.getMessage());
             assertTrue(e.getCause() == exception);
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 f010ea07d99..39eabc5c512 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
@@ -7,6 +7,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -99,4 +100,27 @@ public class YumTest {
                 .converge();
         fail();
     }
+
+    @Test
+    public void testUnknownPackages() {
+        terminal.expectCommand(
+                "yum install --assumeyes package-1 package-2 package-3 2>&1",
+                0,
+                "Loaded plugins: fastestmirror, langpacks\n" +
+                        "Loading mirror speeds from cached hostfile\n" +
+                        "No package package-1 available.\n" +
+                        "No package package-2 available.\n" +
+                        "Nothing to do\n");
+
+        Yum yum = new Yum(taskContext, terminal);
+        Yum.GenericYumCommand install = yum.install("package-1", "package-2", "package-3");
+
+        try {
+            install.converge();
+            fail();
+        } catch (Exception e) {
+            assertTrue(e.getCause() != null);
+            assertEquals("Unknown package: package-1", e.getCause().getMessage());
+        }
+    }
 }
\ No newline at end of file