Skip to content

Commit

Permalink
feat: Add startup-grace-period-seconds and graceful-startup-path
Browse files Browse the repository at this point in the history
  • Loading branch information
ilpianista committed Apr 4, 2024
1 parent b609660 commit 547d97c
Show file tree
Hide file tree
Showing 15 changed files with 277 additions and 10 deletions.
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 shutdown.
DefaultGracefulStartupPath = "/graceful_startup"

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

Expand Down
30 changes: 30 additions & 0 deletions control-plane/connect-inject/lifecycle/lifecycle_configuration.go
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 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) {
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
}
119 changes: 119 additions & 0 deletions control-plane/connect-inject/lifecycle/lifecycle_configuration_test.go
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",
},
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
10 changes: 8 additions & 2 deletions control-plane/connect-inject/webhook/consul_dataplane_sidecar.go
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)
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

0 comments on commit 547d97c

Please sign in to comment.