Skip to content

Commit

Permalink
Enable containerV2 by default
Browse files Browse the repository at this point in the history
wip: Make containerV2 default to true

wip: Make containerV2 default to true

wip: re-add containerv2 flag to test configs.

add debug logs

remove more containerV2 flags

try omitempty

use setToDefaults

outparamcheck

use field.Addr().Interface()

pass addr of conf

don't take addr of unaddressable value

use *bool

set default

update config_test

set containerv2: false for existing configs

nil check

update test expectation

Remove debug logs. Add nl
  • Loading branch information
Pritham Marupaka committed Jan 9, 2024
1 parent af0cb89 commit 4f135ba
Show file tree
Hide file tree
Showing 18 changed files with 45 additions and 7 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ variable):
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``.

TODO(pmarupaka): Update this.
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
Expand Down
2 changes: 1 addition & 1 deletion integration_test/go_java_launcher_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ configVersion: 1
jvmOpts:
- '-Xmx1g'
dangerousDisableContainerSupport: true
experimental:
containerV2: false
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,3 @@ configType: java
configVersion: 1
jvmOpts:
- '-XX:MaxRAMPercentage=70.0'

Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@ jvmOpts:
- '-Xmx1g'
- '-XX:InitialRAMPercentage=79.9'
- '-XX:MaxRAMPercentage=80.9'
experimental:
containerV2: false
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ configVersion: 1
jvmOpts:
- '-Xmx1g'
- '-XX:InitialRAMPercentage=79.9'
experimental:
containerV2: false
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ configType: java
configVersion: 1
jvmOpts:
- '-XX:MaxRAM=1001'
experimental:
containerV2: false
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ configVersion: 1
jvmOpts:
- '-Xmx1g'
- '-XX:MaxRAMPercentage=79.9'
experimental:
containerV2: false
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ subProcesses:
SLEEP_TIME: "200"
jvmOpts:
- '-Xmx1g'
experimental:
containerV2: false
2 changes: 2 additions & 0 deletions integration_test/testdata/launcher-custom-multiprocess.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ subProcesses:
configType: java
jvmOpts:
- '-Xmx1g'
experimental:
containerV2: false
2 changes: 2 additions & 0 deletions integration_test/testdata/launcher-custom.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ configType: java
configVersion: 1
jvmOpts:
- '-Xmx1g'
experimental:
containerV2: false
2 changes: 2 additions & 0 deletions integration_test/testdata/launcher-static-bad-java-home.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ jvmOpts:
args:
- arg1
javaHome: /foo/bar
experimental:
containerV2: false
2 changes: 2 additions & 0 deletions integration_test/testdata/launcher-static-multiprocess.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@ subProcesses:
- ./testdata/
jvmOpts:
- '-Xmx4M'
experimental:
containerV2: false
2 changes: 2 additions & 0 deletions integration_test/testdata/launcher-static-with-dirs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ args:
dirs:
- foo
- bar/baz
experimental:
containerV2: false
2 changes: 2 additions & 0 deletions integration_test/testdata/launcher-static.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@ jvmOpts:
- '-Xmx4M'
args:
- arg1
experimental:
containerV2: false
6 changes: 5 additions & 1 deletion launchlib/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ type CustomLauncherConfig struct {
}

type ExperimentalLauncherConfig struct {
ContainerV2 bool `yaml:"containerV2"`
ContainerV2 *bool `yaml:"containerV2"`
}

type PrimaryCustomLauncherConfig struct {
Expand Down Expand Up @@ -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
}

Expand Down
14 changes: 11 additions & 3 deletions launchlib/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -188,6 +190,7 @@ jvmOpts:
},
JvmOpts: []string{"jvmOpt1", "jvmOpt2"},
DisableContainerSupport: false,
Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue},
},
},
},
Expand All @@ -208,7 +211,8 @@ jvmOpts:
TypedConfig: TypedConfig{
Type: "java",
},
JvmOpts: []string{"jvmOpt1", "jvmOpt2"},
JvmOpts: []string{"jvmOpt1", "jvmOpt2"},
Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue},
},
},
},
Expand All @@ -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},
},
},
},
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -306,6 +312,7 @@ dangerousDisableContainerSupport: true
},
JvmOpts: []string{"jvmOpt1", "jvmOpt2"},
DisableContainerSupport: true,
Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue},
},
},
},
Expand All @@ -330,6 +337,7 @@ env:
"SOME_ENV_VAR": "/etc/profile",
"OTHER_ENV_VAR": "/etc/redhat-release",
},
Experimental: ExperimentalLauncherConfig{ContainerV2: &trueValue},
},
},
},
Expand Down
4 changes: 3 additions & 1 deletion launchlib/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,9 @@ 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 {
Expand Down

0 comments on commit 4f135ba

Please sign in to comment.