Garbage collect unidentified Kubernetes containers. #5395
Conversation
|
Verified changes on a new cluster. PTAL @yujuhong @dchen1107 when you get a chance :) |
| name string | ||
| } | ||
|
|
||
| unidentifiedContainers := make([]unidentifiedContainer, 0) |
There was a problem hiding this comment.
Are you removing the user created dead containers which are not managed by kubernetes. Is that what we want?
There was a problem hiding this comment.
Ahh, never mind. Expanding the code a little, the logic is against kubernetes containers only.
|
LGTM except one nit. |
These containers may be caused by a change in the Kubernetes naming convention. The old containers are killed, the new ones started, but the old ones are never GC'd. This change makes Kubelet GC all Kubernetes containers, old and new. Fixes kubernetes#5372.
| uidToIDMap := map[string][]string{} | ||
| for _, container := range containers { | ||
| _, uid, name, _ := dockertools.ParseDockerName(container.Names[0]) | ||
| if uid == "" && name == "" { |
There was a problem hiding this comment.
I am not sure if this condition covers all cases (and you probably know better than me). It'd be more clear if we can get an error from ParseDockerName() if anything goes wrong, which will help if we ever change the naming scheme again. I see you have a TODO there though, so I guess it's fine for now.
There was a problem hiding this comment.
I am equally unhappy and I agree the error route is more correct. I guess I should tackle that sooner rather than later :P I'll send a PR your way sometime today.
|
LGTM otherwise, and thanks for fixing this! |
Garbage collect unidentified Kubernetes containers.
These containers may be caused by a change in the Kubernetes naming
convention. The old containers are killed, the new ones started, but the
old ones are never GC'd. This change makes Kubelet GC all Kubernetes
containers, old and new.
Fixes #5372.