From fb8c1687518e82f404c21e05080ffa0f3e4fae4f Mon Sep 17 00:00:00 2001 From: Jake Spain Date: Thu, 23 Feb 2023 08:37:33 -0500 Subject: [PATCH] Add rspec tests --- lib/vmpooler/dns/gcp/clouddns.rb | 54 ++++-- spec/clouddns_helper.rb | 26 +++ spec/helpers.rb | 14 ++ spec/spec_helper.rb | 6 + spec/unit/dns/clouddns_spec.rb | 294 +++++++++++++++++++++++++++++++ vmpooler-dns-gcp.gemspec | 1 + 6 files changed, 376 insertions(+), 19 deletions(-) create mode 100644 spec/clouddns_helper.rb create mode 100644 spec/helpers.rb create mode 100644 spec/unit/dns/clouddns_spec.rb diff --git a/lib/vmpooler/dns/gcp/clouddns.rb b/lib/vmpooler/dns/gcp/clouddns.rb index 476da08..3db831e 100644 --- a/lib/vmpooler/dns/gcp/clouddns.rb +++ b/lib/vmpooler/dns/gcp/clouddns.rb @@ -2,6 +2,7 @@ require 'googleauth' require 'google/cloud/dns' +require 'vmpooler/dns/base' module Vmpooler class PoolManager @@ -54,30 +55,38 @@ module Vmpooler end def create_or_replace_record(hostname) + retries = 0 ip = get_ip(hostname) - begin - change = connection.zone(zone_name).add(hostname, 'A', 60, ip) - debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address added") if change - rescue Google::Cloud::AlreadyExistsError => _e - # the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.net. (A)' already exists - # the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.net. (A)' already exists - change = connection.zone(zone_name).replace(hostname, 'A', 60, ip) - debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address previously existed and was replaced") if change - rescue Google::Cloud::FailedPreconditionError => e - debug_logger("DNS create failed, retrying error: #{e}") - retry if (retries += 1) < 30 + if ip.nil? + debug_logger("An IP Address was not recorded for #{hostname}") + else + begin + change = connection.zone(zone_name).add(hostname, 'A', 60, ip) + debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address added") if change + rescue Google::Cloud::AlreadyExistsError => _e + # the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.net. (A)' already exists + # the error is Google::Cloud::AlreadyExistsError: alreadyExists: The resource 'entity.change.additions[0]' named 'instance-8.test.vmpooler.net. (A)' already exists + change = connection.zone(zone_name).replace(hostname, 'A', 60, ip) + debug_logger("#{change.id} - #{change.started_at} - #{change.status} DNS address previously existed and was replaced") if change + rescue Google::Cloud::FailedPreconditionError => e + debug_logger("DNS create failed, retrying error: #{e}") + sleep 5 + retry if (retries += 1) < 30 + end end end def delete_record(hostname) retries = 0 - connection.zone(zone_name).remove(hostname, 'A') - 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("GCP DNS delete_record failed, retrying error: #{e}") - sleep 5 - retry if (retries += 1) < 30 + begin + connection.zone(zone_name).remove(hostname, 'A') + 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("GCP DNS delete_record failed, retrying error: #{e}") + sleep 5 + retry if (retries += 1) < 30 + end end def connection @@ -87,10 +96,17 @@ module Vmpooler end def ensured_gcp_connection(connection_pool_object) - connection_pool_object[:connection] = connect_to_gcp unless connection_pool_object[:connection] + connection_pool_object[:connection] = connect_to_gcp unless gcp_connection_ok?(connection_pool_object[:connection]) connection_pool_object[:connection] end + def gcp_connection_ok?(connection) + _result = connection.id + true + rescue StandardError + false + end + def connect_to_gcp max_tries = global_config[:config]['max_tries'] || 3 retry_factor = global_config[:config]['retry_factor'] || 10 diff --git a/spec/clouddns_helper.rb b/spec/clouddns_helper.rb new file mode 100644 index 0000000..9d072d8 --- /dev/null +++ b/spec/clouddns_helper.rb @@ -0,0 +1,26 @@ +MockDnsZone = Struct.new( + # https://github.com/googleapis/google-cloud-ruby/blob/main/google-cloud-dns/lib/google/cloud/dns/zone.rb + :service, :gapi, :id, :name, :dns, :description, :name_servers, :name_server_set, :created_at +) do + def add(name, type, ttl, data) + change = MockDnsChange.new + change.additions(name, type, ttl, data) + # return name + end +end + +# -------------------- +# Main GoogleCloudDnsProject Object +# -------------------- +MockGoogleCloudDnsProjectConnection = Struct.new( + # https://cloud.google.com/ruby/docs/reference/google-cloud-dns/latest/Google-Cloud#Google__Cloud_dns_instance_ + :scope, :retries, :timeout +) do + def zone + MockDnsZone.new + end +end + +def mock_Google_Cloud_Dns_Project_Connection(options = {}) + MockGoogleCloudDnsProjectConnection.new() +end \ No newline at end of file diff --git a/spec/helpers.rb b/spec/helpers.rb new file mode 100644 index 0000000..478a869 --- /dev/null +++ b/spec/helpers.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'mock_redis' + +def redis + @redis ||= MockRedis.new + @redis +end + +# Mock an object which represents a Logger. This stops the proliferation +# of allow(logger).to .... expectations in tests. +class MockLogger + def log(_level, string); end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4cc72cf..8693400 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,6 +4,12 @@ require 'simplecov' SimpleCov.start do add_filter '/spec/' end +require 'helpers' +require 'rspec' +require 'vmpooler' +require 'redis' +require 'vmpooler/metrics' +require 'clouddns_helper' def project_root_dir File.dirname(File.dirname(__FILE__)) diff --git a/spec/unit/dns/clouddns_spec.rb b/spec/unit/dns/clouddns_spec.rb new file mode 100644 index 0000000..cba2f23 --- /dev/null +++ b/spec/unit/dns/clouddns_spec.rb @@ -0,0 +1,294 @@ +require 'spec_helper' +require 'mock_redis' +require 'vmpooler/dns/gcp/clouddns' + +describe 'Vmpooler::PoolManager::Dns::Gcp' do + let(:logger) { MockLogger.new } + let(:metrics) { Vmpooler::Metrics::DummyStatsd.new } + let(:poolname) { 'debian-9' } + let(:options) { { 'param' => 'value' } } + let(:name) { 'gcp' } + let(:zone_name) { 'vmpooler-example-com' } + let(:config) { YAML.load(<<~EOT + --- + :config: + max_tries: 3 + retry_factor: 10 + :dns_configs: + :#{name}: + dns_class: gcp-clouddns + project: vmpooler-example + domain: vmpooler.example.com + zone_name: vmpooler-example-com + dns_zone_resource_name: vmpooler-example-com + :providers: + :dummy: + filename: '/tmp/dummy-backing.yaml' + :pools: + - name: '#{poolname}' + alias: [ 'mockpool' ] + template: 'Templates/debian-9-x86_64' + size: 5 + timeout: 10 + ready_ttl: 1440 + provider: 'dummy' + dns_config: '#{name}' +EOT + ) + } + + let(:vmname) { 'spicy-proton' } + let(:connection_options) {{}} + let(:connection) { mock_Google_Cloud_Dns_Project_Connection(connection_options) } + let(:redis_connection_pool) do + Vmpooler::PoolManager::GenericConnectionPool.new( + metrics: metrics, + connpool_type: 'redis_connection_pool', + connpool_provider: 'testprovider', + size: 1, + timeout: 5 + ) { MockRedis.new } + end + + subject { Vmpooler::PoolManager::Dns::Gcp::Clouddns.new(config, logger, metrics, redis_connection_pool, name, options) } + + describe '#name' do + it 'should be gcp-clouddns' do + expect(subject.name).to eq('gcp-clouddns') + end + end + + describe '#project' do + it 'should be the project specified in the dns config' do + expect(subject.project).to eq('vmpooler-example') + end + end + + describe '#zone_name' do + it 'should be the zone_name specified in the dns config' do + expect(subject.zone_name).to eq('vmpooler-example-com') + end + end + + describe '#create_or_replace_record' do + let(:hostname) { 'spicy-proton' } + let(:zone) { MockDnsZone.new } + let(:ip) { '169.254.255.255' } + + context 'when adding a record' do + before(:each) do + allow(Google::Cloud::Dns).to receive(:configure) + allow(Google::Cloud::Dns).to receive(:new).and_return(connection) + allow(connection).to receive(:zone).and_return(zone) + allow(subject).to receive(:get_ip).and_return(ip) + end + + it 'should attempt to add a record' do + expect(zone).to receive(:add).with(hostname, 'A', 60, ip) + result = subject.create_or_replace_record(hostname) + end + end + + context 'when record already exists' do + before(:each) do + allow(Google::Cloud::Dns).to receive(:configure) + allow(Google::Cloud::Dns).to receive(:new).and_return(connection) + allow(connection).to receive(:zone).and_return(zone) + allow(subject).to receive(:get_ip).and_return(ip) + end + + it 'should attempt to replace a record' do + allow(zone).to receive(:add).with(:hostname, 'A', 60, ip).and_raise(Google::Cloud::AlreadyExistsError,'MockError') + expect(zone).to receive(:replace).with(:hostname, 'A', 60, ip) + allow(subject).to receive(:get_ip).and_return(ip) + result = subject.create_or_replace_record(:hostname) + end + end + + context 'when add record fails' do + before(:each) do + allow(Google::Cloud::Dns).to receive(:configure) + allow(Google::Cloud::Dns).to receive(:new).and_return(connection) + allow(connection).to receive(:zone).and_return(zone) + allow(subject).to receive(:get_ip).and_return(ip) + end + + it 'should retry' do + allow(zone).to receive(:add).with(:hostname, 'A', 60, ip).and_raise(Google::Cloud::FailedPreconditionError,'MockError') + expect(zone).to receive(:add).with(:hostname, 'A', 60, ip).exactly(30).times + allow(subject).to receive(:sleep) + result = subject.create_or_replace_record(:hostname) + end + end + + context 'when IP does not exist' do + let(:ip) { nil } + + before(:each) do + allow(Google::Cloud::Dns).to receive(:configure) + allow(Google::Cloud::Dns).to receive(:new).and_return(connection) + allow(connection).to receive(:zone).and_return(zone) + allow(subject).to receive(:get_ip).and_return(ip) + end + + it 'should not attempt to add a record' do + allow(zone).to receive(:add).with(:hostname, 'A', 60, ip).and_raise(Google::Cloud::AlreadyExistsError,'MockError') + expect(zone).to_not have_received(:add) + allow(subject).to receive(:get_ip).and_return(ip) + result = subject.create_or_replace_record(:hostname) + end + end + end + + describe "#delete_record" do + let(:hostname) { 'spicy-proton' } + let(:zone) { MockDnsZone.new } + + context 'when removing a record' do + before(:each) do + allow(Google::Cloud::Dns).to receive(:configure) + allow(Google::Cloud::Dns).to receive(:new).and_return(connection) + allow(connection).to receive(:zone).and_return(zone) + end + + it 'should attempt to remove a record' do + expect(zone).to receive(:remove).with(:hostname, 'A') + result = subject.delete_record(:hostname) + end + end + + context 'when removing a record fails' do + before(:each) do + allow(Google::Cloud::Dns).to receive(:configure) + allow(Google::Cloud::Dns).to receive(:new).and_return(connection) + allow(connection).to receive(:zone).and_return(zone) + end + + it 'should retry' do + allow(zone).to receive(:remove).with(:hostname, 'A').and_raise(Google::Cloud::FailedPreconditionError,'MockError') + expect(zone).to receive(:remove).with(:hostname, 'A').exactly(30).times + allow(subject).to receive(:sleep) + result = subject.delete_record(:hostname) + end + end + end + + describe '#ensured_gcp_connection' do + let(:connection1) { mock_Google_Cloud_Dns_Project_Connection(connection_options) } + let(:connection2) { mock_Google_Cloud_Dns_Project_Connection(connection_options) } + + before(:each) do + allow(subject).to receive(:connect_to_gcp).and_return(connection1) + end + + it 'should return the same connection object when calling the pool multiple times' do + subject.connection_pool.with_metrics do |pool_object| + expect(pool_object[:connection]).to be(connection1) + end + subject.connection_pool.with_metrics do |pool_object| + expect(pool_object[:connection]).to be(connection1) + end + subject.connection_pool.with_metrics do |pool_object| + expect(pool_object[:connection]).to be(connection1) + end + end + + context 'when the connection breaks' do + before(:each) do + # Emulate the connection state being good, then bad, then good again + expect(subject).to receive(:gcp_connection_ok?).and_return(true, false, true) + expect(subject).to receive(:connect_to_gcp).and_return(connection1, connection2) + end + + it 'should restore the connection' do + subject.connection_pool.with_metrics do |pool_object| + # This line needs to be added to all instances of the connection_pool allocation + connection = subject.ensured_gcp_connection(pool_object) + + expect(connection).to be(connection1) + end + + subject.connection_pool.with_metrics do |pool_object| + connection = subject.ensured_gcp_connection(pool_object) + # The second connection would have failed. This test ensures that a + # new connection object was created. + expect(connection).to be(connection2) + end + + subject.connection_pool.with_metrics do |pool_object| + connection = subject.ensured_gcp_connection(pool_object) + expect(connection).to be(connection2) + end + end + end + end + + describe '#connect_to_gcp' do + before(:each) do + allow(Google::Cloud::Dns).to receive(:configure) + allow(Google::Cloud::Dns).to receive(:new).and_return(connection) + end + + context 'successful connection' do + it 'should return the connection object' do + result = subject.connect_to_gcp + + expect(result).to be(connection) + end + + it 'should increment the connect.open counter' do + expect(metrics).to receive(:increment).with('connect.open') + subject.connect_to_gcp + end + end + + context 'connection is initially unsuccessful' do + before(:each) do + # Simulate a failure and then success + allow(Google::Cloud::Dns).to receive(:configure) + expect(Google::Cloud::Dns).to receive(:new).and_raise(RuntimeError,'MockError') + allow(subject).to receive(:sleep) + end + + it 'should return the connection object' do + result = subject.connect_to_gcp + + expect(result).to be(connection) + end + + it 'should increment the connect.fail and then connect.open counter' do + expect(metrics).to receive(:increment).with('connect.fail').exactly(1).times + expect(metrics).to receive(:increment).with('connect.open').exactly(1).times + subject.connect_to_gcp + end + end + + context 'connection is always unsuccessful' do + before(:each) do + allow(Google::Cloud::Dns).to receive(:configure) + allow(Google::Cloud::Dns).to receive(:new).exactly(3).times.and_raise(RuntimeError,'MockError') + allow(subject).to receive(:sleep) + end + + it 'should retry the connection attempt config.max_tries times' do + expect(Google::Cloud::Dns).to receive(:new).exactly(config[:config]['max_tries']).times + + begin + # Swallow any errors + subject.connect_to_gcp + rescue + end + end + + it 'should increment the connect.fail counter config.max_tries times' do + expect(metrics).to receive(:increment).with('connect.fail').exactly(config[:config]['max_tries']).times + + begin + # Swallow any errors + subject.connect_to_gcp + rescue + end + end + end + end +end \ No newline at end of file diff --git a/vmpooler-dns-gcp.gemspec b/vmpooler-dns-gcp.gemspec index 9a7a44c..fe416c7 100644 --- a/vmpooler-dns-gcp.gemspec +++ b/vmpooler-dns-gcp.gemspec @@ -25,6 +25,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'vmpooler', '>= 1.3.0', '~> 2.3' # Testing dependencies + spec.add_development_dependency 'mock_redis', '>= 0.17.0' spec.add_development_dependency 'rspec', '>= 3.2' spec.add_development_dependency 'rubocop', '~> 1.1.0' spec.add_development_dependency 'simplecov', '>= 0.11.2'