libnet: add ep name in 'has active endpoints' error #49773
Conversation
735a527 to
d64a887
Compare
libnetwork/error.go
Outdated
| if len(aee.endpoints) > 1 { | ||
| s = "s" | ||
| } | ||
| return fmt.Sprintf("network %s has %d active endpoint%s (%s)", aee.name, len(aee.endpoints), s, strings.Join(aee.endpoints, ", ")) |
There was a problem hiding this comment.
Looking at https://grep.app/search?q=%22has+active+endpoints%22, I see that there are 8 repos doing string matching:
- moby/moby
- 1Panel-dev/1Panel (active)
- kubernetes/minikube (active)
- AliyunContainerService/pouch (looks inactive)
- Tecnativa/doodba-copier-template (active)
- kreuzwerker/terraform-provider-docker (looks inactive)
- moby/swarmkit (active)
- docker-archive/docker-ce (archived)
So I'm in favor of changing this error message. I can open issues on active repos to signal that their code will break with the next minor release.
There was a problem hiding this comment.
Or, just drop/move the %d, so the has active endpoints part of the string is unchanged? The count isn't really necessary when the list of endpoint names is included anyway?
There was a problem hiding this comment.
Agree with Rob. We shouldn't make breaking changes in non-major release. Especially when the breakage can be easily avoided by rephrasing the error message.
d64a887 to
cd5d103
Compare
There have been numerous reports of the "has active endpoints" error over the years. Historically, there were some faulty code paths that could lead to this error, but we believe they all have been fixed by now. However, users are still facing this error from time to time. Either because they forgot that some containers are still running, or because we still have bugs lying around. To help users figure whether this error is legitimate, and what triggers it, add endpoint names (which are just container names) to the error message. Signed-off-by: Albin Kerouanton <albinker@gmail.com>
cd5d103 to
241d685
Compare
|
Only the error message format changed since Rob's approval, so let me bring it in. |
networknamehas error has active endpoints #42119- What I did
There have been numerous reports of the "has active endpoints" error over the years. Historically, there were some faulty code paths that could lead to this error, but we believe they all have been fixed by now.
However, users are still facing this error from time to time. Either because they forgot that some containers are still running, or because we still have bugs lying around.
To help users figure whether this error is legitimate, and what triggers it, add endpoint names (which are just container names) to the error message.
- How to verify it
- Human readable description for the release notes
- Improve the "has active endpoints" error message by including the name of endpoints still connected to the network being deleted