openshift / puppet-openshift_origin

Puppet module to create OpenShift Brokers and Nodes. Can be used to create a full OpenShift Origin deployment.
http://forge.puppetlabs.com/openshift/openshift_origin
Other
45 stars 128 forks source link

node_ip_addr parameter really should be host_public_ip_addr #307

Open detiber opened 9 years ago

detiber commented 9 years ago

the node_ip_addr parameter is used with register_dns (https://github.com/openshift/puppet-openshift_origin/blob/283861443df79f929331b7d3fbd8e809af0bda69/templates/register_dns.erb#L4) to set the host dns entry.

Originally the variable was only used for setting the PUBLIC_IP in node.conf, hence the name, but it has a more generic use now.

This also highlights an issue with the usage of the variable in oo-install (https://github.com/openshift/openshift-extras/blob/4823853f7df32af6e1a2689826276e00e84e4d0a/oo-install/workflows/origin_deploy/originator.rb#L378) instead, originator.rb should be setting it to the value of host_instance.ip_addr for each host.

sdodson commented 9 years ago

@detiber What's our stance on backwards compatibility? Can we change this globally and expect people to just change when they notice catalog failures?

If not I think we'd have to do something like the following which I think is a bit of a hack class openshift_origin ( ... node_ip_addr = $::ipaddr, host_public_ip_addr = $node_ip_addr ... )

detiber commented 9 years ago

hmm, good point, I think we should possibly maintain backwards compatibility at least for a little bit... maybe add a deprecation warning to the puppet output to let users know (in addition to the comments/README)