On 22/02/12 10:10 +0100, jprovazn(a)redhat.com wrote: > From: Jan Provaznik <jprovazn(a)redhat.com&gt; > > Theis patch allows locally stopping of instances or deployments if > the instance's/deployment's provider is not accessible. > > It fixes a bug in passing user param to queue_action method, also > simplifies > used error messages - only (stop|reboot)_invalid_action can be raised. > > This patch doesn't cover deletion of deployments which are not > accessible - this is difficult > because there is a before_destroy hook on deployment which tries to > destroy instances > on provider side. > --- > src/app/controllers/deployments_controller.rb | 43 ++++++++---- > src/app/controllers/instances_controller.rb | 52 +++++++------- > src/app/models/deployment.rb | 17 +++++ > src/app/models/instance.rb | 35 ++++++++-- > src/app/models/provider.rb | 15 +++- > src/app/stylesheets/custom.scss | 2 +- > .../views/deployments/confirm_terminate.html.haml | 16 +++++ > .../views/instances/confirm_terminate.html.haml | 16 +++++ > .../views/providers/confirm_terminate.html.haml | 5 +- > src/config/database.yml.0 | 72 > ++++++++++++++++++++ > src/config/locales/cs.yml | 1 - > src/config/locales/en.yml | 12 ++-- > src/features/deployment.feature | 12 +++ > src/features/instance.feature | 11 +++ > src/features/step_definitions/deployment_steps.rb | 7 ++ > src/features/step_definitions/instance_steps.rb | 8 ++ > 16 files changed, 265 insertions(+), 59 deletions(-) > create mode 100644 src/app/views/deployments/confirm_terminate.html.haml > create mode 100644 src/app/views/instances/confirm_terminate.html.haml > create mode 100644 src/config/database.yml.0 > > diff --git a/src/app/controllers/deployments_controller.rb > b/src/app/controllers/deployments_controller.rb > index 588e88e..15050fc 100644 > --- a/src/app/controllers/deployments_controller.rb > +++ b/src/app/controllers/deployments_controller.rb > @@ -18,6 +18,7 @@ class DeploymentsController < ApplicationController > before_filter :require_user > before_filter :load_deployments, :only => [:index, :show] > before_filter :load_deployment, :only => [:edit, :update] > + before_filter :check_inaccessible_instances, :only => :multi_stop > > > viewstate :show do |default| > @@ -267,6 +268,7 @@ class DeploymentsController < ApplicationController > def multi_destroy > destroyed = [] > failed = [] > + > Deployment.find(params[:deployments_selected] || []).each do > |deployment| > if check_privilege(Privilege::MODIFY, deployment) > begin > @@ -296,28 +298,28 @@ class DeploymentsController < > ApplicationController > def multi_stop > notices = [] > errors = [] > - Deployment.find(params[:deployments_selected] || []).each do > |deployment| > + > + @deployments_to_stop.each do |deployment| > deployment.instances.each do |instance| > + log_prefix = "#{t('deployments.deployment')}: > #{instance.deployment.name}, #{t('instances.instance')}: > #{instance.name}" > begin > - require_privilege(Privilege::USE,instance) > - unless instance.valid_action?('stop') > - raise > ActionError.new(t('deployments.errors.stop_invalid_action')) > - end > - > - #permissons check here > - @task = instance.queue_action(current_user, 'stop') > - unless @task > - raise ActionError.new(t('deployments.errors.cannot_stop')) > + require_privilege(Privilege::USE, instance) > + if @inaccessible_instances.include?(instance) > + instance.force_stop(current_user) > + notices << "#{log_prefix}: > #{t('instances.flash.notice.forced_stop')}" > + else > + instance.stop(current_user) > + notices << "#{log_prefix}: > #{t('instances.flash.notice.stop')}" > end > - Taskomatic.stop_instance(@task) > - notices << "#{t('deployments.deployment')}: > #{instance.deployment.name}, #{t('instances.instance')}: > #{instance.name}: #{t('deployments.flash.notice.stop')}" > rescue Exception => err > - errors << "#{t('deployments.deployment')}: > #{instance.deployment.name}, #{t('instances.instance')}: > #{instance.name}: " + err > + errors << "#{log_prefix}: #{err}" > + logger.error err.message > + logger.error err.backtrace.join("\n ") > end > end > end > # If nothing is selected, display an error message: > - errors = t('deployments.flash.error.none_selected') if > errors.blank? && notices.blank? > + errors = t('deployments.flash.error.none_selected') if > notices.blank? and errors.blank? > flash[:notice] = notices unless notices.blank? > flash[:error] = errors unless errors.blank? > respond_to do |format| > @@ -350,6 +352,19 @@ class DeploymentsController < ApplicationController > > private > > + def check_inaccessible_instances > + @deployments_to_stop = > Deployment.find(params[:deployments_selected] || []) > + @inaccessible_instances = > Deployment.stoppable_inaccessible_instances(@deployments_to_stop) > + if params[:terminate].blank? and @inaccessible_instances.any? > + respond_to do |format| > + format.html { render :action => :confirm_terminate } > + format.json { render :json => {:inaccessbile_instances => > @inaccessible_instances}, :status => :unprocessable_entity } > + end > + return false > + end > + return true > + end > + > def load_deployments > @deployments_header = [ > { :name => 'checkbox', :class => 'checkbox', :sortable => false }, > diff --git a/src/app/controllers/instances_controller.rb > b/src/app/controllers/instances_controller.rb > index a44e7be..987ff9f 100644 > --- a/src/app/controllers/instances_controller.rb > +++ b/src/app/controllers/instances_controller.rb > @@ -18,6 +18,7 @@ class InstancesController < ApplicationController > before_filter :require_user > before_filter :load_instance, :only => [:show, :key, :edit, :update] > before_filter :set_view_vars, :only => [:show, :index, :export_events] > + before_filter :check_inaccessible_instances, :only => :multi_stop > > def index > @params = params > @@ -139,27 +140,26 @@ class InstancesController < ApplicationController > end > > def multi_stop > - notices = "" > - errors = "" > - Instance.find(params[:instance_selected] || []).each do |instance| > + notices = [] > + errors = [] > + > + @instances_to_stop.each do |instance| > begin > require_privilege(Privilege::USE,instance) > - unless instance.valid_action?('stop') > - raise ActionError.new(t('instances.errors.stop_invalid')) > - end > > - #permissons check here > - @task = instance.queue_action(current_user, 'stop') > - unless @task > - raise > ActionError.new(t('instances.errors.stop_not_be_performed')) > + if @inaccessible_instances.include?(instance) > + instance.force_stop(current_user) > + notices << "#{instance.name}: > #{t('instances.flash.notice.forced_stop')}" > + else > + instance.stop(current_user) > + notices << "#{instance.name}: > #{t('instances.flash.notice.stop')}" > end > - Taskomatic.stop_instance(@task) > - notices << "#{instance.name}: > #{t('instances.flash.notice.stop')}" > rescue Exception => err > errors << "#{instance.name}: " + err > + logger.error err.message > + logger.error err.backtrace.join("\n ") > end > end > - # If nothing is selected, display an error message: > errors = t('instances.none_selected') if errors.blank? && > notices.blank? > flash[:notice] = notices unless notices.blank? > flash[:error] = errors unless errors.blank? > @@ -176,11 +176,11 @@ class InstancesController < ApplicationController > end > > def stop > - do_operation(:stop) > + do_operation(current_user, :stop) > end > > def reboot > - do_operation(:reboot) > + do_operation(current_user, :reboot) > end > > def multi_reboot > @@ -189,7 +189,7 @@ class InstancesController < ApplicationController > Instance.find(params[:instance_selected] || []).each do |instance| > begin > require_privilege(Privilege::USE,instance) > - instance.reboot > + instance.reboot(current_user) > notices << "#{instance.name}: > #{t('instances.flash.notice.reboot', :name => instance.name)}" > rescue Exception => err > errors << "#{instance.name}: " + err > @@ -250,16 +250,16 @@ class InstancesController < ApplicationController > end > end > > - def do_operation(operation) > - instance = Instance.find(params[:id]) > - begin > - instance.send(operation) > - flash[:notice] = t("instances.flash.notice.#{operation}", > :name => instance.name) > - rescue Exception => err > - flash[:error] = t('instance.error', :name => instance.name, > :err => err) > - end > - respond_to do |format| > - format.html { redirect_to deployment_path(instance.deployment, > :details_tab => 'instances')} > + def check_inaccessible_instances > + @instances_to_stop = Instance.find(params[:instance_selected] || > []) > + @inaccessible_instances = > Instance.stoppable_inaccessible_instances(@instances_to_stop) > + if params[:terminate].blank? and @inaccessible_instances.any? > + respond_to do |format| > + format.html { render :action => :confirm_terminate } > + format.json { render :json => {:inaccessbile_instances => > @inaccessible_instances}, :status => :unprocessable_entity } > + end > + return false > end > + return true > end > end > diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb > index 9593645..880afdd 100644 > --- a/src/app/models/deployment.rb > +++ b/src/app/models/deployment.rb > @@ -171,6 +171,18 @@ class Deployment < ActiveRecord::Base > end > end > > + def self.stoppable_inaccessible_instances(deployments) > + failed_accounts = {} > + res = [] > + deployments.each do |d| > + next unless acc = d.provider_account > + failed_accounts[acc.id] = acc.connect.nil? unless > failed_accounts.has_key?(acc.id) > + next unless failed_accounts[acc.id] > + res += d.instances.stoppable_inaccessible > + end > + res > + end > + > def launch_parameters > @launch_parameters ||= {} > end > @@ -329,6 +341,11 @@ class Deployment < ActiveRecord::Base > inst && inst.provider_account && inst.provider_account.provider > end > > + def provider_account > + inst = instances.joins(:provider_account).first > + inst && inst.provider_account > + end > + > # we try to create an instance for each assembly and check > # if a match is found > def check_assemblies_matches(user) > diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb > index 0042074..e6ce567 100644 > --- a/src/app/models/instance.rb > +++ b/src/app/models/instance.rb > @@ -114,6 +114,8 @@ class Instance < ActiveRecord::Base > STATE_SHUTTING_DOWN, STATE_STOPPED, STATE_CREATE_FAILED, > STATE_ERROR, STATE_VANISHED] > > + STOPPABLE_INACCESSIBLE_STATES = [STATE_NEW, STATE_PENDING, > STATE_RUNNING, STATE_SHUTTING_DOWN] > + > scope :deployed, :conditions => { :state => [STATE_RUNNING, > STATE_SHUTTING_DOWN] } > # FIXME: "pending" is misleading as it doesn't just cover > STATE_PENDING > scope :pending, :conditions => { :state => [STATE_NEW, > STATE_PENDING] } > @@ -122,6 +124,7 @@ class Instance < ActiveRecord::Base > scope :stopped, :conditions => {:state => STATE_STOPPED} > scope :not_stopped, :conditions => "state <> 'stopped'" > scope :stopable, :conditions => { :state => [STATE_NEW, > STATE_PENDING, STATE_RUNNING] } > + scope :stoppable_inaccessible, :conditions => { :state => > STOPPABLE_INACCESSIBLE_STATES } > scope :ascending_by_name, :order => 'instances.name ASC' > > > @@ -434,12 +437,20 @@ class Instance < ActiveRecord::Base > not deployment.instances.deployed.any? {|i| i != self} > end > > - def stop > - do_operation('stop') > + def stop(user) > + do_operation(user, 'stop') > + end > + > + def reboot(user) > + do_operation(user, 'reboot') > end > > - def reboot > - do_operation('reboot') > + def force_stop(user) > + self.state = STATE_STOPPED > + save! > + event = Event.create!(:source => self, :event_time => Time.now, > + :summary => "Instance is not accessible, > state changed to stopped", > + :status_code => "force_stop") > end > > def deployed? > @@ -466,6 +477,16 @@ class Instance < ActiveRecord::Base > end > end > > + def self.stoppable_inaccessible_instances(instances) > + failed_accounts = {} > + instances.select do |i| > + next unless STOPPABLE_INACCESSIBLE_STATES.include?(i.state) > + next unless i.provider_account > + failed_accounts[i.provider_account.id] = > i.provider_account.connect.nil? unless > failed_accounts.has_key?(i.provider_account.id) > + failed_accounts[i.provider_account.id] > + end > + end > + > private > > def self.apply_search_filter(search) > @@ -488,10 +509,10 @@ class Instance < ActiveRecord::Base > self[:uuid] = UUIDTools::UUID.timestamp_create.to_s > end > > - def do_operation(operation) > - @task = self.queue_action(@current_user, operation) > + def do_operation(user, operation) > + @task = self.queue_action(user, operation) > unless @task > - raise I18n.t("instances.errors.#{operation}_not_be_performed") > + raise I18n.t("instances.errors.#{operation}_invalid_action") > end > Taskomatic.send("#{operation}_instance", @task) > end > diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb > index 22e4e95..9d5877c 100644 > --- a/src/app/models/provider.rb > +++ b/src/app/models/provider.rb > @@ -117,8 +117,16 @@ class Provider < ActiveRecord::Base > 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) > + res[:failed_to_terminate] = instances_to_terminate.select do |i| > + begin > + i.update_attributes(:state => Instance::STATE_STOPPED) > + false > + rescue > + true > + # this should never happen, so display an error only in > log file > + logger.warn "failed to stop instance #{i.name}: > #{$!.message}" > + logger.warn $!.backtrace.join("\n ") > + end > end > end > if res[:failed_to_stop].blank? and res[:failed_to_terminate].blank? > @@ -128,7 +136,8 @@ class Provider < ActiveRecord::Base > end > > def instances_to_terminate > - valid_framework? ? [] : stoppable_instances > + return [] if valid_framework? > + provider_accounts.inject([]) {|all, pa| all += > pa.instances.stoppable_inaccessible} > end > > def populate_realms > diff --git a/src/app/stylesheets/custom.scss > b/src/app/stylesheets/custom.scss > index ee1ce34..be3e1ec 100644 > --- a/src/app/stylesheets/custom.scss > +++ b/src/app/stylesheets/custom.scss > @@ -245,7 +245,7 @@ form.generic section.services { > margin-bottom: 30px; > } > > -.instance_details { > +.instance_details, .confirm_instances_termination { > > h3{ > font-size: 22px; > diff --git a/src/app/views/deployments/confirm_terminate.html.haml > b/src/app/views/deployments/confirm_terminate.html.haml > new file mode 100644 > index 0000000..2ac762f > --- /dev/null > +++ b/src/app/views/deployments/confirm_terminate.html.haml > @@ -0,0 +1,16 @@ > +%header.page-header > + %h1{:class => controller.controller_name}= t > "instances.confirm_terminate.terminate_instances" > + .corner &nbsp; > + > +%section.content-section > + .content.confirm_instances_termination > + .align-center > + %strong= > t('instances.confirm_terminate.terminate_instances_description') > + = form_tag multi_stop_deployments_path, :method => :post do > + - @deployments_to_stop.each do |i| > + = hidden_field_tag 'deployments_selected[]', i.id > + %ul > + - @inaccessible_instances.each do |i| > + %li= i.name > + = link_to t('cancel'), pools_path(:view => 'filter', > :details_tab => 'deployments'), :class => 'button' > + = submit_tag t('instances.confirm_terminate.terminate'), :name > => 'terminate', :class => 'button', :id => 'terminate_button' > diff --git a/src/app/views/instances/confirm_terminate.html.haml > b/src/app/views/instances/confirm_terminate.html.haml > new file mode 100644 > index 0000000..ea680f2 > --- /dev/null > +++ b/src/app/views/instances/confirm_terminate.html.haml > @@ -0,0 +1,16 @@ > +%header.page-header > + %h1{:class => controller.controller_name}= t ".terminate_instances" > + .corner &nbsp; > + > +%section.content-section > + .content.confirm_instances_termination > + .align-center > + %strong= t('.terminate_instances_description') > + = form_tag multi_stop_instances_path, :method => :post do > + - @instances_to_stop.each do |i| > + = hidden_field_tag 'instance_selected[]', i.id > + %ul > + - @inaccessible_instances.each do |i| > + %li= i.name > + = link_to t('cancel'), pools_path(:view => 'filter', > :details_tab => 'instances'), :class => 'button' > + = submit_tag t('.terminate'), :name => 'terminate', :class => > 'button', :id => 'terminate_button' > diff --git a/src/app/views/providers/confirm_terminate.html.haml > b/src/app/views/providers/confirm_terminate.html.haml > index 9df53f5..ef60061 100644 > --- a/src/app/views/providers/confirm_terminate.html.haml > +++ b/src/app/views/providers/confirm_terminate.html.haml > @@ -5,8 +5,9 @@ > %header > %h2= t(".disable_provider?") > > - .content > - = t ".terminate_instances" > + .content.confirm_instances_termination > + .align-center > + %strong= > t('instances.confirm_terminate.terminate_instances_description') > %ul > - @instances_to_terminate.each do |i| > %li= i.name > diff --git a/src/config/database.yml.0 b/src/config/database.yml.0 > new file mode 100644 > index 0000000..2deb8ca > --- /dev/null > +++ b/src/config/database.yml.0 > @@ -0,0 +1,72 @@ > +# PostgresSQL. Versions 8.2 recommended. > +# > +# Fedora Install Instructions: > +# sudo yum install ruby-postgres postgresql-server > +# sudo /sbin/service postgresql initdb > +# > +# On a local system you may need to edit this file to have only > the following > +# /var/lib/pgsql/data/pg_hba.conf > +# local all all trust > +# host all all 127.0.0.1/32 trust > +# > +# sudo /sbin/service postgresql start > +# sudo su - postgres > +# $ psql > +# $-# CREATE USER aeolus WITH PASSWORD 'v23zj59an' CREATEDB; > +# $-# \q > +# $ exit > +# > +# To start off a new environment, use the following commands. > +# If running from /usr/share/aeolus-conductor, use 'sudo' or run as > root > +# as the migrate will write to a file. > +# > +# rake db:drop:all > +# rake db:create:all > +# > +# If you are running this in /usr/share/aeolus-conductor, use: > +# sudo RAILS_ENV=production rake db:migrate > +# > +# or else just: > +# rake db:migrate > +# > +# rake db:test:prepare > +# > +# And when there are changes to the database, simply run: > +# > +# rake db:migrate > +# rake db:test:prepare > +# for the environment you wish to update. > +# > + > + > +development: > + adapter: postgresql > + database: conductor_development > + username: aeolus > + password: v23zj59an > + host: localhost > + allow_concurrency: true > + min_messages: warning > + > +# Warning: The database defined as 'test' will be erased and > +# re-generated from your development database when you run 'rake test'. > +# Do not set this db to the same as development or production. > +test: &TEST > + adapter: postgresql > + database: conductor_test > + username: aeolus > + password: v23zj59an > + host: localhost > + allow_concurrency: true > + min_messages: warning > + > +production: > + adapter: postgresql > + database: conductor > + username: aeolus > + password: v23zj59an > + host: localhost > + allow_concurrency: true > + > +cucumber: > + <<: *TEST > diff --git a/src/config/locales/cs.yml b/src/config/locales/cs.yml > index 7ada0cc..6b0f7b4 100644 > --- a/src/config/locales/cs.yml > +++ b/src/config/locales/cs.yml > @@ -476,7 +476,6 @@ cs: > invalid_state: okhbeh kigib bog mhknokdmj klonm kcolb ndjhmhk > neafi > no_config_server_available: "%{account_name}: ae hmenoo lidmgk > mboinakkm boc cnfcjbli abicnag" > user_quota_reached: kone bdgjo kbggdbl > - stop_not_be_performed: cdim lfnggh nk ogldenhne gk mhde makmngjfl > provider_account_quota_too_low: "%{match_provider_account} > ifcjl omcdh lkm dke gi gkcomo efehjbfgnl" > provider_account_quota_reached: "%{account_name}: boefladm > ebbjhmd caidj klmghik" > image_not_pushed_to_provider: "%{account_name}: ofeif km fef > minkbc gc bcei gncmokeo hgeickk" > diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml > index addb68e..2e4f8c9 100644 > --- a/src/config/locales/en.yml > +++ b/src/config/locales/en.yml > @@ -279,7 +279,6 @@ en: > other: "The deployments %{list} were successfully updated." > errors: > config_server_connection: 'Cannot connect to the config server' > - stop_invalid_action: "stop is an invalid action." > cannot_stop: "stop cannot be performed on this instance." > all_stopped: "All instances must be stopped or running" > not_valid_deployable_xml: "seems to be not valid deployable > XML: %{msg}" > @@ -343,6 +342,10 @@ en: > shared_to: "Shared to: " > error: "Error: " > n_a: "N/A" > + confirm_terminate: > + terminate: "Terminate" > + terminate_instances: "Terminate Instances" > + terminate_instances_description: "Provider is not accessible, > status of following instances will be changed to 'stopped' but their > actual state is unknown." > flash: > warning: > ssh_key_not_found: "SSH Key not found for this Instance." > @@ -363,6 +366,7 @@ en: > other: "The instances %{list} were successfully deleted." > notice: > stop: "stop action was successfully queued." > + forced_stop: "state changed to stopped." > reboot: "%{name}: reboot action was successfully queued." > errors: > image_not_found: "No image build was found with uuid %{b_uuid} > and no image was found with uuid %{i_uuid}" > @@ -378,9 +382,8 @@ en: > no_config_server_available: "%{account_name}: no config server > available for provider account" > realm_not_mapped: "%{account_name}: realm > %{frontend_realm_name} is not mapped to any applicable provider or > provider realm" > provider_account_quota_too_low: "%{match_provider_account} > quota limit too low to launch deployable" > - stop_invalid: "stop is an invalid action." > - stop_not_be_performed: "stop cannot be performed on this > instance." > - reboot_not_be_performed: "cannot be rebooted because it's not > currently running." > + stop_invalid_action: "stop is an invalid action." > + reboot_invalid_action: "reboot is an invalid action." > must_be_enabled: "%{account_name}: provider must be enabled" > provider_not_available: "%{account_name}: provider is not > available" > cannot_destroy: "destroy cannot be performed on this instance." > @@ -1007,7 +1010,6 @@ en: > 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: > diff --git a/src/features/deployment.feature > b/src/features/deployment.feature > index 7b6a663..b9e195e 100644 > --- a/src/features/deployment.feature > +++ b/src/features/deployment.feature > @@ -82,6 +82,18 @@ Feature: Manage Deployments > And I press "stop_button" > Then I should see "testdeployment" > > + Scenario: Stop inaccessible deployments > + Given there is a deployment named "testdeployment" belonging to > "testdeployable" owned by "testuser" > + And "testdeployment" deployment's provider is not accessible > + When I go to the deployments page > + Then I should see "testdeployment" > + When I check "testdeployment" deployment > + And I press "stop_button" > + Then I should see "Terminate Instances" > + When I press "terminate_button" > + Then I should see "state changed to stopped" > + > + > Scenario: Stop a deployment over XHR > Given there is a deployment named "testdeployment" belonging to > "testdeployable" owned by "testuser" > And I request XHR > diff --git a/src/features/instance.feature > b/src/features/instance.feature > index 1d91333..446ee53 100644 > --- a/src/features/instance.feature > +++ b/src/features/instance.feature > @@ -71,6 +71,17 @@ Feature: Manage Instances > Then I should be on the instances page > And I should see "mock1: stop action was successfully queued" > > + Scenario: Stop inaccessible instance > + Given there is a "mock1" running instance > + And I am on the instances page > + And "mock1" instance's provider is not accessible > + When I check "mock1" instance > + And I press "stop_selected_instances" > + Then I should see "Terminate Instances" > + And I should see "mock1" > + When I press "terminate_button" > + Then I should see "mock1: state changed to stopped" > + > Scenario: Stop multiple instances > Given there is a "mock1" running instance > And there is a "mock2" running instance > diff --git a/src/features/step_definitions/deployment_steps.rb > b/src/features/step_definitions/deployment_steps.rb > index 5eedc1d..0640d9d 100644 > --- a/src/features/step_definitions/deployment_steps.rb > +++ b/src/features/step_definitions/deployment_steps.rb > @@ -38,6 +38,13 @@ Given /^the deployment "([^"]*)" has an instance > named "([^"]*)"$/ do |d_name, i > deployment.instances << FactoryGirl.create(:instance, :name => > i_name, :pool => deployment.pool) > end > > +Given /^"([^"]*)" deployment's provider is not accessible$/ do |arg1| > + # FIXME: didn't find a way how to create an inaccessible provider > + # cleanly > + provider = @deployment.provider > + provider.update_attribute(:url, 'http://localhost:3002/invalid_api') > +end > + > When /^I am viewing the deployment "([^"]*)"$/ do |arg1| > visit deployment_url(Deployment.find_by_name(arg1)) > end > diff --git a/src/features/step_definitions/instance_steps.rb > b/src/features/step_definitions/instance_steps.rb > index 268fcdb..5f4d3bb 100644 > --- a/src/features/step_definitions/instance_steps.rb > +++ b/src/features/step_definitions/instance_steps.rb > @@ -39,6 +39,14 @@ Given /^the instance "([^"]*)" is in the (\w*) > state$/ do |instance, state| > instance.save! > end > > +Given /^"([^"]*)" instance's provider is not accessible$/ do |arg1| > + instance = Instance.find_by_name(arg1) > + # FIXME: didn't find a way how to create an inaccessible provider > + # cleanly > + provider = instance.provider_account.provider > + provider.update_attribute(:url, 'http://localhost:3002/invalid_api') > +end > + > When /^I am viewing the pending instance detail$/ do > visit instance_url(pending_instance) > end > -- > 1.7.7.6 > ACK, worked like a champ

On 02/24/2012 10:46 PM, Jim Meyering wrote: > jprovazn(a)redhat.com wrote: >> From: Jan Provaznik&lt;jprovazn(a)redhat.com&gt; >> >> Theis patch allows locally stopping of instances or deployments if >> the instance's/deployment's provider is not accessible. >> >> It fixes a bug in passing user param to queue_action method, also simplifies >> used error messages - only (stop|reboot)_invalid_action can be raised. >> >> This patch doesn't cover deletion of deployments which are not >> accessible - this is difficult >> because there is a before_destroy hook on deployment which tries to >> destroy instances >> on provider side. > ... > > Hi Jan, > > I noticed that these regexps can be simplified: > >> diff --git a/src/features/step_definitions/deployment_steps.rb >> b/src/features/step_definitions/deployment_steps.rb > ... >> +Given /^"([^"]*)" deployment's provider is not accessible$/ do |arg1| > > Using non-greedy matching like this is equivalent, and more readable: > > Given /^"(.*?)" deployment's provider is not accessible$/ do |arg1| > > ... >> diff --git a/src/features/step_definitions/instance_steps.rb >> b/src/features/step_definitions/instance_steps.rb > ... >> +Given /^"([^"]*)" instance's provider is not accessible$/ do |arg1| > > Given /^"(.*?)" instance's provider is not accessible$/ do |arg1| Hi Jim, this is nice tip. Cucumber pregenerates current version (Given /^([^"]*)"" ...$/ do |arg1|) and prints it on stderr when it finds a test with missing step definition - so a user can just copy&paste this into step definition file. So this improvement could be ideally done on upper (cucumber) level.

From a27fedddeb29d2f8903e210e2180fec80a3a0193 Mon Sep 17 00:00:00 2001