Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

min sidecar graceperoid #43

Draft
wants to merge 2 commits into
base: release-1.22.9-lyft.5
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions pkg/kubelet/kuberuntime/kuberuntime_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ func (m *kubeGenericRuntimeManager) generateContainerConfig(container *v1.Contai

command, args := kubecontainer.ExpandContainerCommandAndArgs(container, opts.Envs)
logDir := BuildContainerLogsDirectory(pod.Namespace, pod.Name, pod.UID, container.Name)
err = m.osInterface.MkdirAll(logDir, 0755)
err = m.osInterface.MkdirAll(logDir, 0o755)
if err != nil {
return nil, cleanupAction, fmt.Errorf("create container log directory for container %s failed: %v", container.Name, err)
}
Expand Down Expand Up @@ -413,7 +413,7 @@ func (m *kubeGenericRuntimeManager) makeMounts(opts *kubecontainer.RunContainerO
// open(2) to create the file, so the final mode used is "mode &
// ~umask". But we want to make sure the specified mode is used
// in the file no matter what the umask is.
if err := m.osInterface.Chmod(containerLogPath, 0666); err != nil {
if err := m.osInterface.Chmod(containerLogPath, 0o666); err != nil {
utilruntime.HandleError(fmt.Errorf("unable to set termination-log file permissions %q: %v", containerLogPath, err))
}

Expand Down Expand Up @@ -782,9 +782,16 @@ func (m *kubeGenericRuntimeManager) killContainersWithSyncResult(pod *v1.Pod, ru
}
nonSidecarsWg.Wait()

// If non-sidecar containers eat up all of the gracePerioidDuration
// We still want to add a bit of time to give sidecars a chance to gracefully shutdown
// The default value is 10 seconds
// To orveride the default specify the annotation with a duration
// sidecars.lyft.net/sidecar-min-graceperoid=[time in seconds]
minimumGracePeriod := getSidecarMinGraceperoid(pod.Annotations)

gracePeriodDuration = gracePeriodDuration - time.Since(start)
if gracePeriodDuration < 0 {
gracePeriodDuration = 0
if gracePeriodDuration < minimumGracePeriod {
gracePeriodDuration = minimumGracePeriod
}

// then sidecars
Expand Down
24 changes: 22 additions & 2 deletions pkg/kubelet/kuberuntime/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"encoding/json"
"runtime"
"strconv"
"time"

v1 "k8s.io/api/core/v1"
kubetypes "k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -47,8 +48,9 @@ const (
// xref: https://github.com/kubernetes/kubernetes/pull/99576/commits/42fb66073214eed6fe43fa8b1586f396e30e73e3#r635392090
// Currently, ContainerD on Windows does not yet fully support HostProcess containers
// but will pass annotations to hcsshim which does have support.
windowsHostProcessContainer = "microsoft.com/hostprocess-container"
containerSidecarLabel = "com.lyft.sidecars.container-lifecycle"
windowsHostProcessContainer = "microsoft.com/hostprocess-container"
containerSidecarLabel = "com.lyft.sidecars.container-lifecycle"
sidecarMinGraceperoidAnnotation = "sidecars.lyft.net/sidecar-min-graceperoid"
)

type labeledPodSandboxInfo struct {
Expand Down Expand Up @@ -326,3 +328,21 @@ func getJSONObjectFromLabel(labels map[string]string, label string, value interf
// If the label is not found, return not found.
return false, nil
}

// getSidecarMinGraceperoid returns if set the minimum graceperoid for sidecars
// In the event of a parse failure, or the annotation is not present, return a default value.
func getSidecarMinGraceperoid(annotations map[string]string) time.Duration {
const defaultGraceperoid = 10 * time.Second

if min, ok := annotations[sidecarMinGraceperoidAnnotation]; ok {
minGraceperoid, err := strconv.Atoi(min)
if err != nil {
// if for whatever reason this is not parseable
// return a default value
return defaultGraceperoid
}
return time.Duration(minGraceperoid) * time.Second
}

return defaultGraceperoid
}
35 changes: 33 additions & 2 deletions pkg/kubelet/kuberuntime/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ package kuberuntime
import (
"reflect"
"testing"
"time"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
Expand Down Expand Up @@ -64,7 +65,7 @@ func TestContainerLabels(t *testing.T) {
},
}

var tests = []struct {
tests := []struct {
description string
expected *labeledContainerInfo
}{
Expand Down Expand Up @@ -237,3 +238,33 @@ func TestPodAnnotations(t *testing.T) {
t.Errorf("expected %v, got %v", expected, podSandboxInfo)
}
}

func TestGetSidecarMinGraceperoid(t *testing.T) {
tests := []struct {
id string
annotations map[string]string
expect time.Duration
}{
{
id: "default value",
annotations: map[string]string{},
expect: time.Duration(10) * time.Second,
},
{
id: "overrided",
annotations: map[string]string{
sidecarMinGraceperoidAnnotation: "100",
},
expect: time.Duration(100) * time.Second,
},
}

for _, test := range tests {
t.Run(test.id, func(t *testing.T) {
actual := getSidecarMinGraceperoid(test.annotations)
if test.expect != actual {
t.Errorf("expected: %v, got %v", test.expect, actual)
}
})
}
}