daemon: Ignore nonexistent containers when listing containers#33883
daemon: Ignore nonexistent containers when listing containers#33883aaronlehmann wants to merge 1 commit intomoby:masterfrom
Conversation
The name/ID relationships are maintained separately from the memdb and can be out of sync from any particular memdb snapshot. If a container does not exist in the memdb, we must accept this as normal and not fail the listing. This is consistent with what the code used to do before memdb was introduced. Signed-off-by: Aaron Lehmann <[email protected]>
| return nil, err | ||
| } | ||
| if c != nil { | ||
| if err == nil { |
There was a problem hiding this comment.
view.Get() returns a "no such container" error if the container wasn't found; https://github.com/moby/moby/pull/33882/files#diff-a41e475815729f1ce9540241ddc3942fR138
Guess we should check for that error here, and return other errors?
There was a problem hiding this comment.
I agree with @thaJeztah it'd be better to also check for err != ErrNoSuchContainer and error out instead of silencing the error. Rationale: when someone adds a new error type that view.Get could return (i know it's unlikely but still), they wouldn't have to update all callers.
There was a problem hiding this comment.
I've updated #33882 to return a typed error. Once that is merged, I can update this one to check for it specifically.
|
@tiborvass @fabiokung @cpuguy83 carried in #33908 (Aaron is enjoying a couple of days off) |
5 participants
The name/ID relationships are maintained separately from the memdb and can be out of sync from any particular memdb snapshot. If a container does not exist in the memdb, we must accept this as normal and not fail the listing. This is consistent with what the code used to do before memdb was introduced.
In combination with #33882, this fixes #33863
cc @fabiokung