Skip to content

Commit

Permalink
Actually delete registry settings on VM deletion
Browse files Browse the repository at this point in the history
- since the `human_readable_vm_name` feature, the key under which
  information are stored in the registry, is not necessarily the
  VM name
- this was not considered in `delete_vm`

[#144344945](https://www.pivotaltracker.com/story/show/144344945)
  • Loading branch information
friegger committed May 10, 2017
1 parent 550e927 commit 715272c
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 12 deletions.
13 changes: 9 additions & 4 deletions src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb
Expand Up @@ -328,8 +328,9 @@ 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') {
@logger.info("Deleting settings for server `#{server.id}'...")
@registry.delete_settings(server.name)
registry_key = registry_key_for(server)
@logger.info("Deleting settings for server `#{server.id}' with registry_key `#{registry_key}' ...")
@registry.delete_settings(registry_key)
})
else
@logger.info("Server `#{server_id}' not found. Skipping.")
Expand Down Expand Up @@ -677,8 +678,7 @@ def is_v3
# @param [Fog::Compute::OpenStack::Server] server OpenStack server
def update_agent_settings(server)
raise ArgumentError, 'Block is not provided' unless block_given?
registry_key_metadatum = server.metadata.get(REGISTRY_KEY_TAG)
registry_key = registry_key_metadatum ? registry_key_metadatum.value : server.name
registry_key = registry_key_for(server)
@logger.info("Updating settings for server `#{server.id}' with registry key `#{registry_key}'...")
settings = @registry.read_settings(registry_key)
yield settings
Expand All @@ -693,6 +693,11 @@ def info

private

def registry_key_for(server)
registry_key_metadatum = server.metadata.get(REGISTRY_KEY_TAG)
registry_key_metadatum ? registry_key_metadatum.value : server.name
end

def not_existing_net_ids(nics)
result = []
begin
Expand Down
25 changes: 17 additions & 8 deletions src/bosh_openstack_cpi/spec/unit/delete_vm_spec.rb
Expand Up @@ -2,10 +2,13 @@

describe Bosh::OpenStackCloud::Cloud do

let(:registry_key) { 'vm-registry-key' }

let(:server_metadata) do
[
double('metadatum', :key => 'lbaas_pool_0', :value => 'pool-id-0/membership-id-0'),
double('metadatum', :key => 'job', :value => 'bosh')
double('metadatum', :key => 'job', :value => 'bosh'),
double('metadatum', key: :registry_key, value: registry_key)
]
end

Expand All @@ -26,6 +29,9 @@
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 @@ -68,11 +74,12 @@
expect(server).to have_received(:destroy)
expect(cloud.openstack).to have_received(:wait_resource).with(server, [:terminated, :deleted], :state, true)
expect(Bosh::OpenStackCloud::NetworkConfigurator).to have_received(:cleanup_ports).with(any_args, ['port_id'])
expect(@registry).to have_received(:delete_settings).with('i-foobar')
expect(@registry).to have_received(:delete_settings).with(registry_key)
expect(loadbalancer_configurator).to have_received(:cleanup_memberships).with(
{
'lbaas_pool_0' => 'pool-id-0/membership-id-0',
'job' => 'bosh'
'job' => 'bosh',
:registry_key => 'vm-registry-key'
}
)
end
Expand Down Expand Up @@ -105,11 +112,12 @@
cloud.delete_vm('i-foobar')
}.to raise_error(/BOOM!/)

expect(@registry).to have_received(:delete_settings).with('i-foobar')
expect(@registry).to have_received(:delete_settings).with(registry_key)
expect(loadbalancer_configurator).to have_received(:cleanup_memberships).with(
{
'lbaas_pool_0' => 'pool-id-0/membership-id-0',
'job' => 'bosh'
'job' => 'bosh',
:registry_key => 'vm-registry-key'
}
)
end
Expand All @@ -125,7 +133,7 @@


expect(Bosh::OpenStackCloud::NetworkConfigurator).to have_received(:cleanup_ports).with(any_args, ['port_id'])
expect(@registry).to have_received(:delete_settings).with('i-foobar')
expect(@registry).to have_received(:delete_settings).with(registry_key)
end
end

Expand All @@ -143,10 +151,11 @@
expect(loadbalancer_configurator).to have_received(:cleanup_memberships).with(
{
'lbaas_pool_0' => 'pool-id-0/membership-id-0',
'job' => 'bosh'
'job' => 'bosh',
:registry_key => 'vm-registry-key'
}
)
expect(@registry).to have_received(:delete_settings).with('i-foobar')
expect(@registry).to have_received(:delete_settings).with(registry_key)
end
end

Expand Down

0 comments on commit 715272c

Please sign in to comment.