From 123845da8864a2eaf7768cda486ca646e8216501 Mon Sep 17 00:00:00 2001 From: Tomas Tormo Date: Thu, 11 May 2023 08:03:20 +0000 Subject: [PATCH 1/3] Log a warning if a ImagePullSecrets does not exist --- pkg/kubelet/kubelet_pods.go | 6 ++++++ pkg/kubelet/kubelet_pods_test.go | 31 ++++++++++++++++++++++++++++++ pkg/kubelet/secret/fake_manager.go | 29 +++++++++++++++++++++++++--- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 7d53bdcf01b..7482415f334 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -876,6 +876,7 @@ func (kl *Kubelet) makePodDataDirs(pod *v1.Pod) error { // secrets. func (kl *Kubelet) getPullSecretsForPod(pod *v1.Pod) []v1.Secret { pullSecrets := []v1.Secret{} + failedPullSecrets := []string{} for _, secretRef := range pod.Spec.ImagePullSecrets { if len(secretRef.Name) == 0 { @@ -886,12 +887,17 @@ func (kl *Kubelet) getPullSecretsForPod(pod *v1.Pod) []v1.Secret { secret, err := kl.secretManager.GetSecret(pod.Namespace, secretRef.Name) if err != nil { klog.InfoS("Unable to retrieve pull secret, the image pull may not succeed.", "pod", klog.KObj(pod), "secret", klog.KObj(secret), "err", err) + failedPullSecrets = append(failedPullSecrets, secretRef.Name) continue } pullSecrets = append(pullSecrets, *secret) } + if len(failedPullSecrets) > 0 { + kl.recorder.Eventf(pod, v1.EventTypeWarning, "FailedToRetrieveImagePullSecret", "Unable to retrieve image pull secrets %s, the image pull may not succeed.", strings.Join(failedPullSecrets, ", ")) + } + return pullSecrets } diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index 753e6959c51..418b141136d 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -54,6 +54,7 @@ import ( "k8s.io/kubernetes/pkg/kubelet/cri/streaming/remotecommand" "k8s.io/kubernetes/pkg/kubelet/metrics" "k8s.io/kubernetes/pkg/kubelet/prober/results" + "k8s.io/kubernetes/pkg/kubelet/secret" "k8s.io/kubernetes/pkg/kubelet/status" kubetypes "k8s.io/kubernetes/pkg/kubelet/types" netutils "k8s.io/utils/net" @@ -5396,3 +5397,33 @@ func testMetric(t *testing.T, metricName string, expectedMetric string) { t.Error(err) } } + +func TestGetNonExistentImagePullSecret(t *testing.T) { + secrets := make([]*v1.Secret, 0) + fakeRecorder := record.NewFakeRecorder(1) + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) + testKubelet.kubelet.recorder = fakeRecorder + testKubelet.kubelet.secretManager = secret.NewFakeManagerWithSecrets(secrets) + defer testKubelet.Cleanup() + + expectedEvent := "Warning FailedToRetrieveImagePullSecret Unable to retrieve image pull secrets secretFoo, the image pull may not succeed." + testPod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nsFoo", + Name: "podFoo", + Annotations: map[string]string{}, + }, + Spec: v1.PodSpec{ + ImagePullSecrets: []v1.LocalObjectReference{ + {Name: "secretFoo"}, + }, + }, + } + + pullSecrets := testKubelet.kubelet.getPullSecretsForPod(testPod) + assert.Equal(t, 0, len(pullSecrets)) + + assert.Equal(t, 1, len(fakeRecorder.Events)) + event := <-fakeRecorder.Events + assert.Equal(t, event, expectedEvent) +} diff --git a/pkg/kubelet/secret/fake_manager.go b/pkg/kubelet/secret/fake_manager.go index 4f4bf9fe4cd..0753d3dbb0a 100644 --- a/pkg/kubelet/secret/fake_manager.go +++ b/pkg/kubelet/secret/fake_manager.go @@ -16,11 +16,16 @@ limitations under the License. package secret -import v1 "k8s.io/api/core/v1" +import ( + "fmt" + + v1 "k8s.io/api/core/v1" +) // fakeManager implements Manager interface for testing purposes. // simple operations to apiserver. type fakeManager struct { + secrets []*v1.Secret } // NewFakeManager creates empty/fake secret manager @@ -28,9 +33,27 @@ func NewFakeManager() Manager { return &fakeManager{} } -// GetSecret returns a nil secret for testing +// NewFakeManagerWithSecrets creates a fake secret manager with the provided secrets +func NewFakeManagerWithSecrets(secrets []*v1.Secret) Manager { + return &fakeManager{ + secrets: secrets, + } +} + +// GetSecret function returns the searched secret if it was provided during the manager initialization, otherwise, it returns an error. +// If the manager was initialized without any secrets, it returns a nil secret." func (s *fakeManager) GetSecret(namespace, name string) (*v1.Secret, error) { - return nil, nil + if s.secrets == nil { + return nil, nil + } + + for _, secret := range s.secrets { + if secret.Name == name { + return secret, nil + } + } + + return nil, fmt.Errorf("secret %s not found", name) } // RegisterPod implements the RegisterPod method for testing purposes. From 5a75a03a774c17b287ad6166cfc4dd99e16f96e4 Mon Sep 17 00:00:00 2001 From: Tomas Tormo Date: Wed, 17 May 2023 19:25:54 +0200 Subject: [PATCH 2/3] Improve warning message Co-authored-by: Steven E. Harris --- pkg/kubelet/kubelet_pods.go | 2 +- pkg/kubelet/kubelet_pods_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 7482415f334..866d438bd06 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -895,7 +895,7 @@ func (kl *Kubelet) getPullSecretsForPod(pod *v1.Pod) []v1.Secret { } if len(failedPullSecrets) > 0 { - kl.recorder.Eventf(pod, v1.EventTypeWarning, "FailedToRetrieveImagePullSecret", "Unable to retrieve image pull secrets %s, the image pull may not succeed.", strings.Join(failedPullSecrets, ", ")) + kl.recorder.Eventf(pod, v1.EventTypeWarning, "FailedToRetrieveImagePullSecret", "Unable to retrieve image pull secrets %s; attempting to pull the image may not succeed.", strings.Join(failedPullSecrets, ", ")) } return pullSecrets diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index 418b141136d..cbb21f564c7 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -5406,7 +5406,7 @@ func TestGetNonExistentImagePullSecret(t *testing.T) { testKubelet.kubelet.secretManager = secret.NewFakeManagerWithSecrets(secrets) defer testKubelet.Cleanup() - expectedEvent := "Warning FailedToRetrieveImagePullSecret Unable to retrieve image pull secrets secretFoo, the image pull may not succeed." + expectedEvent := "Warning FailedToRetrieveImagePullSecret Unable to retrieve image pull secrets secretFoo; attempting to pull the image may not succeed." testPod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: "nsFoo", From a10ff53d8e60f35a974e3e6d15ca6ab1c9401fe8 Mon Sep 17 00:00:00 2001 From: Tomas Tormo Date: Thu, 18 May 2023 15:43:59 +0200 Subject: [PATCH 3/3] Reword the event message to read better for a single secret Co-authored-by: Steven E. Harris --- pkg/kubelet/kubelet_pods.go | 2 +- pkg/kubelet/kubelet_pods_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 866d438bd06..f2f99efce52 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -895,7 +895,7 @@ func (kl *Kubelet) getPullSecretsForPod(pod *v1.Pod) []v1.Secret { } if len(failedPullSecrets) > 0 { - kl.recorder.Eventf(pod, v1.EventTypeWarning, "FailedToRetrieveImagePullSecret", "Unable to retrieve image pull secrets %s; attempting to pull the image may not succeed.", strings.Join(failedPullSecrets, ", ")) + kl.recorder.Eventf(pod, v1.EventTypeWarning, "FailedToRetrieveImagePullSecret", "Unable to retrieve some image pull secrets (%s); attempting to pull the image may not succeed.", strings.Join(failedPullSecrets, ", ")) } return pullSecrets diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index cbb21f564c7..ca1ee492feb 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -5406,7 +5406,8 @@ func TestGetNonExistentImagePullSecret(t *testing.T) { testKubelet.kubelet.secretManager = secret.NewFakeManagerWithSecrets(secrets) defer testKubelet.Cleanup() - expectedEvent := "Warning FailedToRetrieveImagePullSecret Unable to retrieve image pull secrets secretFoo; attempting to pull the image may not succeed." + expectedEvent := "Warning FailedToRetrieveImagePullSecret Unable to retrieve some image pull secrets (secretFoo); attempting to pull the image may not succeed." + testPod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: "nsFoo",