Add rspec tests

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,7 +55,11 @@ module Vmpooler
           end
 
           def create_or_replace_record(hostname)
+            retries = 0
             ip = get_ip(hostname)
+            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
@@ -65,12 +70,15 @@ module Vmpooler
                 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
+            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
@@ -79,6 +87,7 @@ module Vmpooler
               sleep 5
               retry if (retries += 1) < 30
             end
+          end
 
           def connection
             @connection_pool.with_metrics do |pool_object|
@@ -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

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

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

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__))

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

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'