-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide clearer warning when Name is used when ID is needed instead #10861
Comments
@dnephin : I'm not actually sure this is accurate
The docs for deregister do say that the node argument must be "the ID of the node", but I think the code suggests that it's the node name that is used, not node ID. Node name is supposed to be unique within the cluster, so that seems reasonable? Q: What's your read on whether node name or node ID is used for Below is my attempt to trace through the code, though I feel like I went wrong somewhere... (because I ended up in a services table, not a node table)
|
Sorry, just catching up on this now. I think the second line you link ( The first place I see the consul/agent/consul/catalog_endpoint.go Lines 403 to 405 in 01e9740
Our acl docs for node rule say it is expecting a node name, and I see other callers passing a node name, so definitely the ACL authorization appears to expect node name (not ID). After that it goes into consul/agent/consul/fsm/commands_oss.go Line 172 in 01e9740
And into the state store here: consul/agent/consul/state/catalog.go Lines 546 to 548 in 01e9740
This query for So it does seem our docs are wrong, this API uses node name, not node ID. |
Regarding:
The current error message is "Unknown service ID: <service_id>". This makes it clear that an ID is needed, not name. |
Feature Description
Some entities within Consul have an ID and a Name (e.g., services). When the CLI or an API endpoint for such an entity specifically requires an ID, and will not work with a Name, Consul should provide an error message that makes this mistake clear to the user.
Example taken from #3122: let's say a user has a service with ID
web-service-id
and Nameweb-service-name
. If the user executes/v1/agent/service/deregister/web-service-name
, the following is output to the log:That error message is very misleading, as a service with that name does exist... just not a service with that id. The user might reasonably conclude something is wrong with the Consul catalog / state, when the actual problem is that the wrong field was used in the API call. Perhaps if Consul fails to lookup a service with that id, it could lookup whether any services exist with that name and, if some do, output a clearer warning message... such as:
This description will be updated to include relevant cases in the list below.
API:
/agent/service/:service_id
: requires service ID, not name/agent/service/deregister/:service_id
: requires service ID, not name (potentially related issues: Unable to delete unwanted services #9861, service did not deregistered properly #3122, Unable to deregister service if multiple nodes available #9415). See PR Clarify service and health check not found error messages #10894./catalog/deregister
: requires node ID, not name (user has tried to deregister using the node name in this issue and in the past Zombie agent trapped in catalog #10848)... EDIT: the docs are wrong, node name is required, not ID. Need to update the docs.Use Case(s)
Ease troubleshooting of any CLI or API endpoints that accept an ID but not a name.
The text was updated successfully, but these errors were encountered: