Updated Readme for --server-connect-attribute option

[NimishaS]

w.r.t #379

[adamedx]

@btm, can you take a look?

[btm]

We're losing the benefit of @ssh_connect_host being an instance variable as written, because we're still stepping through the entire conditional every time ssh_connect_host is called.

If it's not convenient to assign the variable from the output of the conditional like it previously did, you could wrap that in another conditional. I think this looks simpler:

unless @ssh_connect_host
  if config[:server_connect_attribute]
    connect_attribute = config[:server_connect_attribute]
  else
    if vpc_mode? && !(config[:associate_public_ip] || config[:associate_eip])
      connect_attribute = "private_ip_address"
    else
      connect_attribute = server.dns_name ? "dns_name" : "public_ip_address"
    end
  end

  @ssh_connect_host = server.send(connect_attribute)
end

puts "\nSSH Target Address: #{@ssh_connect_host}(#{connect_attribute})"
@ssh_connect_host

[btm]

as in my example in my first comment, I'd combine these two lines, and instead put the thing we're connecting to first with the attribute we used to get that in the parentheses.

[NimishaS]

@btm, fixed the review comments. Please check.

[btm]

EIP means Elastic IP (which is a public IP). Not all instances have an Elastic IP assigned to them. So this isn't true.

I would say "If --server-connect-attribute is not specified, knife attempts to determine if connecting to the instance's public or private IP is most appropriate based on other settings."

[NimishaS]

@btm , please verify again

[btm]

👍