Skip to content

Commit

Permalink
Set termination grace period upon function updates
Browse files Browse the repository at this point in the history
This was missed from #869 and fixes the controller so that
it updates the termination grace period for functions.

The problem would be that if a user updated the
write_timeout variable used to compute the grace
period, it would be ignored.

The operator uses common code for deploy/update, so does
not need a separate change.

Thanks to @kevin-lindsay-1 for doing exploratory testing
to find this.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
  • Loading branch information
alexellis committed Nov 5, 2021
1 parent f0a81e6 commit 3ab2d57
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 17 deletions.
39 changes: 22 additions & 17 deletions pkg/handlers/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,23 +174,8 @@ func makeDeploymentSpec(request types.FunctionDeployment, existingSecrets map[st
return nil, err
}

// 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 {
period, err := time.ParseDuration(v)
if err != nil {
glog.Warningf("Function %s failed to parse write_timeout: %s",
request.Service, err.Error())
}

terminationGracePeriod = period + jitter
}
}

terminationGracePeriodSeconds := int64(terminationGracePeriod.Seconds())
terminationGracePeriodSeconds :=
getTerminationGracePeriodSeconds(request.EnvVars, request.Service)

enableServiceLinks := false
allowPrivilegeEscalation := false
Expand Down Expand Up @@ -277,6 +262,26 @@ func makeDeploymentSpec(request types.FunctionDeployment, existingSecrets map[st
return deploymentSpec, nil
}

func getTerminationGracePeriodSeconds(envVars map[string]string, name string) int64 {
// add 2s jitter to avoid a race condition between write_timeout and grace period
jitter := time.Second * 2
terminationGracePeriod := time.Second*30 + jitter

if envVars != nil {
if v, ok := envVars["write_timeout"]; ok && len(v) > 0 {
period, err := time.ParseDuration(v)
if err != nil {
glog.Warningf("Function %s failed to parse write_timeout: %s",
name, err.Error())
}

terminationGracePeriod = period + jitter
}
}

return int64(terminationGracePeriod.Seconds())
}

func makeServiceSpec(request types.FunctionDeployment, factory k8s.FunctionFactory) *corev1.Service {

serviceSpec := &corev1.Service{
Expand Down
5 changes: 5 additions & 0 deletions pkg/handlers/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ func updateDeploymentSpec(
deployment.Spec.Template.Spec.Containers[0].LivenessProbe = probes.Liveness
deployment.Spec.Template.Spec.Containers[0].ReadinessProbe = probes.Readiness

terminationGracePeriodSeconds :=
getTerminationGracePeriodSeconds(request.EnvVars, request.Service)

deployment.Spec.Template.Spec.TerminationGracePeriodSeconds = &terminationGracePeriodSeconds

// compare the annotations from args to the cache copy of the deployment annotations
// at this point we have already updated the annotations to the new value, if we
// compare to that it will produce an empty list
Expand Down

0 comments on commit 3ab2d57

Please sign in to comment.