Skip to content
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

[NET-8938] feat: Add startup-grace-period-seconds and graceful-startup-path #3878

Merged
merged 8 commits into from
Apr 9, 2024
3 changes: 3 additions & 0 deletions .changelog/3878.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
Add support for configuring graceful startup proxy lifecycle management settings.
```
2 changes: 2 additions & 0 deletions charts/consul/templates/connect-inject-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
54 changes: 54 additions & 0 deletions charts/consul/test/unit/connect-inject-deployment.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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

Expand Down
6 changes: 6 additions & 0 deletions charts/consul/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions cli/helm/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions control-plane/connect-inject/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 startup.
DefaultGracefulStartupPath = "/graceful_startup"

// DefaultWANPort is the default port that consul-dataplane uses for WAN.
DefaultWANPort = 8443

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,6 +65,20 @@ func (lc Config) ShutdownGracePeriodSeconds(pod corev1.Pod) (int, error) {
return shutdownGracePeriodSeconds, nil
}

// 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
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) {
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
},
ilpianista marked this conversation as resolved.
Show resolved Hide resolved
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{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
ilpianista marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down
Loading
Loading