Enable containersV2 by default (#369)

Enable `containersV2` by default.

README.md

@@ -150,7 +150,16 @@ All output from `go-java-launcher` itself, and from the launch of all processes
 
 By _default_, when starting a java process inside a container (as indicated by the presence of ``CONTAINER`` env
 variable):
+1. If the `-XX:ActiveProcessorCount` is unset, it will remain unset.
+1. Args with prefix``-Xmx|-Xms`` in both static and custom jvm opts will be filtered out. If neither
+   ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or custom jvm opts
+   ``-Xmx|-Xms`` will both be set to be 75% of the cgroups memory limit minus 3mb per processor, with a minimum value of
+   50% of the heap.
 
+This will cause the JVM 11+ to discover the ``MaxRAM`` value using Linux cgroups, and calculate the heap sizes as the specified
+percentage of ``MaxRAM`` value, e.g. ``max-heap-size = MaxRAM * MaxRamPercentage``.
+
+If the experimental flag `containerV2` is set to `false`, the behavior will be as follows:
 1. If `-XX:ActiveProcessorCount` is unset, it will be set based on the discovered cgroup configurations and host
    information to a value between 2 and the number of processors reported by the runtime. You can read more about the
    reasoning behind this [here](https://github.com/palantir/go-java-launcher/issues/313).
@@ -159,16 +168,6 @@ variable):
    custom jvm opts, both will be set to ``75.0`` (i.e. ``-XX:InitialRAMPercentage=75.0 -XX:MaxRAMPercentage=75.0 `` will
    be appended after all the other jvm opts).
 
-This will cause the JVM 11+ to discover the ``MaxRAM`` value using Linux cgroups, and calculate the heap sizes as the specified
-percentage of ``MaxRAM`` value, e.g. ``max-heap-size = MaxRAM * MaxRamPercentage``.
-
-If the experimental flag `containerV2` is set:
-1. The `-XX:ActiveProcessorCount` is unset, it will remain unset.
-1. Args with prefix``-Xmx|-Xms`` in both static and custom jvm opts will be filtered out. If neither 
- ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or custom jvm opts 
- ``-Xmx|-Xms`` will both be set to be 75% of the cgroups memory limit minus 3mb per processor, with a minimum value of 
-  50% of the heap.
-
 ### Overriding default values
 
 Developers can override the heap percentage in containers by specifying both ``-XX:MaxRAMPercentage=``

changelog/@unreleased/pr-369.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Enable `containersV2` by default.
+  links:
+  - https://github.com/palantir/go-java-launcher/pull/369

integration_test/go_java_launcher_integration_test.go

@@ -144,7 +144,7 @@ func TestMainMethodWithoutCustomConfig(t *testing.T) {
 }
 
 func TestMainMethodContainerWithoutCustomConfig(t *testing.T) {
-	output := testContainerSupportEnabled(t, "foo", "-XX\\:InitialRAMPercentage=75.0 -XX\\:MaxRAMPercentage=75.0 -XX\\:ActiveProcessorCount=2", []string{})
+	output := testContainerSupportEnabled(t, "foo", "", []string{"-Xmx", "-Xms"})
 	assert.Regexp(t, `Failed to read custom config file, assuming no custom config: foo`, output)
 }
 

integration_test/testdata/launcher-custom-dangerous-disable-container-support.yml

@@ -3,3 +3,5 @@ configVersion: 1
 jvmOpts:
   - '-Xmx1g'
 dangerousDisableContainerSupport: true
+experimental:
+  containerV2: false

integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml

@@ -2,5 +2,3 @@ configType: java
 configVersion: 1
 jvmOpts:
   - '-XX:InitialRAMPercentage=70.0'
-experimental:
-  containerV2: true

integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml

@@ -2,5 +2,3 @@ configType: java
 configVersion: 1
 jvmOpts:
   - '-XX:MaxRAMPercentage=70.0'
-experimental:
-  containerV2: true

integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml

@@ -3,5 +3,3 @@ configVersion: 1
 jvmOpts:
   - '-Xms1g'
   - '-Xmx1g'
-experimental:
-  containerV2: true

integration_test/testdata/launcher-custom-experimental-container-v2.yml

@@ -1,5 +1,3 @@
 configType: java
 configVersion: 1
 jvmOpts: []
-experimental:
-  containerV2: true

integration_test/testdata/launcher-custom-initial-and-max-ram-percentage-override.yml

@@ -4,3 +4,5 @@ jvmOpts:
   - '-Xmx1g'
   - '-XX:InitialRAMPercentage=79.9'
   - '-XX:MaxRAMPercentage=80.9'
+experimental:
+  containerV2: false

integration_test/testdata/launcher-custom-initial-ram-percentage-override.yml

@@ -3,3 +3,5 @@ configVersion: 1
 jvmOpts:
   - '-Xmx1g'
   - '-XX:InitialRAMPercentage=79.9'
+experimental:
+  containerV2: false

integration_test/testdata/launcher-custom-max-ram-override.yml

@@ -2,3 +2,5 @@ configType: java
 configVersion: 1
 jvmOpts:
   - '-XX:MaxRAM=1001'
+experimental:
+  containerV2: false

integration_test/testdata/launcher-custom-max-ram-percentage-override.yml

@@ -3,3 +3,5 @@ configVersion: 1
 jvmOpts:
   - '-Xmx1g'
   - '-XX:MaxRAMPercentage=79.9'
+experimental:
+  containerV2: false

integration_test/testdata/launcher-custom-multiprocess-long-sub-process.yml

@@ -9,3 +9,5 @@ subProcesses:
       SLEEP_TIME: "200"
     jvmOpts:
       - '-Xmx1g'
+experimental:
+  containerV2: false

integration_test/testdata/launcher-custom-multiprocess.yml

@@ -7,3 +7,5 @@ subProcesses:
     configType: java
     jvmOpts:
       - '-Xmx1g'
+experimental:
+  containerV2: false

integration_test/testdata/launcher-custom.yml

@@ -2,3 +2,5 @@ configType: java
 configVersion: 1
 jvmOpts:
   - '-Xmx1g'
+experimental:
+  containerV2: false

integration_test/testdata/launcher-static-bad-java-home.yml

@@ -9,3 +9,5 @@ jvmOpts:
 args:
   - arg1
 javaHome: /foo/bar
+experimental:
+  containerV2: false

integration_test/testdata/launcher-static-multiprocess.yml

@@ -16,3 +16,5 @@ subProcesses:
       - ./testdata/
     jvmOpts:
       - '-Xmx4M'
+experimental:
+  containerV2: false

integration_test/testdata/launcher-static-with-dirs.yml

@@ -11,3 +11,5 @@ args:
 dirs:
   - foo
   - bar/baz
+experimental:
+  containerV2: false

integration_test/testdata/launcher-static.yml

@@ -8,3 +8,5 @@ jvmOpts:
   - '-Xmx4M'
 args:
   - arg1
+experimental:
+  containerV2: false

launchlib/config.go

@@ -72,7 +72,7 @@ type CustomLauncherConfig struct {
 }
 
 type ExperimentalLauncherConfig struct {
-	ContainerV2 bool `yaml:"containerV2"`
+	ContainerV2 *bool `yaml:"containerV2"`
 }
 
 type PrimaryCustomLauncherConfig struct {
@@ -267,6 +267,10 @@ func parseCustomConfig(yamlString []byte) (PrimaryCustomLauncherConfig, error) {
 				"subProcess config %s", name)
 		}
 	}
+	if config.Experimental.ContainerV2 == nil {
+		var trueVal = true
+		config.Experimental.ContainerV2 = &trueVal
+	}
 	return config, nil
 }
 

launchlib/config_test.go

@@ -20,6 +20,8 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+var trueValue = true
+
 func TestParseStaticConfig(t *testing.T) {
 	for i, currCase := range []struct {
 		name string
@@ -188,6 +190,7 @@ jvmOpts:
 					},
 					JvmOpts:                 []string{"jvmOpt1", "jvmOpt2"},
 					DisableContainerSupport: false,
+					Experimental:            ExperimentalLauncherConfig{ContainerV2: &trueValue},
 				},
 			},
 		},
@@ -208,7 +211,8 @@ jvmOpts:
 					TypedConfig: TypedConfig{
 						Type: "java",
 					},
-					JvmOpts: []string{"jvmOpt1", "jvmOpt2"},
+					JvmOpts:      []string{"jvmOpt1", "jvmOpt2"},
+					Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue},
 				},
 			},
 		},
@@ -234,7 +238,8 @@ jvmOpts:
 					Env: map[string]string{
 						"SOME_ENV_VAR": "{{CWD}}/etc/profile",
 					},
-					JvmOpts: []string{"jvmOpt1", "jvmOpt2"},
+					JvmOpts:      []string{"jvmOpt1", "jvmOpt2"},
+					Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue},
 				},
 			},
 		},
@@ -272,7 +277,8 @@ subProcesses:
 					Env: map[string]string{
 						"SOME_ENV_VAR": "{{CWD}}/etc/profile",
 					},
-					JvmOpts: []string{"jvmOpt1", "jvmOpt2"},
+					JvmOpts:      []string{"jvmOpt1", "jvmOpt2"},
+					Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue},
 				},
 				SubProcesses: map[string]CustomLauncherConfig{
 					"envoy": {
@@ -306,6 +312,7 @@ dangerousDisableContainerSupport: true
 					},
 					JvmOpts:                 []string{"jvmOpt1", "jvmOpt2"},
 					DisableContainerSupport: true,
+					Experimental:            ExperimentalLauncherConfig{ContainerV2: &trueValue},
 				},
 			},
 		},
@@ -330,6 +337,7 @@ env:
 						"SOME_ENV_VAR":  "/etc/profile",
 						"OTHER_ENV_VAR": "/etc/redhat-release",
 					},
+					Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue},
 				},
 			},
 		},

launchlib/launcher.go

@@ -281,7 +281,12 @@ func delim(str string) string {
 func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, logger io.WriteCloser) []string {
 	if isEnvVarSet("CONTAINER") && !customConfig.DisableContainerSupport && !hasMaxRAMOverride(combinedJvmOpts) {
 		_, _ = fmt.Fprintln(logger, "Container support enabled")
-		if customConfig.Experimental.ContainerV2 {
+		// If the containerV2 field is nil, there was a failure reading the custom config file, and we use the default
+		// behavior of enabling containerV2 behavior.
+		if customConfig.Experimental.ContainerV2 != nil && !*customConfig.Experimental.ContainerV2 {
+			combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts)
+			combinedJvmOpts = ensureActiveProcessorCount(combinedJvmOpts, logger)
+		} else {
 			jvmOptsWithUpdatedHeapSizeArgs, err := filterHeapSizeArgsV2(combinedJvmOpts)
 			if err != nil {
 				// When we fail to get the memory limit from the cgroups files, fallback to using percentage-based heap
@@ -294,9 +299,6 @@ func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig,
 			} else {
 				combinedJvmOpts = jvmOptsWithUpdatedHeapSizeArgs
 			}
-		} else {
-			combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts)
-			combinedJvmOpts = ensureActiveProcessorCount(combinedJvmOpts, logger)
 		}
 		return combinedJvmOpts
 	}
@@ -334,7 +336,7 @@ func filterHeapSizeArgs(args []string) []string {
 	return filtered
 }
 
-// Used when the containerV2 flag is set
+// Used when the containerV2 flag is set to `true`. This is the default behavior.
 func filterHeapSizeArgsV2(args []string) ([]string, error) {
 	var filtered []string
 	var hasMaxRAMPercentage, hasInitialRAMPercentage bool
@@ -425,8 +427,8 @@ func isInitialRAMPercentage(arg string) bool {
 	return strings.HasPrefix(arg, "-XX:InitialRAMPercentage=")
 }
 
-// ComputeJVMHeapSizeInBytes If the experimental `ContainerV2` is set, compute the heap size to be 75% of
-// the heap minus 3mb per processor, with a minimum value of 50% of the heap.
+// ComputeJVMHeapSizeInBytes If the experimental `ContainerV2` is set to `true` (which it is by default), compute the
+// heap size to be 75% of the heap minus 3mb per processor, with a minimum value of 50% of the heap.
 func ComputeJVMHeapSizeInBytes(hostProcessorCount int, cgroupMemoryLimitInBytes uint64) (uint64, error) {
 	if cgroupMemoryLimitInBytes > 1_000_000*BytesInMebibyte {
 		return 0, errors.New("cgroups memory limit is unusually high. Not computing JVM heap size")