Skip to content

Commit 7429566

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#127918 from saschagrunert/backoff-status
Use image pull error in `message` during back-off
2 parents b0d0a15 + 0fc4b74 commit 7429566

File tree

2 files changed

+90
-35
lines changed

2 files changed

+90
-35
lines changed

‎pkg/kubelet/images/image_manager.go‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"strings"
23+
"sync"
2324
"time"
2425

2526
v1 "k8s.io/api/core/v1"
@@ -43,9 +44,11 @@ type ImagePodPullingTimeRecorder interface {
4344

4445
// imageManager provides the functionalities for image pulling.
4546
type imageManager struct {
46-
recorder record.EventRecorder
47-
imageService kubecontainer.ImageService
48-
backOff *flowcontrol.Backoff
47+
recorder record.EventRecorder
48+
imageService kubecontainer.ImageService
49+
backOff *flowcontrol.Backoff
50+
prevPullErrMsg sync.Map
51+
4952
// It will check the presence of the image, and report the 'image pulling', image pulled' events correspondingly.
5053
puller imagePuller
5154

@@ -154,8 +157,20 @@ func (m *imageManager) EnsureImageExists(ctx context.Context, objRef *v1.ObjectR
154157
if m.backOff.IsInBackOffSinceUpdate(backOffKey, m.backOff.Clock.Now()) {
155158
msg := fmt.Sprintf("Back-off pulling image %q", imgRef)
156159
m.logIt(objRef, v1.EventTypeNormal, events.BackOffPullImage, logPrefix, msg, klog.Info)
160+
161+
// Wrap the error from the actual pull if available.
162+
// This information is populated to the pods
163+
// .status.containerStatuses[*].state.waiting.message.
164+
prevPullErrMsg, ok := m.prevPullErrMsg.Load(backOffKey)
165+
if ok {
166+
msg = fmt.Sprintf("%s: %s", msg, prevPullErrMsg)
167+
}
168+
157169
return "", msg, ErrImagePullBackOff
158170
}
171+
// Ensure that the map cannot grow indefinitely.
172+
m.prevPullErrMsg.Delete(backOffKey)
173+
159174
m.podPullingTimeRecorder.RecordImageStartedPulling(pod.UID)
160175
m.logIt(objRef, v1.EventTypeNormal, events.PullingImage, logPrefix, fmt.Sprintf("Pulling image %q", imgRef), klog.Info)
161176
startTime := time.Now()
@@ -167,6 +182,11 @@ func (m *imageManager) EnsureImageExists(ctx context.Context, objRef *v1.ObjectR
167182
m.backOff.Next(backOffKey, m.backOff.Clock.Now())
168183

169184
msg, err := evalCRIPullErr(imgRef, imagePullResult.err)
185+
186+
// Store the actual pull error for providing that information during
187+
// the image pull back-off.
188+
m.prevPullErrMsg.Store(backOffKey, fmt.Sprintf("%s: %s", err, msg))
189+
170190
return "", msg, err
171191
}
172192
m.podPullingTimeRecorder.RecordImageFinishedPulling(pod.UID)

‎pkg/kubelet/images/image_manager_test.go‎

Lines changed: 67 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ type pullerExpects struct {
4949
shouldRecordStartedPullingTime bool
5050
shouldRecordFinishedPullingTime bool
5151
events []v1.Event
52+
msg string
5253
}
5354

5455
type pullerTestCase struct {
@@ -77,7 +78,7 @@ func pullerTestCases() []pullerTestCase {
7778
[]v1.Event{
7879
{Reason: "Pulling"},
7980
{Reason: "Pulled"},
80-
}},
81+
}, ""},
8182
}},
8283

8384
{ // image present, don't pull
@@ -92,15 +93,15 @@ func pullerTestCases() []pullerTestCase {
9293
{[]string{"GetImageRef"}, nil, false, false,
9394
[]v1.Event{
9495
{Reason: "Pulled"},
95-
}},
96+
}, ""},
9697
{[]string{"GetImageRef"}, nil, false, false,
9798
[]v1.Event{
9899
{Reason: "Pulled"},
99-
}},
100+
}, ""},
100101
{[]string{"GetImageRef"}, nil, false, false,
101102
[]v1.Event{
102103
{Reason: "Pulled"},
103-
}},
104+
}, ""},
104105
}},
105106
// image present, pull it
106107
{containerImage: "present_image",
@@ -115,17 +116,17 @@ func pullerTestCases() []pullerTestCase {
115116
[]v1.Event{
116117
{Reason: "Pulling"},
117118
{Reason: "Pulled"},
118-
}},
119+
}, ""},
119120
{[]string{"PullImage", "GetImageSize"}, nil, true, true,
120121
[]v1.Event{
121122
{Reason: "Pulling"},
122123
{Reason: "Pulled"},
123-
}},
124+
}, ""},
124125
{[]string{"PullImage", "GetImageSize"}, nil, true, true,
125126
[]v1.Event{
126127
{Reason: "Pulling"},
127128
{Reason: "Pulled"},
128-
}},
129+
}, ""},
129130
}},
130131
// missing image, error PullNever
131132
{containerImage: "missing_image",
@@ -139,15 +140,15 @@ func pullerTestCases() []pullerTestCase {
139140
{[]string{"GetImageRef"}, ErrImageNeverPull, false, false,
140141
[]v1.Event{
141142
{Reason: "ErrImageNeverPull"},
142-
}},
143+
}, ""},
143144
{[]string{"GetImageRef"}, ErrImageNeverPull, false, false,
144145
[]v1.Event{
145146
{Reason: "ErrImageNeverPull"},
146-
}},
147+
}, ""},
147148
{[]string{"GetImageRef"}, ErrImageNeverPull, false, false,
148149
[]v1.Event{
149150
{Reason: "ErrImageNeverPull"},
150-
}},
151+
}, ""},
151152
}},
152153
// missing image, unable to inspect
153154
{containerImage: "missing_image",
@@ -161,15 +162,15 @@ func pullerTestCases() []pullerTestCase {
161162
{[]string{"GetImageRef"}, ErrImageInspect, false, false,
162163
[]v1.Event{
163164
{Reason: "InspectFailed"},
164-
}},
165+
}, ""},
165166
{[]string{"GetImageRef"}, ErrImageInspect, false, false,
166167
[]v1.Event{
167168
{Reason: "InspectFailed"},
168-
}},
169+
}, ""},
169170
{[]string{"GetImageRef"}, ErrImageInspect, false, false,
170171
[]v1.Event{
171172
{Reason: "InspectFailed"},
172-
}},
173+
}, ""},
173174
}},
174175
// missing image, unable to fetch
175176
{containerImage: "typo_image",
@@ -184,29 +185,29 @@ func pullerTestCases() []pullerTestCase {
184185
[]v1.Event{
185186
{Reason: "Pulling"},
186187
{Reason: "Failed"},
187-
}},
188+
}, ""},
188189
{[]string{"GetImageRef", "PullImage"}, ErrImagePull, true, false,
189190
[]v1.Event{
190191
{Reason: "Pulling"},
191192
{Reason: "Failed"},
192-
}},
193+
}, ""},
193194
{[]string{"GetImageRef"}, ErrImagePullBackOff, false, false,
194195
[]v1.Event{
195196
{Reason: "BackOff"},
196-
}},
197+
}, ""},
197198
{[]string{"GetImageRef", "PullImage"}, ErrImagePull, true, false,
198199
[]v1.Event{
199200
{Reason: "Pulling"},
200201
{Reason: "Failed"},
201-
}},
202+
}, ""},
202203
{[]string{"GetImageRef"}, ErrImagePullBackOff, false, false,
203204
[]v1.Event{
204205
{Reason: "BackOff"},
205-
}},
206+
}, ""},
206207
{[]string{"GetImageRef"}, ErrImagePullBackOff, false, false,
207208
[]v1.Event{
208209
{Reason: "BackOff"},
209-
}},
210+
}, ""},
210211
}},
211212
// image present, non-zero qps, try to pull
212213
{containerImage: "present_image",
@@ -221,17 +222,17 @@ func pullerTestCases() []pullerTestCase {
221222
[]v1.Event{
222223
{Reason: "Pulling"},
223224
{Reason: "Pulled"},
224-
}},
225+
}, ""},
225226
{[]string{"PullImage", "GetImageSize"}, nil, true, true,
226227
[]v1.Event{
227228
{Reason: "Pulling"},
228229
{Reason: "Pulled"},
229-
}},
230+
}, ""},
230231
{[]string{"PullImage", "GetImageSize"}, nil, true, true,
231232
[]v1.Event{
232233
{Reason: "Pulling"},
233234
{Reason: "Pulled"},
234-
}},
235+
}, ""},
235236
}},
236237
// image present, non-zero qps, try to pull when qps exceeded
237238
{containerImage: "present_image",
@@ -246,16 +247,16 @@ func pullerTestCases() []pullerTestCase {
246247
[]v1.Event{
247248
{Reason: "Pulling"},
248249
{Reason: "Failed"},
249-
}},
250+
}, ""},
250251
{[]string(nil), ErrImagePull, true, false,
251252
[]v1.Event{
252253
{Reason: "Pulling"},
253254
{Reason: "Failed"},
254-
}},
255+
}, ""},
255256
{[]string(nil), ErrImagePullBackOff, false, false,
256257
[]v1.Event{
257258
{Reason: "BackOff"},
258-
}},
259+
}, ""},
259260
}},
260261
// error case if image name fails validation due to invalid reference format
261262
{containerImage: "FAILED_IMAGE",
@@ -269,7 +270,7 @@ func pullerTestCases() []pullerTestCase {
269270
{[]string(nil), ErrInvalidImageName, false, false,
270271
[]v1.Event{
271272
{Reason: "InspectFailed"},
272-
}},
273+
}, ""},
273274
}},
274275
// error case if image name contains http
275276
{containerImage: "http://url",
@@ -283,7 +284,7 @@ func pullerTestCases() []pullerTestCase {
283284
{[]string(nil), ErrInvalidImageName, false, false,
284285
[]v1.Event{
285286
{Reason: "InspectFailed"},
286-
}},
287+
}, ""},
287288
}},
288289
// error case if image name contains sha256
289290
{containerImage: "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad",
@@ -297,7 +298,39 @@ func pullerTestCases() []pullerTestCase {
297298
{[]string(nil), ErrInvalidImageName, false, false,
298299
[]v1.Event{
299300
{Reason: "InspectFailed"},
300-
}},
301+
}, ""},
302+
}},
303+
{containerImage: "typo_image",
304+
testName: "image missing, SignatureValidationFailed",
305+
policy: v1.PullIfNotPresent,
306+
inspectErr: nil,
307+
pullerErr: crierrors.ErrSignatureValidationFailed,
308+
qps: 0.0,
309+
burst: 0,
310+
expected: []pullerExpects{
311+
{[]string{"GetImageRef", "PullImage"}, crierrors.ErrSignatureValidationFailed, true, false,
312+
[]v1.Event{
313+
{Reason: "Pulling"},
314+
{Reason: "Failed"},
315+
}, "image pull failed for typo_image because the signature validation failed"},
316+
{[]string{"GetImageRef", "PullImage"}, crierrors.ErrSignatureValidationFailed, true, false,
317+
[]v1.Event{
318+
{Reason: "Pulling"},
319+
{Reason: "Failed"},
320+
}, "image pull failed for typo_image because the signature validation failed"},
321+
{[]string{"GetImageRef"}, ErrImagePullBackOff, false, false,
322+
[]v1.Event{
323+
{Reason: "BackOff"},
324+
}, "Back-off pulling image \"typo_image\": SignatureValidationFailed: image pull failed for typo_image because the signature validation failed"},
325+
{[]string{"GetImageRef", "PullImage"}, crierrors.ErrSignatureValidationFailed, true, false,
326+
[]v1.Event{
327+
{Reason: "Pulling"},
328+
{Reason: "Failed"},
329+
}, "image pull failed for typo_image because the signature validation failed"},
330+
{[]string{"GetImageRef"}, ErrImagePullBackOff, false, false,
331+
[]v1.Event{
332+
{Reason: "BackOff"},
333+
}, "Back-off pulling image \"typo_image\": SignatureValidationFailed: image pull failed for typo_image because the signature validation failed"},
301334
}},
302335
}
303336
}
@@ -372,11 +405,12 @@ func TestParallelPuller(t *testing.T) {
372405
fakeRuntime.CalledFunctions = nil
373406
fakeClock.Step(time.Second)
374407

375-
_, _, err := puller.EnsureImageExists(ctx, nil, pod, container.Image, nil, nil, "", container.ImagePullPolicy)
408+
_, msg, err := puller.EnsureImageExists(ctx, nil, pod, container.Image, nil, nil, "", container.ImagePullPolicy)
376409
fakeRuntime.AssertCalls(expected.calls)
377410
assert.Equal(t, expected.err, err)
378411
assert.Equal(t, expected.shouldRecordStartedPullingTime, fakePodPullingTimeRecorder.startedPullingRecorded)
379412
assert.Equal(t, expected.shouldRecordFinishedPullingTime, fakePodPullingTimeRecorder.finishedPullingRecorded)
413+
assert.Contains(t, msg, expected.msg)
380414
fakePodPullingTimeRecorder.reset()
381415
}
382416
})
@@ -404,11 +438,12 @@ func TestSerializedPuller(t *testing.T) {
404438
fakeRuntime.CalledFunctions = nil
405439
fakeClock.Step(time.Second)
406440

407-
_, _, err := puller.EnsureImageExists(ctx, nil, pod, container.Image, nil, nil, "", container.ImagePullPolicy)
441+
_, msg, err := puller.EnsureImageExists(ctx, nil, pod, container.Image, nil, nil, "", container.ImagePullPolicy)
408442
fakeRuntime.AssertCalls(expected.calls)
409443
assert.Equal(t, expected.err, err)
410444
assert.Equal(t, expected.shouldRecordStartedPullingTime, fakePodPullingTimeRecorder.startedPullingRecorded)
411445
assert.Equal(t, expected.shouldRecordFinishedPullingTime, fakePodPullingTimeRecorder.finishedPullingRecorded)
446+
assert.Contains(t, msg, expected.msg)
412447
fakePodPullingTimeRecorder.reset()
413448
}
414449
})
@@ -456,7 +491,7 @@ func TestPullAndListImageWithPodAnnotations(t *testing.T) {
456491
inspectErr: nil,
457492
pullerErr: nil,
458493
expected: []pullerExpects{
459-
{[]string{"GetImageRef", "PullImage", "GetImageSize"}, nil, true, true, nil},
494+
{[]string{"GetImageRef", "PullImage", "GetImageSize"}, nil, true, true, nil, ""},
460495
}}
461496

462497
useSerializedEnv := true
@@ -512,7 +547,7 @@ func TestPullAndListImageWithRuntimeHandlerInImageCriAPIFeatureGate(t *testing.T
512547
inspectErr: nil,
513548
pullerErr: nil,
514549
expected: []pullerExpects{
515-
{[]string{"GetImageRef", "PullImage", "GetImageSize"}, nil, true, true, nil},
550+
{[]string{"GetImageRef", "PullImage", "GetImageSize"}, nil, true, true, nil, ""},
516551
}}
517552

518553
useSerializedEnv := true

0 commit comments

Comments
 (0)