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

Add MesosDnsId field to Marathon App struct #205

Merged
merged 5 commits into from
Jun 7, 2016

Conversation

porcupie
Copy link
Contributor

@porcupie porcupie commented May 6, 2016

Hello,

I added an additional string field MesosDnsId to marathon App struct. Marathon AppId Groups are reversed and appended with hyphens to create a DNS friendly name, similar to how Mesos DNS, Consul and some other tools operate.

Not sure if this is useful or the most efficient go, but I could not figure out how to do this simple string munging within the haproxy template file itself, so I resorted to adding a field to the Marathon App struct. Perhaps there is a better way but I wanted to draft this Pull Request in case others had a similar need.

Example: Marathon App ID: "/sample/group/appname" becomes "appname-group-sample" in mesos dns so I can refer to this field as $app.MesosDnsId inside the haproxy template to write ACL rules like hdr_dom(host) -i {{ $app.MesosDnsId }}.

I will make any other changes needed.

Idea came from a coworker Michael Kuriger.

Damian Martinez added 2 commits May 4, 2016 16:02
…rathon group names are reversed and appended to appname. '/a/b/app' becomes app-b-a as a service name
@swagatha-christie
Copy link

By analyzing the blame information on this pull request, we identified @bluepeppers, @activars and @rasputnik to be potential reviewers

@coveralls
Copy link

Coverage Status

Coverage increased (+2.6%) to 38.448% when pulling cef6f49 on porcupie:master into 01da91b on QubitProducts:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+4.3%) to 40.18% when pulling d825591 on porcupie:master into 01da91b on QubitProducts:master.

@lclarkmichalek
Copy link
Contributor

Looks good - thanks for adding tests. I'm thinking it might be worth documenting this somewhere? Maybe in the readme?

@j1n6
Copy link
Contributor

j1n6 commented May 6, 2016

👍

@porcupie
Copy link
Contributor Author

porcupie commented May 6, 2016

Thanks for taking a look @activars, @bluepeppers . Added some notes to README but was not sure exactly where to put it. Let me know if you'd like more/less information or edits.

@coveralls
Copy link

Coverage Status

Coverage increased (+4.3%) to 40.18% when pulling 8b0f970 on porcupie:master into 01da91b on QubitProducts:master.

@j1n6 j1n6 merged commit c5ee788 into QubitProducts:master Jun 7, 2016
@j1n6
Copy link
Contributor

j1n6 commented Jun 7, 2016

@porcupie thank you.

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.

None yet

5 participants