From aadbf221d2a403d57da2f221ad40a1ccf3e14c63 Mon Sep 17 00:00:00 2001 From: Victor Marmol Date: Tue, 24 Feb 2015 11:52:05 -0800 Subject: [PATCH] Kill infra pod when a container is restarted. Revert 0e20f7d73659c8dcec771f8d72f525360c26fcc3. Fixes #4759. --- pkg/kubelet/kubelet.go | 30 ++++++++++++------------------ pkg/kubelet/kubelet_test.go | 3 ++- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index e4f7aedccf8..9d91a462d62 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1081,7 +1081,6 @@ func (kl *Kubelet) syncPod(pod *api.BoundPod, dockerContainers dockertools.Docke glog.Errorf("Unable to get pod with name %q and uid %q info with error(%v)", podFullName, uid, err) } - podChanged := false for _, container := range pod.Spec.Containers { expectedHash := dockertools.HashContainer(&container) dockerContainerName := dockertools.BuildDockerName(uid, podFullName, &container) @@ -1090,8 +1089,8 @@ func (kl *Kubelet) syncPod(pod *api.BoundPod, dockerContainers dockertools.Docke glog.V(3).Infof("pod %q container %q exists as %v", podFullName, container.Name, containerID) // look for changes in the container. - containerChanged := hash != 0 && hash != expectedHash - if !containerChanged { + podChanged := hash != 0 && hash != expectedHash + if !podChanged { // TODO: This should probably be separated out into a separate goroutine. // If the container's liveness probe is unsuccessful, set readiness to false. If liveness is succesful, do a readiness check and set // readiness accordingly. If the initalDelay since container creation on liveness probe has not passed the probe will return Success. @@ -1123,7 +1122,6 @@ func (kl *Kubelet) syncPod(pod *api.BoundPod, dockerContainers dockertools.Docke } glog.Infof("pod %q container %q is unhealthy. Container will be killed and re-created.", podFullName, container.Name, live) } else { - podChanged = true glog.Infof("pod %q container %q hash changed (%d vs %d). Container will be killed and re-created.", podFullName, container.Name, hash, expectedHash) } if err := kl.killContainer(dockerContainer); err != nil { @@ -1131,6 +1129,16 @@ func (kl *Kubelet) syncPod(pod *api.BoundPod, dockerContainers dockertools.Docke continue } killedContainers[containerID] = empty{} + + if podChanged { + // Also kill associated pod infra container if the pod changed. + if podInfraContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uid, dockertools.PodInfraContainerName); found { + if err := kl.killContainer(podInfraContainer); err != nil { + glog.V(1).Infof("Failed to kill pod infra container %q: %v", podInfraContainer.ID, err) + continue + } + } + } } // Check RestartPolicy for container @@ -1193,20 +1201,6 @@ func (kl *Kubelet) syncPod(pod *api.BoundPod, dockerContainers dockertools.Docke containersToKeep[containerID] = empty{} } - if podChanged { - // Also kill associated pod infra container if the pod changed. - if podInfraContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uid, dockertools.PodInfraContainerName); found { - if err := kl.killContainer(podInfraContainer); err != nil { - glog.V(1).Infof("Failed to kill pod infra container %q: %v", podInfraContainer.ID, err) - } - } - podInfraContainerID, err = kl.createPodInfraContainer(pod) - if err != nil { - glog.Errorf("Failed to recreate pod infra container: %v for pod %q", err, podFullName) - } - containersToKeep[podInfraContainerID] = empty{} - } - // Kill any containers in this pod which were not identified above (guards against duplicates). for id, container := range dockerContainers { curPodFullName, curUUID, _, _ := dockertools.ParseDockerName(container.Names[0]) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index c1a792f9f51..d662939045f 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -934,7 +934,8 @@ func TestSyncPodBadHash(t *testing.T) { t.Errorf("unexpected error: %v", err) } - verifyCalls(t, fakeDocker, []string{"list", "stop", "list", "create", "start", "stop", "create", "start", "inspect_container"}) + //verifyCalls(t, fakeDocker, []string{"list", "stop", "list", "create", "start", "stop", "create", "start", "inspect_container"}) + verifyCalls(t, fakeDocker, []string{"list", "stop", "stop", "list", "create", "start"}) // A map interation is used to delete containers, so must not depend on // order here.