Fixes No ref for container in probes after kubelet restart

[EricMountain]

What type of PR is this?

/kind bug

What this PR does / why we need it:

After restarting, the kubelet synchs previously running containers. However it does not populate the container RefManager map (pkg/kubelet/container/container_reference_manager.go).

Any probes that fail against containers that were running before the kubelet restarted do not generate events, and instead log:

No ref for container "containerd://..." (...)

So for instance kubectl describe pod ... with a container (started before kubelet restart) failing its readiness probe will show:

...
Containers:
  xxxxxx:
...
    State:          Running
      Started:      Mon, 16 Sep 2019 11:28:31 +0200
    Ready:          False
    Restart Count:  0
...
    Liveness:   tcp-socket :6379 delay=30s timeout=8s period=10s #success=1 #failure=60
    Readiness:  http-get http://:ready/ready delay=30s timeout=8s period=10s #success=1 #failure=3
...
Conditions:
  Type           Status
  Initialized    True
  Ready          False
  PodScheduled   True
...
Events:          <none>

I.e. a non-ready container, however no matching events. If a container is not-ready, then it is because it is failing its Readiness probe and we expect corresponding events.

Liveness probe events are similarly not created in such a situation, though the container is restarted. So we see short container existence time and restarts, but not the matching liveness probe failure events.

Which issue(s) this PR fixes

Fixes the No ref log mentionned in #83336, though probably not the underlying issue.

Special notes for your reviewer:

2 commits:

• One to fix the No ref issue and ensure events are created by using almost the same recordContainerEvent() function as implemented in pkg/kubelet/kuberuntime/kuberuntime_container.go. The difference is that we don't do container ID/name substitution as the container ID is not present in the messages passed in.
• One to remove RefManager (pkg/kubelet/container/container_reference_manager.go): it was only used by the prober to retrieve container ID - since we generate the ID on the fly now, it is unnecessary.

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

N/A

[k8s-ci-robot]

Welcome @EricMountain!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @EricMountain. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[EricMountain]

/sig node

[EricMountain]

/assign @vishh

[mattjmcnaughton]

/ok-to-test

Thanks for your work here!

If you know it, can you share more around the original motivation for the container reference manager? In other words, why did we add this manager, instead of generating references on the fly every time?
Are there benefits offered by the container reference manager that we may loose if we get rid of it entirely?

If there may be benefits, an alternative option would be "falling back" to generating the reference on the fly only if it does not exist in the container manage after the kubelet restart? If I'm understanding correctly, such a change would require less code change... we would just need to update RefManager.GetRef.

[EricMountain]

/retest

[EricMountain]

/test pull-kubernetes-e2e-kind

[EricMountain]

Working on understanding the test failure.

[EricMountain]

Retest due to failure creating kind cluster.

/test pull-kubernetes-e2e-kind

[EricMountain]

/test pull-kubernetes-kubemark-e2e-gce-big

[EricMountain]

/test pull-kubernetes-node-e2e-containerd

[mattjmcnaughton]

/lgtm

Thanks for your work here :) I agree with merging this change for the short-term, and will take a look at your other diff.

[mattjmcnaughton]

/assign @tallclair

[EricMountain]

Hi @tallclair! Would you take a look at this PR? Thanks!

[dims]

/priority important-soon
/milestone v1.18

[gianarb]

Hello @matt-tyler @tallclair !
Bug Triage team here for the 1.18 release. This is a friendly reminder that code freeze is scheduled for 5 March. Is this issue still intended for milestone 1.18, or should it extend to the next one? Thanks in advance!

[tallclair]

/approve
/hold

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: EricMountain, tallclair

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• pkg/kubelet/OWNERS [tallclair]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[gianarb]

Hello @EricMountain, @tallclair !
Bug Triage team here for the 1.18 release. This is a friendly reminder that code freeze is scheduled for 5 March. I saw you approved this PR already but it got some new code, can you review again in order to try to get it merged before code freeze?
Thanks

[tallclair]

/lgtm
/hold cancel