search

att-comdev / openstack-helm

PROJECT HAS MOVED TO OPENSTACK
https://github.com/openstack/openstack-helm
69 stars 41 forks source link

Add NodePort to remainder of services #314

Closed alraddarla closed 7 years ago

alraddarla commented 7 years ago

What is the purpose of this pull request?: Allows a NodePort for each service to be exposed.

What issue does this pull request address?: Fixes #297

Notes for reviewers to consider: After speaking with Brandon, he said we would want to mimic what the Heat files look like. This is why there are changes in the network section of the values file for each service on top of adding the NodePort boolean.

Specific reviewers for pull request: @alanmeadows @v1k0d3n @larryrensing @intlabs @wilkers-steve

wilkers-steve commented 7 years ago

This looks good @alraddarla. I can't see anything else beyond @intlabs's comments. Once those get addressed, this should be good to go.

intlabs commented 7 years ago

LGTM :+1:

alraddarla commented 7 years ago

DO NOT MERGE --- this crashes keystone currently

alanmeadows commented 7 years ago

@alraddarla This is a great enhancement with a lot of good consistency cleanup embedded in this. The project needs this kind of attention across a large number of areas. Nice work. The only issue is the {{ if .. or .. }} cleanup @wilkers-steve pointed out, but otherwise looks good to me.