From b194b8e6b441a872fb961109d04861f36f250f71 Mon Sep 17 00:00:00 2001
From: Tom Kiemes <tom.kiemes@sap.com>
Date: Fri, 19 May 2017 16:31:20 +0200
Subject: [PATCH] Delete registry settings when deleting VM

- consider registry_key to decide which key to use
  (server.name vs registry_key)
- previous implementation had a bug, where the metadata
  was not available any more, thus server.name was used

[#144344945](https://www.pivotaltracker.com/story/show/144344945)

Signed-off-by: Felix Riegger <felix.riegger@sap.com>
---
 .../lib/cloud/openstack/cloud.rb              |  4 +--
 .../spec/unit/delete_vm_spec.rb               | 27 ++++++++++++++-----
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb b/src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
index da2858ea..9c24281d 100644
--- a/src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
+++ b/src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
@@ -328,7 +328,7 @@ def delete_vm(server_id)
             catch_error('Removing ports') { NetworkConfigurator.cleanup_ports(@openstack, server_port_ids) },
             catch_error('Removing lbaas pool memberships') { LoadbalancerConfigurator.new(@openstack, @logger).cleanup_memberships(server_tags) },
             catch_error('Deleting registry settings') {
-              registry_key = registry_key_for(server)
+              registry_key = server_tags.fetch(REGISTRY_KEY_TAG.to_s, server.name)
               @logger.info("Deleting settings for server `#{server.id}' with registry_key `#{registry_key}' ...")
               @registry.delete_settings(registry_key)
             })
@@ -694,7 +694,7 @@ def info
     private
 
     def registry_key_for(server)
-      registry_key_metadatum = server.metadata.get(REGISTRY_KEY_TAG)
+      registry_key_metadatum = @openstack.with_openstack { server.metadata.get(REGISTRY_KEY_TAG) }
       registry_key_metadatum ? registry_key_metadatum.value : server.name
     end
 
diff --git a/src/bosh_openstack_cpi/spec/unit/delete_vm_spec.rb b/src/bosh_openstack_cpi/spec/unit/delete_vm_spec.rb
index 9936861e..6e8dd7f1 100644
--- a/src/bosh_openstack_cpi/spec/unit/delete_vm_spec.rb
+++ b/src/bosh_openstack_cpi/spec/unit/delete_vm_spec.rb
@@ -8,7 +8,7 @@
     [
       double('metadatum', :key => 'lbaas_pool_0', :value => 'pool-id-0/membership-id-0'),
       double('metadatum', :key => 'job', :value => 'bosh'),
-      double('metadatum', key: :registry_key, value: registry_key)
+      double('metadatum', key: 'registry_key', value: registry_key)
     ]
   end
 
@@ -29,9 +29,6 @@
      allow(server).to receive(:destroy)
      allow(cloud.openstack).to receive(:wait_resource)
      allow(@registry).to receive(:delete_settings)
-     allow(server_metadata).to receive(:get) { |param|
-       server_metadata.find { |metadatum| metadatum.key == param}
-     }
      allow(Bosh::OpenStackCloud::LoadbalancerConfigurator).to receive(:new).and_return(loadbalancer_configurator)
      allow(loadbalancer_configurator).to receive(:cleanup_memberships)
    end
@@ -79,7 +76,7 @@
       {
         'lbaas_pool_0' => 'pool-id-0/membership-id-0',
         'job' => 'bosh',
-        :registry_key => 'vm-registry-key'
+        'registry_key' => 'vm-registry-key'
       }
     )
   end
@@ -117,7 +114,7 @@
         {
           'lbaas_pool_0' => 'pool-id-0/membership-id-0',
           'job' => 'bosh',
-          :registry_key => 'vm-registry-key'
+          'registry_key' => 'vm-registry-key'
         }
       )
     end
@@ -152,7 +149,7 @@
         {
           'lbaas_pool_0' => 'pool-id-0/membership-id-0',
           'job' => 'bosh',
-          :registry_key => 'vm-registry-key'
+          'registry_key' => 'vm-registry-key'
         }
       )
       expect(@registry).to have_received(:delete_settings).with(registry_key)
@@ -177,4 +174,20 @@
       }.to raise_error(Bosh::Clouds::CloudError, expected_error_msg.chomp)
     end
   end
+
+  context 'when server is not tagged with `registry_key`' do
+
+    let(:server_metadata) do
+      [
+          double('metadatum', :key => 'lbaas_pool_0', :value => 'pool-id-0/membership-id-0'),
+          double('metadatum', :key => 'job', :value => 'bosh'),
+      ]
+    end
+
+    it 'uses the server name to delete registry settings' do
+      cloud.delete_vm('i-foobar')
+
+      expect(@registry).to have_received(:delete_settings).with(server.name)
+    end
+  end
 end