On Thursday 07 June 2012 15:38:00 jprovazn(a)redhat.com wrote:
From: Jan Provaznik <jprovazn(a)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.