diff --git a/pkg/kubelet/dockershim/helpers.go b/pkg/kubelet/dockershim/helpers.go index a83e9de33a8..63eeacfcc02 100644 --- a/pkg/kubelet/dockershim/helpers.go +++ b/pkg/kubelet/dockershim/helpers.go @@ -39,8 +39,7 @@ const ( ) var ( - conflictRE = regexp.MustCompile(`Conflict. (?:.)+ is already in use by container ([0-9a-z]+)`) - noContainerRE = regexp.MustCompile(`No such container: [0-9a-z]+`) + conflictRE = regexp.MustCompile(`Conflict. (?:.)+ is already in use by container ([0-9a-z]+)`) ) // apiVersion implements kubecontainer.Version interface by implementing @@ -313,8 +312,8 @@ func recoverFromCreationConflictIfNeeded(client dockertools.DockerInterface, cre return nil, err } else { glog.Errorf("Failed to remove the conflicting container %q: %v", id, rmErr) - // Return if the error is not "No such container" error. - if !noContainerRE.MatchString(rmErr.Error()) { + // Return if the error is not container not found error. + if !dockertools.IsContainerNotFoundError(rmErr) { return nil, err } } diff --git a/pkg/kubelet/dockershim/helpers_test.go b/pkg/kubelet/dockershim/helpers_test.go index aef94951fe1..d5900cf19a1 100644 --- a/pkg/kubelet/dockershim/helpers_test.go +++ b/pkg/kubelet/dockershim/helpers_test.go @@ -236,12 +236,3 @@ func TestParsingCreationConflictError(t *testing.T) { require.Len(t, matches, 2) require.Equal(t, matches[1], "24666ab8c814d16f986449e504ea0159468ddf8da01897144a770f66dce0e14e") } - -func TestParsingRemovalNoContainerError(t *testing.T) { - // Expected error message from docker. - match := "Error response from daemon: No such container: 96e914f31579e44fe49b239266385330a9b2125abeb9254badd9fca74580c95a" - notMatch := "Error response from daemon: Other errors" - - assert.True(t, noContainerRE.MatchString(match)) - assert.False(t, noContainerRE.MatchString(notMatch)) -} diff --git a/pkg/kubelet/dockertools/BUILD b/pkg/kubelet/dockertools/BUILD index 2eca4d90708..9af2ed22251 100644 --- a/pkg/kubelet/dockertools/BUILD +++ b/pkg/kubelet/dockertools/BUILD @@ -90,6 +90,7 @@ go_test( "docker_manager_test.go", "docker_test.go", "images_test.go", + "kube_docker_client_test.go", "labels_test.go", ], data = [ diff --git a/pkg/kubelet/dockertools/docker_manager.go b/pkg/kubelet/dockertools/docker_manager.go index 6f086a7e4af..ade66493bcf 100644 --- a/pkg/kubelet/dockertools/docker_manager.go +++ b/pkg/kubelet/dockertools/docker_manager.go @@ -2428,7 +2428,7 @@ func (dm *DockerManager) pruneInitContainersBeforeStart(pod *v1.Pod, podStatus * // TODO: we may not need aggressive pruning glog.V(4).Infof("Removing init container %q instance %q %d", status.Name, status.ID.ID, count) if err := dm.client.RemoveContainer(status.ID.ID, dockertypes.ContainerRemoveOptions{RemoveVolumes: true}); err != nil { - if _, ok := err.(containerNotFoundError); ok { + if IsContainerNotFoundError(err) { count-- continue } @@ -2671,7 +2671,7 @@ func (dm *DockerManager) GetPodStatus(uid kubetypes.UID, name, namespace string) } result, ip, err := dm.inspectContainer(c.ID, name, namespace) if err != nil { - if _, ok := err.(containerNotFoundError); ok { + if IsContainerNotFoundError(err) { // https://github.com/kubernetes/kubernetes/issues/22541 // Sometimes when docker's state is corrupt, a container can be listed // but couldn't be inspected. We fake a status for this container so diff --git a/pkg/kubelet/dockertools/docker_manager_test.go b/pkg/kubelet/dockertools/docker_manager_test.go index 1febf1326f6..77f0d727ad1 100644 --- a/pkg/kubelet/dockertools/docker_manager_test.go +++ b/pkg/kubelet/dockertools/docker_manager_test.go @@ -1820,7 +1820,7 @@ func TestGetPodStatusNoSuchContainer(t *testing.T) { Running: false, }, }) - fakeDocker.InjectErrors(map[string]error{"inspect_container": containerNotFoundError{}}) + fakeDocker.InjectErrors(map[string]error{"inspect_container": fmt.Errorf("Error: No such container: %s", noSuchContainerID)}) runSyncPod(t, dm, fakeDocker, pod, nil, false) // Verify that we will try to start new contrainers even if the inspections diff --git a/pkg/kubelet/dockertools/kube_docker_client.go b/pkg/kubelet/dockertools/kube_docker_client.go index 1f332138835..37533789159 100644 --- a/pkg/kubelet/dockertools/kube_docker_client.go +++ b/pkg/kubelet/dockertools/kube_docker_client.go @@ -23,6 +23,7 @@ import ( "fmt" "io" "io/ioutil" + "regexp" "sync" "time" @@ -123,9 +124,6 @@ func (d *kubeDockerClient) InspectContainer(id string) (*dockertypes.ContainerJS return nil, ctxErr } if err != nil { - if dockerapi.IsErrContainerNotFound(err) { - return nil, containerNotFoundError{ID: id} - } return nil, err } return &containerJSON, nil @@ -607,15 +605,12 @@ func (e operationTimeout) Error() string { return fmt.Sprintf("operation timeout: %v", e.err) } -// containerNotFoundError is the error returned by InspectContainer when container not found. We -// add this error type for testability. We don't use the original error returned by engine-api -// because dockertypes.containerNotFoundError is private, we can't create and inject it in our test. -type containerNotFoundError struct { - ID string -} +// containerNotFoundErrorRegx is the regexp of container not found error message. +var containerNotFoundErrorRegx = regexp.MustCompile(`No such container: [0-9a-z]+`) -func (e containerNotFoundError) Error() string { - return fmt.Sprintf("no such container: %q", e.ID) +// IsContainerNotFoundError checks whether the error is container not found error. +func IsContainerNotFoundError(err error) bool { + return containerNotFoundErrorRegx.MatchString(err.Error()) } // imageNotFoundError is the error returned by InspectImage when image not found. diff --git a/pkg/kubelet/dockertools/kube_docker_client_test.go b/pkg/kubelet/dockertools/kube_docker_client_test.go new file mode 100644 index 00000000000..42f1ac4fdea --- /dev/null +++ b/pkg/kubelet/dockertools/kube_docker_client_test.go @@ -0,0 +1,33 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package dockertools + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsContainerNotFoundError(t *testing.T) { + // Expected error message from docker. + containerNotFoundError := fmt.Errorf("Error response from daemon: No such container: 96e914f31579e44fe49b239266385330a9b2125abeb9254badd9fca74580c95a") + otherError := fmt.Errorf("Error response from daemon: Other errors") + + assert.True(t, IsContainerNotFoundError(containerNotFoundError)) + assert.False(t, IsContainerNotFoundError(otherError)) +}