Project

General

Profile

« Previous | Next » 

Revision 91f8ffb1

Added by Daniel Lobato Garcia almost 7 years ago

Fixes #19631 - ComputeResource with KeyPair can be removed

The concern for removing the compute resources with keypairs was trying
to destroy the compute resource too early. Also, this deletion already
happened because of the has_one :dependency => :destroy relation

This fixes the problem and adds tests to ensure the relation works.

View differences:

app/controllers/key_pairs_controller.rb
def destroy
key_to_delete = params[:id]
return not_found unless key_to_delete
if @compute_resource.delete_key_pair(key_to_delete)
if @compute_resource.delete_key_from_resource(key_to_delete)
process_success :success_msg => _('Successfully delete %s') % key_to_delete,
:success_redirect => compute_resource_path(@compute_resource)
else
app/models/concerns/key_pair_compute_resource.rb
setup_key_pair
end
def delete_key_pair(kay_pair_name)
delete_key_from_resource(kay_pair_name)
def delete_key_from_resource(remote_key_pair = key_pair.name)
logger.info "removing key from compute resource #{name} "\
"(#{provider_friendly_name}): #{remote_key_pair}"
client.key_pairs.get(remote_key_pair).try(:destroy)
rescue => e
Foreman::Logging.exception(
"Failed to delete key pair from #{provider_friendly_name}: #{name}, you "\
"might need to cleanup manually: #{e}",
e,
:level => :warn
)
end
private
......
end
def destroy_key_pair
return unless key_pair
delete_key_from_resource(key_pair.name)
key_pair.destroy!
end
def delete_key_from_resource(key_pair_name)
logger.info "removing #{provider_friendly_name}: #{name} key #{key_pair_name}"
key = client.key_pairs.get(key_pair_name)
key.nil? || key.destroy
rescue => e
logger.warn "failed to delete key pair from #{provider_friendly_name}: #{name}, you might need to cleanup manually : #{e}"
return unless key_pair.present?
delete_key_from_resource
# If the key pair could not be removed, it will be logged.
# Returning 'true' allows this method to not halt the deletion
# of the Compute Resource even if the key pair could not be
# deleted for some reason (permissions, not found, etc...)
true
end
end
test/models/compute_resources/ec2_test.rb
require 'test_helper'
require 'models/compute_resources/compute_resource_test_helpers'
class EC2Test < ActiveSupport::TestCase
include ComputeResourceTestHelpers
test "#associated_host matches any NIC" do
host = FactoryGirl.create(:host, :ip => '10.0.0.154')
cr = FactoryGirl.build(:ec2_cr)
iface = mock('iface1', :public_ip_address => '10.0.0.154', :private_ip_address => "10.1.1.1")
assert_equal host, as_admin { cr.associated_host(iface) }
end
describe "find_vm_by_uuid" do
it "raises RecordNotFound when the vm does not exist" do
cr = mock_cr_servers(Foreman::Model::EC2.new, empty_servers)
assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
end
module Foreman
module Model
class EC2Test < ActiveSupport::TestCase
include ComputeResourceTestHelpers
should have_one(:key_pair).with_foreign_key('compute_resource_id').
dependent(:destroy)
test "#associated_host matches any NIC" do
host = FactoryGirl.create(:host, :ip => '10.0.0.154')
cr = FactoryGirl.build(:ec2_cr)
iface = mock('iface1', :public_ip_address => '10.0.0.154', :private_ip_address => "10.1.1.1")
assert_equal host, as_admin { cr.associated_host(iface) }
end
it "raises RecordNotFound when the compute raises EC2 error" do
cr = mock_cr_servers(Foreman::Model::EC2.new, servers_raising_exception(Fog::Compute::AWS::Error))
assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
end
end
describe "find_vm_by_uuid" do
it "raises RecordNotFound when the vm does not exist" do
cr = mock_cr_servers(Foreman::Model::EC2.new, empty_servers)
assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
end
context "key pairs" do
setup do
@aws_key_pairs = []
3.times do |i|
@aws_key_pairs << AWSKeyPair.new("foreman-#{i}", "13:01:73:0#{i}")
it "raises RecordNotFound when the compute raises EC2 error" do
cr = mock_cr_servers(Foreman::Model::EC2.new, servers_raising_exception(Fog::Compute::AWS::Error))
assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
end
end
end
test "#get_compute_key_pairs" do
cr = FactoryGirl.build(:ec2_cr)
key_pair = FactoryGirl.build(:key_pair)
cr.key_pair = key_pair
Foreman::Model::EC2.any_instance.stubs(:key_pairs).returns(@aws_key_pairs)
assert_kind_of(ComputeResourceKeyPair, cr.get_compute_key_pairs.first)
end
test "should be capable of key_pair" do
cr = FactoryGirl.create(:ec2_cr)
assert_includes(cr.capabilities, :key_pair)
context "key pairs" do
test "should be capable of key_pair" do
cr = FactoryGirl.create(:ec2_cr)
assert_includes(cr.capabilities, :key_pair)
end
end
end
end
end
# We can't use 'Fog::Compute::AWS::KeyPair' model
# This class mocks it.
class AWSKeyPair
attr_reader :name, :fingerprint, :private_key
def initialize(name, fingerprint, private_key = nil)
@name = name
@fingerprint = fingerprint
@private_key = private_key
end
end
test/models/compute_resources/openstack_test.rb
require 'test_helper'
require 'models/compute_resources/compute_resource_test_helpers'
class OpenstackTest < ActiveSupport::TestCase
include ComputeResourceTestHelpers
module Foreman
module Model
class OpenstackTest < ActiveSupport::TestCase
include ComputeResourceTestHelpers
setup do
@compute_resource = FactoryGirl.build(:openstack_cr)
end
should have_one(:key_pair).with_foreign_key('compute_resource_id').
dependent(:destroy)
teardown do
Fog.unmock!
end
setup do
@compute_resource = FactoryGirl.build(:openstack_cr)
end
test "#associated_host matches any NIC" do
host = FactoryGirl.create(:host, :ip => '10.0.0.154')
iface = mock('iface1', :floating_ip_address => '10.0.0.154', :private_ip_address => "10.1.1.1")
assert_equal host, as_admin { @compute_resource.associated_host(iface) }
end
teardown do
Fog.unmock!
end
test "boot_from_volume does not get triggered when a string 'false' is passed as argument" do
Fog.mock!
@compute_resource.stubs(:key_pair).returns(mocked_key_pair)
@compute_resource.expects(:boot_from_volume).never
@compute_resource.create_vm(:boot_from_volume => 'false', :nics => [""],
:flavor_ref => 'foo_flavor', :image_ref => 'foo_image')
end
test "#associated_host matches any NIC" do
host = FactoryGirl.create(:host, :ip => '10.0.0.154')
iface = mock('iface1', :floating_ip_address => '10.0.0.154', :private_ip_address => "10.1.1.1")
assert_equal host, as_admin { @compute_resource.associated_host(iface) }
end
describe "formatting hints" do
it "formats well when set to ServerGroupAntiAffinity" do
args = {
:scheduler_hint_filter => "ServerGroupAntiAffinity",
:scheduler_hint_data => {
:scheduler_hint_value => "some-uuid"
test "boot_from_volume does not get triggered when a string 'false' is passed as argument" do
Fog.mock!
@compute_resource.stubs(:key_pair).returns(mocked_key_pair)
@compute_resource.expects(:boot_from_volume).never
@compute_resource.create_vm(:boot_from_volume => 'false', :nics => [""],
:flavor_ref => 'foo_flavor', :image_ref => 'foo_image')
end
describe "formatting hints" do
it "formats well when set to ServerGroupAntiAffinity" do
args = {
:scheduler_hint_filter => "ServerGroupAntiAffinity",
:scheduler_hint_data => {
:scheduler_hint_value => "some-uuid"
}
}
}
desired = {
:os_scheduler_hints => {
:group => "some-uuid"
}
}
@compute_resource.format_scheduler_hint_filter(args)
assert_equal(desired, args)
end
desired = {
:os_scheduler_hints => {
:group => "some-uuid"
}
}
@compute_resource.format_scheduler_hint_filter(args)
assert_equal(desired, args)
end
it "formats well when set to ServerGroupAffinity" do
args = {
:scheduler_hint_filter => "ServerGroupAffinity",
:scheduler_hint_data => {
:scheduler_hint_value => "some-uuid"
it "formats well when set to ServerGroupAffinity" do
args = {
:scheduler_hint_filter => "ServerGroupAffinity",
:scheduler_hint_data => {
:scheduler_hint_value => "some-uuid"
}
}
}
desired = {
:os_scheduler_hints => {
:group => "some-uuid"
}
}
@compute_resource.format_scheduler_hint_filter(args)
assert_equal(desired, args)
end
desired = {
:os_scheduler_hints => {
:group => "some-uuid"
}
}
@compute_resource.format_scheduler_hint_filter(args)
assert_equal(desired, args)
end
it "formats well when set to Raw" do
args = {
:scheduler_hint_filter => "Raw",
:scheduler_hint_data => {
:scheduler_hint_value => '{"key": "value"}'
it "formats well when set to Raw" do
args = {
:scheduler_hint_filter => "Raw",
:scheduler_hint_data => {
:scheduler_hint_value => '{"key": "value"}'
}
}
}
desired = {
:os_scheduler_hints => {
'key' => "value"
}
}
@compute_resource.format_scheduler_hint_filter(args)
assert_equal(desired, args)
end
desired = {
:os_scheduler_hints => {
'key' => "value"
}
}
@compute_resource.format_scheduler_hint_filter(args)
assert_equal(desired, args)
end
it "Should raise exception if set to Raw and malformed json" do
args = {
:scheduler_hint_filter => "Raw",
:scheduler_hint_data => {
:scheduler_hint_value => '{"key": }'
it "Should raise exception if set to Raw and malformed json" do
args = {
:scheduler_hint_filter => "Raw",
:scheduler_hint_data => {
:scheduler_hint_value => '{"key": }'
}
}
}
assert_raise ::JSON::ParserError do
@compute_resource.format_scheduler_hint_filter(args)
end
end
assert_raise ::JSON::ParserError do
@compute_resource.format_scheduler_hint_filter(args)
end
end
it "Should raise exception if no hint data provided" do
args = {
:scheduler_hint_filter => "Raw"
}
e = assert_raise(::Foreman::Exception) do
@compute_resource.format_scheduler_hint_filter(args)
it "Should raise exception if no hint data provided" do
args = {
:scheduler_hint_filter => "Raw"
}
e = assert_raise(::Foreman::Exception) do
@compute_resource.format_scheduler_hint_filter(args)
end
assert_equal("ERF42-4598 [Foreman::Exception]: Hint data is missing", e.message)
end
end
assert_equal("ERF42-4598 [Foreman::Exception]: Hint data is missing", e.message)
end
end
describe "find_vm_by_uuid" do
it "raises RecordNotFound when the vm does not exist" do
cr = mock_cr_servers(Foreman::Model::Openstack.new, empty_servers)
assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
end
end
describe "find_vm_by_uuid" do
it "raises RecordNotFound when the vm does not exist" do
cr = mock_cr_servers(Foreman::Model::Openstack.new, empty_servers)
assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
end
end
private
private
def mocked_key_pair
key_pair = mock
key_pair.stubs(:name).returns('foo_key')
key_pair
def mocked_key_pair
key_pair = mock
key_pair.stubs(:name).returns('foo_key')
key_pair
end
end
end
end
test/models/concerns/key_pair_compute_resource_test.rb
require 'test_helper'
class DummyComputeResource < ComputeResource
include KeyPairComputeResource
include Mocha::API
def client
@client ||= mock('client')
end
end
class KeyPairComputeResourceTest < ActiveSupport::TestCase
# We can't use 'Fog::Compute::AWS::KeyPair' model
# This class mocks it.
class FakeKeyPair
attr_reader :name, :fingerprint, :private_key
def initialize(name, fingerprint, private_key = nil)
@name = name
@fingerprint = fingerprint
@private_key = private_key
end
end
test "#get_compute_key_pairs" do
@key_pairs = []
3.times do |i|
@key_pairs << FakeKeyPair.new("foreman-#{i}", "13:01:73:0#{i}")
end
cr = DummyComputeResource.new
key_pair = FactoryGirl.build(:key_pair)
cr.key_pair = key_pair
DummyComputeResource.any_instance.stubs(:key_pairs).returns(@key_pairs)
assert_kind_of(ComputeResourceKeyPair, cr.get_compute_key_pairs.first)
end
test 'should remove the key pair on compute resource deletion' do
cr = DummyComputeResource.new
key_pair = FactoryGirl.build(:key_pair)
cr.key_pair = key_pair
mock_key_pairs = mock('mock_key_pairs')
fog_key_pair = mock('fog_key_pair')
cr.send(:client).expects(:key_pairs).returns(mock_key_pairs)
mock_key_pairs.expects(:get).with(key_pair.name).returns(fog_key_pair)
key_pair.expects(:destroy).once
fog_key_pair.expects(:destroy).once
assert cr.destroy!
end
end

Also available in: Unified diff