Skip to content

Commit

Permalink
Merge pull request kubernetes#48194 from k82cn/k8s_48173
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 47327, 48194)

Checked container spec when killing container.

**What this PR does / why we need it**:
Checked container spec when getting container, return error if failed.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes kubernetes#48173 

**Release note**:
```release-note-none
```
  • Loading branch information
Kubernetes Submit Queue authored Jul 5, 2017
2 parents 67da2da + 549360c commit 145976f
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 2 deletions.
11 changes: 9 additions & 2 deletions pkg/kubelet/kuberuntime/kuberuntime_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,10 @@ func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandb
msg, handlerErr := m.runner.Run(kubeContainerID, pod, container, container.Lifecycle.PostStart)
if handlerErr != nil {
m.recordContainerEvent(pod, container, kubeContainerID.ID, v1.EventTypeWarning, events.FailedPostStartHook, msg)
m.killContainer(pod, kubeContainerID, container.Name, "FailedPostStartHook", nil)
if err := m.killContainer(pod, kubeContainerID, container.Name, "FailedPostStartHook", nil); err != nil {
glog.Errorf("Failed to kill container %q(id=%q) in pod %q: %v, %v",
container.Name, kubeContainerID.String(), format.Pod(pod), ErrPostStartHook, err)
}
return msg, ErrPostStartHook
}
}
Expand Down Expand Up @@ -547,7 +550,10 @@ func (m *kubeGenericRuntimeManager) restoreSpecsFromContainerLabels(containerID
func (m *kubeGenericRuntimeManager) killContainer(pod *v1.Pod, containerID kubecontainer.ContainerID, containerName string, reason string, gracePeriodOverride *int64) error {
var containerSpec *v1.Container
if pod != nil {
containerSpec = kubecontainer.GetContainerSpec(pod, containerName)
if containerSpec = kubecontainer.GetContainerSpec(pod, containerName); containerSpec == nil {
return fmt.Errorf("failed to get containerSpec %q(id=%q) in pod %q when killing container for reason %q",
containerName, containerID.String(), format.Pod(pod), reason)
}
} else {
// Restore necessary information if one of the specs is nil.
restoredPod, restoredContainer, err := m.restoreSpecsFromContainerLabels(containerID)
Expand All @@ -556,6 +562,7 @@ func (m *kubeGenericRuntimeManager) killContainer(pod *v1.Pod, containerID kubec
}
pod, containerSpec = restoredPod, restoredContainer
}

// From this point , pod and container must be non-nil.
gracePeriod := int64(minimumGracePeriodInSeconds)
switch {
Expand Down
35 changes: 35 additions & 0 deletions pkg/kubelet/kuberuntime/kuberuntime_container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,41 @@ func TestRemoveContainer(t *testing.T) {
assert.Empty(t, containers)
}

// TestKillContainer tests killing the container in a Pod.
func TestKillContainer(t *testing.T) {
_, _, m, _ := createTestRuntimeManager()

tests := []struct {
caseName string
pod *v1.Pod
containerID kubecontainer.ContainerID
containerName string
reason string
gracePeriodOverride int64
succeed bool
}{
{
caseName: "Failed to find container in pods, expect to return error",
pod: &v1.Pod{
ObjectMeta: metav1.ObjectMeta{UID: "pod1_id", Name: "pod1", Namespace: "default"},
Spec: v1.PodSpec{Containers: []v1.Container{{Name: "empty_container"}}},
},
containerID: kubecontainer.ContainerID{Type: "docker", ID: "not_exist_container_id"},
containerName: "not_exist_container",
reason: "unknown reason",
gracePeriodOverride: 0,
succeed: false,
},
}

for _, test := range tests {
err := m.killContainer(test.pod, test.containerID, test.containerName, test.reason, &test.gracePeriodOverride)
if test.succeed != (err == nil) {
t.Errorf("%s: expected %v, got %v (%v)", test.caseName, test.succeed, (err == nil), err)
}
}
}

// TestToKubeContainerStatus tests the converting the CRI container status to
// the internal type (i.e., toKubeContainerStatus()) for containers in
// different states.
Expand Down

0 comments on commit 145976f

Please sign in to comment.