LOg or fail deployments with invalid JVM GC options

1 files changed, 35 insertions, 13 deletions

diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JvmOptionsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JvmOptionsTest.java
index 4d34df98fb7..a1b27299d56 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JvmOptionsTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JvmOptionsTest.java
@@ -12,7 +12,9 @@ import com.yahoo.config.model.test.MockApplicationPackage;
 import com.yahoo.search.config.QrStartConfig;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.container.ContainerCluster;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.w3c.dom.Element;
 import org.xml.sax.SAXException;
 
@@ -22,6 +24,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.logging.Level;
 
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -32,6 +35,8 @@ import static org.junit.Assert.assertTrue;
  */
 public class JvmOptionsTest extends ContainerModelBuilderTestBase {
 
+    @Rule public ExpectedException expectedException = ExpectedException.none();
+
     @Test
     public void verify_jvm_tag_with_attributes() throws IOException, SAXException {
         String servicesXml =
@@ -171,26 +176,23 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase {
         verifyLoggingOfJvmGcOptions(false, "-XX:+UseConcMarkSweepGC");
     }
 
+    @Test
+    public void requireThatInvalidJvmGcOptionsFailDeployment() throws IOException, SAXException {
+        expectedException.expect(IllegalArgumentException.class);
+        expectedException.expectMessage(containsString("Invalid JVM GC options from services.xml: bar,foo"));
+        buildModelWithJvmOptions(new TestProperties().setHostedVespa(true).failDeploymentWithInvalidJvmOptions(true),
+                                    new TestLogger(),
+                                    "gc-options",
+                                    "-XX:+ParallelGCThreads=8 foo     bar");
+    }
 
     private void verifyLoggingOfJvmGcOptions(boolean isHosted, String override, String... invalidOptions) throws IOException, SAXException  {
         verifyLoggingOfJvmOptions(isHosted, "gc-options", override, invalidOptions);
     }
 
     private void verifyLoggingOfJvmOptions(boolean isHosted, String optionName, String override, String... invalidOptions) throws IOException, SAXException  {
-        String servicesXml =
-                "<container version='1.0'>" +
-                "  <nodes>" +
-                "    <jvm " + optionName + "='" + override + "'/>" +
-                "    <node hostalias='mockhost'/>" +
-                "  </nodes>" +
-                "</container>";
-        ApplicationPackage app = new MockApplicationPackage.Builder().withServices(servicesXml).build();
         TestLogger logger = new TestLogger();
-        new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder()
-                .applicationPackage(app)
-                .deployLogger(logger)
-                .properties(new TestProperties().setHostedVespa(isHosted))
-                .build());
+        buildModelWithJvmOptions(isHosted, logger, optionName, override);
 
         List<String> strings = Arrays.asList(invalidOptions.clone());
         if (strings.isEmpty()) return;
@@ -202,6 +204,26 @@ public class JvmOptionsTest extends ContainerModelBuilderTestBase {
                              "options from services.xml: " + String.join(",", strings), firstOption.getSecond());
     }
 
+    private void buildModelWithJvmOptions(boolean isHosted, TestLogger logger, String optionName, String override) throws IOException, SAXException {
+        buildModelWithJvmOptions(new TestProperties().setHostedVespa(isHosted), logger, optionName, override);
+    }
+
+    private void buildModelWithJvmOptions(TestProperties properties, TestLogger logger, String optionName, String override) throws IOException, SAXException {
+        String servicesXml =
+                "<container version='1.0'>" +
+                        "  <nodes>" +
+                        "    <jvm " + optionName + "='" + override + "'/>" +
+                        "    <node hostalias='mockhost'/>" +
+                        "  </nodes>" +
+                        "</container>";
+        ApplicationPackage app = new MockApplicationPackage.Builder().withServices(servicesXml).build();
+        new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder()
+                .applicationPackage(app)
+                .deployLogger(logger)
+                .properties(properties)
+                .build());
+    }
+
     @Test
     public void requireThatJvmOptionsAreLogged()  throws IOException, SAXException {
         verifyLoggingOfJvmOptions(true, "options", "-Xms2G");