Identify services using both the ID and the Node

[alvaroaleman]

This aligns Fabio with the Consul docs[0] which state that
a serviceID has to be unique per agent but not cluster wide.

Fixes #383

[0] https://www.consul.io/api/agent/service.html#id

[magiconair]

That looks simple enough. Can you update registry/consul/passing_test.go? It is time for me to also write a test for service.go

[alvaroaleman]

Updated passing_test.go.

Thanks btw for your awesome work and really fast reaction on issues/PRs!

[ygersie]

I think in general c.Node == svc.Node should be the first thing to evaluate, right? The Node will always be the most specific condition in that loop, while for example serfHealth will be hit for every consul agent.

[ygersie]

If you want to align with what Consul servers internally do:
fmt.Sprintf("%s.%s", check.Node, check.ServiceID)

[magiconair]

I'll fix that in a subsequent commit.

[alvaroaleman]

@dropje86 Does that actually result in Go not evaluating the other conditionals?

@magiconair Is there still something missing from your POV?

[ygersie]

@alvaroaleman Although not clearly documented, Go does use "Short circuit evaluation". See here: https://kuree.gitbooks.io/the-go-programming-language-report/content/22/text.html

[alvaroaleman]

Interesting, thanks for the link @dropje86!

I've added a second commit that simplifies the conditional logic in passing.go and avoids unnecessary evaluations, depending on what @magiconair thinks I can either squash it or put it in a separate PR.

[magiconair]

Sorry, got a bit sick. I'll do this probably tomorrow and then try to run a release right away. Time to use goreleaser.

[magiconair]

@alvaroaleman Thanks for the patch. Merged it and made some small changes in subsequent commits.