Skip to content

Commit 3fa38a2

Browse files
bug(quota): handle ResourcesChanged on resource quota filter
1 parent 530278b commit 3fa38a2

File tree

2 files changed

+166
-0
lines changed

2 files changed

+166
-0
lines changed

pkg/quota/v1/install/update_filter.go

+25
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ package install
1818

1919
import (
2020
v1 "k8s.io/api/core/v1"
21+
apiequality "k8s.io/apimachinery/pkg/api/equality"
2122
"k8s.io/apimachinery/pkg/runtime/schema"
23+
"k8s.io/apiserver/pkg/util/feature"
24+
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
25+
"k8s.io/kubernetes/pkg/features"
2226
"k8s.io/kubernetes/pkg/quota/v1/evaluator/core"
2327
"k8s.io/utils/clock"
2428
)
@@ -30,6 +34,10 @@ func DefaultUpdateFilter() func(resource schema.GroupVersionResource, oldObj, ne
3034
case schema.GroupResource{Resource: "pods"}:
3135
oldPod := oldObj.(*v1.Pod)
3236
newPod := newObj.(*v1.Pod)
37+
// when Resources changed
38+
if feature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) && hasResourcesChanged(oldPod, newPod) {
39+
return true
40+
}
3341
return core.QuotaV1Pod(oldPod, clock.RealClock{}) && !core.QuotaV1Pod(newPod, clock.RealClock{})
3442
case schema.GroupResource{Resource: "services"}:
3543
oldService := oldObj.(*v1.Service)
@@ -44,3 +52,20 @@ func DefaultUpdateFilter() func(resource schema.GroupVersionResource, oldObj, ne
4452
return false
4553
}
4654
}
55+
56+
// hasResourcesChanged function to compare resources in container statuses
57+
func hasResourcesChanged(oldPod *v1.Pod, newPod *v1.Pod) bool {
58+
for _, oldStatus := range oldPod.Status.ContainerStatuses {
59+
newStatus, ok := podutil.GetContainerStatus(newPod.Status.ContainerStatuses, oldStatus.Name)
60+
if ok && !apiequality.Semantic.DeepEqual(oldStatus.Resources, newStatus.Resources) {
61+
return true
62+
}
63+
}
64+
for _, oldInitContainerStatus := range oldPod.Status.InitContainerStatuses {
65+
newInitContainerStatus, ok := podutil.GetContainerStatus(newPod.Status.InitContainerStatuses, oldInitContainerStatus.Name)
66+
if ok && !apiequality.Semantic.DeepEqual(oldInitContainerStatus.Resources, newInitContainerStatus.Resources) {
67+
return true
68+
}
69+
}
70+
return false
71+
}
+141
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/*
2+
Copyright 2024 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package install
18+
19+
import (
20+
"testing"
21+
22+
v1 "k8s.io/api/core/v1"
23+
"k8s.io/apimachinery/pkg/api/resource"
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25+
)
26+
27+
var (
28+
containerFoo = v1.Container{
29+
30+
Name: "foo",
31+
Resources: v1.ResourceRequirements{
32+
Requests: v1.ResourceList{
33+
v1.ResourceCPU: resource.MustParse("2"),
34+
},
35+
},
36+
}
37+
containerFooInitialStatus = v1.ContainerStatus{
38+
Name: "foo",
39+
Resources: &v1.ResourceRequirements{
40+
Requests: v1.ResourceList{
41+
v1.ResourceCPU: resource.MustParse("2"),
42+
},
43+
},
44+
}
45+
containerFooChangedStatus = v1.ContainerStatus{
46+
Name: "foo",
47+
Resources: &v1.ResourceRequirements{
48+
Requests: v1.ResourceList{
49+
v1.ResourceCPU: resource.MustParse("3"),
50+
},
51+
},
52+
}
53+
)
54+
55+
func TestHasResourcesChanged(t *testing.T) {
56+
57+
oldNotChangedPod := &v1.Pod{
58+
ObjectMeta: metav1.ObjectMeta{},
59+
Spec: v1.PodSpec{
60+
Containers: []v1.Container{
61+
containerFoo,
62+
},
63+
},
64+
Status: v1.PodStatus{
65+
Phase: v1.PodRunning,
66+
ContainerStatuses: []v1.ContainerStatus{
67+
containerFooInitialStatus,
68+
},
69+
},
70+
}
71+
newNotChangedPod := &v1.Pod{
72+
ObjectMeta: metav1.ObjectMeta{},
73+
Spec: v1.PodSpec{
74+
Containers: []v1.Container{
75+
containerFoo,
76+
},
77+
},
78+
Status: v1.PodStatus{
79+
Phase: v1.PodRunning,
80+
ContainerStatuses: []v1.ContainerStatus{
81+
containerFooInitialStatus,
82+
},
83+
},
84+
}
85+
86+
oldChangedPod := &v1.Pod{
87+
ObjectMeta: metav1.ObjectMeta{},
88+
Spec: v1.PodSpec{
89+
Containers: []v1.Container{
90+
containerFoo,
91+
},
92+
},
93+
Status: v1.PodStatus{
94+
Phase: v1.PodRunning,
95+
ContainerStatuses: []v1.ContainerStatus{
96+
containerFooInitialStatus,
97+
},
98+
},
99+
}
100+
newChangedPod := &v1.Pod{
101+
ObjectMeta: metav1.ObjectMeta{},
102+
Spec: v1.PodSpec{
103+
Containers: []v1.Container{
104+
containerFoo,
105+
},
106+
},
107+
Status: v1.PodStatus{
108+
Phase: v1.PodRunning,
109+
ContainerStatuses: []v1.ContainerStatus{
110+
containerFooChangedStatus,
111+
},
112+
},
113+
}
114+
115+
tests := []struct {
116+
name string
117+
oldPod *v1.Pod
118+
newPod *v1.Pod
119+
expected bool
120+
}{
121+
{
122+
name: "not-changed-pod",
123+
oldPod: oldNotChangedPod,
124+
newPod: newNotChangedPod,
125+
expected: false,
126+
},
127+
{
128+
name: "changed-pod",
129+
oldPod: oldChangedPod,
130+
newPod: newChangedPod,
131+
expected: true,
132+
},
133+
}
134+
for _, test := range tests {
135+
t.Run(test.name, func(t *testing.T) {
136+
if got := hasResourcesChanged(test.oldPod, test.newPod); got != test.expected {
137+
t.Errorf("TestHasResourcesChanged = %v, expected %v", got, test.expected)
138+
}
139+
})
140+
}
141+
}

0 commit comments

Comments
 (0)