From 6304743240720d3f8c3e4b38ef2b8b555859f023 Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Fri, 29 May 2020 11:51:23 -0500 Subject: [PATCH 1/2] (POOLER-166) Check for stale dns records --- lib/vmpooler/pool_manager.rb | 33 +++++++++++++++++++++++++++++---- spec/unit/pool_manager_spec.rb | 26 ++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 65847e8f7..51ef27866 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -2,6 +2,7 @@ require 'vmpooler/providers' require 'spicy-proton' +require 'resolv' # ruby standard lib module Vmpooler class PoolManager @@ -295,22 +296,46 @@ def generate_and_check_hostname(_pool_name) end def find_unique_hostname(pool_name) + # generate hostname that is not already in use in vmpooler + # also check that no dns record already exists hostname_retries = 0 max_hostname_retries = 3 while hostname_retries < max_hostname_retries hostname, available = generate_and_check_hostname(pool_name) - break if available + domain = $config[:config]['domain'] + dns_ip, dns_available = check_dns_available(hostname, domain) + break if available && dns_available hostname_retries += 1 - $metrics.increment("errors.duplicatehostname.#{pool_name}") - $logger.log('s', "[!] [#{pool_name}] Generated hostname #{hostname} was not unique (attempt \##{hostname_retries} of #{max_hostname_retries})") + + if !available + $metrics.increment("errors.duplicatehostname.#{pool_name}") + $logger.log('s', "[!] [#{pool_name}] Generated hostname #{hostname} was not unique (attempt \##{hostname_retries} of #{max_hostname_retries})") + elsif !dns_available + $metrics.increment("errors.staledns.#{hostname}") + $logger.log('s', "[!] [#{pool_name}] Generated hostname #{hostname} already exists in DNS records (#{dns_ip}), stale DNS") + end end - raise "Unable to generate a unique hostname after #{hostname_retries} attempts. The last hostname checked was #{hostname}" unless available + raise "Unable to generate a unique hostname after #{hostname_retries} attempts. The last hostname checked was #{hostname}" unless available && dns_available hostname end + def check_dns_available(vm_name, domain = nil) + # Query the DNS for the name we want to create and if it already exists, mark it unavailable + # This protects against stale DNS records + vm_name = "#{vm_name}.#{domain}" if domain + begin + dns_ip = Resolv.getaddress(vm_name) + rescue Resolv::ResolvError + # this is the expected case, swallow the error + # eg "no address for blah-daisy" + return ['', true] + end + [dns_ip, false] + end + def _clone_vm(pool_name, provider) new_vmname = find_unique_hostname(pool_name) diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index e3218f639..737980f04 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -681,6 +681,28 @@ expect{subject._clone_vm(pool,provider)}.to raise_error(/MockError/) end end + + context 'with #check_dns_available' do + before(:each) do + allow(logger).to receive(:log) + end + it 'should error out if DNS already exists' do + vm_name = "foo" + resolv = class_double("Resolv").as_stubbed_const(:transfer_nested_constants => true) + expect(subject).to receive(:generate_and_check_hostname).exactly(3).times.and_return([vm_name, true]) #skip this, make it available all times + expect(resolv).to receive(:getaddress).exactly(3).times.and_return("1.2.3.4") + expect(metrics).to receive(:increment).with("errors.staledns.#{vm_name}").exactly(3).times + expect{subject._clone_vm(pool,provider)}.to raise_error(/Unable to generate a unique hostname after/) + end + it 'should be successful if DNS does not exist' do + vm_name = "foo" + resolv = class_double("Resolv").as_stubbed_const(:transfer_nested_constants => true) + expect(subject).to receive(:generate_and_check_hostname).and_return([vm_name, true]) + expect(resolv).to receive(:getaddress).exactly(1).times.and_raise(Resolv::ResolvError) + expect(provider).to receive(:create_vm).with(pool, String) + subject._clone_vm(pool,provider) + end + end end describe '#destroy_vm' do @@ -2747,7 +2769,7 @@ let(:loop_delay) { 1 } # Note a maxloop of zero can not be tested as it never terminates before(:each) do - + allow(subject).to receive(:check_disk_queue) allow(subject).to receive(:check_snapshot_queue) allow(subject).to receive(:check_pool) @@ -3639,7 +3661,7 @@ # Modify the pool size to 1 and add a VM in the queue redis.sadd("vmpooler__#{queue_name}__#{pool}",vm) pool_size = 1 - + subject.repopulate_pool_vms(pool, provider, pool_check_response, pool_size) end end From 41f097cc3e62428b51ceb3817c2293f1b1240b07 Mon Sep 17 00:00:00 2001 From: Samuel Beaulieu Date: Fri, 29 May 2020 12:14:29 -0500 Subject: [PATCH 2/2] with the addition of dns_available, renaming available to hostname_available --- lib/vmpooler/pool_manager.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 51ef27866..1bdb70454 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -301,14 +301,14 @@ def find_unique_hostname(pool_name) hostname_retries = 0 max_hostname_retries = 3 while hostname_retries < max_hostname_retries - hostname, available = generate_and_check_hostname(pool_name) + hostname, hostname_available = generate_and_check_hostname(pool_name) domain = $config[:config]['domain'] dns_ip, dns_available = check_dns_available(hostname, domain) - break if available && dns_available + break if hostname_available && dns_available hostname_retries += 1 - if !available + if !hostname_available $metrics.increment("errors.duplicatehostname.#{pool_name}") $logger.log('s', "[!] [#{pool_name}] Generated hostname #{hostname} was not unique (attempt \##{hostname_retries} of #{max_hostname_retries})") elsif !dns_available @@ -317,7 +317,7 @@ def find_unique_hostname(pool_name) end end - raise "Unable to generate a unique hostname after #{hostname_retries} attempts. The last hostname checked was #{hostname}" unless available && dns_available + raise "Unable to generate a unique hostname after #{hostname_retries} attempts. The last hostname checked was #{hostname}" unless hostname_available && dns_available hostname end