Revision 91f8ffb1
Added by Daniel Lobato Garcia almost 7 years ago
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
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.