From a471843246aff6cd69a73a205424bd6804601e9f Mon Sep 17 00:00:00 2001 From: knight42 Date: Fri, 7 Aug 2020 16:54:38 +0800 Subject: [PATCH] fix(kubelet): protect `containerCleanupInfos` from concurrent map writes Signed-off-by: knight42 --- pkg/kubelet/dockershim/docker_container.go | 25 +++++++++++++++++++--- pkg/kubelet/dockershim/docker_service.go | 3 ++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/pkg/kubelet/dockershim/docker_container.go b/pkg/kubelet/dockershim/docker_container.go index 01b865f9d2a..3c61e13cf91 100644 --- a/pkg/kubelet/dockershim/docker_container.go +++ b/pkg/kubelet/dockershim/docker_container.go @@ -83,6 +83,25 @@ func (ds *dockerService) ListContainers(_ context.Context, r *runtimeapi.ListCon return &runtimeapi.ListContainersResponse{Containers: result}, nil } +func (ds *dockerService) getContainerCleanupInfo(containerID string) (*containerCleanupInfo, bool) { + ds.cleanupInfosLock.RLock() + defer ds.cleanupInfosLock.RUnlock() + info, ok := ds.containerCleanupInfos[containerID] + return info, ok +} + +func (ds *dockerService) setContainerCleanupInfo(containerID string, info *containerCleanupInfo) { + ds.cleanupInfosLock.Lock() + defer ds.cleanupInfosLock.Unlock() + ds.containerCleanupInfos[containerID] = info +} + +func (ds *dockerService) clearContainerCleanupInfo(containerID string) { + ds.cleanupInfosLock.Lock() + defer ds.cleanupInfosLock.Unlock() + delete(ds.containerCleanupInfos, containerID) +} + // CreateContainer creates a new container in the given PodSandbox // Docker cannot store the log to an arbitrary location (yet), so we create an // symlink at LogPath, linking to the actual path of the log. @@ -185,7 +204,7 @@ func (ds *dockerService) CreateContainer(_ context.Context, r *runtimeapi.Create // we don't perform the clean up just yet at that could destroy information // needed for the container to start (e.g. Windows credentials stored in // registry keys); instead, we'll clean up when the container gets removed - ds.containerCleanupInfos[containerID] = cleanupInfo + ds.setContainerCleanupInfo(containerID, cleanupInfo) } return &runtimeapi.CreateContainerResponse{ContainerId: containerID}, nil } @@ -461,11 +480,11 @@ func (ds *dockerService) UpdateContainerResources(_ context.Context, r *runtimea } func (ds *dockerService) performPlatformSpecificContainerForContainer(containerID string) (errors []error) { - if cleanupInfo, present := ds.containerCleanupInfos[containerID]; present { + if cleanupInfo, present := ds.getContainerCleanupInfo(containerID); present { errors = ds.performPlatformSpecificContainerCleanupAndLogErrors(containerID, cleanupInfo) if len(errors) == 0 { - delete(ds.containerCleanupInfos, containerID) + ds.clearContainerCleanupInfo(containerID) } } diff --git a/pkg/kubelet/dockershim/docker_service.go b/pkg/kubelet/dockershim/docker_service.go index d9665698cc3..74a7b5d0812 100644 --- a/pkg/kubelet/dockershim/docker_service.go +++ b/pkg/kubelet/dockershim/docker_service.go @@ -31,7 +31,7 @@ import ( dockertypes "github.com/docker/docker/api/types" "k8s.io/klog/v2" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" "k8s.io/kubernetes/pkg/kubelet/checkpointmanager" @@ -316,6 +316,7 @@ type dockerService struct { // (see `applyPlatformSpecificDockerConfig` and `performPlatformSpecificContainerCleanup` // methods for more info). containerCleanupInfos map[string]*containerCleanupInfo + cleanupInfosLock sync.RWMutex } // TODO: handle context.