From 0ade15960d6b0f448194f0ff198c877393740dd9 Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Tue, 29 Aug 2017 09:19:42 -0400 Subject: [PATCH 1/8] Fix missing nil check https://bugzilla.redhat.com/show_bug.cgi?id=1450109 --- app/models/container_node.rb | 11 +++--- app/models/vm/operations.rb | 10 ++++-- spec/models/vm/operations_spec.rb | 59 +++++++++++++++++++++++++------ 3 files changed, 61 insertions(+), 19 deletions(-) diff --git a/app/models/container_node.rb b/app/models/container_node.rb index 7985e422c9a..9e934fd39c9 100644 --- a/app/models/container_node.rb +++ b/app/models/container_node.rb @@ -81,12 +81,13 @@ def kubernetes_hostname def cockpit_url URI::HTTP.build(:host => kubernetes_hostname, :port => 9090) address = kubernetes_hostname || name - miq_server = ext_management_system.nil? ? nil : ext_management_system.zone.remote_cockpit_ws_miq_server - if miq_server - - end + miq_server = if ext_management_system.nil? || ext_management_system.zone.remote_cockpit_ws_miq_server.nil? + nil + else + ext_management_system.zone.remote_cockpit_ws_miq_server + end MiqCockpit::WS.url(miq_server, - MiqCockpitWsWorker.fetch_worker_settings_from_server(miq_server), + miq_server.nil? ? nil : MiqCockpitWsWorker.fetch_worker_settings_from_server(miq_server), address) end diff --git a/app/models/vm/operations.rb b/app/models/vm/operations.rb index f53c406d525..b82561255b9 100644 --- a/app/models/vm/operations.rb +++ b/app/models/vm/operations.rb @@ -15,10 +15,14 @@ module Vm::Operations def cockpit_url return if ipaddresses.blank? - miq_server = ext_management_system.nil? ? nil : ext_management_system.zone.remote_cockpit_ws_miq_server + miq_server = if ext_management_system.nil? || ext_management_system.zone.remote_cockpit_ws_miq_server.nil? + nil + else + ext_management_system.zone.remote_cockpit_ws_miq_server + end MiqCockpit::WS.url(miq_server, - MiqCockpitWsWorker.fetch_worker_settings_from_server(miq_server), - ipv4_address || ipaddresses.first) + miq_server.nil? ? nil : MiqCockpitWsWorker.fetch_worker_settings_from_server(miq_server), + ipv4_address || ipaddresses.first).to_s end def ipv4_address diff --git a/spec/models/vm/operations_spec.rb b/spec/models/vm/operations_spec.rb index e7ffd6f3f2d..f208c358c41 100644 --- a/spec/models/vm/operations_spec.rb +++ b/spec/models/vm/operations_spec.rb @@ -1,25 +1,62 @@ describe 'VM::Operations' do before(:each) do - miq_server = EvmSpecHelper.local_miq_server - @ems = FactoryGirl.create(:ems_vmware, :zone => miq_server.zone) - @vm = FactoryGirl.create(:vm_vmware, :ems_id => @ems.id) - allow(@vm).to receive(:ipaddresses).and_return(@ipaddresses) + @miq_server = EvmSpecHelper.local_miq_server + @ems = FactoryGirl.create(:ems_vmware, :zone => @miq_server.zone) + @vm = FactoryGirl.create(:vm_vmware, :ems_id => @ems.id) + ipaddresses = %w(fe80::21a:4aff:fe22:dde5 127.0.0.1) + allow(@vm).to receive(:ipaddresses).and_return(ipaddresses) + + @hardware = FactoryGirl.create(:hardware) + @hardware.ipaddresses << '10.142.0.2' + @hardware.ipaddresses << '35.190.140.48' end context '#cockpit_url' do it '#returns a valid Cockpit url' do - @ipaddresses = %w(fe80::21a:4aff:fe22:dde5 127.0.0.1) - expect(@vm).to receive(:cockpit_url).and_return('http://127.0.0.1:9090') - @vm.send(:cockpit_url) + url = @vm.send(:cockpit_url) + expect(url).to eq('http://127.0.0.1:9090') end end context '#ipv4_address' do - after(:each) { @vm.send(:return_ipv4_address) } - it 'returns the existing ipv4 address' do - @ipaddresses = %w(fe80::21a:4aff:fe22:dde5 127.0.0.1) - expect(@vm).to receive(:return_ipv4_address).and_return('127.0.0.1') + url = @vm.send(:ipv4_address) + expect(url).to eq('127.0.0.1') + end + + context 'cloud providers' do + before(:each) { @ipaddresses = %w(10.10.1.121 35.190.140.48) } + it 'returns the public ipv4 address for AWS' do + ems = FactoryGirl.create(:ems_google, :project => 'manageiq-dev') + az = FactoryGirl.create(:availability_zone_google) + vm = FactoryGirl.create(:vm_google, + :ext_management_system => ems, + :ems_ref => 123, + :availability_zone => az, + :hardware => @hardware) + allow(vm).to receive(:ipaddresses).and_return(@ipaddresses) + url = vm.send(:ipv4_address) + expect(url).to eq('35.190.140.48') + end + + it 'returns the public ipv4 address for GCE' do + ems = FactoryGirl.create(:ems_amazon) + vm = FactoryGirl.create(:vm_amazon, :ext_management_system => ems, :hardware => @hardware) + allow(vm).to receive(:ipaddresses).and_return(@ipaddresses) + url = vm.send(:ipv4_address) + expect(url).to eq('35.190.140.48') + end + end + end + + context '#public_address' do + it 'returns a public ipv4 address' do + ipaddresses = %w(10.10.1.121 35.190.140.48) + ems = FactoryGirl.create(:ems_amazon) + vm = FactoryGirl.create(:vm_amazon, :ext_management_system => ems, :hardware => @hardware) + allow(vm).to receive(:ipaddresses).and_return(ipaddresses) + url = vm.send(:public_address) + expect(url).to eq('35.190.140.48') end end end From 41b564d98adf5ad2317dec129a1910b0f6634d82 Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Tue, 29 Aug 2017 09:22:52 -0400 Subject: [PATCH 2/8] Make AWS and GCE connect on public ips for Web Console https://bugzilla.redhat.com/show_bug.cgi?id=1450109 --- app/models/vm/operations.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/models/vm/operations.rb b/app/models/vm/operations.rb index b82561255b9..f35853772c7 100644 --- a/app/models/vm/operations.rb +++ b/app/models/vm/operations.rb @@ -26,7 +26,15 @@ def cockpit_url end def ipv4_address - ipaddresses.find { |ip| IPAddr.new(ip).ipv4? } + if %w(amazon google).include?(vendor.downcase) + public_address + else + ipaddresses.find { |ip| IPAddr.new(ip).ipv4? } + end + end + + def public_address + ipaddresses.find { |ip| !Addrinfo.tcp(ip, 80).ipv4_private? } end def validate_collect_running_processes From 1e1e0b3d0adca5787a38385e2e2facc4fd600777 Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Tue, 29 Aug 2017 09:25:41 -0400 Subject: [PATCH 3/8] Enable Web Console on RHOS provider https://bugzilla.redhat.com/show_bug.cgi?id=1451655 --- app/models/vm/operations.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/vm/operations.rb b/app/models/vm/operations.rb index f35853772c7..1bd963e820a 100644 --- a/app/models/vm/operations.rb +++ b/app/models/vm/operations.rb @@ -28,6 +28,8 @@ def cockpit_url def ipv4_address if %w(amazon google).include?(vendor.downcase) public_address + elsif %w(openstack).include?(vendor.downcase) + public_address.nil? ? floating_ip_addresses.first : public_address else ipaddresses.find { |ip| IPAddr.new(ip).ipv4? } end From ae845c60ef371312ce1d1172995a1bc8ebf4f8d0 Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Fri, 1 Sep 2017 13:13:42 -0400 Subject: [PATCH 4/8] Conflict resolution https://bugzilla.redhat.com/show_bug.cgi?id=1450109 https://bugzilla.redhat.com/show_bug.cgi?id=1451655 --- app/models/container_node.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/app/models/container_node.rb b/app/models/container_node.rb index 9e934fd39c9..1a01a917b9b 100644 --- a/app/models/container_node.rb +++ b/app/models/container_node.rb @@ -6,6 +6,7 @@ class ContainerNode < ApplicationRecord include TenantIdentityMixin include SupportsFeatureMixin include ArchivedMixin + include CockpitMixin include_concern 'Purging' EXTERNAL_LOGGING_PATH = "/#/discover?_g=()&_a=(columns:!(hostname,level,kubernetes.pod_name,message),filters:!((meta:(disabled:!f,index:'%{index}',key:hostname,negate:!f),%{query})),index:'%{index}',interval:auto,query:(query_string:(analyze_wildcard:!t,query:'*')),sort:!(time,desc))".freeze @@ -81,14 +82,7 @@ def kubernetes_hostname def cockpit_url URI::HTTP.build(:host => kubernetes_hostname, :port => 9090) address = kubernetes_hostname || name - miq_server = if ext_management_system.nil? || ext_management_system.zone.remote_cockpit_ws_miq_server.nil? - nil - else - ext_management_system.zone.remote_cockpit_ws_miq_server - end - MiqCockpit::WS.url(miq_server, - miq_server.nil? ? nil : MiqCockpitWsWorker.fetch_worker_settings_from_server(miq_server), - address) + MiqCockpit::WS.url(cockpit_server, cockpit_worker, address) end def evaluate_alert(_alert_id, _event) From dc1731fc020d0947f47e30db1bac741071a489ca Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Wed, 30 Aug 2017 13:52:46 -0400 Subject: [PATCH 5/8] Refactoring https://bugzilla.redhat.com/show_bug.cgi?id=1450109 https://bugzilla.redhat.com/show_bug.cgi?id=1451655 --- app/models/mixins/cockpit_mixin.rb | 10 ++++++++++ app/models/vm/operations.rb | 20 +++++--------------- 2 files changed, 15 insertions(+), 15 deletions(-) create mode 100644 app/models/mixins/cockpit_mixin.rb diff --git a/app/models/mixins/cockpit_mixin.rb b/app/models/mixins/cockpit_mixin.rb new file mode 100644 index 00000000000..327c68fec1c --- /dev/null +++ b/app/models/mixins/cockpit_mixin.rb @@ -0,0 +1,10 @@ +module CockpitMixin + extend ActiveSupport::Concern + def cockpit_server + ext_management_system.try(:zone).try(:remote_cockpit_ws_miq_server) + end + + def cockpit_worker + cockpit_server.nil? ? nil : MiqCockpitWsWorker.fetch_worker_settings_from_server(miq_server) + end +end diff --git a/app/models/vm/operations.rb b/app/models/vm/operations.rb index 1bd963e820a..3d70b7f6432 100644 --- a/app/models/vm/operations.rb +++ b/app/models/vm/operations.rb @@ -1,6 +1,8 @@ module Vm::Operations extend ActiveSupport::Concern + include CockpitMixin + include_concern 'Guest' include_concern 'Power' include_concern 'Lifecycle' @@ -15,24 +17,12 @@ module Vm::Operations def cockpit_url return if ipaddresses.blank? - miq_server = if ext_management_system.nil? || ext_management_system.zone.remote_cockpit_ws_miq_server.nil? - nil - else - ext_management_system.zone.remote_cockpit_ws_miq_server - end - MiqCockpit::WS.url(miq_server, - miq_server.nil? ? nil : MiqCockpitWsWorker.fetch_worker_settings_from_server(miq_server), - ipv4_address || ipaddresses.first).to_s + MiqCockpit::WS.url(cockpit_server, cockpit_worker, ipv4_address || ipaddresses.first) end def ipv4_address - if %w(amazon google).include?(vendor.downcase) - public_address - elsif %w(openstack).include?(vendor.downcase) - public_address.nil? ? floating_ip_addresses.first : public_address - else - ipaddresses.find { |ip| IPAddr.new(ip).ipv4? } - end + return public_address unless public_address.nil? + ipaddresses.find { |ip| IPAddr.new(ip).ipv4? unless ip.starts_with?('192') } end def public_address From 1ca5c494b9c79cd5a7ebb209b862cd752a867f85 Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Wed, 30 Aug 2017 14:42:29 -0400 Subject: [PATCH 6/8] Fix rubocop error https://bugzilla.redhat.com/show_bug.cgi?id=1450109 https://bugzilla.redhat.com/show_bug.cgi?id=1451655 --- app/models/mixins/cockpit_mixin.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/mixins/cockpit_mixin.rb b/app/models/mixins/cockpit_mixin.rb index 327c68fec1c..a91cb1a4070 100644 --- a/app/models/mixins/cockpit_mixin.rb +++ b/app/models/mixins/cockpit_mixin.rb @@ -5,6 +5,6 @@ def cockpit_server end def cockpit_worker - cockpit_server.nil? ? nil : MiqCockpitWsWorker.fetch_worker_settings_from_server(miq_server) + cockpit_server.nil? ? nil : MiqCockpitWsWorker.fetch_worker_settings_from_server(cockpit_server) end end From 106f7c597bac160e043fb5a4ccdc39e7b7c08bce Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Thu, 31 Aug 2017 09:35:18 -0400 Subject: [PATCH 7/8] Fix travis failures https://bugzilla.redhat.com/show_bug.cgi?id=1450109 https://bugzilla.redhat.com/show_bug.cgi?id=1451655 --- app/models/vm/operations.rb | 2 +- spec/models/vm/operations_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/vm/operations.rb b/app/models/vm/operations.rb index 3d70b7f6432..6dd342ebecd 100644 --- a/app/models/vm/operations.rb +++ b/app/models/vm/operations.rb @@ -26,7 +26,7 @@ def ipv4_address end def public_address - ipaddresses.find { |ip| !Addrinfo.tcp(ip, 80).ipv4_private? } + ipaddresses.find { |ip| !Addrinfo.tcp(ip, 80).ipv4_private? && IPAddr.new(ip).ipv4? } end def validate_collect_running_processes diff --git a/spec/models/vm/operations_spec.rb b/spec/models/vm/operations_spec.rb index f208c358c41..68c81182107 100644 --- a/spec/models/vm/operations_spec.rb +++ b/spec/models/vm/operations_spec.rb @@ -14,7 +14,7 @@ context '#cockpit_url' do it '#returns a valid Cockpit url' do url = @vm.send(:cockpit_url) - expect(url).to eq('http://127.0.0.1:9090') + expect(url).to eq(URI::HTTP.build(:host => "127.0.0.1", :port => 9090)) end end From 47bcea1e6bcba474142c48313f349ed0ee69f423 Mon Sep 17 00:00:00 2001 From: Brian McLaughlin Date: Thu, 31 Aug 2017 14:25:27 -0400 Subject: [PATCH 8/8] Refactoring https://bugzilla.redhat.com/show_bug.cgi?id=1450109 https://bugzilla.redhat.com/show_bug.cgi?id=1451655 --- app/models/vm/operations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/vm/operations.rb b/app/models/vm/operations.rb index 6dd342ebecd..0599c16b717 100644 --- a/app/models/vm/operations.rb +++ b/app/models/vm/operations.rb @@ -22,7 +22,7 @@ def cockpit_url def ipv4_address return public_address unless public_address.nil? - ipaddresses.find { |ip| IPAddr.new(ip).ipv4? unless ip.starts_with?('192') } + ipaddresses.find { |ip| IPAddr.new(ip).ipv4? && !ip.starts_with?('192') } end def public_address