Improve exception for package with multiple installed versions

3 files changed, 31 insertions, 7 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 d8a131f5ed1..72bc0fd4215 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
@@ -22,7 +22,7 @@ public class Yum {
     }
 
     public Optional<YumPackageName> queryInstalled(TaskContext context, String packageName) {
-        return YumCommand.queryInstalled(terminal, context, packageName);
+        return YumCommand.queryInstalled(terminal, context, YumPackageName.fromString(packageName));
     }
 
     /** Lock and install, or if necessary downgrade, a package to a given version. */
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
index ba9ba80ccb7..1d105057e02 100644
--- 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
@@ -241,10 +241,11 @@ public abstract class YumCommand<T extends YumCommand<T>> {
     }
 
     protected boolean isInstalled(TaskContext context, YumPackageName yumPackage) {
-        return queryInstalled(terminal, context, yumPackage.getName()).map(yumPackage::isSubsetOf).orElse(false);
+        return queryInstalled(terminal, context, yumPackage).map(yumPackage::isSubsetOf).orElse(false);
     }
 
-    static Optional<YumPackageName> queryInstalled(Terminal terminal, TaskContext context, String packageName) {
+    static Optional<YumPackageName> queryInstalled(Terminal terminal, TaskContext context, YumPackageName yumPackage) {
+        String packageName = yumPackage.toName();
         CommandResult commandResult = terminal.newCommandLine(context)
                 .add("rpm", "-q", packageName, "--queryformat", RPM_QUERYFORMAT)
                 .ignoreExitCode()
@@ -255,8 +256,8 @@ public abstract class YumCommand<T extends YumCommand<T>> {
         YumPackageName.Builder builder = new YumPackageName.Builder();
         List<Function<String, YumPackageName.Builder>> builders = PACKAGE_NAME_BUILDERS_GENERATOR.apply(builder);
         List<Optional<String>> lines = commandResult.mapEachLine(line -> Optional.of(line).filter(s -> !"(none)".equals(s)));
-        if (lines.size() != builders.size()) throw new IllegalStateException(String.format(
-                "Unexpected response from rpm, expected %d lines, got %s", builders.size(), commandResult.getOutput()));
+        if (lines.size() % builders.size() != 0) throw new IllegalStateException(String.format("Unexpected response from rpm, expected %d lines, got '%s'", builders.size(), commandResult.getOutput()));
+        if (lines.size() > builders.size()) throw new IllegalArgumentException("Found multiple installed packages for '" + packageName + "'. Version is required to match package exactly");
 
         IntStream.range(0, builders.size()).forEach(i -> lines.get(i).ifPresent(builders.get(i)::apply));
         return Optional.of(builder.build());
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 62fd7410e95..2c8a2535676 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
@@ -31,7 +31,7 @@ public class YumTest {
     }
 
     @Test
-    public void testQueryInstalledNevra() {
+    public void testQueryInstalled() {
         terminal.expectCommand(
                 "rpm -q docker --queryformat \"%{NAME}\\\\n%{EPOCH}\\\\n%{VERSION}\\\\n%{RELEASE}\\\\n%{ARCH}\" 2>&1",
                 0,
@@ -77,6 +77,29 @@ public class YumTest {
     }
 
     @Test
+    public void testQueryInstalledMultiplePackages() {
+        terminal.expectCommand(
+                "rpm -q kernel-devel --queryformat \"%{NAME}\\\\n%{EPOCH}\\\\n%{VERSION}\\\\n%{RELEASE}\\\\n%{ARCH}\" 2>&1",
+                0,
+                "kernel-devel\n" +
+                "(none)\n" +
+                "4.18.0\n" +
+                "305.7.1.el8_4\n" +
+                "x86_64\n" +
+                "kernel-devel\n" +
+                "(none)\n" +
+                "4.18.0\n" +
+                "240.15.1.el8_3\n" +
+                "x86_64\n");
+        try {
+            yum.queryInstalled(taskContext, "kernel-devel");
+            fail("Expected exception");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Found multiple installed packages for 'kernel-devel'. Version is required to match package exactly", e.getMessage());
+        }
+    }
+
+    @Test
     public void testAlreadyInstalled() {
         mockRpmQuery("package-1", null);
         terminal.expectCommand(
@@ -133,7 +156,7 @@ public class YumTest {
 
     @Test
     public void skipsYumInstallIfInRpm() {
-        mockRpmQuery("package-1", YumPackageName.fromString("package-1-1.2.3-1"));
+        mockRpmQuery("package-1-0:1.2.3-1", YumPackageName.fromString("package-1-1.2.3-1"));
         mockRpmQuery("package-2", YumPackageName.fromString("1:package-2-1.2.3-1.el7.x86_64"));
         assertFalse(yum.install("package-1-1.2.3-1", "package-2").converge(taskContext));
     }