[PATCH conductor] Fixed deployment rollback
Jozef Zigmund
jzigmund at redhat.com
Thu Jun 7 15:26:04 UTC 2012
On Thursday 07 June 2012 15:38:00 jprovazn at redhat.com wrote:
> From: Jan Provaznik <jprovazn at redhat.com>
>
> When rollback was initiated, 'stop' request was sent to instances
> in 'pending' state too. But it seems that 'stop' is not valid action
> for 'pending' state. Now we wait until pending instances are running
> and send 'stop' request then.
>
> This is fix for already pushed task:
> https://www.aeolusproject.org/redmine/issues/3320
> ---
> src/app/models/deployment.rb | 14 +++++++++-----
> src/app/models/instance.rb | 1 +
> src/app/models/instance_observer.rb | 2 +-
> src/spec/models/deployment_spec.rb | 2 ++
> 4 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
> index 0cf1137..7b66e56 100644
> --- a/src/app/models/deployment.rb
> +++ b/src/app/models/deployment.rb
> @@ -654,6 +654,8 @@ class Deployment < ActiveRecord::Base
> if Instance::ACTIVE_FAILED_STATES.include?(instance.state)
> # if this instance stop failed, whole deployment rollback failed
> self.state = STATE_ROLLBACK_FAILED
> + elsif instance.state == Instance::STATE_RUNNING
> + deployment_rollback
> elsif instances.all? {|i| i.inactive?}
> # some other instances might be failed (because their
> # launch failed), but it shouldn't be a problem if all
> @@ -663,17 +665,19 @@ class Deployment < ActiveRecord::Base
> end
>
> def deployment_rollback
> - stoppable_instances = instances.stoppable
> - if stoppable_instances.empty?
> + unless self.state == STATE_ROLLBACK_IN_PROGRESS
> + self.state = STATE_ROLLBACK_IN_PROGRESS
> + save!
> + end
> +
> + if instances.all? {|i| i.inactive?}
needs to add 'or i == Instance::STATE_NEW' then it works properly as I tested
> self.state = STATE_ROLLBACK_COMPLETE
> save!
> return
> end
>
> - self.state = STATE_ROLLBACK_IN_PROGRESS
> - save!
> error_occured = false
> - stoppable_instances.each do |instance|
> + instances.running.each do |instance|
> begin
> instance.stop(nil)
> rescue
> diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
> index aee01a1..3806d16 100644
> --- a/src/app/models/instance.rb
> +++ b/src/app/models/instance.rb
> @@ -128,6 +128,7 @@ class Instance < ActiveRecord::Base
> 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] } + scope :running, :conditions => { :state
> => [STATE_RUNNING] }
> scope :pending_or_deployed, :conditions => { :state => [STATE_NEW,
> STATE_PENDING, STATE_RUNNING, STATE_SHUTTING_DOWN] } # FIXME: "failed" is
> misleading too...
> scope :failed, :conditions => { :state => FAILED_STATES }
> diff --git a/src/app/models/instance_observer.rb
> b/src/app/models/instance_observer.rb index 77b2e12..d42fbe6 100644
> --- a/src/app/models/instance_observer.rb
> +++ b/src/app/models/instance_observer.rb
> @@ -95,7 +95,6 @@ class InstanceObserver < ActiveRecord::Observer
> # This can get stale, so reload it -- if it exists
> instance.deployment.reload if instance.deployment
> if instance.state_changed?
> - instance.deployment.update_state(instance) if instance.deployment
> event = Event.new(:source => instance, :event_time => DateTime.now,
>
> :summary => "state changed to #{instance.state}",
> :change_hash => instance.changes, :status_code =>
> :instance.state)
>
> @@ -126,6 +125,7 @@ class InstanceObserver < ActiveRecord::Observer
>
> :status_code => 'vanished', :summary => "Instance disappeared
> :from cloud provider"
> )
> end
> + instance.deployment.update_state(instance) if instance.deployment
>
> end
> end
> diff --git a/src/spec/models/deployment_spec.rb
> b/src/spec/models/deployment_spec.rb index 3ffbae9..48df0ad 100644
> --- a/src/spec/models/deployment_spec.rb
> +++ b/src/spec/models/deployment_spec.rb
> @@ -428,6 +428,8 @@ describe Deployment do
> end
>
> it "should stop all running instances if an instance fails" do
> + @inst1.state = Instance::STATE_RUNNING
> + @inst1.save!
> Instance.any_instance.stub(:stop).and_return(true)
> Instance.any_instance.should_receive(:stop)
> @inst2.state = Instance::STATE_ERROR
ACK works for me, pls check inline note.
More information about the aeolus-devel
mailing list