Skip to content

Upload etcd keys by template. #76

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

bheatwole
Copy link

Currently the etcd backend only creates a very specific style of key in etcd. If you need etcd to contain more information (such as tags and attributes) or for the format to be different for different types of services, you're stuck.

This pull requests adds an additional etcd backend that creates keys based on templates with access to all the information from docker. For example the template below uses the exposed port from the container as part of the key, but uses the published host IP and port in the value.

/services/{{.Published.ExposedPort}}/{{.Name}} {{.Published.HostIP}}:{{.Published.HostPort}}

Since templates allow if...else, you can write add a template that only adds a key if a specific attribute is present.

{{if .Attrs.proxy}}/proxies/{{.Name}} {{.Published.HostIP}}:{{.Published.HostPort}}{{end}}

Usage:
Start registrator with one or more ETCD_TMPL_XXXX environment variables. These variables should contain a template of the key value pair you wish published to etcd. Also be sure to use the 'etcd-tmpl' scheme.

docker run -d -v /var/run/docker.sock:/tmp/docker.sock \
   -e ETCD_TMPL_PROXY="{{if .Attrs.proxy}}/load_balancer/{{.Published.ExposedPort}}/{{.Name}} {{.Published.HostIP}}:{{.Published.HostPort}}{{end}}" \
   -e ETCD_TMPL_HTTP="{{if .Published.HostPort eq 80}}/http/{{.Name}} {{.Published.HostIP}}:{{.Published.HostPort}}{{end}}" \
   -e ETCD_TMPL_ALL="/services/{{.Name}} {{.Published.HostIP}}:{{.Published.HostPort}}" \
   progrium/registrator etcd-tmpl://${PRIVATE_IPV4}:4001/

@bryanlarsen
Copy link

I did something similar for consul as a docker plugin: https://github.com/bryanlarsen/docker-plugin-kv-consul

Discussion: progrium/docker-plugins#3

@progrium
Copy link
Contributor

Ah right, that was you. This is a big addition, so it might take even
longer to get reviewed. Sorry I'm so backed up!

On Fri, Dec 19, 2014 at 6:46 AM, Bryan Larsen [email protected]
wrote:

I did something similar for consul as a docker plugin:
https://github.com/bryanlarsen/docker-plugin-kv-consul

Discussion: progrium/docker-plugins#3
progrium/docker-plugins#3


Reply to this email directly or view it on GitHub
https://github.com/progrium/registrator/pull/76#issuecomment-67633949.

Jeff Lindsay
http://progrium.com

@tsaikd
Copy link

tsaikd commented Dec 23, 2014

This PR is a good idea. It solve my problem for customizing the etcd K/V rule.
👍

@subicura
Copy link

👍

@llinder
Copy link

llinder commented Feb 24, 2015

👍

1 similar comment
@benguillet
Copy link

👍

@vidarl
Copy link

vidarl commented May 4, 2015

👍

@vidarl
Copy link

vidarl commented May 21, 2015

In my option, registrator should definitely have the possibility for storing stuff in a flexible way in etcd like this PR proposes. In addition, this PR makes it possible to store Attrs in etcd too, something the default etcd backend currently does not.

So, why has this PR not been merged yet?
Are there objections to the approach? or is it because of the current merge conflicts? or just lack of time?

Is there anything I can do in order to get this merged?

@progrium
Copy link
Contributor

Help write tests for existing code? That's one of our blockers for more
active development and frequent releases. Docs are our first priority so
help with tests would be great.

On Thu, May 21, 2015 at 3:49 AM, Vidar [email protected] wrote:

In my option, registrator should defiantly have the possibility for
storing stuff in a flexible way in etcd like this PR proposes. In addition,
this PR makes it possible to store Attrs in etcd too, something the default
etcd backend currently does not.

So, why has this PR not been merged yet?
Are there objections to the approach? or is it because of the current
merge conflicts? or just lack of time?

Is there anything I can do in order to get this merged?


Reply to this email directly or view it on GitHub
#76 (comment)
.

Jeff Lindsay
http://progrium.com

@hyperbolic2346
Copy link

@progrium I've been thinking about your comment above regarding tests and I'm a little torn by it. It seems that you are holding meaningful features hostage here. I totally understand the desire to have tests and to automate, I don't have any beef there. My concern is that without these features you will push users away and that could ultimately result in a dead branch. Like anyone else looking at these issues and investigating registrator, I don't want that to happen. It is really close to exactly what I need. The only reason I am currently still searching for a solution for service announcement and discovery is this missing feature.

@bheatwole
Copy link
Author

I checked to see how far off the current code base my pull request is, and there are some significant issues to work through. However, I will do the work to bring the pull request up to master if there's some hope it will be quickly reviewed.

@progrium progrium self-assigned this Sep 1, 2015
@progrium progrium added this to the v7 milestone Sep 1, 2015
@aramalipoor
Copy link

👍

1 similar comment
@majidgolshadi
Copy link

👍

break
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small improvement which makes it a lot easier to write the templates ...

-       } 
+       } else {
+           log.Println("Error processing template :", err)
+       }

_, err = r.client2.Delete(path, false)
toSet, err := r.executeTemplates(service)
if err == nil {
for key, value := range toSet {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix for compile error "'value declared and not used'"

-for key, value := range toSet {
+for key := range toSet {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.