Skip to content

Commit e5ffba1

Browse files
jralmarazjosekasna
jralmaraz
authored and
josekasna
committed
Report event for the cases when probe returned Unknown result
keep log success
1 parent 8ebfb8c commit e5ffba1

File tree

2 files changed

+158
-35
lines changed

2 files changed

+158
-35
lines changed

‎pkg/kubelet/prober/prober.go‎

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -98,24 +98,37 @@ func (pb *prober) probe(ctx context.Context, probeType probeType, pod *v1.Pod, s
9898
}
9999

100100
result, output, err := pb.runProbeWithRetries(ctx, probeType, probeSpec, pod, status, container, containerID, maxProbeRetries)
101-
if err != nil || (result != probe.Success && result != probe.Warning) {
102-
// Probe failed in one way or another.
103-
if err != nil {
104-
klog.V(1).ErrorS(err, "Probe errored", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name)
105-
pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, events.ContainerUnhealthy, "%s probe errored: %v", probeType, err)
106-
} else { // result != probe.Success
107-
klog.V(1).InfoS("Probe failed", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name, "probeResult", result, "output", output)
108-
pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, events.ContainerUnhealthy, "%s probe failed: %s", probeType, output)
109-
}
101+
102+
if err != nil {
103+
// Handle probe error
104+
klog.V(1).ErrorS(err, "Probe errored", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name, "probeResult", result)
105+
pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, events.ContainerUnhealthy, "%s probe errored and resulted in %s state: %s", probeType, result, err)
110106
return results.Failure, err
111107
}
112-
if result == probe.Warning {
108+
109+
switch result {
110+
case probe.Success:
111+
klog.V(3).InfoS("Probe succeeded", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name)
112+
return results.Success, nil
113+
114+
case probe.Warning:
113115
pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, events.ContainerProbeWarning, "%s probe warning: %s", probeType, output)
114116
klog.V(3).InfoS("Probe succeeded with a warning", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name, "output", output)
115-
} else {
116-
klog.V(3).InfoS("Probe succeeded", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name)
117+
return results.Success, nil
118+
119+
case probe.Failure:
120+
klog.V(1).InfoS("Probe failed", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name, "probeResult", result, "output", output)
121+
pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, events.ContainerUnhealthy, "%s probe failed: %s", probeType, output)
122+
return results.Failure, nil
123+
124+
case probe.Unknown:
125+
klog.V(1).InfoS("Probe unknown without error", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name, "probeResult", result)
126+
return results.Failure, nil
127+
128+
default:
129+
klog.V(1).InfoS("Unsupported probe result", "probeType", probeType, "pod", klog.KObj(pod), "podUID", pod.UID, "containerName", container.Name, "probeResult", result)
130+
return results.Failure, nil
117131
}
118-
return results.Success, nil
119132
}
120133

121134
// runProbeWithRetries tries to probe the container in a finite loop, it returns the last result
@@ -144,6 +157,8 @@ func (pb *prober) runProbe(ctx context.Context, probeType probeType, p *v1.Probe
144157
case p.HTTPGet != nil:
145158
req, err := httpprobe.NewRequestForHTTPGetAction(p.HTTPGet, &container, status.PodIP, "probe")
146159
if err != nil {
160+
// Log and record event for Unknown result
161+
klog.V(4).InfoS("HTTP-Probe failed to create request", "error", err)
147162
return probe.Unknown, "", err
148163
}
149164
if klogV4 := klog.V(4); klogV4.Enabled() {
@@ -152,13 +167,14 @@ func (pb *prober) runProbe(ctx context.Context, probeType probeType, p *v1.Probe
152167
path := req.URL.Path
153168
scheme := req.URL.Scheme
154169
headers := p.HTTPGet.HTTPHeaders
155-
klogV4.InfoS("HTTP-Probe", "scheme", scheme, "host", host, "port", port, "path", path, "timeout", timeout, "headers", headers)
170+
klogV4.InfoS("HTTP-Probe", "scheme", scheme, "host", host, "port", port, "path", path, "timeout", timeout, "headers", headers, "probeType", probeType)
156171
}
157172
return pb.http.Probe(req, timeout)
158173

159174
case p.TCPSocket != nil:
160175
port, err := probe.ResolveContainerPort(p.TCPSocket.Port, &container)
161176
if err != nil {
177+
klog.V(4).InfoS("TCP-Probe failed to resolve port", "error", err)
162178
return probe.Unknown, "", err
163179
}
164180
host := p.TCPSocket.Host
@@ -178,7 +194,7 @@ func (pb *prober) runProbe(ctx context.Context, probeType probeType, p *v1.Probe
178194
return pb.grpc.Probe(host, service, int(p.GRPC.Port), timeout)
179195

180196
default:
181-
klog.InfoS("Failed to find probe builder for container", "containerName", container.Name)
197+
klog.V(4).InfoS("Failed to find probe builder for container", "containerName", container.Name)
182198
return probe.Unknown, "", fmt.Errorf("missing probe handler for %s:%s", format.Pod(pod), container.Name)
183199
}
184200
}

‎pkg/kubelet/prober/prober_test.go‎

Lines changed: 127 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,20 @@ import (
2525
"strings"
2626
"testing"
2727

28+
"github.com/stretchr/testify/assert"
29+
"github.com/stretchr/testify/require"
2830
v1 "k8s.io/api/core/v1"
31+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2932
"k8s.io/apimachinery/pkg/util/intstr"
3033
"k8s.io/client-go/tools/record"
34+
"k8s.io/kubernetes/pkg/api/legacyscheme"
3135
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
3236
containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
3337
"k8s.io/kubernetes/pkg/kubelet/prober/results"
3438
"k8s.io/kubernetes/pkg/kubelet/util/ioutils"
3539
"k8s.io/kubernetes/pkg/probe"
3640
execprobe "k8s.io/kubernetes/pkg/probe/exec"
41+
"k8s.io/kubernetes/test/utils/ktesting"
3742
)
3843

3944
func TestGetURLParts(t *testing.T) {
@@ -141,6 +146,7 @@ func TestProbe(t *testing.T) {
141146
Exec: &v1.ExecAction{},
142147
},
143148
}
149+
144150
tests := []struct {
145151
probe *v1.Probe
146152
env []v1.EnvVar
@@ -149,6 +155,7 @@ func TestProbe(t *testing.T) {
149155
execResult probe.Result
150156
expectedResult results.Result
151157
expectCommand []string
158+
unsupported bool
152159
}{
153160
{ // No probe
154161
probe: nil,
@@ -174,18 +181,25 @@ func TestProbe(t *testing.T) {
174181
execResult: probe.Warning,
175182
expectedResult: results.Success,
176183
},
177-
{ // Probe result is unknown
184+
{ // Probe result is unknown with no error
178185
probe: execProbe,
179186
execResult: probe.Unknown,
187+
expectError: false,
180188
expectedResult: results.Failure,
181189
},
182-
{ // Probe has an error
190+
{ // Probe result is unknown with an error
183191
probe: execProbe,
184192
execError: true,
185193
expectError: true,
186194
execResult: probe.Unknown,
187195
expectedResult: results.Failure,
188196
},
197+
{ // Unsupported probe type
198+
probe: nil,
199+
expectedResult: results.Failure,
200+
expectError: true,
201+
unsupported: true,
202+
},
189203
{ // Probe arguments are passed through
190204
probe: &v1.Probe{
191205
ProbeHandler: v1.ProbeHandler{
@@ -216,13 +230,17 @@ func TestProbe(t *testing.T) {
216230
}
217231

218232
for i, test := range tests {
219-
for _, probeType := range [...]probeType{liveness, readiness, startup} {
233+
for _, pType := range [...]probeType{liveness, readiness, startup} {
234+
235+
if test.unsupported {
236+
pType = probeType(666)
237+
}
220238
prober := &prober{
221239
recorder: &record.FakeRecorder{},
222240
}
223-
testID := fmt.Sprintf("%d-%s", i, probeType)
241+
testID := fmt.Sprintf("%d-%s", i, pType)
224242
testContainer := v1.Container{Env: test.env}
225-
switch probeType {
243+
switch pType {
226244
case liveness:
227245
testContainer.LivenessProbe = test.probe
228246
case readiness:
@@ -236,25 +254,22 @@ func TestProbe(t *testing.T) {
236254
prober.exec = fakeExecProber{test.execResult, nil}
237255
}
238256

239-
result, err := prober.probe(ctx, probeType, &v1.Pod{}, v1.PodStatus{}, testContainer, containerID)
240-
if test.expectError && err == nil {
241-
t.Errorf("[%s] Expected probe error but no error was returned.", testID)
242-
}
243-
if !test.expectError && err != nil {
244-
t.Errorf("[%s] Didn't expect probe error but got: %v", testID, err)
245-
}
246-
if test.expectedResult != result {
247-
t.Errorf("[%s] Expected result to be %v but was %v", testID, test.expectedResult, result)
257+
result, err := prober.probe(ctx, pType, &v1.Pod{}, v1.PodStatus{}, testContainer, containerID)
258+
259+
if test.expectError {
260+
require.Error(t, err, "[%s] Expected probe error but no error was returned.", testID)
261+
} else {
262+
require.NoError(t, err, "[%s] Didn't expect probe error", testID)
248263
}
249264

265+
require.Equal(t, test.expectedResult, result, "[%s] Expected result to be %v but was %v", testID, test.expectedResult, result)
266+
250267
if len(test.expectCommand) > 0 {
251268
prober.exec = execprobe.New()
252269
prober.runner = &containertest.FakeContainerCommandRunner{}
253-
_, err := prober.probe(ctx, probeType, &v1.Pod{}, v1.PodStatus{}, testContainer, containerID)
254-
if err != nil {
255-
t.Errorf("[%s] Didn't expect probe error but got: %v", testID, err)
256-
continue
257-
}
270+
_, err := prober.probe(ctx, pType, &v1.Pod{}, v1.PodStatus{}, testContainer, containerID)
271+
require.NoError(t, err, "[%s] Didn't expect probe error ", testID)
272+
258273
if !reflect.DeepEqual(test.expectCommand, prober.runner.(*containertest.FakeContainerCommandRunner).Cmd) {
259274
t.Errorf("[%s] unexpected probe arguments: %v", testID, prober.runner.(*containertest.FakeContainerCommandRunner).Cmd)
260275
}
@@ -264,7 +279,7 @@ func TestProbe(t *testing.T) {
264279
}
265280

266281
func TestNewExecInContainer(t *testing.T) {
267-
ctx := context.Background()
282+
ctx := ktesting.Init(t)
268283
limit := 1024
269284
tenKilobyte := strings.Repeat("logs-123", 128*10)
270285

@@ -333,3 +348,95 @@ func TestNewExecInContainer(t *testing.T) {
333348
}
334349
}
335350
}
351+
352+
func TestNewProber(t *testing.T) {
353+
runner := &containertest.FakeContainerCommandRunner{}
354+
recorder := &record.FakeRecorder{}
355+
prober := newProber(runner, recorder)
356+
357+
assert.NotNil(t, prober, "Expected prober to be non-nil")
358+
assert.Equal(t, runner, prober.runner, "Expected prober runner to match")
359+
assert.Equal(t, recorder, prober.recorder, "Expected prober recorder to match")
360+
361+
assert.NotNil(t, prober.exec, "exec probe initialized")
362+
assert.NotNil(t, prober.http, "http probe initialized")
363+
assert.NotNil(t, prober.tcp, "tcp probe initialized")
364+
assert.NotNil(t, prober.grpc, "grpc probe initialized")
365+
366+
}
367+
368+
func TestRecordContainerEventUnknownStatus(t *testing.T) {
369+
370+
err := v1.AddToScheme(legacyscheme.Scheme)
371+
require.NoError(t, err, "failed to add v1 to scheme")
372+
373+
pod := &v1.Pod{
374+
ObjectMeta: metav1.ObjectMeta{
375+
UID: "test-probe-pod",
376+
},
377+
Spec: v1.PodSpec{
378+
Containers: []v1.Container{
379+
{
380+
Name: "test-probe-container",
381+
},
382+
},
383+
},
384+
}
385+
386+
container := pod.Spec.Containers[0]
387+
output := "probe output"
388+
389+
testCases := []struct {
390+
name string
391+
probeType probeType
392+
result probe.Result
393+
expected []string
394+
}{
395+
{
396+
name: "Readiness Probe Unknown",
397+
probeType: readiness,
398+
result: probe.Unknown,
399+
expected: []string{
400+
"Warning ContainerProbeWarning Readiness probe warning: probe output",
401+
"Warning ContainerProbeWarning Unknown Readiness probe status: unknown",
402+
},
403+
},
404+
{
405+
name: "Liveness Probe Unknown",
406+
probeType: liveness,
407+
result: probe.Unknown,
408+
expected: []string{
409+
"Warning ContainerProbeWarning Liveness probe warning: probe output",
410+
"Warning ContainerProbeWarning Unknown Liveness probe status: unknown",
411+
},
412+
},
413+
{
414+
name: "Startup Probe Unknown",
415+
probeType: startup,
416+
result: probe.Unknown,
417+
expected: []string{
418+
"Warning ContainerProbeWarning Startup probe warning: probe output",
419+
"Warning ContainerProbeWarning Unknown Startup probe status: unknown",
420+
},
421+
},
422+
}
423+
424+
for _, tc := range testCases {
425+
t.Run(tc.name, func(t *testing.T) {
426+
bufferSize := len(tc.expected) + 1
427+
fakeRecorder := record.NewFakeRecorder(bufferSize)
428+
429+
pb := &prober{
430+
recorder: fakeRecorder,
431+
}
432+
433+
pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, "ContainerProbeWarning", "%s probe warning: %s", tc.probeType, output)
434+
pb.recordContainerEvent(pod, &container, v1.EventTypeWarning, "ContainerProbeWarning", "Unknown %s probe status: %s", tc.probeType, tc.result)
435+
436+
assert.Equal(t, len(tc.expected), len(fakeRecorder.Events), "unexpected number of events")
437+
for _, expected := range tc.expected {
438+
assert.Equal(t, expected, <-fakeRecorder.Events)
439+
}
440+
})
441+
}
442+
}

0 commit comments

Comments
 (0)