From 547d97c752fc3e7e522b8e699746a25d4bfc2cd9 Mon Sep 17 00:00:00 2001 From: Andrea Scarpino Date: Thu, 4 Apr 2024 14:23:10 +0200 Subject: [PATCH 1/8] feat: Add startup-grace-period-seconds and graceful-startup-path --- .../templates/connect-inject-deployment.yaml | 2 + .../test/unit/connect-inject-deployment.bats | 54 ++++++++ charts/consul/values.yaml | 6 + cli/helm/values.go | 2 + .../constants/annotations_and_labels.go | 2 + .../connect-inject/constants/constants.go | 3 + .../lifecycle/lifecycle_configuration.go | 30 +++++ .../lifecycle/lifecycle_configuration_test.go | 119 ++++++++++++++++++ .../webhook/consul_dataplane_sidecar.go | 10 +- .../webhook/consul_dataplane_sidecar_test.go | 20 ++- .../webhookv2/consul_dataplane_sidecar.go | 10 +- .../consul_dataplane_sidecar_test.go | 20 ++- .../subcommand/inject-connect/command.go | 5 + .../inject-connect/v1controllers.go | 2 + .../inject-connect/v2controllers.go | 2 + 15 files changed, 277 insertions(+), 10 deletions(-) diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index 1565e92811..fe07c2581a 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -260,8 +260,10 @@ spec: -default-enable-sidecar-proxy-lifecycle-shutdown-drain-listeners=false \ {{- end }} -default-sidecar-proxy-lifecycle-shutdown-grace-period-seconds={{ .Values.connectInject.sidecarProxy.lifecycle.defaultShutdownGracePeriodSeconds }} \ + -default-sidecar-proxy-lifecycle-startup-grace-period-seconds={{ .Values.connectInject.sidecarProxy.lifecycle.defaultStartupGracePeriodSeconds }} \ -default-sidecar-proxy-lifecycle-graceful-port={{ .Values.connectInject.sidecarProxy.lifecycle.defaultGracefulPort }} \ -default-sidecar-proxy-lifecycle-graceful-shutdown-path="{{ .Values.connectInject.sidecarProxy.lifecycle.defaultGracefulShutdownPath }}" \ + -default-sidecar-proxy-lifecycle-graceful-startup-path="{{ .Values.connectInject.sidecarProxy.lifecycle.defaultGracefulStartupPath }}" \ -default-sidecar-proxy-startup-failure-seconds={{ .Values.connectInject.sidecarProxy.defaultStartupFailureSeconds }} \ -default-sidecar-proxy-liveness-failure-seconds={{ .Values.connectInject.sidecarProxy.defaultLivenessFailureSeconds }} \ {{- if .Values.connectInject.initContainer }} diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index 1e6397e39c..d5c9704446 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -1315,6 +1315,33 @@ load _helpers [ "${actual}" = "true" ] } +@test "connectInject/Deployment: by default sidecar proxy lifecycle management startup grace period is set to 0 seconds" { + cd `chart_dir` + local cmd=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo "$cmd" | + yq 'any(contains("-default-sidecar-proxy-lifecycle-startup-grace-period-seconds=0"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "connectInject/Deployment: sidecar proxy lifecycle management startup grace period can be set" { + cd `chart_dir` + local cmd=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'connectInject.sidecarProxy.lifecycle.defaultstartupGracePeriodSeconds=13' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo "$cmd" | + yq 'any(contains("-default-sidecar-proxy-lifecycle-startup-grace-period-seconds=13"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + @test "connectInject/Deployment: by default sidecar proxy lifecycle management port is set to 20600" { cd `chart_dir` local cmd=$(helm template \ @@ -1369,6 +1396,33 @@ load _helpers [ "${actual}" = "true" ] } +@test "connectInject/Deployment: by default sidecar proxy lifecycle management graceful startup path is set to /graceful_startup" { + cd `chart_dir` + local cmd=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo "$cmd" | + yq 'any(contains("-default-sidecar-proxy-lifecycle-graceful-startup-path=\"/graceful_startup\""))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + +@test "connectInject/Deployment: sidecar proxy lifecycle management graceful startup path can be set" { + cd `chart_dir` + local cmd=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'connectInject.sidecarProxy.lifecycle.defaultGracefulstartupPath=/start' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo "$cmd" | + yq 'any(contains("-default-sidecar-proxy-lifecycle-graceful-startup-path=\"/start\""))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + #-------------------------------------------------------------------- # priorityClassName diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 7d4f00210c..9d06d56c1a 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -2868,8 +2868,10 @@ connectInject: # - `consul.hashicorp.com/enable-sidecar-proxy-lifecycle` # - `consul.hashicorp.com/enable-sidecar-proxy-shutdown-drain-listeners` # - `consul.hashicorp.com/sidecar-proxy-lifecycle-shutdown-grace-period-seconds` + # - `consul.hashicorp.com/sidecar-proxy-lifecycle-startup-grace-period-seconds` # - `consul.hashicorp.com/sidecar-proxy-lifecycle-graceful-port` # - `consul.hashicorp.com/sidecar-proxy-lifecycle-graceful-shutdown-path` + # - `consul.hashicorp.com/sidecar-proxy-lifecycle-graceful-startup-path` # @type: map lifecycle: # @type: boolean @@ -2879,9 +2881,13 @@ connectInject: # @type: integer defaultShutdownGracePeriodSeconds: 30 # @type: integer + defaultStartupGracePeriodSeconds: 0 + # @type: integer defaultGracefulPort: 20600 # @type: string defaultGracefulShutdownPath: "/graceful_shutdown" + # @type: string + defaultGracefulStartupPath: "/graceful_startup" # Configures how long the k8s startup probe will wait before the proxy is considered to be unhealthy and the container is restarted. # A value of zero disables the probe. diff --git a/cli/helm/values.go b/cli/helm/values.go index 19e19d8d05..527843fd6d 100644 --- a/cli/helm/values.go +++ b/cli/helm/values.go @@ -438,6 +438,8 @@ type Lifecycle struct { DefaultShutdownGracePeriodSeconds int `yaml:"defaultShutdownGracePeriodSeconds"` DefaultGracefulPort int `yaml:"defaultGracefulPort"` DefaultGracefulShutdownPath string `yaml:"defaultGracefulShutdownPath"` + DefaultStartupGracePeriodSeconds int `yaml:"defaultStartupGracePeriodSeconds"` + DefaultGracefulStartupPath string `yaml:"defaultGracefulStartupPath"` } type ConnectInject struct { diff --git a/control-plane/connect-inject/constants/annotations_and_labels.go b/control-plane/connect-inject/constants/annotations_and_labels.go index 236beadeb0..dca3c523a3 100644 --- a/control-plane/connect-inject/constants/annotations_and_labels.go +++ b/control-plane/connect-inject/constants/annotations_and_labels.go @@ -118,8 +118,10 @@ const ( AnnotationEnableSidecarProxyLifecycle = "consul.hashicorp.com/enable-sidecar-proxy-lifecycle" AnnotationEnableSidecarProxyLifecycleShutdownDrainListeners = "consul.hashicorp.com/enable-sidecar-proxy-lifecycle-shutdown-drain-listeners" AnnotationSidecarProxyLifecycleShutdownGracePeriodSeconds = "consul.hashicorp.com/sidecar-proxy-lifecycle-shutdown-grace-period-seconds" + AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds = "consul.hashicorp.com/sidecar-proxy-lifecycle-startup-grace-period-seconds" AnnotationSidecarProxyLifecycleGracefulPort = "consul.hashicorp.com/sidecar-proxy-lifecycle-graceful-port" AnnotationSidecarProxyLifecycleGracefulShutdownPath = "consul.hashicorp.com/sidecar-proxy-lifecycle-graceful-shutdown-path" + AnnotationSidecarProxyLifecycleGracefulStartupPath = "consul.hashicorp.com/sidecar-proxy-lifecycle-graceful-startup-path" // annotations for sidecar volumes. AnnotationConsulSidecarUserVolume = "consul.hashicorp.com/consul-sidecar-user-volume" diff --git a/control-plane/connect-inject/constants/constants.go b/control-plane/connect-inject/constants/constants.go index d4d109ade5..627b7bf5d2 100644 --- a/control-plane/connect-inject/constants/constants.go +++ b/control-plane/connect-inject/constants/constants.go @@ -62,6 +62,9 @@ const ( // DefaultGracefulShutdownPath is the default path that consul-dataplane uses for graceful shutdown. DefaultGracefulShutdownPath = "/graceful_shutdown" + // DefaultGracefulStartupPath is the default path that consul-dataplane uses for graceful shutdown. + DefaultGracefulStartupPath = "/graceful_startup" + // DefaultWANPort is the default port that consul-dataplane uses for WAN. DefaultWANPort = 8443 diff --git a/control-plane/connect-inject/lifecycle/lifecycle_configuration.go b/control-plane/connect-inject/lifecycle/lifecycle_configuration.go index 651d4eecae..488412ec9d 100644 --- a/control-plane/connect-inject/lifecycle/lifecycle_configuration.go +++ b/control-plane/connect-inject/lifecycle/lifecycle_configuration.go @@ -17,8 +17,10 @@ type Config struct { DefaultEnableProxyLifecycle bool DefaultEnableShutdownDrainListeners bool DefaultShutdownGracePeriodSeconds int + DefaultStartupGracePeriodSeconds int DefaultGracefulPort string DefaultGracefulShutdownPath string + DefaultGracefulStartupPath string } // EnableProxyLifecycle returns whether proxy lifecycle management is enabled either via the default value in the meshWebhook, or if it's been @@ -63,6 +65,20 @@ func (lc Config) ShutdownGracePeriodSeconds(pod corev1.Pod) (int, error) { return shutdownGracePeriodSeconds, nil } +// StartupGracePeriodSeconds returns how long the sidecar proxy should wait after startup, either via the default value in the meshWebhook, or if it's been +// overridden via the annotation. +func (lc Config) StartupGracePeriodSeconds(pod corev1.Pod) (int, error) { + startupGracePeriodSeconds := lc.DefaultStartupGracePeriodSeconds + if startupGracePeriodSecondsAnnotation, ok := pod.Annotations[constants.AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds]; ok { + val, err := strconv.ParseUint(startupGracePeriodSecondsAnnotation, 10, 64) + if err != nil { + return 0, fmt.Errorf("unable to parse annotation %q: %w", constants.AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds, err) + } + startupGracePeriodSeconds = int(val) + } + return startupGracePeriodSeconds, nil +} + // GracefulPort returns the port on which consul-dataplane should serve the proxy lifecycle management HTTP endpoints, either via the default value in the meshWebhook, or // if it's been overridden via the annotation. It also validates the port is in the unprivileged port range. func (lc Config) GracefulPort(pod corev1.Pod) (int, error) { @@ -93,3 +109,17 @@ func (lc Config) GracefulShutdownPath(pod corev1.Pod) string { return lc.DefaultGracefulShutdownPath } + +// GracefulStartupPath returns the path on which consul-dataplane should serve the graceful startup HTTP endpoint, either via the default value in the meshWebhook, or +// if it's been overridden via the annotation. +func (lc Config) GracefulStartupPath(pod corev1.Pod) string { + if raw, ok := pod.Annotations[constants.AnnotationSidecarProxyLifecycleGracefulStartupPath]; ok && raw != "" { + return raw + } + + if lc.DefaultGracefulStartupPath == "" { + return constants.DefaultGracefulStartupPath + } + + return lc.DefaultGracefulStartupPath +} diff --git a/control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go b/control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go index 64157a3d55..7d792321f2 100644 --- a/control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go +++ b/control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go @@ -200,6 +200,72 @@ func TestLifecycleConfig_ShutdownGracePeriodSeconds(t *testing.T) { } } +func TestLifecycleConfig_StartupGracePeriodSeconds(t *testing.T) { + cases := []struct { + Name string + Pod func(*corev1.Pod) *corev1.Pod + LifecycleConfig Config + Expected int + Err string + }{ + { + Name: "Sidecar proxy startup grace period set via meshWebhook", + Pod: func(pod *corev1.Pod) *corev1.Pod { + return pod + }, + LifecycleConfig: Config{ + DefaultStartupGracePeriodSeconds: 10, + }, + Expected: 10, + Err: "", + }, + { + Name: "Sidecar proxy startup grace period set via annotation", + Pod: func(pod *corev1.Pod) *corev1.Pod { + pod.Annotations[constants.AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds] = "20" + return pod + }, + LifecycleConfig: Config{ + DefaultStartupGracePeriodSeconds: 10, + }, + Expected: 20, + Err: "", + }, + { + Name: "Sidecar proxy startup grace period configured via invalid annotation, negative number", + Pod: func(pod *corev1.Pod) *corev1.Pod { + pod.Annotations[constants.AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds] = "-1" + return pod + }, + Err: "unable to parse annotation \"consul.hashicorp.com/sidecar-proxy-lifecycle-startup-grace-period-seconds\": strconv.ParseUint: parsing \"-1\": invalid syntax", + }, + { + Name: "Sidecar proxy startup grace period configured via invalid annotation, not-parseable string", + Pod: func(pod *corev1.Pod) *corev1.Pod { + pod.Annotations[constants.AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds] = "not-int" + return pod + }, + Err: "unable to parse annotation \"consul.hashicorp.com/sidecar-proxy-lifecycle-startup-grace-period-seconds\": strconv.ParseUint: parsing \"not-int\": invalid syntax", + }, + } + + for _, tt := range cases { + t.Run(tt.Name, func(t *testing.T) { + require := require.New(t) + lc := tt.LifecycleConfig + + actual, err := lc.StartupGracePeriodSeconds(*tt.Pod(minimal())) + + if tt.Err == "" { + require.Equal(tt.Expected, actual) + require.NoError(err) + } else { + require.EqualError(err, tt.Err) + } + }) + } +} + func TestLifecycleConfig_GracefulPort(t *testing.T) { cases := []struct { Name string @@ -327,6 +393,59 @@ func TestLifecycleConfig_GracefulShutdownPath(t *testing.T) { } } +func TestLifecycleConfig_GracefulStartupPath(t *testing.T) { + cases := []struct { + Name string + Pod func(*corev1.Pod) *corev1.Pod + LifecycleConfig Config + Expected string + Err string + }{ + { + Name: "Sidecar proxy lifecycle graceful startup path defaults to /graceful_startup", + Pod: func(pod *corev1.Pod) *corev1.Pod { + return pod + }, + Expected: "/graceful_startup", + Err: "", + }, + { + Name: "Sidecar proxy lifecycle graceful startup path set via meshWebhook", + Pod: func(pod *corev1.Pod) *corev1.Pod { + return pod + }, + LifecycleConfig: Config{ + DefaultGracefulStartupPath: "/quit", + }, + Expected: "/quit", + Err: "", + }, + { + Name: "Sidecar proxy lifecycle graceful port set via annotation", + Pod: func(pod *corev1.Pod) *corev1.Pod { + pod.Annotations[constants.AnnotationSidecarProxyLifecycleGracefulStartupPath] = "/custom-startup-path" + return pod + }, + LifecycleConfig: Config{ + DefaultGracefulStartupPath: "/quit", + }, + Expected: "/custom-startup-path", + Err: "", + }, + } + + for _, tt := range cases { + t.Run(tt.Name, func(t *testing.T) { + require := require.New(t) + lc := tt.LifecycleConfig + + actual := lc.GracefulStartupPath(*tt.Pod(minimal())) + + require.Equal(tt.Expected, actual) + }) + } +} + func minimal() *corev1.Pod { return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ diff --git a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go index 0c617c5c51..dc9ca0d0bf 100644 --- a/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go +++ b/control-plane/connect-inject/webhook/consul_dataplane_sidecar.go @@ -376,10 +376,16 @@ func (w *MeshWebhook) getContainerSidecarArgs(namespace corev1.Namespace, mpi mu args = append(args, fmt.Sprintf("-shutdown-grace-period-seconds=%d", shutdownGracePeriodSeconds)) gracefulShutdownPath := w.LifecycleConfig.GracefulShutdownPath(pod) + args = append(args, fmt.Sprintf("-graceful-shutdown-path=%s", gracefulShutdownPath)) + + startupGracePeriodSeconds, err := w.LifecycleConfig.StartupGracePeriodSeconds(pod) if err != nil { - return nil, fmt.Errorf("unable to determine proxy lifecycle graceful shutdown path: %w", err) + return nil, fmt.Errorf("unable to determine proxy lifecycle startup grace period: %w", err) } - args = append(args, fmt.Sprintf("-graceful-shutdown-path=%s", gracefulShutdownPath)) + args = append(args, fmt.Sprintf("-startup-grace-period-seconds=%d", startupGracePeriodSeconds)) + + gracefulStartupPath := w.LifecycleConfig.GracefulStartupPath(pod) + args = append(args, fmt.Sprintf("-graceful-startup-path=%s", gracefulStartupPath)) } // Set a default scrape path that can be overwritten by the annotation. diff --git a/control-plane/connect-inject/webhook/consul_dataplane_sidecar_test.go b/control-plane/connect-inject/webhook/consul_dataplane_sidecar_test.go index c7045ac477..4a8386c493 100644 --- a/control-plane/connect-inject/webhook/consul_dataplane_sidecar_test.go +++ b/control-plane/connect-inject/webhook/consul_dataplane_sidecar_test.go @@ -1481,8 +1481,10 @@ func TestHandlerConsulDataplaneSidecar_Metrics(t *testing.T) { func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { gracefulShutdownSeconds := 10 + gracefulStartupSeconds := 10 gracefulPort := "20307" gracefulShutdownPath := "/exit" + gracefulStartupPath := "/start" cases := []struct { name string @@ -1504,12 +1506,14 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { DefaultEnableProxyLifecycle: true, DefaultEnableShutdownDrainListeners: true, DefaultShutdownGracePeriodSeconds: gracefulShutdownSeconds, + DefaultStartupGracePeriodSeconds: gracefulStartupSeconds, DefaultGracefulPort: gracefulPort, DefaultGracefulShutdownPath: gracefulShutdownPath, + DefaultGracefulStartupPath: gracefulStartupPath, }, }, annotations: nil, - expCmdArgs: "graceful-port=20307 -shutdown-drain-listeners -shutdown-grace-period-seconds=10 -graceful-shutdown-path=/exit", + expCmdArgs: "graceful-port=20307 -shutdown-drain-listeners -shutdown-grace-period-seconds=10 -graceful-shutdown-path=/exit -startup-grace-period-seconds=10 -graceful-startup-path=/start", }, { name: "no defaults, all annotations", @@ -1518,10 +1522,12 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { constants.AnnotationEnableSidecarProxyLifecycle: "true", constants.AnnotationEnableSidecarProxyLifecycleShutdownDrainListeners: "true", constants.AnnotationSidecarProxyLifecycleShutdownGracePeriodSeconds: fmt.Sprint(gracefulShutdownSeconds), + constants.AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds: fmt.Sprint(gracefulStartupSeconds), constants.AnnotationSidecarProxyLifecycleGracefulPort: gracefulPort, constants.AnnotationSidecarProxyLifecycleGracefulShutdownPath: gracefulShutdownPath, + constants.AnnotationSidecarProxyLifecycleGracefulStartupPath: gracefulStartupPath, }, - expCmdArgs: "-graceful-port=20307 -shutdown-drain-listeners -shutdown-grace-period-seconds=10 -graceful-shutdown-path=/exit", + expCmdArgs: "-graceful-port=20307 -shutdown-drain-listeners -shutdown-grace-period-seconds=10 -graceful-shutdown-path=/exit -startup-grace-period-seconds=10 -graceful-startup-path=/start", }, { name: "annotations override defaults", @@ -1530,18 +1536,22 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { DefaultEnableProxyLifecycle: false, DefaultEnableShutdownDrainListeners: true, DefaultShutdownGracePeriodSeconds: gracefulShutdownSeconds, + DefaultStartupGracePeriodSeconds: gracefulStartupSeconds, DefaultGracefulPort: gracefulPort, DefaultGracefulShutdownPath: gracefulShutdownPath, + DefaultGracefulStartupPath: gracefulStartupPath, }, }, annotations: map[string]string{ constants.AnnotationEnableSidecarProxyLifecycle: "true", constants.AnnotationEnableSidecarProxyLifecycleShutdownDrainListeners: "false", constants.AnnotationSidecarProxyLifecycleShutdownGracePeriodSeconds: fmt.Sprint(gracefulShutdownSeconds + 5), + constants.AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds: fmt.Sprint(gracefulStartupSeconds + 5), constants.AnnotationSidecarProxyLifecycleGracefulPort: "20317", constants.AnnotationSidecarProxyLifecycleGracefulShutdownPath: "/foo", + constants.AnnotationSidecarProxyLifecycleGracefulStartupPath: "/bar", }, - expCmdArgs: "-graceful-port=20317 -shutdown-grace-period-seconds=15 -graceful-shutdown-path=/foo", + expCmdArgs: "-graceful-port=20317 -shutdown-grace-period-seconds=15 -graceful-shutdown-path=/foo -startup-grace-period-seconds=15 -graceful-startup-path=/bar", }, { name: "lifecycle disabled, no annotations", @@ -1550,8 +1560,10 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { DefaultEnableProxyLifecycle: false, DefaultEnableShutdownDrainListeners: true, DefaultShutdownGracePeriodSeconds: gracefulShutdownSeconds, + DefaultStartupGracePeriodSeconds: gracefulStartupSeconds, DefaultGracefulPort: gracefulPort, DefaultGracefulShutdownPath: gracefulShutdownPath, + DefaultGracefulStartupPath: gracefulStartupPath, }, }, annotations: nil, @@ -1574,8 +1586,10 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { DefaultEnableProxyLifecycle: true, DefaultEnableShutdownDrainListeners: true, DefaultShutdownGracePeriodSeconds: gracefulShutdownSeconds, + DefaultStartupGracePeriodSeconds: gracefulStartupSeconds, DefaultGracefulPort: gracefulPort, DefaultGracefulShutdownPath: gracefulShutdownPath, + DefaultGracefulStartupPath: gracefulStartupPath, }, }, annotations: map[string]string{ diff --git a/control-plane/connect-inject/webhookv2/consul_dataplane_sidecar.go b/control-plane/connect-inject/webhookv2/consul_dataplane_sidecar.go index d3ba5095ac..d94dbeaaac 100644 --- a/control-plane/connect-inject/webhookv2/consul_dataplane_sidecar.go +++ b/control-plane/connect-inject/webhookv2/consul_dataplane_sidecar.go @@ -297,10 +297,16 @@ func (w *MeshWebhook) getContainerSidecarArgs(namespace corev1.Namespace, bearer args = append(args, fmt.Sprintf("-shutdown-grace-period-seconds=%d", shutdownGracePeriodSeconds)) gracefulShutdownPath := w.LifecycleConfig.GracefulShutdownPath(pod) + args = append(args, fmt.Sprintf("-graceful-shutdown-path=%s", gracefulShutdownPath)) + + startupGracePeriodSeconds, err := w.LifecycleConfig.StartupGracePeriodSeconds(pod) if err != nil { - return nil, fmt.Errorf("unable to determine proxy lifecycle graceful shutdown path: %w", err) + return nil, fmt.Errorf("unable to determine proxy lifecycle startup grace period: %w", err) } - args = append(args, fmt.Sprintf("-graceful-shutdown-path=%s", gracefulShutdownPath)) + args = append(args, fmt.Sprintf("-startup-grace-period-seconds=%d", startupGracePeriodSeconds)) + + gracefulStartupPath := w.LifecycleConfig.GracefulStartupPath(pod) + args = append(args, fmt.Sprintf("-graceful-startup-path=%s", gracefulStartupPath)) } // Set a default scrape path that can be overwritten by the annotation. diff --git a/control-plane/connect-inject/webhookv2/consul_dataplane_sidecar_test.go b/control-plane/connect-inject/webhookv2/consul_dataplane_sidecar_test.go index cf9124c673..3b5fb3c0c7 100644 --- a/control-plane/connect-inject/webhookv2/consul_dataplane_sidecar_test.go +++ b/control-plane/connect-inject/webhookv2/consul_dataplane_sidecar_test.go @@ -1123,8 +1123,10 @@ func TestHandlerConsulDataplaneSidecar_Metrics(t *testing.T) { func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { gracefulShutdownSeconds := 10 + gracefulStartupSeconds := 10 gracefulPort := "20307" gracefulShutdownPath := "/exit" + gracefulStartupPath := "/start" cases := []struct { name string @@ -1146,12 +1148,14 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { DefaultEnableProxyLifecycle: true, DefaultEnableShutdownDrainListeners: true, DefaultShutdownGracePeriodSeconds: gracefulShutdownSeconds, + DefaultStartupGracePeriodSeconds: gracefulStartupSeconds, DefaultGracefulPort: gracefulPort, DefaultGracefulShutdownPath: gracefulShutdownPath, + DefaultGracefulStartupPath: gracefulStartupPath, }, }, annotations: nil, - expCmdArgs: "graceful-port=20307 -shutdown-drain-listeners -shutdown-grace-period-seconds=10 -graceful-shutdown-path=/exit", + expCmdArgs: "graceful-port=20307 -shutdown-drain-listeners -shutdown-grace-period-seconds=10 -graceful-shutdown-path=/exit -startup-grace-period-seconds=10 -graceful-startup-path=/start", }, { name: "no defaults, all annotations", @@ -1160,10 +1164,12 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { constants.AnnotationEnableSidecarProxyLifecycle: "true", constants.AnnotationEnableSidecarProxyLifecycleShutdownDrainListeners: "true", constants.AnnotationSidecarProxyLifecycleShutdownGracePeriodSeconds: fmt.Sprint(gracefulShutdownSeconds), + constants.AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds: fmt.Sprint(gracefulStartupSeconds), constants.AnnotationSidecarProxyLifecycleGracefulPort: gracefulPort, constants.AnnotationSidecarProxyLifecycleGracefulShutdownPath: gracefulShutdownPath, + constants.AnnotationSidecarProxyLifecycleGracefulStartupPath: gracefulStartupPath, }, - expCmdArgs: "-graceful-port=20307 -shutdown-drain-listeners -shutdown-grace-period-seconds=10 -graceful-shutdown-path=/exit", + expCmdArgs: "-graceful-port=20307 -shutdown-drain-listeners -shutdown-grace-period-seconds=10 -graceful-shutdown-path=/exit -startup-grace-period-seconds=10 -graceful-startup-path=/start", }, { name: "annotations override defaults", @@ -1172,18 +1178,22 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { DefaultEnableProxyLifecycle: false, DefaultEnableShutdownDrainListeners: true, DefaultShutdownGracePeriodSeconds: gracefulShutdownSeconds, + DefaultStartupGracePeriodSeconds: gracefulStartupSeconds, DefaultGracefulPort: gracefulPort, DefaultGracefulShutdownPath: gracefulShutdownPath, + DefaultGracefulStartupPath: gracefulStartupPath, }, }, annotations: map[string]string{ constants.AnnotationEnableSidecarProxyLifecycle: "true", constants.AnnotationEnableSidecarProxyLifecycleShutdownDrainListeners: "false", constants.AnnotationSidecarProxyLifecycleShutdownGracePeriodSeconds: fmt.Sprint(gracefulShutdownSeconds + 5), + constants.AnnotationSidecarProxyLifecycleStartupGracePeriodSeconds: fmt.Sprint(gracefulStartupSeconds + 5), constants.AnnotationSidecarProxyLifecycleGracefulPort: "20317", constants.AnnotationSidecarProxyLifecycleGracefulShutdownPath: "/foo", + constants.AnnotationSidecarProxyLifecycleGracefulStartupPath: "/bar", }, - expCmdArgs: "-graceful-port=20317 -shutdown-grace-period-seconds=15 -graceful-shutdown-path=/foo", + expCmdArgs: "-graceful-port=20317 -shutdown-grace-period-seconds=15 -graceful-shutdown-path=/foo -startup-grace-period-seconds=15 -graceful-startup-path=/bar", }, { name: "lifecycle disabled, no annotations", @@ -1192,8 +1202,10 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { DefaultEnableProxyLifecycle: false, DefaultEnableShutdownDrainListeners: true, DefaultShutdownGracePeriodSeconds: gracefulShutdownSeconds, + DefaultStartupGracePeriodSeconds: gracefulStartupSeconds, DefaultGracefulPort: gracefulPort, DefaultGracefulShutdownPath: gracefulShutdownPath, + DefaultGracefulStartupPath: gracefulStartupPath, }, }, annotations: nil, @@ -1216,8 +1228,10 @@ func TestHandlerConsulDataplaneSidecar_Lifecycle(t *testing.T) { DefaultEnableProxyLifecycle: true, DefaultEnableShutdownDrainListeners: true, DefaultShutdownGracePeriodSeconds: gracefulShutdownSeconds, + DefaultStartupGracePeriodSeconds: gracefulStartupSeconds, DefaultGracefulPort: gracefulPort, DefaultGracefulShutdownPath: gracefulShutdownPath, + DefaultGracefulStartupPath: gracefulStartupPath, }, }, annotations: map[string]string{ diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index e0c05933b8..192058cf04 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -93,6 +93,9 @@ type Command struct { flagDefaultSidecarProxyStartupFailureSeconds int flagDefaultSidecarProxyLivenessFailureSeconds int + flagDefaultSidecarProxyLifecycleStartupGracePeriodSeconds int + flagDefaultSidecarProxyLifecycleGracefulStartupPath string + // Metrics settings. flagDefaultEnableMetrics bool flagEnableGatewayMetrics bool @@ -254,8 +257,10 @@ func (c *Command) init() { c.flagSet.BoolVar(&c.flagDefaultEnableSidecarProxyLifecycle, "default-enable-sidecar-proxy-lifecycle", false, "Default for enabling sidecar proxy lifecycle management.") c.flagSet.BoolVar(&c.flagDefaultEnableSidecarProxyLifecycleShutdownDrainListeners, "default-enable-sidecar-proxy-lifecycle-shutdown-drain-listeners", false, "Default for enabling sidecar proxy listener draining of inbound connections during shutdown.") c.flagSet.IntVar(&c.flagDefaultSidecarProxyLifecycleShutdownGracePeriodSeconds, "default-sidecar-proxy-lifecycle-shutdown-grace-period-seconds", 0, "Default sidecar proxy shutdown grace period in seconds.") + c.flagSet.IntVar(&c.flagDefaultSidecarProxyLifecycleStartupGracePeriodSeconds, "default-sidecar-proxy-lifecycle-startup-grace-period-seconds", 0, "Default sidecar proxy startup grace period in seconds.") c.flagSet.StringVar(&c.flagDefaultSidecarProxyLifecycleGracefulPort, "default-sidecar-proxy-lifecycle-graceful-port", strconv.Itoa(constants.DefaultGracefulPort), "Default port for sidecar proxy lifecycle management HTTP endpoints.") c.flagSet.StringVar(&c.flagDefaultSidecarProxyLifecycleGracefulShutdownPath, "default-sidecar-proxy-lifecycle-graceful-shutdown-path", "/graceful_shutdown", "Default sidecar proxy lifecycle management graceful shutdown path.") + c.flagSet.StringVar(&c.flagDefaultSidecarProxyLifecycleGracefulStartupPath, "default-sidecar-proxy-lifecycle-graceful-startup-path", "/graceful_startup", "Default sidecar proxy lifecycle management graceful startup path.") c.flagSet.IntVar(&c.flagDefaultSidecarProxyStartupFailureSeconds, "default-sidecar-proxy-startup-failure-seconds", 0, "Default number of seconds for the k8s startup probe to fail before the proxy container is restarted. Zero disables the probe.") c.flagSet.IntVar(&c.flagDefaultSidecarProxyLivenessFailureSeconds, "default-sidecar-proxy-liveness-failure-seconds", 0, "Default number of seconds for the k8s liveness probe to fail before the proxy container is restarted. Zero disables the probe.") diff --git a/control-plane/subcommand/inject-connect/v1controllers.go b/control-plane/subcommand/inject-connect/v1controllers.go index 37ed344dba..288ba92189 100644 --- a/control-plane/subcommand/inject-connect/v1controllers.go +++ b/control-plane/subcommand/inject-connect/v1controllers.go @@ -39,8 +39,10 @@ func (c *Command) configureV1Controllers(ctx context.Context, mgr manager.Manage DefaultEnableProxyLifecycle: c.flagDefaultEnableSidecarProxyLifecycle, DefaultEnableShutdownDrainListeners: c.flagDefaultEnableSidecarProxyLifecycleShutdownDrainListeners, DefaultShutdownGracePeriodSeconds: c.flagDefaultSidecarProxyLifecycleShutdownGracePeriodSeconds, + DefaultStartupGracePeriodSeconds: c.flagDefaultSidecarProxyLifecycleStartupGracePeriodSeconds, DefaultGracefulPort: c.flagDefaultSidecarProxyLifecycleGracefulPort, DefaultGracefulShutdownPath: c.flagDefaultSidecarProxyLifecycleGracefulShutdownPath, + DefaultGracefulStartupPath: c.flagDefaultSidecarProxyLifecycleGracefulStartupPath, } metricsConfig := metrics.Config{ diff --git a/control-plane/subcommand/inject-connect/v2controllers.go b/control-plane/subcommand/inject-connect/v2controllers.go index fce6968ad6..20fbbb5119 100644 --- a/control-plane/subcommand/inject-connect/v2controllers.go +++ b/control-plane/subcommand/inject-connect/v2controllers.go @@ -52,8 +52,10 @@ func (c *Command) configureV2Controllers(ctx context.Context, mgr manager.Manage DefaultEnableProxyLifecycle: c.flagDefaultEnableSidecarProxyLifecycle, DefaultEnableShutdownDrainListeners: c.flagDefaultEnableSidecarProxyLifecycleShutdownDrainListeners, DefaultShutdownGracePeriodSeconds: c.flagDefaultSidecarProxyLifecycleShutdownGracePeriodSeconds, + DefaultStartupGracePeriodSeconds: c.flagDefaultSidecarProxyLifecycleStartupGracePeriodSeconds, DefaultGracefulPort: c.flagDefaultSidecarProxyLifecycleGracefulPort, DefaultGracefulShutdownPath: c.flagDefaultSidecarProxyLifecycleGracefulShutdownPath, + DefaultGracefulStartupPath: c.flagDefaultSidecarProxyLifecycleGracefulStartupPath, } metricsConfig := metrics.Config{ From 3d2f3ae1b947ecd7a4bfc772eb9efe85258316f2 Mon Sep 17 00:00:00 2001 From: Andrea Scarpino Date: Thu, 4 Apr 2024 14:37:58 +0200 Subject: [PATCH 2/8] Add changelog --- .changelog/3878.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/3878.txt diff --git a/.changelog/3878.txt b/.changelog/3878.txt new file mode 100644 index 0000000000..81b7c842b4 --- /dev/null +++ b/.changelog/3878.txt @@ -0,0 +1,3 @@ +```release-note:feature +Add support for configuring graceful startup proxy lifecycle management settings. +``` From 468d846c2887630d92345bc1da0fb94d507b081c Mon Sep 17 00:00:00 2001 From: Andrea Scarpino Date: Mon, 8 Apr 2024 19:52:40 +0200 Subject: [PATCH 3/8] Update control-plane/connect-inject/constants/constants.go Co-authored-by: Michael Zalimeni --- control-plane/connect-inject/constants/constants.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control-plane/connect-inject/constants/constants.go b/control-plane/connect-inject/constants/constants.go index 627b7bf5d2..57f276f949 100644 --- a/control-plane/connect-inject/constants/constants.go +++ b/control-plane/connect-inject/constants/constants.go @@ -62,7 +62,7 @@ const ( // DefaultGracefulShutdownPath is the default path that consul-dataplane uses for graceful shutdown. DefaultGracefulShutdownPath = "/graceful_shutdown" - // DefaultGracefulStartupPath is the default path that consul-dataplane uses for graceful shutdown. + // DefaultGracefulStartupPath is the default path that consul-dataplane uses for graceful startup. DefaultGracefulStartupPath = "/graceful_startup" // DefaultWANPort is the default port that consul-dataplane uses for WAN. From 0978ad70f1ae23ca3c55ab1fe937a7271ba7d01d Mon Sep 17 00:00:00 2001 From: Andrea Scarpino Date: Mon, 8 Apr 2024 19:52:54 +0200 Subject: [PATCH 4/8] Update charts/consul/test/unit/connect-inject-deployment.bats Co-authored-by: Michael Zalimeni --- charts/consul/test/unit/connect-inject-deployment.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index d5c9704446..921040792a 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -1333,7 +1333,7 @@ load _helpers local cmd=$(helm template \ -s templates/connect-inject-deployment.yaml \ --set 'connectInject.enabled=true' \ - --set 'connectInject.sidecarProxy.lifecycle.defaultstartupGracePeriodSeconds=13' \ + --set 'connectInject.sidecarProxy.lifecycle.defaultStartupGracePeriodSeconds=13' \ . | tee /dev/stderr | yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) From 0037da798aea24e64047d79203d4701f28743707 Mon Sep 17 00:00:00 2001 From: Andrea Scarpino Date: Mon, 8 Apr 2024 19:53:02 +0200 Subject: [PATCH 5/8] Update charts/consul/test/unit/connect-inject-deployment.bats Co-authored-by: Michael Zalimeni --- charts/consul/test/unit/connect-inject-deployment.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index 921040792a..a25bcfeee7 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -1414,7 +1414,7 @@ load _helpers local cmd=$(helm template \ -s templates/connect-inject-deployment.yaml \ --set 'connectInject.enabled=true' \ - --set 'connectInject.sidecarProxy.lifecycle.defaultGracefulstartupPath=/start' \ + --set 'connectInject.sidecarProxy.lifecycle.defaultGracefulStartupPath=/start' \ . | tee /dev/stderr | yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) From a1fb6b80825472f6f2544e95662e49a345305ca2 Mon Sep 17 00:00:00 2001 From: Andrea Scarpino Date: Mon, 8 Apr 2024 19:53:09 +0200 Subject: [PATCH 6/8] Update control-plane/connect-inject/lifecycle/lifecycle_configuration.go Co-authored-by: Michael Zalimeni --- .../connect-inject/lifecycle/lifecycle_configuration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control-plane/connect-inject/lifecycle/lifecycle_configuration.go b/control-plane/connect-inject/lifecycle/lifecycle_configuration.go index 488412ec9d..54000a2b01 100644 --- a/control-plane/connect-inject/lifecycle/lifecycle_configuration.go +++ b/control-plane/connect-inject/lifecycle/lifecycle_configuration.go @@ -65,7 +65,7 @@ func (lc Config) ShutdownGracePeriodSeconds(pod corev1.Pod) (int, error) { return shutdownGracePeriodSeconds, nil } -// StartupGracePeriodSeconds returns how long the sidecar proxy should wait after startup, either via the default value in the meshWebhook, or if it's been +// StartupGracePeriodSeconds returns how long to block application startup waiting for the sidecar proxy to be ready, either via the default value in the meshWebhook, or if it's been // overridden via the annotation. func (lc Config) StartupGracePeriodSeconds(pod corev1.Pod) (int, error) { startupGracePeriodSeconds := lc.DefaultStartupGracePeriodSeconds From e75cb9e117d48d3523ef97c09541bde0eee45ba2 Mon Sep 17 00:00:00 2001 From: Andrea Scarpino Date: Mon, 8 Apr 2024 19:54:31 +0200 Subject: [PATCH 7/8] Group flags --- control-plane/subcommand/inject-connect/command.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index 192058cf04..a9db25f3d8 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -87,15 +87,14 @@ type Command struct { flagDefaultEnableSidecarProxyLifecycle bool flagDefaultEnableSidecarProxyLifecycleShutdownDrainListeners bool flagDefaultSidecarProxyLifecycleShutdownGracePeriodSeconds int + flagDefaultSidecarProxyLifecycleStartupGracePeriodSeconds int flagDefaultSidecarProxyLifecycleGracefulPort string flagDefaultSidecarProxyLifecycleGracefulShutdownPath string + flagDefaultSidecarProxyLifecycleGracefulStartupPath string flagDefaultSidecarProxyStartupFailureSeconds int flagDefaultSidecarProxyLivenessFailureSeconds int - flagDefaultSidecarProxyLifecycleStartupGracePeriodSeconds int - flagDefaultSidecarProxyLifecycleGracefulStartupPath string - // Metrics settings. flagDefaultEnableMetrics bool flagEnableGatewayMetrics bool From 0ff7cd6c100b015edb04fb1ae1268c1849742052 Mon Sep 17 00:00:00 2001 From: Andrea Scarpino Date: Mon, 8 Apr 2024 21:05:29 +0200 Subject: [PATCH 8/8] Update control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go Co-authored-by: Michael Zalimeni --- .../lifecycle/lifecycle_configuration_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go b/control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go index 7d792321f2..4da4d47171 100644 --- a/control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go +++ b/control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go @@ -415,19 +415,19 @@ func TestLifecycleConfig_GracefulStartupPath(t *testing.T) { return pod }, LifecycleConfig: Config{ - DefaultGracefulStartupPath: "/quit", + DefaultGracefulStartupPath: "/start", }, - Expected: "/quit", + Expected: "/start", Err: "", }, { - Name: "Sidecar proxy lifecycle graceful port set via annotation", + Name: "Sidecar proxy lifecycle graceful startup path set via annotation", Pod: func(pod *corev1.Pod) *corev1.Pod { pod.Annotations[constants.AnnotationSidecarProxyLifecycleGracefulStartupPath] = "/custom-startup-path" return pod }, LifecycleConfig: Config{ - DefaultGracefulStartupPath: "/quit", + DefaultGracefulStartupPath: "/start", }, Expected: "/custom-startup-path", Err: "",