cloudfoundry-attic / consul-release

This is a BOSH release for consul.
Apache License 2.0
10 stars 30 forks source link

AZ tags #56

Closed jasonkeene closed 7 years ago

jasonkeene commented 7 years ago

This dynamically adds tags for the availability zones when bosh renders out the confab template for the job.

cfdreddbot commented 7 years ago

Hey jasonkeene!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

cf-gitbot commented 7 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/135972993

The labels on this github issue will be updated when the story is started.

christianang commented 7 years ago

Hi @jasonkeene,

Thanks for the PR. However to merge this code I would say we need the logic to be inside of confab. confab is essentially our process to bootstrap consul. confab deals with creating the the consul config files, including service definitions, and starting consul.

So to not duplicate logic and keep it tested, the best place for this logic would be in our service definer. More specifically here: https://github.com/cloudfoundry-incubator/consul-release/blob/master/src/confab/config/service_definer.go#L83, which is where the default tags are set.

I would just pass in the spec.az into the confab.json.erb and generate the service in confab.service-definer.

-Christian

jasonkeene commented 7 years ago

Sounds good. We will look into it and re-submit.

jtarchie commented 7 years ago

Please close this PR in favor of #57.