Merge pull request #20587 from vespa-engine/hmusum/log-or-fail-deployment-with-invalid-jvm-gc-options

Log or fail deployment with invalid jvm gc options

2 files changed, 18 insertions, 6 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 b24f63148c7..e645fec5520 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;
     private double tlsSizeFraction = 0.07;
 
     @Override public ModelContext.FeatureFlags featureFlags() { return this; }
@@ -133,6 +134,7 @@ 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; }
     @Override public double tlsSizeFraction() { return tlsSizeFraction; }
 
     public TestProperties maxUnCommittedMemory(int maxUnCommittedMemory) {
@@ -349,6 +351,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
         return this;
     }
 
+    public TestProperties failDeploymentWithInvalidJvmOptions(boolean fail) {
+        failDeploymentWithInvalidJvmOptions = fail;
+        return this;
+    }
+
     public TestProperties tlsSizeFraction(double tlsSizeFraction) {
         this.tlsSizeFraction = tlsSizeFraction;
         return this;
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..c8b2197fc29 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
@@ -1133,19 +1133,21 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
      */
     private static class JvmGcOptions {
 
-        private static final Pattern validPattern = Pattern.compile("-XX:[+-][a-zA-z0-9=]+");
+        private static final Pattern validPattern = Pattern.compile("-XX:[+-]*[a-zA-z0-9=]+");
         private static final Pattern invalidCMSPattern = Pattern.compile("-XX:[+-]\\w*CMS[a-zA-z0-9=]+");
 
         private final DeployState deployState;
         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);
         }
 
     }