kubernetes-exec is now a flag #2814
Conversation
DrJosh9000
force-pushed
the
deexperimentify-k8s
branch
from
972efcc to
e163007
Compare
DrJosh9000
force-pushed
the
deexperimentify-k8s
branch
from
e163007 to
33e5aa2
Compare
agent/tags.go
Outdated
| @@ -78,7 +77,7 @@ type tagFetcher struct { | |||
| func (t *tagFetcher) Fetch(ctx context.Context, l logger.Logger, conf FetchTagsConfig) []string { | |||
| tags := conf.Tags | |||
|
|
|||
| if experiments.IsEnabled(ctx, experiments.KubernetesExec) { | |||
| if t.k8s != nil { | |||
There was a problem hiding this comment.
Is this line right? the t.k8s seems to be set always?
There was a problem hiding this comment.
Good point, on one hand this is defensive in case we set t.k8s optionally, on the other YAGNI.
There was a problem hiding this comment.
I changed it so that it used the same logic as before (only run t.k8s to create tags if kubernetes-exec is enabled).
| @@ -488,6 +494,10 @@ func (r *JobRunner) createEnvironment(ctx context.Context) ([]string, error) { | |||
| env["BUILDKITE_STRICT_SINGLE_HOOKS"] = fmt.Sprintf("%t", r.conf.AgentConfiguration.StrictSingleHooks) | |||
| env["BUILDKITE_SIGNAL_GRACE_PERIOD_SECONDS"] = fmt.Sprintf("%d", int(r.conf.AgentConfiguration.SignalGracePeriod/time.Second)) | |||
|
|
|||
| if r.conf.KubernetesExec { | |||
| env["BUILDKITE_KUBERNETES_EXEC"] = "true" | |||
There was a problem hiding this comment.
This seems to be new. Just for my curiosity, what is this impacting?
There was a problem hiding this comment.
Previously, if kubernetes-exec were enabled (as an experiment), it would be passed down to the subprocess via the BUILDKITE_AGENT_EXPERIMENTS variable. Now it could be set in the parent process via a flag, we should ensure that if enabled it should be passed to the subprocess.
But we'll make the k8s controller set this anyway, so it's probably unnecessary.
| @@ -165,6 +165,7 @@ type AgentStartConfig struct { | |||
| Experiments []string `cli:"experiment" normalize:"list"` | |||
| Profile string `cli:"profile"` | |||
| StrictSingleHooks bool `cli:"strict-single-hooks"` | |||
| KuberentesExec bool `cli:"kubernetes-exec"` | |||
There was a problem hiding this comment.
Heads up there's a typo here KuberentesExec
There was a problem hiding this comment.
lol, whoops. i've just put a PR in to fix it, but there should be no impact
Description
Replaces the
kubernetes-exec"experiment" with a proper flag, environment variable, etc.Context
The
kubernetes-exec"experiment" was not really an experiment, but rather misused the experiments system as a way to pass a Boolean flag into various parts of the agent without setting up a flag, environment variable, etc.Testing
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go fmt ./...)