Add ability to find init Container IDs in cpumanager reconcileState()

The cpumanager loops through all init Containers and app Containers when
reconciling its state. However, the current implementation of
findContainerIDByName(), which is call by the reconciler, does not
resolve for init Containers.

This patch updates findContainerIDByName() to account for init
Containers and adds a regression test that fails before the change and
succeeds after.
This commit is contained in:
Kevin Klues 2019-04-27 05:57:03 -07:00
parent 41f9f31592
commit ef27f5f1a5
2 changed files with 88 additions and 30 deletions

View File

@ -294,7 +294,9 @@ func (m *manager) reconcileState() (success []reconciledContainer, failure []rec
} }
func findContainerIDByName(status *v1.PodStatus, name string) (string, error) { func findContainerIDByName(status *v1.PodStatus, name string) (string, error) {
for _, container := range status.ContainerStatuses { allStatuses := status.InitContainerStatuses
allStatuses = append(allStatuses, status.ContainerStatuses...)
for _, container := range allStatuses {
if container.Name == name && container.ContainerID != "" { if container.Name == name && container.ContainerID != "" {
cid := &kubecontainer.ContainerID{} cid := &kubecontainer.ContainerID{}
err := cid.ParseString(container.ContainerID) err := cid.ParseString(container.ContainerID)

View File

@ -359,6 +359,7 @@ func TestReconcileState(t *testing.T) {
stAssignments state.ContainerCPUAssignments stAssignments state.ContainerCPUAssignments
stDefaultCPUSet cpuset.CPUSet stDefaultCPUSet cpuset.CPUSet
updateErr error updateErr error
expectSucceededContainerName string
expectFailedContainerName string expectFailedContainerName string
}{ }{
{ {
@ -392,6 +393,41 @@ func TestReconcileState(t *testing.T) {
}, },
stDefaultCPUSet: cpuset.NewCPUSet(3, 4, 5, 6, 7), stDefaultCPUSet: cpuset.NewCPUSet(3, 4, 5, 6, 7),
updateErr: nil, updateErr: nil,
expectSucceededContainerName: "fakeName",
expectFailedContainerName: "",
},
{
description: "cpu manager reconcile init container - no error",
activePods: []*v1.Pod{
{
ObjectMeta: metav1.ObjectMeta{
Name: "fakePodName",
UID: "fakeUID",
},
Spec: v1.PodSpec{
InitContainers: []v1.Container{
{
Name: "fakeName",
},
},
},
},
},
pspPS: v1.PodStatus{
InitContainerStatuses: []v1.ContainerStatus{
{
Name: "fakeName",
ContainerID: "docker://fakeID",
},
},
},
pspFound: true,
stAssignments: state.ContainerCPUAssignments{
"fakeID": cpuset.NewCPUSet(1, 2),
},
stDefaultCPUSet: cpuset.NewCPUSet(3, 4, 5, 6, 7),
updateErr: nil,
expectSucceededContainerName: "fakeName",
expectFailedContainerName: "", expectFailedContainerName: "",
}, },
{ {
@ -416,6 +452,7 @@ func TestReconcileState(t *testing.T) {
stAssignments: state.ContainerCPUAssignments{}, stAssignments: state.ContainerCPUAssignments{},
stDefaultCPUSet: cpuset.NewCPUSet(), stDefaultCPUSet: cpuset.NewCPUSet(),
updateErr: nil, updateErr: nil,
expectSucceededContainerName: "",
expectFailedContainerName: "fakeName", expectFailedContainerName: "fakeName",
}, },
{ {
@ -447,6 +484,7 @@ func TestReconcileState(t *testing.T) {
stAssignments: state.ContainerCPUAssignments{}, stAssignments: state.ContainerCPUAssignments{},
stDefaultCPUSet: cpuset.NewCPUSet(), stDefaultCPUSet: cpuset.NewCPUSet(),
updateErr: nil, updateErr: nil,
expectSucceededContainerName: "",
expectFailedContainerName: "fakeName", expectFailedContainerName: "fakeName",
}, },
{ {
@ -480,6 +518,7 @@ func TestReconcileState(t *testing.T) {
}, },
stDefaultCPUSet: cpuset.NewCPUSet(1, 2, 3, 4, 5, 6, 7), stDefaultCPUSet: cpuset.NewCPUSet(1, 2, 3, 4, 5, 6, 7),
updateErr: nil, updateErr: nil,
expectSucceededContainerName: "",
expectFailedContainerName: "fakeName", expectFailedContainerName: "fakeName",
}, },
{ {
@ -513,6 +552,7 @@ func TestReconcileState(t *testing.T) {
}, },
stDefaultCPUSet: cpuset.NewCPUSet(3, 4, 5, 6, 7), stDefaultCPUSet: cpuset.NewCPUSet(3, 4, 5, 6, 7),
updateErr: fmt.Errorf("fake container update error"), updateErr: fmt.Errorf("fake container update error"),
expectSucceededContainerName: "",
expectFailedContainerName: "fakeName", expectFailedContainerName: "fakeName",
}, },
} }
@ -538,7 +578,22 @@ func TestReconcileState(t *testing.T) {
}, },
} }
_, failure := mgr.reconcileState() success, failure := mgr.reconcileState()
if testCase.expectSucceededContainerName != "" {
// Search succeeded reconciled containers for the supplied name.
foundSucceededContainer := false
for _, reconciled := range success {
if reconciled.containerName == testCase.expectSucceededContainerName {
foundSucceededContainer = true
break
}
}
if !foundSucceededContainer {
t.Errorf("%v", testCase.description)
t.Errorf("Expected reconciliation success for container: %s", testCase.expectSucceededContainerName)
}
}
if testCase.expectFailedContainerName != "" { if testCase.expectFailedContainerName != "" {
// Search failed reconciled containers for the supplied name. // Search failed reconciled containers for the supplied name.
@ -550,6 +605,7 @@ func TestReconcileState(t *testing.T) {
} }
} }
if !foundFailedContainer { if !foundFailedContainer {
t.Errorf("%v", testCase.description)
t.Errorf("Expected reconciliation failure for container: %s", testCase.expectFailedContainerName) t.Errorf("Expected reconciliation failure for container: %s", testCase.expectFailedContainerName)
} }
} }