libnet: add ep name in 'has active endpoints' error#49773
Merged
akerouanton merged 1 commit intomoby:masterfrom Apr 9, 2025
Merged
libnet: add ep name in 'has active endpoints' error#49773akerouanton merged 1 commit intomoby:masterfrom
akerouanton merged 1 commit intomoby:masterfrom
Conversation
akerouanton
added
status/2-code-review
kind/enhancement
robmry
approved these changes
Apr 8, 2025
akerouanton
force-pushed
the
improve-has-active-endpoints-error
branch
2 times, most recently
from
735a527 to
d64a887
Compare
akerouanton
commented
Apr 8, 2025
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, ", ")) |
Member
Author
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.
Contributor
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?
Contributor
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.
akerouanton
force-pushed
the
improve-has-active-endpoints-error
branch
from
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>
akerouanton
force-pushed
the
improve-has-active-endpoints-error
branch
from
cd5d103 to
241d685
Compare
vvoland
approved these changes
Apr 9, 2025
Member
Author
|
Only the error message format changed since Rob's approval, so let me bring it in. |
This was referenced Apr 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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