Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add notifications for VM destroy, Cloud Volume and Cloud Volume Snapshot actions #85

Merged
merged 5 commits into from
Oct 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ class ManageIQ::Providers::Openstack::CloudManager::CloudVolume < ::CloudVolume
include_concern 'Operations'

include SupportsFeatureMixin
include ManageIQ::Providers::Openstack::HelperMethods

supports :create
supports :backup_create
Expand All @@ -18,9 +19,14 @@ def self.raw_create_volume(ext_management_system, options)

# provide display_name for Cinder V1
options[:display_name] |= options[:name]
ext_management_system.with_provider_connection(cinder_connection_options(cloud_tenant)) do |service|
volume = service.volumes.new(options)
volume.save
with_notification(:cloud_volume_create,
:options => {
:volume_name => options[:name],
}) do
ext_management_system.with_provider_connection(cinder_connection_options(cloud_tenant)) do |service|
volume = service.volumes.new(options)
volume.save
end
end
{:ems_ref => volume.id, :status => volume.status, :name => options[:name]}
rescue => e
Expand All @@ -33,9 +39,14 @@ def validate_update_volume
end

def raw_update_volume(options)
with_provider_object do |volume|
volume.attributes.merge!(options)
volume.save
with_notification(:cloud_volume_update,
:options => {
:subject => self,
}) do
with_provider_object do |volume|
volume.attributes.merge!(options)
volume.save
end
end
rescue => e
_log.error "volume=[#{name}], error: #{e}"
Expand All @@ -52,7 +63,12 @@ def validate_delete_volume
end

def raw_delete_volume
with_provider_object(&:destroy)
with_notification(:cloud_volume_delete,
:options => {
:subject => self,
}) do
with_provider_object(&:destroy)
end
rescue => e
_log.error "volume=[#{name}], error: #{e}"
raise MiqException::MiqVolumeDeleteError, e.to_s, e.backtrace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,26 @@ def validate_detach_volume

def raw_attach_volume(server_ems_ref, device = nil)
device = nil if device.try(:empty?)
ext_management_system.with_provider_connection(connection_options) do |service|
service.servers.get(server_ems_ref).attach_volume(ems_ref, device)
with_notification(:cloud_volume_attach,
:options => {
:subject => self,
:instance_name => server_ems_ref,
}) do
ext_management_system.with_provider_connection(connection_options) do |service|
service.servers.get(server_ems_ref).attach_volume(ems_ref, device)
end
end
end

def raw_detach_volume(server_ems_ref)
ext_management_system.with_provider_connection(connection_options) do |service|
service.servers.get(server_ems_ref).detach_volume(ems_ref)
with_notification(:cloud_volume_detach,
:options => {
:subject => self,
:instance_name => server_ems_ref,
}) do
ext_management_system.with_provider_connection(connection_options) do |service|
service.servers.get(server_ems_ref).detach_volume(ems_ref)
end
end
end
end
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class ManageIQ::Providers::Openstack::CloudManager::CloudVolumeSnapshot < ::CloudVolumeSnapshot
include SupportsFeatureMixin
include ManageIQ::Providers::Openstack::HelperMethods

supports :create
supports :update
Expand Down Expand Up @@ -41,8 +42,14 @@ def self.create_snapshot(cloud_volume, options = {})
cloud_tenant = cloud_volume.cloud_tenant
snapshot = nil
options[:volume_id] = cloud_volume.ems_ref
ext_management_system.with_provider_connection(connection_options(cloud_tenant)) do |service|
snapshot = service.snapshots.create(options)
with_notification(:cloud_volume_snapshot_create,
:options => {
:snapshot_name => options[:name],
:volume_name => cloud_volume.name,
}) do
ext_management_system.with_provider_connection(connection_options(cloud_tenant)) do |service|
snapshot = service.snapshots.create(options)
end
end

create(
Expand Down Expand Up @@ -111,19 +118,24 @@ def delete_snapshot_queue(userid = "system", _options = {})
end

def delete_snapshot(_options = {})
with_provider_object do |snapshot|
if snapshot
snapshot.destroy
else
_log.warn "snapshot=[#{name}] already deleted"
with_notification(:cloud_volume_snapshot_delete,
:options => {
:subject => self,
:volume_name => cloud_volume.name,
}) do
with_provider_object do |snapshot|
if snapshot
snapshot.destroy
else
_log.warn("snapshot=[#{name}] already deleted")
end
end
end
rescue => e
_log.error "snapshot=[#{name}], error: #{e}"
raise MiqException::MiqVolumeSnapshotDeleteError, e.to_s, e.backtrace
end


def self.connection_options(cloud_tenant = nil)
connection_options = { :service => 'Volume' }
connection_options[:tenant_name] = cloud_tenant.name if cloud_tenant
Expand Down
2 changes: 2 additions & 0 deletions app/models/manageiq/providers/openstack/cloud_manager/vm.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ class ManageIQ::Providers::Openstack::CloudManager::Vm < ManageIQ::Providers::Cl
include_concern 'AssociateIp'
include_concern 'ManageSecurityGroups'

include ManageIQ::Providers::Openstack::HelperMethods

supports :smartstate_analysis do
feature_supported, reason = check_feature_support('smartstate_analysis')
unless feature_supported
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ module ManageIQ::Providers::Openstack::CloudManager::Vm::Operations

def raw_destroy
raise "VM has no #{ui_lookup(:table => "ext_management_systems")}, unable to destroy VM" unless ext_management_system
with_provider_object(&:destroy)
with_notification(:vm_destroy,
:options => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, the approach looks good to me.

I think, :instance_name => ... should not be wrapped in :options => ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact it should - with_notification uses keyword parameter :options not the ruby "options hash".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks, I forgot about this Ruby 2.0 update.

:subject => self,
}) do
with_provider_object(&:destroy)
end
self.update_attributes!(:raw_power_state => "DELETED")
end
end
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
module ManageIQ::Providers::Openstack::CloudManager::Vm::Operations::Relocation
extend ActiveSupport::Concern

include ManageIQ::Providers::Openstack::HelperMethods

included do
supports :live_migrate do
unsupported_reason_add(:live_migrate, unsupported_reason(:control)) unless supports_control?
Expand Down
24 changes: 24 additions & 0 deletions app/models/manageiq/providers/openstack/helper_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ def parse_error_message_from_neutron_response(exception)
self.class.parse_error_message_from_neutron_response(exception)
end

def with_notification(type, options: {}, &block)
self.class.with_notification(type, :options => options, &block)
end

module ClassMethods
def parse_error_message_from_fog_response(exception)
exception_string = exception.to_s
Expand All @@ -19,5 +23,25 @@ def parse_error_message_from_fog_response(exception)
def parse_error_message_from_neutron_response(exception)
JSON.parse(exception.response.body)["NeutronError"]["message"]
end

def with_notification(type, options: {})
# extract success and error options from options
# :success and :error keys respectively
# with all other keys common for both cases
success_options = options.delete(:success) || {}
error_options = options.delete(:error) || {}
success_options.merge!(options)
error_options.merge!(options)
begin
yield
rescue => ex
# Fog specific
error_message = parse_error_message_from_fog_response(ex.to_s)
Notification.create(:type => "#{type}_error".to_sym, :options => error_options.merge(:error_message => error_message))
raise
else
Notification.create(:type => "#{type}_success".to_sym, :options => success_options)
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@

describe "vm actions" do
context "#live_migrate" do
before do
NotificationType.seed
end

it "live migrates with default options" do
expect(handle).to receive(:live_migrate_server).with(vm.ems_ref, nil, false, false)
vm.live_migrate
Expand All @@ -72,10 +68,6 @@
end

context "evacuate" do
before do
NotificationType.seed
end

it "evacuates with default options" do
expect(handle).to receive(:evacuate_server).with(vm.ems_ref, nil, true, nil)
vm.evacuate
Expand Down
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
config.cassette_library_dir = File.join(ManageIQ::Providers::Openstack::Engine.root, 'spec/vcr_cassettes')
end

NotificationType.seed
Copy link
Member

@aufi aufi Oct 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be moved to https://github.com/ManageIQ/manageiq/blob/master/lib/evm_database.rb in following PR, no merge blocker. Otherwise LGTM.


Dir[Rails.root.join("spec/shared/**/*.rb")].each { |f| require f }
Dir[ManageIQ::Providers::Openstack::Engine.root.join("spec/support/**/*.rb")].each { |f| require f }

Expand Down