diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e71ec28..0097c39 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -20,10 +20,10 @@ jobs: draft: false prerelease: false generateReleaseNotes: true - - name: Install Ruby 2.5.8 + - name: Install Ruby jruby-9.3.6.0 uses: ruby/setup-ruby@v1 with: - ruby-version: '2.5.8' + ruby-version: 'jruby-9.3.6.0' - name: Build gem run: gem build *.gemspec - name: Publish gem diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 501403f..068495e 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -18,7 +18,7 @@ jobs: strategy: matrix: ruby-version: - - '2.5.8' + - 'jruby-9.3.6.0' steps: - uses: actions/checkout@v2 - name: Set up Ruby @@ -34,8 +34,7 @@ jobs: strategy: matrix: ruby-version: - - '2.5.8' - - 'jruby-9.2.12.0' + - 'jruby-9.3.6.0' steps: - uses: actions/checkout@v2 - name: Set up Ruby diff --git a/lib/vmpooler-provider-gce/version.rb b/lib/vmpooler-provider-gce/version.rb index cd39329..a332a30 100644 --- a/lib/vmpooler-provider-gce/version.rb +++ b/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 diff --git a/lib/vmpooler/cloud_dns.rb b/lib/vmpooler/cloud_dns.rb new file mode 100644 index 0000000..a98d9ef --- /dev/null +++ b/lib/vmpooler/cloud_dns.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'googleauth' +require 'google/cloud/dns' + +module Vmpooler + class PoolManager + # This class interacts with GCP Cloud DNS to create or delete records. + 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'] + + retries = 0 + 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 + 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[0]' + debug_logger("DNS create failed, retrying error: #{e}") + sleep 5 + retry if (retries += 1) < 30 + 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 diff --git a/lib/vmpooler/providers/gce.rb b/lib/vmpooler/providers/gce.rb index e94c9a3..d656156 100644 --- a/lib/vmpooler/providers/gce.rb +++ b/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 - 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 + dns = Vmpooler::PoolManager::CloudDns.new(project, dns_zone_resource_name) + dns.dns_create_or_replace(created_instance) 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 - - name = created_instance['name'] - change = dns_zone.remove(name, 'A') - debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address removed") if change + dns = Vmpooler::PoolManager::CloudDns.new(project, dns_zone_resource_name) + dns.dns_teardown(created_instance) end def should_be_ignored(item, allowlist) diff --git a/spec/unit/providers/gce_spec.rb b/spec/unit/providers/gce_spec.rb index a66a8c0..c4b8f25 100644 --- a/spec/unit/providers/gce_spec.rb +++ b/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' - domain: 'test.vmpooler.puppet.net' + dns_zone_resource_name: 'vmpooler-test-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.dns_teardown({'name' => vmname}) + # subject.get_vm(poolname, vmname) + subject.dns_teardown({'name' => vmname}) # subject.dns_setup({'name' => vmname, 'ip' => '1.2.3.5'}) end end @@ -267,6 +267,7 @@ EOT result = MockResult.new result.status = 'DONE' allow(connection).to receive(:insert_instance).and_return(result) + allow(subject).to receive(:dns_setup).and_return(true) end it 'should return a hash' do @@ -313,6 +314,7 @@ EOT result.status = 'DONE' allow(subject).to receive(:wait_for_operation).and_return(result) allow(connection).to receive(:delete_instance).and_return(result) + allow(subject).to receive(:dns_teardown).and_return(true) end it 'should return true' do @@ -584,6 +586,7 @@ EOT before(:each) do allow(subject).to receive(:connect_to_gce).and_return(connection) + allow(subject).to receive(:dns_teardown).and_return(true) end context 'with empty allowlist' do