diff options
author | Harald Musum <musum@yahooinc.com> | 2021-12-20 09:59:33 +0100 |
---|---|---|
committer | Harald Musum <musum@yahooinc.com> | 2021-12-20 09:59:33 +0100 |
commit | b7dedde0a83d681966641e2cb4439ad60bf07702 (patch) | |
tree | 1fd489fcee9a2284d44528efc587142ee85cf9d3 /config-model | |
parent | 50580cb73a96f89f37c868e080da6f8a396bddde (diff) |
LOg or fail deployments with invalid JVM GC options
Diffstat (limited to 'config-model')
3 files changed, 53 insertions, 18 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index cf649162c08..b0c1dfadfd7 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -76,6 +76,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private boolean unorderedMergeChaining = false; private List<String> zoneDnsSuffixes = List.of(); private int maxCompactBuffers = 1; + private boolean failDeploymentWithInvalidJvmOptions = false; @Override public ModelContext.FeatureFlags featureFlags() { return this; } @Override public boolean multitenant() { return multitenant; } @@ -132,6 +133,8 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public boolean unorderedMergeChaining() { return unorderedMergeChaining; } @Override public List<String> zoneDnsSuffixes() { return zoneDnsSuffixes; } @Override public int maxCompactBuffers() { return maxCompactBuffers; } + @Override public boolean failDeploymentWithInvalidJvmOptions() { return failDeploymentWithInvalidJvmOptions; } + public TestProperties maxUnCommittedMemory(int maxUnCommittedMemory) { this.maxUnCommittedMemory = maxUnCommittedMemory; @@ -347,6 +350,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea return this; } + public TestProperties failDeploymentWithInvalidJvmOptions(boolean fail) { + failDeploymentWithInvalidJvmOptions = fail; + return this; + } + public static class Spec implements ConfigServerSpec { private final String hostName; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index d7691f4c486..a8d876f904c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -1140,12 +1140,14 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { private final String jvmGcOptions; private final DeployLogger logger; private final boolean isHosted; + private final boolean failDeploymentWithInvalidJvmOptions; public JvmGcOptions(DeployState deployState, String jvmGcOptions) { this.deployState = deployState; this.jvmGcOptions = jvmGcOptions; this.logger = deployState.getDeployLogger(); this.isHosted = deployState.isHosted(); + this.failDeploymentWithInvalidJvmOptions = deployState.featureFlags().failDeploymentWithInvalidJvmOptions(); } private String build() { @@ -1162,13 +1164,12 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { // CMS GC options cannot be used in hosted, CMS is unsupported in JDK 17 invalidOptions.addAll(Arrays.stream(optionList) .filter(option -> !option.isEmpty()) - .filter(option -> Pattern - .matches(invalidCMSPattern.pattern(), option) || + .filter(option -> Pattern.matches(invalidCMSPattern.pattern(), option) || option.equals("-XX:+UseConcMarkSweepGC")) .collect(Collectors.toList())); } - logInvalidOptions(invalidOptions); + logOrFailInvalidOptions(invalidOptions); } if (options == null || options.isEmpty()) @@ -1177,11 +1178,15 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { return options; } - private void logInvalidOptions(List<String> options) { + private void logOrFailInvalidOptions(List<String> options) { if (options.isEmpty()) return; Collections.sort(options); - logger.logApplicationPackage(WARNING, "Invalid JVM GC options from services.xml: " + String.join(",", options)); + String message = "Invalid JVM GC options from services.xml: " + String.join(",", options); + if (failDeploymentWithInvalidJvmOptions) + throw new IllegalArgumentException(message); + else + logger.logApplicationPackage(WARNING, message); } } 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"); |