revisit envoy shutdown settings (envoyproxy#4288)

* Set default minDrainDuration to `10s` . Since the default `readinessProbe.periodSeconds` is `5s`, this gives any LB controller `5s` to update its endpoint pool if its basing it off the k8s API server * Set default `drainTimeout` to `60s`. This ensures clients holding persistent connections, can be closed sooner. Fixes: envoyproxy#4125 * Updates the default `terminationGracePeriodSeconds` to `360s` which is `300s` more than the default drain timeout Signed-off-by: Arko Dasgupta <[email protected]> Co-authored-by: zirain <[email protected]>
zhaohuabing · Sep 21, 2024 · 475cd61 · 475cd61
1 parent f4ae934
commit 475cd61
Show file tree

Hide file tree

Showing 42 changed files with 85 additions and 47 deletions.
diff --git a/api/v1alpha1/envoyproxy_types.go b/api/v1alpha1/envoyproxy_types.go
@@ -256,12 +256,12 @@ type EnvoyProxyProvider struct {
 // ShutdownConfig defines configuration for graceful envoy shutdown process.
 type ShutdownConfig struct {
 	// DrainTimeout defines the graceful drain timeout. This should be less than the pod's terminationGracePeriodSeconds.
-	// If unspecified, defaults to 600 seconds.
+	// If unspecified, defaults to 60 seconds.
 	//
 	// +optional
 	DrainTimeout *metav1.Duration `json:"drainTimeout,omitempty"`
 	// MinDrainDuration defines the minimum drain duration allowing time for endpoint deprogramming to complete.
-	// If unspecified, defaults to 5 seconds.
+	// If unspecified, defaults to 10 seconds.
 	//
 	// +optional
 	MinDrainDuration *metav1.Duration `json:"minDrainDuration,omitempty"`

diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml
@@ -10248,12 +10248,12 @@ spec:
                   drainTimeout:
                     description: |-
                       DrainTimeout defines the graceful drain timeout. This should be less than the pod's terminationGracePeriodSeconds.
-                      If unspecified, defaults to 600 seconds.
+                      If unspecified, defaults to 60 seconds.
                     type: string
                   minDrainDuration:
                     description: |-
                       MinDrainDuration defines the minimum drain duration allowing time for endpoint deprogramming to complete.
-                      If unspecified, defaults to 5 seconds.
+                      If unspecified, defaults to 10 seconds.
                     type: string
                 type: object
               telemetry:

diff --git a/internal/cmd/envoy.go b/internal/cmd/envoy.go
@@ -40,10 +40,10 @@ func getShutdownCommand() *cobra.Command {
 		},
 	}
 
-	cmd.PersistentFlags().DurationVar(&drainTimeout, "drain-timeout", 600*time.Second,
+	cmd.PersistentFlags().DurationVar(&drainTimeout, "drain-timeout", 60*time.Second,
 		"Graceful shutdown timeout. This should be less than the pod's terminationGracePeriodSeconds.")
 
-	cmd.PersistentFlags().DurationVar(&minDrainDuration, "min-drain-duration", 5*time.Second,
+	cmd.PersistentFlags().DurationVar(&minDrainDuration, "min-drain-duration", 10*time.Second,
 		"Minimum drain duration allowing time for endpoint deprogramming to complete.")
 
 	cmd.PersistentFlags().IntVar(&exitAtConnections, "exit-at-connections", 0,

diff --git a/internal/infrastructure/kubernetes/proxy/resource.go b/internal/infrastructure/kubernetes/proxy/resource.go
@@ -183,9 +183,12 @@ func expectedProxyContainers(infra *ir.ProxyInfra,
 		args = append(args, fmt.Sprintf("--component-log-level %s", componentsLogLevel))
 	}
 
+	// Default
+	drainTimeout := 60.0
 	if shutdownConfig != nil && shutdownConfig.DrainTimeout != nil {
-		args = append(args, fmt.Sprintf("--drain-time-s %.0f", shutdownConfig.DrainTimeout.Seconds()))
+		drainTimeout = shutdownConfig.DrainTimeout.Seconds()
 	}
+	args = append(args, fmt.Sprintf("--drain-time-s %.0f", drainTimeout))
 
 	if infra.Config != nil {
 		args = append(args, infra.Config.Spec.ExtraArgs...)

diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider.go b/internal/infrastructure/kubernetes/proxy/resource_provider.go
@@ -442,7 +442,7 @@ func (r *ResourceRender) HorizontalPodAutoscaler() (*autoscalingv2.HorizontalPod
 }
 
 func expectedTerminationGracePeriodSeconds(cfg *egv1a1.ShutdownConfig) *int64 {
-	s := 900 // default
+	s := 360 // default
 	if cfg != nil && cfg.DrainTimeout != nil {
 		s = int(cfg.DrainTimeout.Seconds() + 300) // 5 minutes longer than drain timeout
 	}

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/component-level.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/component-level.yaml
@@ -42,6 +42,7 @@ spec:
         - --cpuset-threads
         - --drain-strategy immediate
         - --component-log-level filter:info
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -185,7 +186,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/custom.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/custom.yaml
@@ -225,6 +225,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -366,7 +367,7 @@ spec:
       securityContext:
         runAsUser: 1000
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default-env.yaml
@@ -224,6 +224,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -365,7 +366,7 @@ spec:
       securityContext:
         runAsUser: 1000
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default.yaml
@@ -209,6 +209,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -352,7 +353,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/disable-prometheus.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/disable-prometheus.yaml
@@ -183,6 +183,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -323,7 +324,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/extension-env.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/extension-env.yaml
@@ -224,6 +224,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -369,7 +370,7 @@ spec:
       securityContext:
         runAsUser: 1000
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/.../infrastructure/kubernetes/proxy/testdata/daemonsets/override-labels-and-annotations.yaml b/.../infrastructure/kubernetes/proxy/testdata/daemonsets/override-labels-and-annotations.yaml
@@ -218,6 +218,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -361,7 +362,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/patch-daemonset.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/patch-daemonset.yaml
@@ -209,6 +209,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -353,7 +354,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/volumes.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/volumes.yaml
@@ -224,6 +224,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -369,7 +370,7 @@ spec:
       securityContext:
         runAsUser: 1000
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-annotations.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-annotations.yaml
@@ -214,6 +214,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -357,7 +358,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-concurrency.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-concurrency.yaml
@@ -42,6 +42,7 @@ spec:
         - --cpuset-threads
         - --drain-strategy immediate
         - --concurrency 4
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -185,7 +186,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-extra-args.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-extra-args.yaml
@@ -209,6 +209,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         - --key1 val1
         - --key2 val2
         command:
@@ -354,7 +355,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-image-pull-secrets.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-image-pull-secrets.yaml
@@ -209,6 +209,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -355,7 +356,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-name.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-name.yaml
@@ -209,6 +209,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -352,7 +353,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-node-selector.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-node-selector.yaml
@@ -209,6 +209,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -355,7 +356,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/...infrastructure/kubernetes/proxy/testdata/daemonsets/with-topology-spread-constraints.yaml b/...infrastructure/kubernetes/proxy/testdata/daemonsets/with-topology-spread-constraints.yaml
@@ -209,6 +209,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -352,7 +353,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       topologySpreadConstraints:
       - labelSelector:
           matchLabels:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml
@@ -45,6 +45,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -188,7 +189,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml
@@ -46,6 +46,7 @@ spec:
         - --cpuset-threads
         - --drain-strategy immediate
         - --component-log-level filter:info
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -189,7 +190,7 @@ spec:
       restartPolicy: Always
       schedulerName: default-scheduler
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret:

diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/custom.yaml
@@ -230,6 +230,7 @@ spec:
         - --log-level warn
         - --cpuset-threads
         - --drain-strategy immediate
+        - --drain-time-s 60
         command:
         - envoy
         env:
@@ -371,7 +372,7 @@ spec:
       securityContext:
         runAsUser: 1000
       serviceAccountName: envoy-default-37a8eec1
-      terminationGracePeriodSeconds: 900
+      terminationGracePeriodSeconds: 360
       volumes:
       - name: certs
         secret: