[PATCH conductor] BZ 743677 - stop/terminate instances when disabling inaccessible provider
jprovazn at redhat.com
jprovazn at redhat.com
Wed Feb 1 10:35:38 UTC 2012
From: Jan Provaznik <jprovazn at redhat.com>
If a provider is not accessible and has running instances, allow a user to stop them
on conductor side (no matter what is their real state).
If a provider is accessible but stop request fails for any reason, the provider is not
disabled (before the provider was disabled in any case).
---
src/app/controllers/providers_controller.rb | 37 ++++++++++---
src/app/models/provider.rb | 58 ++++++++++++++------
.../views/providers/confirm_terminate.html.haml | 14 +++++
src/config/locales/en.yml | 8 +++
src/features/provider.feature | 12 ++++
src/features/step_definitions/provider_steps.rb | 9 +++
src/spec/models/provider_spec.rb | 30 ++++++++++-
7 files changed, 144 insertions(+), 24 deletions(-)
create mode 100644 src/app/views/providers/confirm_terminate.html.haml
diff --git a/src/app/controllers/providers_controller.rb b/src/app/controllers/providers_controller.rb
index 69252cc..969281c 100644
--- a/src/app/controllers/providers_controller.rb
+++ b/src/app/controllers/providers_controller.rb
@@ -119,14 +119,13 @@ class ProvidersController < ApplicationController
require_privilege(Privilege::MODIFY, @provider)
@provider.attributes = params[:provider]
provider_disabled = @provider.enabled_changed? && !@provider.enabled
+
+ if provider_disabled
+ disable_provider
+ return
+ end
+
if @provider.save
- # stop running instances when a provider is disabled
- # it would be better to have this in provider's observer
- # but we need to display error message when an error occurs
- if provider_disabled
- errs = @provider.stop_instances(current_user)
- flash[:error] = errs unless errs.empty?
- end
flash[:notice] = t"providers.flash.notice.updated"
redirect_to edit_provider_path(@provider)
else
@@ -177,6 +176,30 @@ class ProvidersController < ApplicationController
@providers = Provider.list_for_user(current_user, Privilege::VIEW)
end
+ def disable_provider
+ @instances_to_terminate = @provider.instances_to_terminate
+ if @instances_to_terminate.any? and not params[:terminate]
+ render :action => "confirm_terminate"
+ return
+ end
+
+ res = @provider.disable(current_user)
+ if res[:failed_to_stop].present?
+ flash[:error] = {
+ :summary => t("providers.flash.warning.not_stopped_instances"),
+ :failures => res[:failed_to_stop]
+ }
+ elsif res[:failed_to_terminate].present?
+ flash[:error] = {
+ :summary => t("providers.flash.warning.not_terminated_instances"),
+ :failures => res[:failed_to_terminate].map {|i| i.name}
+ }
+ else
+ flash[:notice] = t"providers.flash.notice.disabled"
+ end
+ redirect_to edit_provider_path(@provider)
+ end
+
def provider_alerts(provider)
alerts = []
diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb
index 01c0d3e..44f403c 100644
--- a/src/app/models/provider.rb
+++ b/src/app/models/provider.rb
@@ -108,25 +108,48 @@ class Provider < ActiveRecord::Base
def supported_types
end
+ def disable(user)
+ res = {}
+ if valid_framework?
+ # if we can connect to the provider, try to stop running instances
+ # TODO: now provider is disabled even if stop request fails, is it ok?
+ res[:failed_to_stop] = stop_instances(user)
+ else
+ # if the provider is not accessible and there are no running
+ # instances, we just change state of all instances to stopped
+ res[:failed_to_terminate] = stoppable_instances.select do |i|
+ !i.update_attributes(:state => Instance::STATE_STOPPED)
+ end
+ end
+ if res[:failed_to_stop].blank? and res[:failed_to_terminate].blank?
+ update_attribute(:enabled, false)
+ end
+ res
+ end
+
+ def instances_to_terminate
+ valid_framework? ? [] : stoppable_instances
+ end
+
+ protected
+
def stop_instances(user)
errs = []
- provider_accounts.each do |pa|
- pa.instances.stopable.each do |instance|
- begin
- unless instance.valid_action?('stop')
- raise "stop is an invalid action."
- end
+ stoppable_instances.each do |instance|
+ begin
+ unless instance.valid_action?('stop')
+ raise "stop is an invalid action."
+ end
- unless @task = instance.queue_action(user, 'stop')
- raise "stop cannot be performed on this instance."
- end
- Taskomatic.stop_instance(@task)
- rescue Exception => e
- err = "Error while stopping an instance #{instance.name}: #{e.message}"
- errs << err
- logger.error err
- logger.error e.backtrace.join("\n ")
+ unless @task = instance.queue_action(user, 'stop')
+ raise "stop cannot be performed on this instance."
end
+ Taskomatic.stop_instance(@task)
+ rescue Exception => e
+ err = "Error while stopping an instance #{instance.name}: #{e.message}"
+ errs << err
+ logger.error err
+ logger.error e.backtrace.join("\n ")
end
end
errs
@@ -192,7 +215,10 @@ class Provider < ActiveRecord::Base
end
- protected
+ def stoppable_instances
+ provider_accounts.inject([]) {|all, pa| all += pa.instances.stopable}
+ end
+
def validate_provider
if !nil_or_empty(url)
errors.add("url", "must be a valid provider url") unless valid_framework?
diff --git a/src/app/views/providers/confirm_terminate.html.haml b/src/app/views/providers/confirm_terminate.html.haml
new file mode 100644
index 0000000..9df53f5
--- /dev/null
+++ b/src/app/views/providers/confirm_terminate.html.haml
@@ -0,0 +1,14 @@
+= render :partial => 'layouts/admin_nav'
+= render :partial => 'section_header'
+
+%section.admin-content-section
+ %header
+ %h2= t(".disable_provider?")
+
+ .content
+ = t ".terminate_instances"
+ %ul
+ - @instances_to_terminate.each do |i|
+ %li= i.name
+ = link_to t('cancel'), edit_provider_path(@provider), :class => 'button'
+ = button_to "Disable", provider_path(@provider, :terminate => true, :provider => {:enabled => false}), :class => 'button', :method => :put, :id => 'disable_button'
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 2d1749a..0467072 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -973,18 +973,26 @@ en:
test_connection: Test Connection
required_field: Required field.
caution_image: Caution
+ confirm_terminate:
+ terminate_instances: "Provider is not accessible, status of following instances will be changed to 'stopped' but their actual state is unknown."
+ disabled_provider: "Disable Provider"
+ disable: "Disable"
flash:
notice:
added: "Provider added."
updated: "Provider updated."
+ disabled: "Provider disabled."
connected: "Successfully Connected to Provider"
deleted: "Provider has been successfully deleted."
error:
not_added: "Cannot add the provider."
not_updated: "Cannot update the provider."
+ not_disabled: "Cannot disabled the provider."
not_deleted: "Provider was not deleted"
warning:
connect_failed: "Failed to connect to Provider!"
+ not_stopped_instances: "Provider was not disabled, failed to stop following instances:"
+ not_terminated_instances: "Provider was not disabled, failed to change status to 'stopped' for following instances:"
alerts:
type:
critical: Critical
diff --git a/src/features/provider.feature b/src/features/provider.feature
index bcce43c..85a4568 100644
--- a/src/features/provider.feature
+++ b/src/features/provider.feature
@@ -92,6 +92,18 @@ Feature: Manage Providers
And I should see "Provider is disabled."
And I should not see "Error while stopping an instance"
+ Scenario: Disable an inaccessible provider
+ Given there is a provider named "provider1"
+ And this provider has a provider account with 2 running instances
+ And provider "provider1" is not accessible
+ When I go to the provider1's edit provider page
+ And I press "provider_submit"
+ Then I should see "Provider is not accessible, status of following instances will be changed to 'stopped'"
+ When I press "disable_button"
+ Then I should be on the provider1's edit provider page
+ And I should see "Provider is disabled."
+ And provider "provider1" should have all instances stopped
+
Scenario: Persist selected provider
Given there is a provider named "provider1"
And there is a provider named "provider2"
diff --git a/src/features/step_definitions/provider_steps.rb b/src/features/step_definitions/provider_steps.rb
index 9699211..fc25f87 100644
--- a/src/features/step_definitions/provider_steps.rb
+++ b/src/features/step_definitions/provider_steps.rb
@@ -26,6 +26,10 @@ Given /^there is a provider named "([^\"]*)"$/ do |name|
@provider = FactoryGirl.create(:mock_provider, :name => name)
end
+Given /^provider "([^"]*)" is not accessible$/ do |arg1|
+ Provider.any_instance.stub(:valid_framework?).and_return(false)
+end
+
Then /^I should have a provider named "([^\"]*)"$/ do |name|
Provider.find_by_name(name).should_not be_nil
end
@@ -117,3 +121,8 @@ end
When /^I click on the Providers icon in the menu$/ do
find('#administer_nav a.providers').click
end
+
+Then /^provider "([^"]*)" should have all instances stopped$/ do |arg1|
+ p = Provider.find_by_name(arg1)
+ p.instances_to_terminate.should be_empty
+end
diff --git a/src/spec/models/provider_spec.rb b/src/spec/models/provider_spec.rb
index fbb4729..ca6843e 100644
--- a/src/spec/models/provider_spec.rb
+++ b/src/spec/models/provider_spec.rb
@@ -104,10 +104,20 @@ describe Provider do
pa = FactoryGirl.create(:mock_provider_account, :provider => provider1)
inst1 = FactoryGirl.create(:instance, :provider_account => pa)
inst2 = FactoryGirl.create(:instance, :provider_account => pa)
- errs = provider1.stop_instances(nil)
+ errs = provider1.disable(nil)[:failed_to_stop]
errs.should be_empty
end
+ it "should not disable provider if instance stop action fails" do
+ provider1 = Factory.create(:mock_provider)
+ pa = FactoryGirl.create(:mock_provider_account, :provider => provider1)
+ inst1 = FactoryGirl.create(:instance, :provider_account => pa)
+ Instance.any_instance.stub(:valid_action?).and_return(false)
+ errs = provider1.disable(nil)[:failed_to_stop]
+ errs.should_not be_empty
+ provider1.enabled.should be_true
+ end
+
end
context "(using original connect method)" do
@@ -122,4 +132,22 @@ describe Provider do
end
end
+ context "(inaccessible provider with running instance)" do
+ before(:each) do
+ @provider = FactoryGirl.create(:mock_provider)
+ @provider.stub!(:valid_framework?).and_return(false)
+ acc = FactoryGirl.create(:mock_provider_account)
+ inst = FactoryGirl.create(:instance, :provider_account => acc)
+ @provider.provider_accounts << acc
+ end
+
+ it "should return list of instances to terminate" do
+ @provider.instances_to_terminate.should_not be_empty
+ end
+
+ it "should return empty list of not terminated instances when disabled" do
+ @provider.disable(nil)[:failed_to_terminate].should be_empty
+ end
+ end
+
end
--
1.7.7.5
More information about the aeolus-devel
mailing list