-
Notifications
You must be signed in to change notification settings - Fork 914
Added support for storing service attributes in the Consul KV store. #261
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
base: master
Are you sure you want to change the base?
Conversation
Use by adding a 'servicePrefix=somepath' URL parameter to the consul:// URL
consul/consul.go
Outdated
return r.client.Agent().ServiceDeregister(service.ID) | ||
// pair := &consulapi.KVPair{Key: "service_attribute" + "/" + service.Name + "/" + k, Value: []byte(v)} | ||
success := r.client.Agent().ServiceDeregister(service.ID) | ||
r.client.KV().DeleteTree("service_attribute"+"/"+service.ID, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"service_attribute"
should be r.servicePrefix
It looks exactly like a solution I need at the moment. +1 for merging! |
Bear in mind I am a complete Go n00b, so handle with care ;-) |
I believe this was talked about in another issue. Into this general feature. Anybody want to give feedback on implementation? |
return r.client.Agent().ServiceDeregister(service.ID) | ||
// pair := &consulapi.KVPair{Key: "service_attribute" + "/" + service.Name + "/" + k, Value: []byte(v)} | ||
success := r.client.Agent().ServiceDeregister(service.ID) | ||
r.client.KV().DeleteTree("service_attribute"+"/attributes/"+service.ID, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, shouldn't "service_attribute"+"/attributes/"
be r.servicePrefix
?
The test is passing, however that's because you ain't working with the return value for DeleteTree
. The failure should be at least logged.
👍 |
+1 |
I'd really like to see this implemented. At the moment I'm storing attributes in a special consul tag, but this forces me to awkwardly search for this tag and split it into it's K/V combos every time I want to access one value. This change would really ease my life. 👍 |
Can we get some docs around how this would be used as part of this PR? Eg, in https://github.com/gliderlabs/registrator/tree/master/docs/user |
This pull request can be accepted? Why it is in standby? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some comments that bring up good issues and we need docs for this.
The general Service registration store in Consul does not support service attributes, but it always has a KV-store available.
Use by adding a 'servicePrefix=somepath' URL parameter to the consul:// URL