Skip to content

Commit 37149b0

Browse files
authored
Merge pull request #6214 from mercedes-benz/tilt-prepare-command-and-args
🌱 Move command and arg handling of manager.yaml to tilt-prepare
2 parents b9e68f8 + ed50d79 commit 37149b0

File tree

3 files changed

+129
-75
lines changed

3 files changed

+129
-75
lines changed

Tiltfile

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ settings = {
1212
}
1313

1414
# global settings
15+
tilt_file = "./tilt-settings.yaml" if os.path.exists("./tilt-settings.yaml") else "./tilt-settings.json"
1516
settings.update(read_yaml(
16-
"./tilt-settings.yaml" if os.path.exists("./tilt-settings.yaml") else "./tilt-settings.json",
17+
tilt_file,
1718
default = {},
1819
))
1920

@@ -28,7 +29,6 @@ if settings.get("trigger_mode") == "manual":
2829
default_registry(settings.get("default_registry"))
2930

3031
always_enable_providers = ["core"]
31-
extra_args = settings.get("extra_args", {})
3232

3333
providers = {
3434
"core": {
@@ -235,13 +235,7 @@ def enable_provider(name, debug):
235235
links = []
236236

237237
if debug_port != 0:
238-
# Add delve when debugging. Delve will always listen on the pod side on port 30000.
239-
entrypoint = ["sh", "/start.sh", "/dlv", "--listen=:" + str(30000), "--accept-multiclient", "--api-version=2", "--headless=true", "exec", "--", "/manager"]
240238
port_forwards.append(port_forward(debug_port, 30000))
241-
if debug.get("continue", True):
242-
entrypoint.insert(8, "--continue")
243-
else:
244-
entrypoint = ["sh", "/start.sh", "/manager"]
245239

246240
metrics_port = int(debug.get("metrics_port", 0))
247241
profiler_port = int(debug.get("profiler_port", 0))
@@ -251,21 +245,15 @@ def enable_provider(name, debug):
251245

252246
if profiler_port != 0:
253247
port_forwards.append(port_forward(profiler_port, 6060))
254-
entrypoint.extend(["--profiler-address", ":6060"])
255248
links.append(link("http://localhost:" + str(profiler_port) + "/debug/pprof", "profiler"))
256249

257250
# Set up an image build for the provider. The live update configuration syncs the output from the local_resource
258251
# build into the container.
259-
provider_args = extra_args.get(name)
260-
if provider_args:
261-
entrypoint.extend(provider_args)
262-
263252
docker_build(
264253
ref = p.get("image"),
265254
context = context + "/.tiltbuild/bin/",
266255
dockerfile_contents = dockerfile_contents,
267256
target = "tilt",
268-
entrypoint = entrypoint,
269257
only = "manager",
270258
live_update = [
271259
sync(context + "/.tiltbuild/bin/manager", "/manager"),
@@ -379,20 +367,20 @@ def prepare_all():
379367
if p.get("kustomize_config", True):
380368
context = p.get("context")
381369
debug = ""
382-
if name in settings.get("debug"):
383-
debug = ":debug"
384-
providers_arg = providers_arg + "--providers {name}:{context}{debug} ".format(
370+
providers_arg = providers_arg + "--providers {name}:{context} ".format(
385371
name = name,
386372
context = context,
387-
debug = debug,
388373
)
389374

390-
cmd = "make -B tilt-prepare && ./hack/tools/bin/tilt-prepare {allow_k8s_arg}{tools_arg}{cert_manager_arg}{kustomize_build_arg}{providers_arg}".format(
375+
tilt_settings_file_arg = "--tilt-settings-file " + tilt_file
376+
377+
cmd = "make -B tilt-prepare && ./hack/tools/bin/tilt-prepare {allow_k8s_arg}{tools_arg}{cert_manager_arg}{kustomize_build_arg}{providers_arg}{tilt_settings_file_arg}".format(
391378
allow_k8s_arg = allow_k8s_arg,
392379
tools_arg = tools_arg,
393380
cert_manager_arg = cert_manager_arg,
394381
kustomize_build_arg = kustomize_build_arg,
395382
providers_arg = providers_arg,
383+
tilt_settings_file_arg = tilt_settings_file_arg,
396384
)
397385
local(cmd, env = settings.get("kustomize_substitutions", {}))
398386

hack/tools/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ require (
2020
k8s.io/apimachinery v0.23.0
2121
k8s.io/client-go v0.23.0
2222
k8s.io/klog/v2 v2.30.0
23+
k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b
2324
sigs.k8s.io/cluster-api v0.0.0-00010101000000-000000000000
2425
sigs.k8s.io/cluster-api/test v0.0.0-00010101000000-000000000000
2526
sigs.k8s.io/controller-runtime v0.11.1
@@ -125,7 +126,6 @@ require (
125126
k8s.io/cluster-bootstrap v0.23.0 // indirect
126127
k8s.io/component-base v0.23.0 // indirect
127128
k8s.io/kube-openapi v0.0.0-20211115234752-e816edb12b65 // indirect
128-
k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b // indirect
129129
sigs.k8s.io/json v0.0.0-20211020170558-c049b76a60c6 // indirect
130130
sigs.k8s.io/structured-merge-diff/v4 v4.2.0 // indirect
131131
sigs.k8s.io/yaml v1.3.0 // indirect

hack/tools/tilt-prepare/main.go

Lines changed: 121 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ package main
2222
import (
2323
"bytes"
2424
"fmt"
25+
"io"
2526
"os"
2627
"os/exec"
2728
"path/filepath"
@@ -38,9 +39,11 @@ import (
3839
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3940
kerrors "k8s.io/apimachinery/pkg/util/errors"
4041
"k8s.io/apimachinery/pkg/util/sets"
42+
"k8s.io/apimachinery/pkg/util/yaml"
4143
"k8s.io/client-go/kubernetes/scheme"
4244
"k8s.io/client-go/tools/clientcmd"
4345
"k8s.io/klog/v2"
46+
"k8s.io/utils/pointer"
4447
ctrl "sigs.k8s.io/controller-runtime"
4548

4649
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -57,7 +60,7 @@ Example call for tilt up:
5760
--cert-manager
5861
--kustomize-builds clusterctl.crd:./cmd/clusterctl/config/crd/
5962
--kustomize-builds observability.tools:./hack/observability/
60-
--providers core:.:debug
63+
--providers core:.
6164
--providers kubeadm-bootstrap:./bootstrap/kubeadm
6265
--providers kubeadm-control-plane:./controlplane/kubeadm
6366
--providers docker:./test/infrastructure/docker
@@ -70,10 +73,25 @@ var (
7073
toolsFlag = pflag.StringSlice("tools", []string{}, "list of tools to be created; each value should correspond to a make target")
7174
certManagerFlag = pflag.Bool("cert-manager", false, "prepare cert-manager")
7275
kustomizeBuildsFlag = pflag.StringSlice("kustomize-builds", []string{}, "list of kustomize build to be run; each value should be in the form name:path")
73-
providersBuildsFlag = pflag.StringSlice("providers", []string{}, "list of providers to be installed; each value should be in the form name:path[:debug]")
76+
providersBuildsFlag = pflag.StringSlice("providers", []string{}, "list of providers to be installed; each value should be in the form name:path")
7477
allowK8SContextsFlag = pflag.StringSlice("allow-k8s-contexts", []string{}, "Specifies that Tilt is allowed to run against the specified k8s context name; Kind is automatically allowed")
78+
tiltSettingsFileFlag = pflag.String("tilt-settings-file", "./tilt-settings.yaml", "Path to a tilt-settings.(json|yaml) file")
7579
)
7680

81+
type tiltSettings struct {
82+
Debug map[string]debugConfig `json:"debug,omitempty"`
83+
ExtraArgs map[string]extraArgs `json:"extra_args,omitempty"`
84+
}
85+
86+
type debugConfig struct {
87+
Continue *bool `json:"continue"`
88+
Port *int `json:"port"`
89+
ProfilerPort *int `json:"profiler_port"`
90+
MetricsPort *int `json:"metrics_port"`
91+
}
92+
93+
type extraArgs []string
94+
7795
const (
7896
kustomizePath = "./hack/tools/bin/kustomize"
7997
envsubstPath = "./hack/tools/bin/envsubst"
@@ -106,19 +124,68 @@ func main() {
106124

107125
ctx := ctrl.SetupSignalHandler()
108126

127+
ts, err := readTiltSettings(*tiltSettingsFileFlag)
128+
if err != nil {
129+
klog.Exit(fmt.Sprintf("[main] failed to read tilt settings: %v", err))
130+
}
131+
109132
// Execute a first group of tilt prepare tasks, building all the tools required in subsequent steps/by tilt.
110133
if err := tiltTools(ctx); err != nil {
111134
klog.Exit(fmt.Sprintf("[main] failed to prepare tilt tools: %v", err))
112135
}
113136

114137
// execute a second group of tilt prepare tasks, building all the resources required by tilt.
115-
if err := tiltResources(ctx); err != nil {
138+
if err := tiltResources(ctx, ts); err != nil {
116139
klog.Exit(fmt.Sprintf("[main] failed to prepare tilt resources: %v", err))
117140
}
118141

119142
klog.Infof("[main] completed, elapsed: %s\n", time.Since(start))
120143
}
121144

145+
// readTiltSettings reads a tilt-settings.(json|yaml) file from the given path and sets debug defaults for certain
146+
// fields that are not present.
147+
func readTiltSettings(path string) (*tiltSettings, error) {
148+
f, err := os.Open(filepath.Clean(path))
149+
if err != nil {
150+
return nil, errors.Wrap(err, fmt.Sprintf("failed to open tilt-settings file for path: %s", path))
151+
}
152+
defer f.Close()
153+
154+
data, err := io.ReadAll(f)
155+
if err != nil {
156+
return nil, errors.Wrap(err, "failed to read tilt-settings content")
157+
}
158+
159+
ts := &tiltSettings{}
160+
if err := yaml.Unmarshal(data, ts); err != nil {
161+
return nil, errors.Wrap(err, "failed to unmarshal tilt-settings content")
162+
}
163+
164+
setDebugDefaults(ts)
165+
return ts, nil
166+
}
167+
168+
// setDebugDefaults sets default values for debug related fields in tiltSettings.
169+
func setDebugDefaults(ts *tiltSettings) {
170+
for k := range ts.Debug {
171+
p := ts.Debug[k]
172+
if p.Continue == nil {
173+
p.Continue = pointer.BoolPtr(true)
174+
}
175+
if p.Port == nil {
176+
p.Port = pointer.IntPtr(0)
177+
}
178+
if p.ProfilerPort == nil {
179+
p.ProfilerPort = pointer.IntPtr(0)
180+
}
181+
if p.MetricsPort == nil {
182+
p.MetricsPort = pointer.IntPtr(0)
183+
}
184+
185+
ts.Debug[k] = p
186+
}
187+
}
188+
122189
// allowK8sConfig mimics allow_k8s_contexts; only kind is enabled by default but more can be added.
123190
func allowK8sConfig() error {
124191
config, err := clientcmd.NewDefaultClientConfigLoadingRules().Load()
@@ -157,7 +224,7 @@ func tiltTools(ctx context.Context) error {
157224
}
158225

159226
// tiltResources runs tasks required for building all the resources required by tilt.
160-
func tiltResources(ctx context.Context) error {
227+
func tiltResources(ctx context.Context, ts *tiltSettings) error {
161228
tasks := map[string]taskFunction{}
162229

163230
// If required, all the task to install cert manager.
@@ -184,16 +251,12 @@ func tiltResources(ctx context.Context) error {
184251
// Add a provider task for each name/path defined using the --provider flag.
185252
for _, p := range *providersBuildsFlag {
186253
pValues := strings.Split(p, ":")
187-
if len(pValues) != 2 && len(pValues) != 3 {
188-
return errors.Errorf("[resources] failed to parse --provider flag %s: value should be in the form of name:path[:debug]", p)
254+
if len(pValues) != 2 {
255+
return errors.Errorf("[resources] failed to parse --provider flag %s: value should be in the form of name:path", p)
189256
}
190257
name := pValues[0]
191258
path := pValues[1]
192-
debug := false
193-
if len(pValues) == 3 && pValues[2] == "debug" {
194-
debug = true
195-
}
196-
tasks[name] = providerTask(fmt.Sprintf("%s/config/default", path), fmt.Sprintf("%s.provider.yaml", name), debug)
259+
tasks[name] = providerTask(name, fmt.Sprintf("%s/config/default", path), ts)
197260
}
198261

199262
return runTaskGroup(ctx, "resources", tasks)
@@ -380,7 +443,7 @@ func kustomizeTask(path, out string) taskFunction {
380443
// providerTask generates a task for creating the component yal for a provider and saving the output on a file.
381444
// NOTE: This task has several sub steps including running kustomize, envsubst, fixing components for debugging,
382445
// and adding the Provider resource mimicking what clusterctl init does.
383-
func providerTask(path, out string, debug bool) taskFunction {
446+
func providerTask(name, path string, ts *tiltSettings) taskFunction {
384447
return func(ctx context.Context, prefix string, errCh chan error) {
385448
kustomizeCmd := exec.CommandContext(ctx, kustomizePath, "build", path)
386449
var stdout1, stderr1 bytes.Buffer
@@ -408,15 +471,7 @@ func providerTask(path, out string, debug bool) taskFunction {
408471
errCh <- errors.Wrapf(err, "[%s] failed parse components yaml", prefix)
409472
return
410473
}
411-
412-
if debug {
413-
if err := prepareDeploymentForDebug(prefix, objs); err != nil {
414-
errCh <- err
415-
return
416-
}
417-
}
418-
419-
if err := prepareDeploymentForObservability(prefix, objs); err != nil {
474+
if err := prepareManagerDeployment(name, prefix, objs, ts); err != nil {
420475
errCh <- err
421476
return
422477
}
@@ -434,7 +489,7 @@ func providerTask(path, out string, debug bool) taskFunction {
434489
return
435490
}
436491

437-
if err := writeIfChanged(prefix, filepath.Join(tiltBuildPath, "yaml", out), yaml); err != nil {
492+
if err := writeIfChanged(prefix, filepath.Join(tiltBuildPath, "yaml", fmt.Sprintf("%s.provider.yaml", name)), yaml); err != nil {
438493
errCh <- err
439494
}
440495
}
@@ -470,62 +525,73 @@ func writeIfChanged(prefix string, path string, yaml []byte) error {
470525
return nil
471526
}
472527

473-
// prepareDeploymentForDebug alter controller deployments for working nicely with delve debugger;
474-
// most specifically, liveness and readiness probes are dropper and leader election turned off.
475-
func prepareDeploymentForDebug(prefix string, objs []unstructured.Unstructured) error {
528+
// prepareManagerDeployment sets the Command and Args for the manager container according to the given tiltSettings.
529+
// If there is a debug config given for the provider, we modify Command and Args to work nicely with the delve debugger.
530+
// If there are extra_args given for the provider, we append those to the ones that already exist in the deployment.
531+
// This has the affect that the appended ones will take precedence, as those are read last.
532+
// Finally, we modify the deployment to enable prometheus metrics scraping.
533+
func prepareManagerDeployment(name, prefix string, objs []unstructured.Unstructured, ts *tiltSettings) error {
476534
return updateDeployment(prefix, objs, func(d *appsv1.Deployment) {
477535
for j, container := range d.Spec.Template.Spec.Containers {
478536
if container.Name != "manager" {
479537
// as defined in clusterctl Provider Contract "Controllers & Watching namespace"
480538
continue
481539
}
482540

483-
// Drop liveness and readiness probes.
484-
container.LivenessProbe = nil
485-
container.ReadinessProbe = nil
541+
cmd := []string{"sh", "/start.sh", "/manager"}
542+
args := append(container.Args, []string(ts.ExtraArgs[name])...)
486543

487-
// Drop leader election.
488-
debugArgs := make([]string, 0, len(container.Args))
489-
for _, a := range container.Args {
490-
if a == "--leader-elect" || a == "--leader-elect=true" {
491-
continue
544+
// alter controller deployment for working nicely with delve debugger;
545+
// most specifically, configuring delve, starting the manager with profiling enabled, dropping liveness and
546+
// readiness probes and disabling leader election.
547+
if d, ok := ts.Debug[name]; ok {
548+
cmd = []string{"sh", "/start.sh", "/dlv", "--accept-multiclient", "--api-version=2", "--headless=true", "exec"}
549+
550+
if d.Port != nil && *d.Port > 0 {
551+
cmd = append(cmd, "--listen=:30000")
552+
}
553+
if d.Continue != nil && *d.Continue {
554+
cmd = append(cmd, "--continue")
492555
}
493-
debugArgs = append(debugArgs, a)
494-
}
495-
container.Args = debugArgs
496556

497-
d.Spec.Template.Spec.Containers[j] = container
498-
}
499-
})
500-
}
557+
cmd = append(cmd, []string{"--", "/manager"}...)
501558

502-
// prepareDeploymentForObservability alters controller deployments for working
503-
// nicely with prometheus metrics scraping. Specifically, the metrics endpoint is set to
504-
// listen on all interfaces instead of only localhost, and another port is added to the
505-
// container to expose the metrics endpoint.
506-
func prepareDeploymentForObservability(prefix string, objs []unstructured.Unstructured) error {
507-
return updateDeployment(prefix, objs, func(d *appsv1.Deployment) {
508-
for j, container := range d.Spec.Template.Spec.Containers {
509-
if container.Name != "manager" {
510-
// as defined in clusterctl Provider Contract "Controllers & Watching namespace"
511-
continue
559+
if d.ProfilerPort != nil && *d.ProfilerPort > 0 {
560+
args = append(args, []string{"--profiler-address=:6060"}...)
561+
}
562+
563+
debugArgs := make([]string, 0, len(args))
564+
for _, a := range args {
565+
if a == "--leader-elect" || a == "--leader-elect=true" {
566+
continue
567+
}
568+
debugArgs = append(debugArgs, a)
569+
}
570+
args = debugArgs
571+
572+
container.LivenessProbe = nil
573+
container.ReadinessProbe = nil
512574
}
513575

514-
args := make([]string, 0, len(container.Args))
515-
for _, a := range container.Args {
576+
// alter the controller deployment for working nicely with prometheus metrics scraping. Specifically, the
577+
// metrics endpoint is set to listen on all interfaces instead of only localhost, and another port is added
578+
// to the container to expose the metrics endpoint.
579+
finalArgs := make([]string, 0, len(args))
580+
for _, a := range args {
516581
if strings.HasPrefix(a, "--metrics-bind-addr=") {
517-
args = append(args, "--metrics-bind-addr=0.0.0.0:8080")
582+
finalArgs = append(finalArgs, "--metrics-bind-addr=0.0.0.0:8080")
518583
continue
519584
}
520-
args = append(args, a)
585+
finalArgs = append(finalArgs, a)
521586
}
522-
container.Args = args
523587

524588
container.Ports = append(container.Ports, corev1.ContainerPort{
525589
Name: "metrics",
526590
ContainerPort: 8080,
527591
Protocol: "TCP",
528592
})
593+
container.Command = cmd
594+
container.Args = finalArgs
529595

530596
d.Spec.Template.Spec.Containers[j] = container
531597
}

0 commit comments

Comments
 (0)