From 019ed021b0dc3b1f21b745897c34f6c6ab54e368 Mon Sep 17 00:00:00 2001
From: Brandon High <brandon.high@puppet.com>
Date: Fri, 1 Nov 2019 15:26:51 -0700
Subject: [PATCH] (QENG-7530) Add check for unique hostnames

Prior to this commit the pooler had no awareness of the complete set of
hostnames that are currently in use. This meant that it was possible to
allocate the same hostname twice, which would result in the original
host with that hostname becoming unreachable.

This commit adds a check for the existence of the
`vmpooler__vm__<hostname>` key before attempting to  clone the vm.
This should prevent duplicate hostnames.

If the hostname is already taken, `_clone_vm` will retry with a new
random hostname multiple times before raising an exception.
---
 lib/vmpooler/pool_manager.rb   | 28 ++++++++++++++++++++++++++--
 spec/unit/pool_manager_spec.rb |  2 ++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb
index 6c07bde..25a5027 100644
--- a/lib/vmpooler/pool_manager.rb
+++ b/lib/vmpooler/pool_manager.rb
@@ -267,10 +267,34 @@ module Vmpooler
       end
     end
 
-    def _clone_vm(pool_name, provider)
+    def generate_and_check_hostname(pool_name)
       # Generate a randomized hostname
       random_name = [@name_generator.adjective(max: 7), @name_generator.noun(max: 7)].join('-')
-      new_vmname = $config[:config]['prefix'] + random_name
+      hostname = $config[:config]['prefix'] + random_name
+      available = $redis.hlen('vmpooler__vm__' + hostname) == 0
+
+      return hostname, available
+    end
+
+    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)
+        break if 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})")
+      end
+      
+      raise "Unable to generate a unique hostname after #{hostname_retries} attempts. The last hostname checked was #{hostname}" unless available
+      
+      hostname
+    end
+
+    def _clone_vm(pool_name, provider)
+      new_vmname = find_unique_hostname(pool_name)
 
       # Add VM to Redis inventory ('pending' pool)
       $redis.sadd('vmpooler__pending__' + pool_name, new_vmname)
diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb
index 3b41e62..2fa9b3e 100644
--- a/spec/unit/pool_manager_spec.rb
+++ b/spec/unit/pool_manager_spec.rb
@@ -608,6 +608,7 @@ EOT
         expect(metrics).to receive(:timing).with(/clone\./,/0/)
         expect(provider).to receive(:create_vm).with(pool, String)
         allow(logger).to receive(:log)
+        expect(subject).to receive(:find_unique_hostname).with(pool).and_return(vm)
       end
 
       it 'should create a cloning VM' do
@@ -649,6 +650,7 @@ EOT
       before(:each) do
         expect(provider).to receive(:create_vm).with(pool, String).and_raise('MockError')
         allow(logger).to receive(:log)
+        expect(subject).to receive(:find_unique_hostname).with(pool).and_return(vm)
       end
 
       it 'should not create a cloning VM' do