Skip to content
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

Allow for customizable instance IDs ... and a few fixes #25

Merged
merged 6 commits into from
Sep 10, 2015

Conversation

st3v
Copy link
Contributor

@st3v st3v commented Sep 6, 2015

The main purpose of this PR is to add the ability to use custom instance IDs. This is needed in order to register multiple instances of the same app in a PAAS like Cloud Foundry. In such an environment the host name is the same for multiple instances of an app. Thus, using the host name as the instance ID will cause collisions in the registry. Beside the PAAS-case there are other use cases that require this feature, e.g. docker containers running on the same host.

This functionality has been added to the Java client a while ago, for details see Netflix/eureka#85.

The rest of the commits in this PR were needed to (a) get the Vagrant boxes up and running and (b) have all tests pass.

Let me know if you've got any questions. Thanks!

Convey panics when multiple test suites have the same name. See
smartystreets/goconvey@52529b
InstanceMetadata.MarshalJSON must return proper JSON; i.e. an empty JSON
object should be returned when Raw is not set.

This fixes failing test case TestReregistration in net_test.go
Don't re-use instance between XML and JSON tests as InstanceMetadata.Raw does
not seem to handle this at the moment.
@itsrainy
Copy link
Contributor

itsrainy commented Sep 8, 2015

Thanks for the contribution @st3v. This looks good to me.

@tysonstewart or @llaughlin could I get another set of eyes on this before I merge? It shouldn't affect our usage of fargo since we don't currently set the UniqueId() function anywhere.

@ryansb
Copy link
Contributor

ryansb commented Sep 8, 2015

@jcox92 I'll take a look at this before the end of the week.

@ryansb
Copy link
Contributor

ryansb commented Sep 9, 2015

Looks good to me, 💯

itsrainy pushed a commit that referenced this pull request Sep 10, 2015
Allow for customizable instance IDs ... and a few fixes
@itsrainy itsrainy merged commit ac37d05 into hudl:master Sep 10, 2015
@st3v
Copy link
Contributor Author

st3v commented Sep 10, 2015

Thanks for merging this!

@itsrainy
Copy link
Contributor

Definitely! Thanks for making fargo better!

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

Successfully merging this pull request may close these issues.

3 participants