AWS: make it possible to disable minion public ip assignment #7928
Conversation
|
/cc @justinsb |
|
LGTM. Will merge in the morning unless @justinsb has any comments to add. |
| @@ -98,7 +106,12 @@ function detect-master () { | |||
| function detect-minions () { | |||
| KUBE_MINION_IP_ADDRESSES=() | |||
| for (( i=0; i<${#MINION_NAMES[@]}; i++)); do | |||
| local minion_ip=$(get_instance_public_ip ${MINION_NAMES[$i]}) | |||
| local minion_ip | |||
| if [[ "ENABLE_MINION_PUBLIC_IP" == "true" ]]; then | |||
|
This LGTM, other than the comment about the comparison missing a $. It would be great to add this option to cluster/aws/options.md, but I don't think that should block a merge. I suspect this will actually surface some more problems, because I'm not certain that our networking address choosing logic will tolerate this, but really we won't find those until we merge this, and then we should fix those (hypothetical) problems anyway. (And users must opt-in to the new behaviour). |
|
bad mistake, that missing $ - sorry... |
| @@ -542,6 +555,14 @@ function kube-up { | |||
| grep -v "^#" "${KUBE_ROOT}/cluster/aws/templates/format-disks.sh" | |||
| grep -v "^#" "${KUBE_ROOT}/cluster/aws/templates/salt-minion.sh" | |||
| ) > "${KUBE_TEMP}/minion-start-${i}.sh" | |||
|
|
|||
| local public_ip_option | |||
| if [[ "ENABLE_MINION_PUBLIC_IP" == "true" ]]; then | |||
There was a problem hiding this comment.
Oops... here too.
While you're at it, you probably should do "${ENABLE_MINION_PUBLIC_IP}" (i.e. add curly brackets) - I think it's more in keeping with our bash style.
There was a problem hiding this comment.
I didn't mention the non-quoted/curly braced variables because it kept the same style as the rest of the file, but it would be nice to make them more consistent with our bash style so that these typos are easier to see.
|
Thanks for adding docs. I found another instance of the problem, but once that is fixed LGTM. |
|
oh no... thanks for your attention and patience @justinsb. fixed. |
|
LGTM :-) |
|
Travis is happy; merging. |
AWS: make it possible to disable minion public ip assignment
This patch makes it possible to disable the automatic assignment of a public IP when launching minions. The feature can be controlled by setting the env variable KUBE_ENABLE_MINION_PUBLIC_IP to "false" (instead of "true" - which is the default value).
Use case scenario: