Skip to content

Commit

Permalink
Fixes #18765 - finish script use FQDN when IP is missing
Browse files Browse the repository at this point in the history
This is resurrect of

https://github.com/theforeman/foreman/pull/2171/files

The old patch did not pass review because I was unable to explain the
motivation. The reason for the fallback is that only cloud compute
resources usually provide IP addresses but virtualization do not. Our
users often associate finish templates with libvirt or VMWare but it
does not work at all. This only applies to externally managed networks
where we don't know the IP (no reservation made by Foreman).

Now there is a question of safety, with incorrect DNS setup, Foreman
could run finish script on incorrect server. Therefore I am sending this
patch for discussion - I think I could make this behavior an opt-it via
setting.
  • Loading branch information
lzap authored and dLobatog committed Jun 26, 2018
1 parent c6a311f commit 8397b3c
Showing 1 changed file with 6 additions and 5 deletions.
11 changes: 6 additions & 5 deletions app/models/concerns/orchestration/ssh_provision.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def delSSHProvisionScript
end

def setSSHWaitForResponse
logger.info "Starting SSH provisioning script - waiting for #{provision_ip} to respond"
logger.info "Starting SSH provisioning script - waiting for #{provision_host} to respond"
if compute_resource.respond_to?(:key_pair) && compute_resource.key_pair.try(:secret)
credentials = { :key_data => [compute_resource.key_pair.secret] }
elsif vm.respond_to?(:password) && vm.password.present?
Expand All @@ -54,7 +54,7 @@ def setSSHWaitForResponse
else
raise ::Foreman::Exception.new(N_('Unable to find proper authentication method'))
end
self.client = Foreman::Provision::SSH.new provision_ip, image.username, { :template => template_file.path, :uuid => uuid }.merge(credentials)
self.client = Foreman::Provision::SSH.new provision_host, image.username, { :template => template_file.path, :uuid => uuid }.merge(credentials)
rescue => e
failure _("Failed to login via SSH to %{name}: %{e}") % { :name => name, :e => e }, e
end
Expand All @@ -81,7 +81,7 @@ def delSSHCert
end

def setSSHProvision
logger.info "SSH connection established to #{provision_ip} - executing template"
logger.info "SSH connection established to #{provision_host} - executing template"
if client.deploy!
# since we are in a after_commit callback, we need to fetch our host again, and clean up puppet ca on our own
Host.find(id).built
Expand Down Expand Up @@ -118,7 +118,8 @@ def validate_ssh_provisioning
failure(_("No finish templates were found for this host, make sure you define at least one in your %s settings") % os) unless status
end

def provision_ip
provision_interface.ip
def provision_host
# usually cloud compute resources provide IPs but virtualization do not
provision_interface.ip || provision_interface.fqdn
end
end

0 comments on commit 8397b3c

Please sign in to comment.