Add 2s jitter to termination grace period

The additional 2s should prevent an issue where the grace
period is exactly the same as the timeout, and kills the Pod
before the remaining requests have completed.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <[email protected]>

Author: alexellis

pkg/controller/deployment.go

@@ -57,7 +57,9 @@ func newDeployment(
 		}
 	}
 
-	terminationGracePeriod := time.Second * 30
+	// add 2s jitter to avoid a race condition between write_timeout and grace period
+	jitter := time.Second * 2
+	terminationGracePeriod := time.Second*30 + jitter
 
 	if function.Spec.Environment != nil {
 		e := *function.Spec.Environment
@@ -67,9 +69,11 @@ func newDeployment(
 				glog.Warningf("Function %s failed to parse write_timeout: %s",
 					function.Spec.Name, err.Error())
 			}
-			terminationGracePeriod = period
+
+			terminationGracePeriod = period + jitter
 		}
 	}
+
 	terminationGracePeriodSeconds := int64(terminationGracePeriod.Seconds())
 
 	allowPrivilegeEscalation := false

pkg/controller/deployment_test.go

@@ -13,18 +13,18 @@ import (
 func Test_GracePeriodFromWriteTimeout(t *testing.T) {
 
 	scenarios := []struct {
-		name    string
-		seconds int64
-		envs    map[string]string
+		name        string
+		wantSeconds int64
+		envs        map[string]string
 	}{
-		{"grace period is the default", 30, map[string]string{}},
-		{"grace period is set from write_timeout", 60, map[string]string{"write_timeout": "60s"}},
+		{"grace period is the default", 32, map[string]string{}},
+		{"grace period is set from write_timeout", 62, map[string]string{"write_timeout": "60s"}},
 	}
 
 	for _, s := range scenarios {
 		t.Run(s.name, func(t *testing.T) {
 
-			want := int64(s.seconds)
+			want := int64(s.wantSeconds)
 			function := &faasv1.Function{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "alpine",

pkg/handlers/deploy.go

@@ -174,7 +174,9 @@ func makeDeploymentSpec(request types.FunctionDeployment, existingSecrets map[st
 		return nil, err
 	}
 
-	terminationGracePeriod := time.Second * 30
+	// add 2s jitter to avoid a race condition between write_timeout and grace period
+	jitter := time.Second * 2
+	terminationGracePeriod := time.Second*30 + jitter
 
 	if request.EnvVars != nil {
 		if v, ok := request.EnvVars["write_timeout"]; ok && len(v) > 0 {
@@ -183,7 +185,8 @@ func makeDeploymentSpec(request types.FunctionDeployment, existingSecrets map[st
 				glog.Warningf("Function %s failed to parse write_timeout: %s",
 					request.Service, err.Error())
 			}
-			terminationGracePeriod = period
+
+			terminationGracePeriod = period + jitter
 		}
 	}
 

pkg/handlers/deploy_test.go

@@ -16,12 +16,12 @@ import (
 func Test_GracePeriodFromWriteTimeout(t *testing.T) {
 
 	scenarios := []struct {
-		name    string
-		seconds int64
-		envs    map[string]string
+		name        string
+		wantSeconds int64
+		envs        map[string]string
 	}{
-		{"grace period is the default", 30, map[string]string{}},
-		{"grace period is set from write_timeout", 60, map[string]string{"write_timeout": "60s"}},
+		{"grace period is the default", 32, map[string]string{}},
+		{"grace period is set from write_timeout", 62, map[string]string{"write_timeout": "60s"}},
 	}
 
 	for _, s := range scenarios {
@@ -38,7 +38,7 @@ func Test_GracePeriodFromWriteTimeout(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected makeDeploymentSpec error: %s", err.Error())
 			}
-			want := s.seconds
+			want := s.wantSeconds
 			got := deployment.Spec.Template.Spec.TerminationGracePeriodSeconds
 
 			if got == nil {