Fixing error where aws returns DNS name as empty string #124
Conversation
|
It looks like I also broke backwards compatibility when I removed the |
backwards compatability
| @@ -354,6 +361,8 @@ def hostname(server, interface_type = nil) | |||
| potential_hostname = nil | |||
| INTERFACE_TYPES.values.each do |type| | |||
| potential_hostname ||= server.send(type) | |||
| # AWS returns an empty string if the dns name isn't populated yet | |||
There was a problem hiding this comment.
Why not default to private_ip unless specified?
It seems like it would be the most likely to be available, followed by public_ip and then DNS. Looks like there could be reduced complexity unless there is something I am missing.
What is the behavior of not defining the hostname? (returning nil)
There was a problem hiding this comment.
private_ip is the most likely to be available but not the most likely to be reachable by SSH. The logic here is currently
# if `interface_type` is provided:
# query only that interface_type
# else
# first, try dns
# second, try public_ip
# third, try private_ip
# return the first value that is non-nil and non-emptyThis brings the hostname logic in line with what is in the README. I don't want to break the backwards compatibility with logic that has been there for a while.
I also like the 'try from easiest to SSH into to hardest to SSH into' logic rather than 'try from what is most likely to be available to what is least likely to be available'. The goal is to get a hostname we can SSH into and then install Chef. If a user really wants to use private_ip they can specify interface: private in their .kitchen.yml.
There was a problem hiding this comment.
+1 I was thinking of this soley from inside a VPC, good call. Little short-sighted on my part.
|
LGTM, 👍 |
Fixing error where aws returns DNS name as empty string
Fixes #122
Replaces #123
\cc @fnichol @litjoco @dsavinkov @callmeradical @huntertj