(maint) Refactor cloud dns

Moved the creation and deletion methods to a vmpooler class, to be used by the ec2
provisioner.
Added a check for failed deletion attempts, they are now retried up to 30 times

lib/vmpooler-provider-gce/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module VmpoolerProviderGce
-  VERSION = '0.3.0'
+  VERSION = '0.4.0'
 end

lib/vmpooler/cloud_dns.rb

@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require 'googleauth'
+require 'google/cloud/dns'
+
+module Vmpooler
+  class PoolManager
+    class CloudDns
+      def initialize(project, dns_zone_resource_name)
+        @dns ||= Google::Cloud::Dns.new(project_id: project)
+        @dns_zone_resource_name = dns_zone_resource_name
+      end
+
+      def dns_create_or_replace(created_instance)
+        dns_zone = @dns.zone(@dns_zone_resource_name) if @dns_zone_resource_name
+        return unless dns_zone && created_instance && created_instance['name'] && created_instance['ip']
+
+        name = created_instance['name']
+        begin
+          change = dns_zone.add(name, 'A', 60, [created_instance['ip']])
+          debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address added") if change
+        rescue Google::Cloud::AlreadyExistsError => _e
+          # DNS setup is done only for new instances, so in the rare case where a DNS record already exists (it is stale) and we replace it.
+          # the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.net. (A)' already exists
+          change = dns_zone.replace(name, 'A', 60, [created_instance['ip']])
+          debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address previously existed and was replaced") if change
+        end
+      end
+
+      def dns_teardown(created_instance)
+        dns_zone = @dns.zone(@dns_zone_resource_name) if @dns_zone_resource_name
+        return unless dns_zone && created_instance
+
+        retries = 0
+        name = created_instance['name']
+        change = dns_zone.remove(name, 'A')
+        debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address removed") if change
+      rescue Google::Cloud::FailedPreconditionError => e
+        # this error was experienced intermittently, will retry to see if it can complete successfully
+        # the error is Google::Cloud::FailedPreconditionError: conditionNotMet: Precondition not met for 'entity.change.deletions[1]'
+        debug_logger("DNS teardown failed, retrying error: #{e}")
+        sleep 5
+        retry if (retries += 1) < 30
+      end
+
+      # used in local dev environment, set DEBUG_FLAG=true
+      # this way the upstream vmpooler manager does not get polluted with logs
+      def debug_logger(message, send_to_upstream: false)
+        # the default logger is simple and does not enforce debug levels (the first argument)
+        puts message if ENV['DEBUG_FLAG']
+        logger.log('[g]', message) if send_to_upstream
+      end
+    end
+  end
+end

lib/vmpooler/providers/gce.rb

@@ -2,7 +2,7 @@
 
 require 'googleauth'
 require 'google/apis/compute_v1'
-require 'google/cloud/dns'
+require 'vmpooler/cloud_dns'
 require 'bigdecimal'
 require 'bigdecimal/util'
 require 'vmpooler/providers/base'
@@ -58,11 +58,6 @@ module Vmpooler
           end
         end
 
-        def dns
-          @dns ||= Google::Cloud::Dns.new(project_id: project)
-          @dns
-        end
-
         # main configuration options
         def project
           provider_config['project']
@@ -566,28 +561,13 @@ module Vmpooler
         # END BASE METHODS
 
         def dns_setup(created_instance)
-          dns_zone = dns.zone(dns_zone_resource_name) if dns_zone_resource_name
+          dns = Vmpooler::PoolManager::CloudDns.new(project, dns_zone_resource_name)
-          return unless dns_zone && created_instance && created_instance['name'] && created_instance['ip']
+          dns.dns_create_or_replace(created_instance)
-
-          name = created_instance['name']
-          begin
-            change = dns_zone.add(name, 'A', 60, [created_instance['ip']])
-            debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address added") if change
-          rescue Google::Cloud::AlreadyExistsError => _e
-            # DNS setup is done only for new instances, so in the rare case where a DNS record already exists (it is stale) and we replace it.
-            # the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.net. (A)' already exists
-            change = dns_zone.replace(name, 'A', 60, [created_instance['ip']])
-            debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address previously existed and was replaced") if change
-          end
         end
 
         def dns_teardown(created_instance)
-          dns_zone = dns.zone(dns_zone_resource_name) if dns_zone_resource_name
+          dns = Vmpooler::PoolManager::CloudDns.new(project, dns_zone_resource_name)
-          return unless dns_zone && created_instance
+          dns.dns_teardown(created_instance)
-
-          name = created_instance['name']
-          change = dns_zone.remove(name, 'A')
-          debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address removed") if change
         end
 
         def should_be_ignored(item, allowlist)

spec/unit/providers/gce_spec.rb

@@ -62,7 +62,7 @@ EOT
   describe '#manual tests live' do
     context 'in itsysops' do
       before(:each) { allow(subject).to receive(:dns).and_call_original }
-      let(:vmname) { "instance-27" }
+      let(:vmname) { "instance-31" }
       let(:project) { 'vmpooler-test' }
       let(:config) { YAML.load(<<~EOT
       ---
@@ -75,8 +75,8 @@ EOT
           project: '#{project}'
           zone: '#{zone}'
           network_name: 'projects/itsysopsnetworking/global/networks/shared1'
-          dns_zone_resource_name: 'test-vmpooler-puppet-net'
+          dns_zone_resource_name: 'vmpooler-test-puppet-net'
-          domain: 'test.vmpooler.puppet.net'
+          domain: 'vmpooler-test.puppet.net'
       :pools:
         - name: '#{poolname}'
           alias: [ 'mockpool' ]
@@ -93,8 +93,8 @@ EOT
       skip 'gets a vm' do
         result = subject.create_vm(poolname, vmname)
         #result = subject.destroy_vm(poolname, vmname)
-        subject.get_vm(poolname, vmname)
+        # subject.get_vm(poolname, vmname)
-        #subject.dns_teardown({'name' => vmname})
+        subject.dns_teardown({'name' => vmname})
         # subject.dns_setup({'name' => vmname, 'ip' => '1.2.3.5'})
       end
     end