Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversion Host - Try hostname for SSH and fix MiqSshUtil args #18103

Merged

Conversation

ghost
Copy link

@ghost ghost commented Oct 17, 2018

The IP addresses of a virtual machine or a host are not ordered accordingly to their interface name or role. So, when trying to connect to a conversion host, the IP address might not be correct. Returning the hostname when it's available mimics the behavior of Host.connect_ssh method.

This PR also fixes the parameters for MiqSshUtil.shell_with_su by passing the array correctly.

Associated RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1634029

@ghost
Copy link
Author

ghost commented Oct 17, 2018

@miq-bot add-label transformation, enhancement, hammer/yes
@miq-bot add-reviewer agrare

@@ -34,6 +34,7 @@ def source_transport_method
end

def ipaddress(family = 'ipv4')
return resource.hostname if resource.hostname
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm I wouldn't expect an ipaddress method to return a dns hostname, maybe another method that tries hostname then ipaddress?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a delegation for hostname and updated miq_ssh_util_args to use it if present.

@@ -121,7 +122,7 @@ def check_resource_credentials_openstack

def connect_ssh
require 'MiqSshUtil'
MiqSshUtil.shell_with_su(miq_ssh_util_args) do |ssu, _shell|
MiqSshUtil.shell_with_su(*miq_ssh_util_args) do |ssu, _shell|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops ;)

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 this looks nicer

@miq-bot
Copy link
Member

miq-bot commented Oct 17, 2018

Checked commits fabiendupont/manageiq@c1c93cd~...1de3f2d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@ghost ghost changed the title Conversion Host - Return hostname as IP address Conversion Host - Try hostname for SSH and fix MiqSshUtil args Oct 17, 2018
@agrare agrare merged commit 84a80a0 into ManageIQ:master Oct 17, 2018
@ghost ghost deleted the conversion_host_hostname_as_ipaddress branch October 17, 2018 21:45
simaishi pushed a commit that referenced this pull request Oct 18, 2018
…e_as_ipaddress

Conversion Host - Try hostname for SSH and fix MiqSshUtil args

(cherry picked from commit 84a80a0)

https://bugzilla.redhat.com/show_bug.cgi?id=1634029
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 84a10456ba1270d7a10b91dc607738bd438ac09a
Author: Adam Grare <agrare@redhat.com>
Date:   Wed Oct 17 17:12:03 2018 -0400

    Merge pull request #18103 from fdupont-redhat/conversion_host_hostname_as_ipaddress
    
    Conversion Host - Try hostname for SSH and fix MiqSshUtil args
    
    (cherry picked from commit 84a80a0401d7cfce7e9674ee8ba78da5e3482661)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1634029

@agrare agrare added this to the Sprint 97 Ending Oct 22, 2018 milestone Feb 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants