Add imagePullSecrets: myfakesecret to a pod spec, and pull images from docker.io.

/sig node
(based on the field location)

(and/or)
/sig auth

correct me if not. Pulling private images with nonexists secret, pod will turn to ErrImagePull. However, pulling public image with nonexists secret, no error will display yet.

Similar issue with #88142.

Perhaps a warning event should be created?

correct me if not. Pulling private images with nonexists secret, pod will turn to ErrImagePull. However, pulling public image with nonexists secret, no error will display yet.

Yes that's correct.

Similar issue with #88142.

Perhaps a warning event should be created?

I think so. Working on this later. Will also live up to your expectations, right? @neilharris123

But I check it already have a warnging events if secret not exist and pullImageFail:
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/images/image_manager.go line:147

Now, only info message when pullsecret not exist

maybe direct return and not enter syncPod is better.
return if pullSecret is not existed

@kubernetes/sig-node-bugs

maybe direct return and not enter syncPod is better.
return if pullSecret is not existed

kubernetes/pkg/kubelet/kubelet.go

Lines 1725 to 1729 in 175776d

	// Fetch the pull secrets for the pod
	pullSecrets := kl.getPullSecretsForPod(pod)
	// Call the container runtime's SyncPod callback
	result := kl.containerRuntime.SyncPod(pod, podStatus, pullSecrets, kl.backOff)

@kubernetes/sig-node-bugs

if you pull image from a public repo, the process should not be impacted even no secrets exists. And it's what it is right now.

I think a warning event FailedToRetrieveImagePullSecret like

@adisky:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

/assign @EricWvi

@kerthcet: GitHub didn't allow me to assign the following users: EricWvi.

Note that only kubernetes members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

@calvin0327: GitHub didn't allow me to assign the following users: EricWvi.

Note that only kubernetes members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

I would like to work in this.
/assign

Thanks for wanting to jump in on this, but it's not clear this is a good first issue… making the proposed change would break deployments/pods that are currently working/running, so it's unlikely this would be acceptable from a compatibility perspective.

Requiring image pull secrets to exist would have to be opt-in, which means an API change. I don't think this is worth the addition, but would like to hear sig-node's perspective.

An alternative could be to explore ways to make it more obvious to the user when this situation was occurring without automatically failing the pod

Requiring image pull secrets to exist would have to be opt-in, which means an API change. I don't think this is worth the addition, but would like to hear sig-node's perspective.

An alternative could be to explore ways to make it more obvious to the user when this situation was occurring without automatically failing the pod

hi @liggitt , we will not fail the pod, we just want to add some warning to make it obviously, maybe it's the same as you advised. correct me if not. 🧐

I think a warning event FailedToRetrieveImagePullSecret like

kubernetes/pkg/kubelet/kubelet_pods.go

Line 655 in 175776d

kl.recorder.Eventf(pod, v1.EventTypeWarning, "InvalidEnvironmentVariableNames", "Keys [%s] from the EnvFrom configMap %s/%s were skipped since they are considered invalid environment variable names.", strings.Join(invalidKeys, ", "), pod.Namespace, name)

could be added here

kubernetes/pkg/kubelet/kubelet_pods.go

Lines 891 to 895 in 175776d

	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)
	continue
	}

/good-first-issue
/triage accepted

To be clear here, only adding a warning is marked as good first issue here, not changing any pod behaviour.

gotcha, that seems more reasonable, thanks for clarifying

as a note, make sure the warning events are generated at most once in that function, rather than once per missing secret object (if there are multiple missing secrets)

/good-first-issue

@liggitt:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

Similar issue with #88142.

Perhaps a warning event should be created?

I think so. Working on this later. Will also live up to your expectations, right? @neilharris123

Yes, a warning would be good. I feel an error would have be better, but as has already been mentioned above we are too far along now for this change as it could break some users existing deployments, so a warning makes the most sense now.

/assign

@adisky based on what you have stated above, I see an INFO log already exists! If I'm understanding this correctly, now we need to raise a warning event in the same code block to fulfill this request. Is that right? @liggitt @neilharris123

I think @calvin0327 has created a pr above and labeled /lgtm already.

Hey I don't know anything about how to code but i am interested to contribute on kubernetes
So help me to contribute