Skip to content

Commit d7e5ff8

Browse files
authored
Merge pull request kubernetes#128083 from carlory/fix-126662-kubelet
kubelet: fix a bug where kubelet wrongly drops the QOSClass field of the Pod's s status when it rejects a Pod
2 parents 352056f + c7e384f commit d7e5ff8

File tree

2 files changed

+85
-4
lines changed

2 files changed

+85
-4
lines changed

pkg/kubelet/kubelet.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"k8s.io/client-go/informers"
4343
"k8s.io/mount-utils"
4444

45+
v1qos "k8s.io/kubernetes/pkg/apis/core/v1/helper/qos"
4546
utilfs "k8s.io/kubernetes/pkg/util/filesystem"
4647
netutils "k8s.io/utils/net"
4748

@@ -2292,9 +2293,10 @@ func (kl *Kubelet) deletePod(pod *v1.Pod) error {
22922293
func (kl *Kubelet) rejectPod(pod *v1.Pod, reason, message string) {
22932294
kl.recorder.Eventf(pod, v1.EventTypeWarning, reason, message)
22942295
kl.statusManager.SetPodStatus(pod, v1.PodStatus{
2295-
Phase: v1.PodFailed,
2296-
Reason: reason,
2297-
Message: "Pod was rejected: " + message})
2296+
QOSClass: v1qos.GetPodQOS(pod), // keep it as is
2297+
Phase: v1.PodFailed,
2298+
Reason: reason,
2299+
Message: "Pod was rejected: " + message})
22982300
}
22992301

23002302
// canAdmitPod determines if a pod can be admitted, and gives a reason if it

test/e2e/common/node/pod_admission.go

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ import (
2020
"context"
2121

2222
"github.com/onsi/ginkgo/v2"
23+
"github.com/onsi/gomega"
2324

2425
v1 "k8s.io/api/core/v1"
26+
"k8s.io/apimachinery/pkg/api/resource"
2527
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2628
"k8s.io/apimachinery/pkg/labels"
2729
"k8s.io/kubernetes/test/e2e/framework"
@@ -36,7 +38,7 @@ var _ = SIGDescribe("PodOSRejection", framework.WithNodeConformance(), func() {
3638
f := framework.NewDefaultFramework("pod-os-rejection")
3739
f.NamespacePodSecurityLevel = admissionapi.LevelBaseline
3840
ginkgo.Context("Kubelet", func() {
39-
ginkgo.It("should reject pod when the node OS doesn't match pod's OS", func(ctx context.Context) {
41+
ginkgo.It("[LinuxOnly] should reject pod when the node OS doesn't match pod's OS", func(ctx context.Context) {
4042
linuxNode, err := findLinuxNode(ctx, f)
4143
framework.ExpectNoError(err)
4244
pod := &v1.Pod{
@@ -65,6 +67,83 @@ var _ = SIGDescribe("PodOSRejection", framework.WithNodeConformance(), func() {
6567
})
6668
})
6769

70+
var _ = SIGDescribe("PodRejectionStatus", func() {
71+
f := framework.NewDefaultFramework("pod-rejection-status")
72+
f.NamespacePodSecurityLevel = admissionapi.LevelBaseline
73+
ginkgo.Context("Kubelet", func() {
74+
ginkgo.It("should reject pod when the node didn't have enough resource", func(ctx context.Context) {
75+
node, err := e2enode.GetRandomReadySchedulableNode(ctx, f.ClientSet)
76+
framework.ExpectNoError(err, "Failed to get a ready schedulable node")
77+
78+
// Create a pod that requests more CPU than the node has
79+
pod := &v1.Pod{
80+
ObjectMeta: metav1.ObjectMeta{
81+
Name: "pod-out-of-cpu",
82+
Namespace: f.Namespace.Name,
83+
},
84+
Spec: v1.PodSpec{
85+
Containers: []v1.Container{
86+
{
87+
Name: "pod-out-of-cpu",
88+
Image: imageutils.GetPauseImageName(),
89+
Resources: v1.ResourceRequirements{
90+
Requests: v1.ResourceList{
91+
v1.ResourceCPU: resource.MustParse("1000000000000"), // requests more CPU than any node has
92+
},
93+
},
94+
},
95+
},
96+
},
97+
}
98+
99+
pod = e2epod.NewPodClient(f).Create(ctx, pod)
100+
101+
// Wait for the scheduler to update the pod status
102+
err = e2epod.WaitForPodNameUnschedulableInNamespace(ctx, f.ClientSet, pod.Name, pod.Namespace)
103+
framework.ExpectNoError(err)
104+
105+
// Fetch the pod to get the latest status which should be last one observed by the scheduler
106+
// before it rejected the pod
107+
pod, err = f.ClientSet.CoreV1().Pods(pod.Namespace).Get(ctx, pod.Name, metav1.GetOptions{})
108+
framework.ExpectNoError(err)
109+
110+
// force assign the Pod to a node in order to get rejection status later
111+
binding := &v1.Binding{
112+
ObjectMeta: metav1.ObjectMeta{
113+
Name: pod.Name,
114+
Namespace: pod.Namespace,
115+
UID: pod.UID,
116+
},
117+
Target: v1.ObjectReference{
118+
Kind: "Node",
119+
Name: node.Name,
120+
},
121+
}
122+
err = f.ClientSet.CoreV1().Pods(pod.Namespace).Bind(ctx, binding, metav1.CreateOptions{})
123+
framework.ExpectNoError(err)
124+
125+
// kubelet has rejected the pod
126+
err = e2epod.WaitForPodFailedReason(ctx, f.ClientSet, pod, "OutOfcpu", f.Timeouts.PodStartShort)
127+
framework.ExpectNoError(err)
128+
129+
// fetch the reject Pod and compare the status
130+
gotPod, err := f.ClientSet.CoreV1().Pods(pod.Namespace).Get(ctx, pod.Name, metav1.GetOptions{})
131+
framework.ExpectNoError(err)
132+
133+
// This detects if there are any new fields in Status that were dropped by the pod rejection.
134+
// These new fields either should be kept by kubelet's admission or added explicitly in the list of fields that are having a different value or must be cleared.
135+
expectedStatus := pod.Status.DeepCopy()
136+
expectedStatus.Phase = gotPod.Status.Phase
137+
expectedStatus.Conditions = nil
138+
expectedStatus.Message = gotPod.Status.Message
139+
expectedStatus.Reason = gotPod.Status.Reason
140+
expectedStatus.StartTime = gotPod.Status.StartTime
141+
// expectedStatus.QOSClass keep it as is
142+
gomega.Expect(gotPod.Status).To(gomega.Equal(*expectedStatus))
143+
})
144+
})
145+
})
146+
68147
// findLinuxNode finds a Linux node that is Ready and Schedulable
69148
func findLinuxNode(ctx context.Context, f *framework.Framework) (v1.Node, error) {
70149
selector := labels.Set{"kubernetes.io/os": "linux"}.AsSelector()

0 commit comments

Comments
 (0)