Fix the first alias of container reference #2931
Conversation
Signed-off-by: Qiutong Song <songqt01@gmail.com>
qiutongs
force-pushed
the
containerd-fix
branch
from
900e025
to
2900bf4
Compare
|
Hi @qiutongs. Thanks for your PR. I'm waiting for a google member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
| @@ -112,11 +112,31 @@ func newContainerdContainerHandler( | |||
| rootfs = "/rootfs" | |||
| } | |||
|
|
|||
| var restart uint32 = 0 | |||
| containerName := "POD" | |||
There was a problem hiding this comment.
For clarity, maybe move this into a constant and leave a comment that POD is container name for sandbox container and it is defined here: https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/dockershim/naming.go#L50-L52
| @@ -112,11 +112,31 @@ func newContainerdContainerHandler( | |||
| rootfs = "/rootfs" | |||
| } | |||
|
|
|||
| var restart uint32 = 0 | |||
There was a problem hiding this comment.
nit: you can just do var restart uint32 and use the default value
There was a problem hiding this comment.
Thanks for reminding. I would just explicitly set to 0
| if err != nil { | ||
| return nil, err | ||
| } | ||
| restart = status.Metadata.Attempt |
There was a problem hiding this comment.
leave a comment that sandbox container restart is not tracked as you mentioned in the PR description
For sandbox container (pause), the restart is hardcoded to "0"
|
LGTM, thanks! |
Signed-off-by: Qiutong Song songqt01@gmail.com
Overview
Previous PR: #2919
In Prometheus and Statsd, they use the first container alias as the "container name" in the output metrics. However, docker handler and containerd handler have different logics for the alias.
In docker, it is
k8s_<container-name>_<pod-name>_<namespace>_<pod-uid>_<restart-count>but it is in containerd.This PR is to make containerd behave the same way as docker.
Notes
Testing
Query Prometheus endpont:
localhost:8080/metricsKill the container process (to force restarted) and re-query