Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d9af3c8

Browse filesBrowse files
esotsalesotsal
committed
Address sig-node meeting comments for mustKeepCPUs.
- Revert introduction of API env mustKeepCPUs - Replace mustKeepCPUs with local checkpoint "promised" - Introduce "promised" in CPUManagerCheckpointV3 format - Add logic, refactor with Beta candidate - Fix lint issues - TODO improve alogn resize tests, go through testing, corner cases - TODO improve CPUManagerCheckpointV3 tests - TODO address code review/feedback - TODO check init-containers - TODO check migration from v2 to v3 CPU Manager checkpoint - TODO check kubectl failure when prohibited can this be done earlier? - TODO update CPU Manager tests to use refactpred cpu_manager_test
1 parent 8f7b7f6 commit d9af3c8
Copy full SHA for d9af3c8

17 files changed

+802
-526
lines changed

‎pkg/api/pod/testing/make.go

Copy file name to clipboardExpand all lines: pkg/api/pod/testing/make.go
-6Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -293,12 +293,6 @@ func SetContainerResources(rr api.ResourceRequirements) TweakContainer {
293293
}
294294
}
295295

296-
func SetContainerEnv(env []api.EnvVar) TweakContainer {
297-
return func(cnr *api.Container) {
298-
cnr.Env = env
299-
}
300-
}
301-
302296
func SetContainerPorts(ports ...api.ContainerPort) TweakContainer {
303297
return func(cnr *api.Container) {
304298
cnr.Ports = ports

‎pkg/apis/core/validation/validation.go

Copy file name to clipboardExpand all lines: pkg/apis/core/validation/validation.go
-48Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ import (
6161
"k8s.io/kubernetes/pkg/capabilities"
6262
"k8s.io/kubernetes/pkg/features"
6363
"k8s.io/kubernetes/pkg/fieldpath"
64-
"k8s.io/utils/cpuset"
6564
)
6665

6766
const isNegativeErrorMsg string = apimachineryvalidation.IsNegativeErrorMsg
@@ -5706,7 +5705,6 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
57065705
var newContainers []core.Container
57075706
for ix, container := range originalCPUMemPodSpec.Containers {
57085707
dropCPUMemoryResourcesFromContainer(&container, &oldPod.Spec.Containers[ix])
5709-
allErrs = append(allErrs, dropMustKeepCPUsEnvFromContainer(&container, &oldPod.Spec.Containers[ix], specPath)...)
57105708
if !apiequality.Semantic.DeepEqual(container, oldPod.Spec.Containers[ix]) {
57115709
// This likely means that the user has made changes to resources other than CPU and memory for regular container.
57125710
errs := field.Forbidden(specPath, "only cpu and memory resources are mutable")
@@ -5802,52 +5800,6 @@ func dropCPUMemoryResourcesFromContainer(container *core.Container, oldPodSpecCo
58025800
container.Resources = core.ResourceRequirements{Limits: lim, Requests: req}
58035801
}
58045802

5805-
func removeEnvVar(envs []core.EnvVar, nameToRemove string) []core.EnvVar {
5806-
var newEnvs []core.EnvVar
5807-
for _, env := range envs {
5808-
if env.Name != nameToRemove {
5809-
newEnvs = append(newEnvs, env)
5810-
}
5811-
}
5812-
return newEnvs
5813-
}
5814-
5815-
// dropMustKeepCPUsEnvFromContainer deletes the "mustKeepCPUs" in env from the container, and copies them from the old pod container resources if present.
5816-
func dropMustKeepCPUsEnvFromContainer(container *core.Container, oldPodSpecContainer *core.Container, fldPath *field.Path) field.ErrorList {
5817-
allErrs := field.ErrorList{}
5818-
// the element named "mustKeepCPUs" in env can be update or add
5819-
existNewMustKeepCPUs := false
5820-
existOldMustKeepCPUs := false
5821-
for jx, newEnv := range container.Env {
5822-
if newEnv.Name == "mustKeepCPUs" {
5823-
existNewMustKeepCPUs = true
5824-
_, err := cpuset.Parse(newEnv.Value)
5825-
if err != nil {
5826-
allErrs = append(allErrs, field.Invalid(fldPath, newEnv, "Check mustKeepCPUs format, only number \",\" and \"-\" are allowed"))
5827-
}
5828-
// Change mustKeepCPUs
5829-
for _, oldEnv := range oldPodSpecContainer.Env {
5830-
if oldEnv.Name == "mustKeepCPUs" {
5831-
existOldMustKeepCPUs = true
5832-
container.Env[jx] = oldEnv // +k8s:verify-mutation:reason=clone
5833-
break
5834-
}
5835-
}
5836-
// Add mustKeepCPUs
5837-
if !existOldMustKeepCPUs && (len(container.Env)-len(oldPodSpecContainer.Env)) == 1 {
5838-
// Delete "mustKeepCPUs" in newPod to make newPod equal to oldPod
5839-
container.Env = removeEnvVar(container.Env, "mustKeepCPUs")
5840-
}
5841-
break
5842-
}
5843-
}
5844-
// Delete mustKeepCPUs
5845-
if !existNewMustKeepCPUs && (len(oldPodSpecContainer.Env)-len(container.Env)) == 1 {
5846-
oldPodSpecContainer.Env = removeEnvVar(oldPodSpecContainer.Env, "mustKeepCPUs") // +k8s:verify-mutation:reason=clone
5847-
}
5848-
return allErrs
5849-
}
5850-
58515803
// isPodResizeRequestSupported checks whether the pod is running on a node with InPlacePodVerticalScaling enabled.
58525804
func isPodResizeRequestSupported(pod core.Pod) bool {
58535805
// TODO: Remove this after GA+3 releases of InPlacePodVerticalScaling

‎pkg/apis/core/validation/validation_test.go

Copy file name to clipboardExpand all lines: pkg/apis/core/validation/validation_test.go
-82Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -26661,46 +26661,6 @@ func TestValidatePodResize(t *testing.T) {
2666126661
}))
2666226662
}
2666326663

26664-
mkPodWith1Env := func(envName1, envValue1 string, tweaks ...podtest.Tweak) *core.Pod {
26665-
return podtest.MakePod("pod", append(tweaks,
26666-
podtest.SetContainers(
26667-
podtest.MakeContainer(
26668-
"container",
26669-
podtest.SetContainerEnv(
26670-
[]core.EnvVar{
26671-
{
26672-
Name: envName1,
26673-
Value: envValue1,
26674-
},
26675-
},
26676-
),
26677-
),
26678-
),
26679-
)...)
26680-
}
26681-
26682-
mkPodWith2Env := func(envName1, envValue1, envName2, envValue2 string, tweaks ...podtest.Tweak) *core.Pod {
26683-
return podtest.MakePod("pod", append(tweaks,
26684-
podtest.SetContainers(
26685-
podtest.MakeContainer(
26686-
"container",
26687-
podtest.SetContainerEnv(
26688-
[]core.EnvVar{
26689-
{
26690-
Name: envName1,
26691-
Value: envValue1,
26692-
},
26693-
{
26694-
Name: envName2,
26695-
Value: envValue2,
26696-
},
26697-
},
26698-
),
26699-
),
26700-
),
26701-
)...)
26702-
}
26703-
2670426664
tests := []struct {
2670526665
test string
2670626666
old *core.Pod
@@ -27209,48 +27169,6 @@ func TestValidatePodResize(t *testing.T) {
2720927169
new: mkPodWithInitContainers(getResources("100m", "0", "2Gi", ""), core.ResourceList{}, core.ContainerRestartPolicyAlways, resizePolicy(core.ResourceMemory, core.NotRequired)),
2721027170
err: "spec: Forbidden: only cpu and memory resources are mutable",
2721127171
},
27212-
{
27213-
test: "Pod env:mustKeepCPUs change value",
27214-
old: mkPodWith2Env("env1", "a", "mustKeepCPUs", "0"),
27215-
new: mkPodWith2Env("env1", "a", "mustKeepCPUs", "1"),
27216-
err: "",
27217-
},
27218-
{
27219-
test: "Pod env:mustKeepCPUs add value",
27220-
old: mkPodWith1Env("env1", "a"),
27221-
new: mkPodWith2Env("env1", "a", "mustKeepCPUs", "1"),
27222-
err: "",
27223-
},
27224-
{
27225-
test: "Pod env:mustKeepCPUs delete",
27226-
old: mkPodWith2Env("env1", "a", "mustKeepCPUs", "1"),
27227-
new: mkPodWith1Env("env1", "a"),
27228-
err: "",
27229-
},
27230-
{
27231-
test: "Pod env:env1 change is forbidden",
27232-
old: mkPodWith2Env("env1", "a", "mustKeepCPUs", "0"),
27233-
new: mkPodWith2Env("env1", "b", "mustKeepCPUs", "0"),
27234-
err: "spec: Forbidden: only cpu and memory resources are mutable",
27235-
},
27236-
{
27237-
test: "Pod env:env1 add is forbidden",
27238-
old: mkPodWith1Env("mustKeepCPUs", "0"),
27239-
new: mkPodWith2Env("env1", "a", "mustKeepCPUs", "1"),
27240-
err: "spec: Forbidden: only cpu and memory resources are mutable",
27241-
},
27242-
{
27243-
test: "Pod env:env1 delete is forbidden",
27244-
old: mkPodWith2Env("env1", "a", "mustKeepCPUs", "1"),
27245-
new: mkPodWith1Env("mustKeepCPUs", "0"),
27246-
err: "spec: Forbidden: only cpu and memory resources are mutable",
27247-
},
27248-
{
27249-
test: "Pod env:mustKeepCPUs delete",
27250-
old: mkPodWith2Env("env1", "a", "mustKeepCPUs", "1"),
27251-
new: mkPodWith2Env("env1", "a", "mustKeepCPUs", "1s2"),
27252-
err: "Check mustKeepCPUs format, only number \",\" and \"-\" are allowed",
27253-
},
2725427172
}
2725527173

2725627174
for _, test := range tests {

‎pkg/kubelet/cm/cpumanager/cpu_manager.go

Copy file name to clipboardExpand all lines: pkg/kubelet/cm/cpumanager/cpu_manager.go
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,9 @@ func (m *manager) AddContainer(pod *v1.Pod, container *v1.Container, containerID
275275
if cset, exists := m.state.GetCPUSet(string(pod.UID), container.Name); exists {
276276
m.lastUpdateState.SetCPUSet(string(pod.UID), container.Name, cset)
277277
}
278+
if cset, exists := m.state.GetPromisedCPUSet(string(pod.UID), container.Name); exists {
279+
m.lastUpdateState.SetPromisedCPUSet(string(pod.UID), container.Name, cset)
280+
}
278281
m.containerMap.Add(string(pod.UID), container.Name, containerID)
279282
}
280283

‎pkg/kubelet/cm/cpumanager/cpu_manager_test.go

Copy file name to clipboardExpand all lines: pkg/kubelet/cm/cpumanager/cpu_manager_test.go
+32Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
)
4747

4848
type mockState struct {
49+
promised state.ContainerCPUAssignments
4950
assignments state.ContainerCPUAssignments
5051
defaultCPUSet cpuset.CPUSet
5152
}
@@ -55,6 +56,11 @@ func (s *mockState) GetCPUSet(podUID string, containerName string) (cpuset.CPUSe
5556
return res.Clone(), ok
5657
}
5758

59+
func (s *mockState) GetPromisedCPUSet(podUID string, containerName string) (cpuset.CPUSet, bool) {
60+
res, ok := s.promised[podUID][containerName]
61+
return res.Clone(), ok
62+
}
63+
5864
func (s *mockState) GetDefaultCPUSet() cpuset.CPUSet {
5965
return s.defaultCPUSet.Clone()
6066
}
@@ -66,6 +72,13 @@ func (s *mockState) GetCPUSetOrDefault(podUID string, containerName string) cpus
6672
return s.GetDefaultCPUSet()
6773
}
6874

75+
func (s *mockState) SetPromisedCPUSet(podUID string, containerName string, cset cpuset.CPUSet) {
76+
if _, exists := s.promised[podUID]; !exists {
77+
s.promised[podUID] = make(map[string]cpuset.CPUSet)
78+
}
79+
s.promised[podUID][containerName] = cset
80+
}
81+
6982
func (s *mockState) SetCPUSet(podUID string, containerName string, cset cpuset.CPUSet) {
7083
if _, exists := s.assignments[podUID]; !exists {
7184
s.assignments[podUID] = make(map[string]cpuset.CPUSet)
@@ -82,11 +95,16 @@ func (s *mockState) Delete(podUID string, containerName string) {
8295
if len(s.assignments[podUID]) == 0 {
8396
delete(s.assignments, podUID)
8497
}
98+
delete(s.promised[podUID], containerName)
99+
if len(s.promised[podUID]) == 0 {
100+
delete(s.promised, podUID)
101+
}
85102
}
86103

87104
func (s *mockState) ClearState() {
88105
s.defaultCPUSet = cpuset.CPUSet{}
89106
s.assignments = make(state.ContainerCPUAssignments)
107+
s.promised = make(state.ContainerCPUAssignments)
90108
}
91109

92110
func (s *mockState) SetCPUAssignments(a state.ContainerCPUAssignments) {
@@ -97,6 +115,14 @@ func (s *mockState) GetCPUAssignments() state.ContainerCPUAssignments {
97115
return s.assignments.Clone()
98116
}
99117

118+
func (s *mockState) SetCPUPromised(a state.ContainerCPUAssignments) {
119+
s.promised = a.Clone()
120+
}
121+
122+
func (s *mockState) GetCPUPromised() state.ContainerCPUAssignments {
123+
return s.promised.Clone()
124+
}
125+
100126
type mockPolicy struct {
101127
err error
102128
}
@@ -321,6 +347,7 @@ func TestCPUManagerAdd(t *testing.T) {
321347
mgr := &manager{
322348
policy: testCase.policy,
323349
state: &mockState{
350+
promised: state.ContainerCPUAssignments{},
324351
assignments: state.ContainerCPUAssignments{},
325352
defaultCPUSet: cpuset.New(1, 2, 3, 4),
326353
},
@@ -545,6 +572,7 @@ func TestCPUManagerAddWithInitContainers(t *testing.T) {
545572
policy, _ := NewStaticPolicy(testCase.topo, testCase.numReservedCPUs, cpuset.New(), topologymanager.NewFakeManager(), nil)
546573

547574
mockState := &mockState{
575+
promised: testCase.stAssignments,
548576
assignments: testCase.stAssignments,
549577
defaultCPUSet: testCase.stDefaultCPUSet,
550578
}
@@ -737,6 +765,7 @@ func TestCPUManagerRemove(t *testing.T) {
737765
err: nil,
738766
},
739767
state: &mockState{
768+
promised: state.ContainerCPUAssignments{},
740769
assignments: state.ContainerCPUAssignments{},
741770
defaultCPUSet: cpuset.New(),
742771
},
@@ -1233,6 +1262,7 @@ func TestReconcileState(t *testing.T) {
12331262
mgr := &manager{
12341263
policy: testCase.policy,
12351264
state: &mockState{
1265+
promised: testCase.stAssignments,
12361266
assignments: testCase.stAssignments,
12371267
defaultCPUSet: testCase.stDefaultCPUSet,
12381268
},
@@ -1341,6 +1371,7 @@ func TestCPUManagerAddWithResvList(t *testing.T) {
13411371
mgr := &manager{
13421372
policy: testCase.policy,
13431373
state: &mockState{
1374+
promised: state.ContainerCPUAssignments{},
13441375
assignments: state.ContainerCPUAssignments{},
13451376
defaultCPUSet: cpuset.New(0, 1, 2, 3),
13461377
},
@@ -1490,6 +1521,7 @@ func TestCPUManagerGetAllocatableCPUs(t *testing.T) {
14901521
policy: testCase.policy,
14911522
activePods: func() []*v1.Pod { return nil },
14921523
state: &mockState{
1524+
promised: state.ContainerCPUAssignments{},
14931525
assignments: state.ContainerCPUAssignments{},
14941526
defaultCPUSet: cpuset.New(0, 1, 2, 3),
14951527
},

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.