Skip to content

Commit f854ec2

Browse files
committed
feat: add drain delay to graceful shutdown process
When a pod terminates, the shutdown-manager immediately calls `healthcheck/fail` to fail health checks. This causes the readiness probe to fail and removes the pod from service endpoints. External load balancers that use health checks for deregistration need several failed probes before they stop sending new connections to the pod. On GKE, the L4 passthrough load balancer probes every 3 seconds and needs 2 failures (at the time of writing), so deregistration takes 6 seconds or more. The pod readiness probe fails faster than that, so for a few seconds the load balancer keeps sending connections that kube-proxy no longer routes. Kubernetes has a mechanism for this window. When a service has no other ready endpoints, kube-proxy keeps routing to terminating pods that still pass their readiness probe (KEP-1669). This only works if the pod keeps passing its probe while the load balancer deregisters it. Calling `healthcheck/fail` at the start of shutdown breaks that. This adds a drainDelay field to ShutdownConfig which delays the `healthcheck/fail` call. During the delay the pod stays ready and keeps serving, so the load balancer has time to deregister it. The ready-timeout and termination grace period are extended by the delay. Signed-off-by: Michael Wain <michael@sanity.io>
1 parent 14b11d1 commit f854ec2

10 files changed

Lines changed: 161 additions & 11 deletions

File tree

api/v1alpha1/envoyproxy_helpers.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,17 @@ import (
99
"fmt"
1010
"sort"
1111
"strings"
12+
"time"
1213

1314
autoscalingv2 "k8s.io/api/autoscaling/v2"
1415
corev1 "k8s.io/api/core/v1"
1516
"k8s.io/apimachinery/pkg/api/resource"
1617
)
1718

19+
// DefaultDrainTimeout is the default drain timeout for the graceful drain
20+
// sequence, used when ShutdownConfig.DrainTimeout is not specified.
21+
const DefaultDrainTimeout = 60 * time.Second
22+
1823
// DefaultEnvoyProxyProvider returns a new EnvoyProxyProvider with default settings.
1924
func DefaultEnvoyProxyProvider() *EnvoyProxyProvider {
2025
return &EnvoyProxyProvider{

internal/cmd/envoy.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/spf13/cobra"
1212

13+
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
1314
"github.com/envoyproxy/gateway/internal/cmd/envoy"
1415
)
1516

@@ -28,6 +29,7 @@ func GetEnvoyCommand() *cobra.Command {
2829

2930
// getShutdownCommand returns the shutdown cobra command to be executed.
3031
func getShutdownCommand() *cobra.Command {
32+
var drainDelay time.Duration
3133
var drainTimeout time.Duration
3234
var minDrainDuration time.Duration
3335
var exitAtConnections int
@@ -36,11 +38,14 @@ func getShutdownCommand() *cobra.Command {
3638
Use: "shutdown",
3739
Short: "Gracefully drain open connections prior to pod shutdown.",
3840
RunE: func(_ *cobra.Command, _ []string) error {
39-
return envoy.Shutdown(drainTimeout, minDrainDuration, exitAtConnections)
41+
return envoy.Shutdown(drainDelay, drainTimeout, minDrainDuration, exitAtConnections)
4042
},
4143
}
4244

43-
cmd.PersistentFlags().DurationVar(&drainTimeout, "drain-timeout", 60*time.Second,
45+
cmd.PersistentFlags().DurationVar(&drainDelay, "drain-delay", 0*time.Second,
46+
"Delay before starting the drain process.")
47+
48+
cmd.PersistentFlags().DurationVar(&drainTimeout, "drain-timeout", egv1a1.DefaultDrainTimeout,
4449
"Graceful shutdown timeout. This should be less than the pod's terminationGracePeriodSeconds.")
4550

4651
cmd.PersistentFlags().DurationVar(&minDrainDuration, "min-drain-duration", 10*time.Second,

internal/cmd/envoy/shutdown_manager.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,21 @@ func shutdownReadyHandler(w http.ResponseWriter, readyTimeout time.Duration, rea
118118
// Shutdown is called from a preStop hook on the shutdown-manager container where
119119
// it will initiate a drain sequence on the Envoy proxy and block until
120120
// connections are drained or a timeout is exceeded.
121-
func Shutdown(drainTimeout, minDrainDuration time.Duration, exitAtConnections int) error {
122-
startTime := time.Now()
121+
func Shutdown(drainDelay, drainTimeout, minDrainDuration time.Duration, exitAtConnections int) error {
123122
allowedToExit := false
124123

125124
// Reconfigure logger to write to stdout of main process if running in Kubernetes
126125
if _, k8s := os.LookupEnv("KUBERNETES_SERVICE_HOST"); k8s && os.Getpid() != 1 {
127126
logger = logging.FileLogger("/proc/1/fd/1", "shutdown-manager", egv1a1.LogLevelInfo)
128127
}
129128

129+
if drainDelay > 0 {
130+
logger.Info(fmt.Sprintf("waiting %.0f seconds before starting drain", drainDelay.Seconds()))
131+
time.Sleep(drainDelay)
132+
}
133+
134+
startTime := time.Now()
135+
130136
logger.Info(fmt.Sprintf("initiating drain with %.0f second minimum drain period and %.0f second timeout",
131137
minDrainDuration.Seconds(), drainTimeout.Seconds()))
132138

internal/infrastructure/common/proxy_args.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func BuildProxyArgs(
8484
}
8585

8686
// Default drain timeout.
87-
drainTimeout := 60.0
87+
drainTimeout := egv1a1.DefaultDrainTimeout.Seconds()
8888
if shutdownConfig != nil && shutdownConfig.DrainTimeout != nil {
8989
d, err := time.ParseDuration(string(*shutdownConfig.DrainTimeout))
9090
if err != nil {

internal/infrastructure/kubernetes/proxy/resource.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -266,15 +266,32 @@ func expectedShutdownManagerImage(shutdownManager *egv1a1.ShutdownManager) strin
266266
}
267267

268268
func expectedShutdownManagerArgs(cfg *egv1a1.ShutdownConfig) []string {
269-
args := []string{"envoy", "shutdown-manager"}
270-
if cfg != nil && cfg.DrainTimeout != nil {
269+
args := make([]string, 0, 3)
270+
args = append(args, "envoy", "shutdown-manager")
271+
272+
// Use default --ready-timeout when neither drain field is configured.
273+
if cfg == nil || (cfg.DrainTimeout == nil && cfg.DrainDelay == nil) {
274+
return args
275+
}
276+
277+
readyTimeout := egv1a1.DefaultDrainTimeout
278+
if cfg.DrainTimeout != nil {
271279
d, err := time.ParseDuration(string(*cfg.DrainTimeout))
272280
if err != nil {
273281
return nil
274282
}
275-
args = append(args, fmt.Sprintf("--ready-timeout=%.0fs", d.Seconds()+10))
283+
readyTimeout = d
276284
}
277-
return args
285+
286+
if cfg.DrainDelay != nil {
287+
delay, err := time.ParseDuration(string(*cfg.DrainDelay))
288+
if err != nil {
289+
return nil
290+
}
291+
readyTimeout += delay
292+
}
293+
294+
return append(args, fmt.Sprintf("--ready-timeout=%.0fs", readyTimeout.Seconds()+10))
278295
}
279296

280297
func expectedShutdownPreStopCommand(cfg *egv1a1.ShutdownConfig) []string {
@@ -284,6 +301,14 @@ func expectedShutdownPreStopCommand(cfg *egv1a1.ShutdownConfig) []string {
284301
return command
285302
}
286303

304+
if cfg.DrainDelay != nil {
305+
d, err := time.ParseDuration(string(*cfg.DrainDelay))
306+
if err != nil {
307+
return nil
308+
}
309+
command = append(command, fmt.Sprintf("--drain-delay=%.0fs", d.Seconds()))
310+
}
311+
287312
if cfg.DrainTimeout != nil {
288313
d, err := time.ParseDuration(string(*cfg.DrainTimeout))
289314
if err != nil {

internal/infrastructure/kubernetes/proxy/resource_provider.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,15 @@ func expectedTerminationGracePeriodSeconds(cfg *egv1a1.ShutdownConfig) *int64 {
620620
}
621621
s = int(d.Seconds() + 300) // 5 minutes longer than drain timeout
622622
}
623+
624+
if cfg != nil && cfg.DrainDelay != nil {
625+
d, err := time.ParseDuration(string(*cfg.DrainDelay))
626+
if err != nil {
627+
return nil
628+
}
629+
s += int(d.Seconds())
630+
}
631+
623632
return new(int64(s))
624633
}
625634

internal/infrastructure/kubernetes/proxy/resource_provider_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ func TestDeployment(t *testing.T) {
292292
},
293293
},
294294
shutdown: &egv1a1.ShutdownConfig{
295+
DrainDelay: new(gwapiv1.Duration("15s")),
295296
DrainTimeout: new(gwapiv1.Duration("30s")),
296297
MinDrainDuration: new(gwapiv1.Duration("15s")),
297298
},
@@ -2120,3 +2121,52 @@ func writeTestDataToFile(filename string, resources []any) error {
21202121

21212122
return os.WriteFile(filename, combinedYAML, 0o600)
21222123
}
2124+
2125+
func TestExpectedTerminationGracePeriodSeconds(t *testing.T) {
2126+
tests := []struct {
2127+
name string
2128+
cfg *egv1a1.ShutdownConfig
2129+
expected int64
2130+
}{
2131+
{
2132+
name: "nil config",
2133+
cfg: nil,
2134+
expected: 360,
2135+
},
2136+
{
2137+
name: "empty config",
2138+
cfg: &egv1a1.ShutdownConfig{},
2139+
expected: 360,
2140+
},
2141+
{
2142+
name: "only drainTimeout",
2143+
cfg: &egv1a1.ShutdownConfig{
2144+
DrainTimeout: new(gwapiv1.Duration("30s")),
2145+
},
2146+
expected: 330,
2147+
},
2148+
{
2149+
name: "only drainDelay",
2150+
cfg: &egv1a1.ShutdownConfig{
2151+
DrainDelay: new(gwapiv1.Duration("15s")),
2152+
},
2153+
expected: 375,
2154+
},
2155+
{
2156+
name: "drainDelay and drainTimeout",
2157+
cfg: &egv1a1.ShutdownConfig{
2158+
DrainDelay: new(gwapiv1.Duration("15s")),
2159+
DrainTimeout: new(gwapiv1.Duration("30s")),
2160+
},
2161+
expected: 345,
2162+
},
2163+
}
2164+
2165+
for _, tt := range tests {
2166+
t.Run(tt.name, func(t *testing.T) {
2167+
got := expectedTerminationGracePeriodSeconds(tt.cfg)
2168+
require.NotNil(t, got)
2169+
require.Equal(t, tt.expected, *got)
2170+
})
2171+
}
2172+
}

internal/infrastructure/kubernetes/proxy/resource_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/stretchr/testify/require"
1414
corev1 "k8s.io/api/core/v1"
15+
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
1516

1617
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
1718
"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/resource"
@@ -182,3 +183,50 @@ func TestGetImageTag(t *testing.T) {
182183
})
183184
}
184185
}
186+
187+
func TestExpectedShutdownManagerArgs(t *testing.T) {
188+
tests := []struct {
189+
name string
190+
cfg *egv1a1.ShutdownConfig
191+
expected []string
192+
}{
193+
{
194+
name: "nil config",
195+
cfg: nil,
196+
expected: []string{"envoy", "shutdown-manager"},
197+
},
198+
{
199+
name: "empty config",
200+
cfg: &egv1a1.ShutdownConfig{},
201+
expected: []string{"envoy", "shutdown-manager"},
202+
},
203+
{
204+
name: "only drainTimeout",
205+
cfg: &egv1a1.ShutdownConfig{
206+
DrainTimeout: new(gwapiv1.Duration("30s")),
207+
},
208+
expected: []string{"envoy", "shutdown-manager", "--ready-timeout=40s"},
209+
},
210+
{
211+
name: "only drainDelay",
212+
cfg: &egv1a1.ShutdownConfig{
213+
DrainDelay: new(gwapiv1.Duration("15s")),
214+
},
215+
expected: []string{"envoy", "shutdown-manager", "--ready-timeout=85s"},
216+
},
217+
{
218+
name: "drainDelay and drainTimeout",
219+
cfg: &egv1a1.ShutdownConfig{
220+
DrainDelay: new(gwapiv1.Duration("15s")),
221+
DrainTimeout: new(gwapiv1.Duration("30s")),
222+
},
223+
expected: []string{"envoy", "shutdown-manager", "--ready-timeout=55s"},
224+
},
225+
}
226+
227+
for _, tt := range tests {
228+
t.Run(tt.name, func(t *testing.T) {
229+
require.Equal(t, tt.expected, expectedShutdownManagerArgs(tt.cfg))
230+
})
231+
}
232+
}

internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ spec:
309309
- args:
310310
- envoy
311311
- shutdown-manager
312-
- --ready-timeout=40s
312+
- --ready-timeout=55s
313313
command:
314314
- envoy-gateway
315315
env:
@@ -341,6 +341,7 @@ spec:
341341
- envoy-gateway
342342
- envoy
343343
- shutdown
344+
- --drain-delay=15s
344345
- --drain-timeout=30s
345346
- --min-drain-duration=15s
346347
livenessProbe:
@@ -395,7 +396,7 @@ spec:
395396
restartPolicy: Always
396397
schedulerName: default-scheduler
397398
serviceAccountName: envoy-default-37a8eec1
398-
terminationGracePeriodSeconds: 330
399+
terminationGracePeriodSeconds: 345
399400
volumes:
400401
- name: certs
401402
secret:

release-notes/current.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ security updates: |
1313
new features: |
1414
Added support for authorization path match.
1515
Added a `requestBody` field to the HTTP active health checker in `BackendTrafficPolicy`, allowing a request body payload to be sent during HTTP health checking. The field requires the health check `method` to be `POST` or `PUT`.
16+
Added a `drainDelay` field to `ShutdownConfig`, delaying the start of the Envoy proxy drain sequence on pod termination. During the delay the proxy continues to accept and serve new connections, allowing external load balancers that rely on health-check based de-registration (e.g. L4 passthrough load balancers) to stop sending traffic to the terminating pod before it stops serving.
1617
1718
bug fixes: |
1819
Fixed Backend TLS `alpnProtocols: []` to disable upstream ALPN instead of inheriting EnvoyProxy BackendTLS defaults.

0 commit comments

Comments
 (0)