Skip to content

Commit

Permalink
Delete registry settings when deleting VM
Browse files Browse the repository at this point in the history
- 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>
  • Loading branch information
Kiemes authored and friegger committed May 19, 2017
1 parent 465187a commit b194b8e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 9 deletions.
4 changes: 2 additions & 2 deletions src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down Expand Up @@ -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

Expand Down
27 changes: 20 additions & 7 deletions src/bosh_openstack_cpi/spec/unit/delete_vm_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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

0 comments on commit b194b8e

Please sign in to comment.