-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable containersV2
by default
#369
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
type: improvement | ||
improvement: | ||
description: Enable `containersV2` by default. | ||
links: | ||
- https://github.com/palantir/go-java-launcher/pull/369 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,3 +3,5 @@ configVersion: 1 | |
jvmOpts: | ||
- '-Xmx1g' | ||
dangerousDisableContainerSupport: true | ||
experimental: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting the flag to |
||
containerV2: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,5 +2,3 @@ configType: java | |
configVersion: 1 | ||
jvmOpts: | ||
- '-XX:InitialRAMPercentage=70.0' | ||
experimental: | ||
containerV2: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,5 +2,3 @@ configType: java | |
configVersion: 1 | ||
jvmOpts: | ||
- '-XX:MaxRAMPercentage=70.0' | ||
experimental: | ||
containerV2: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,5 +3,3 @@ configVersion: 1 | |
jvmOpts: | ||
- '-Xms1g' | ||
- '-Xmx1g' | ||
experimental: | ||
containerV2: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,3 @@ | ||
configType: java | ||
configVersion: 1 | ||
jvmOpts: [] | ||
experimental: | ||
containerV2: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 |
---|---|---|
|
@@ -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 |
---|---|---|
|
@@ -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 |
---|---|---|
|
@@ -9,3 +9,5 @@ subProcesses: | |
SLEEP_TIME: "200" | ||
jvmOpts: | ||
- '-Xmx1g' | ||
experimental: | ||
containerV2: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,3 +7,5 @@ subProcesses: | |
configType: java | ||
jvmOpts: | ||
- '-Xmx1g' | ||
experimental: | ||
containerV2: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,5 @@ configType: java | |
configVersion: 1 | ||
jvmOpts: | ||
- '-Xmx1g' | ||
experimental: | ||
containerV2: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,3 +9,5 @@ jvmOpts: | |
args: | ||
- arg1 | ||
javaHome: /foo/bar | ||
experimental: | ||
containerV2: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,3 +16,5 @@ subProcesses: | |
- ./testdata/ | ||
jvmOpts: | ||
- '-Xmx4M' | ||
experimental: | ||
containerV2: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,5 @@ args: | |
dirs: | ||
- foo | ||
- bar/baz | ||
experimental: | ||
containerV2: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,5 @@ jvmOpts: | |
- '-Xmx4M' | ||
args: | ||
- arg1 | ||
experimental: | ||
containerV2: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've seen this pattern elsewhere, but it would be nice if I could make this one line. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think there's anything in the standard library for it :( but there are some pretty small deps for doing something like this: package main
import (
"fmt"
"k8s.io/utils/ptr"
)
func main() {
a := ptr.To(false)
fmt.Printf("%T %v", a, *a)
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. I'm leaning towards not adding the dep since we use this in one place in the implementation (and also once in the tests). |
||
} | ||
return config, nil | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is a no custom config, or there is a failure to read the custom config, we use the new default behavior, hence this change.