WIP integrate canary steps into Deployment Updater

Author: sethboyles

app/models/runtime/deployment_model.rb

@@ -114,6 +114,10 @@ def continuable?
       state == DeploymentModel::PAUSED_STATE
     end
 
+    def current_canary_instance_target
+      canary_step[:canary]
+    end
+
     def canary_step
       raise 'canary_step is only valid for canary deloyments' unless strategy == CANARY_STRATEGY
 

lib/cloud_controller/deployment_updater/actions/canary.rb

@@ -23,7 +23,8 @@ def call
           up_scaler = UpScaler.new(deployment, logger, interim_desired_instance_count, instance_count_summary)
 
           deployment.db.transaction do
-            return unless deployment.lock!.state == DeploymentModel::DEPLOYING_STATE
+            # TODO: write scale test for prepaused state
+            return unless [DeploymentModel::DEPLOYING_STATE, DeploymentModel::PREPAUSED_STATE].include?(deployment.lock!.state)
             return unless up_scaler.can_scale? || down_scaler.can_downscale?
 
             app.lock!

lib/cloud_controller/deployment_updater/actions/scale.rb

@@ -21,6 +21,7 @@ def scale_up
           deployment.update(last_healthy_at: Time.now)
         end
 
+        # TODO: change name to something like has_capacity_to_scale? (or something better)
         def can_scale?
           @starting_instances_count < deployment.max_in_flight &&
             @unhealthy_instances_count == 0 &&

lib/cloud_controller/deployment_updater/actions/up_scaler.rb

@@ -1,7 +1,7 @@
 require 'cloud_controller/deployment_updater/actions/scale'
 require 'cloud_controller/deployment_updater/actions/cancel'
-require 'cloud_controller/deployment_updater/actions/canary'
 require 'cloud_controller/deployment_updater/actions/finalize'
+
 module VCAP::CloudController
   module DeploymentUpdater
     class Updater
@@ -22,7 +22,18 @@ def scale
 
       def canary
         with_error_logging('error-canarying-deployment') do
-          Actions::Canary.new(deployment, logger).call
+          # TODO: do we need to pass in deployment.original_web_process_instance_count + 1 if there is a single canary instance?
+          finished = Actions::Scale.new(deployment, logger, deployment.original_web_process_instance_count, deployment.current_canary_instance_target).call
+          if finished
+            deployment.update(
+              last_healthy_at: Time.now,
+              state: DeploymentModel::PAUSED_STATE,
+              status_value: DeploymentModel::ACTIVE_STATUS_VALUE,
+              status_reason: DeploymentModel::PAUSED_STATUS_REASON
+            )
+            logger.info("paused-canary-deployment-for-#{deployment.guid}")
+          end
+          # Actions::Canary.new(deployment, logger).call
           logger.info("ran-canarying-deployment-for-#{deployment.guid}")
         end
       end

lib/cloud_controller/deployment_updater/updater.rb

@@ -121,7 +121,7 @@ module VCAP::CloudController
       end
     end
 
-    context 'when the canary is not routable routable' do
+    context 'when the canary is not routable' do
       let(:all_instances_results) do
         {
           0 => { state: 'RUNNING', uptime: 50, since: 2, routable: false }

spec/unit/lib/cloud_controller/deployment_updater/actions/canary_spec.rb

@@ -162,6 +162,104 @@ module VCAP::CloudController
     describe '#canary' do
       let(:state) { DeploymentModel::PREPAUSED_STATE }
       let(:current_deploying_instances) { 1 }
+      let(:deployment) do
+        DeploymentModel.make(
+          app: web_process.app,
+          deploying_web_process: deploying_web_process,
+          state: state,
+          strategy: 'canary',
+          original_web_process_instance_count: original_web_process_instance_count,
+          max_in_flight: 1
+        )
+      end
+
+      describe 'canary steps' do
+        let(:max_in_flight) { 1 }
+        let(:original_web_process_instance_count) { 10 }
+        let(:deployment) do
+          DeploymentModel.make(
+            app: web_process.app,
+            deploying_web_process: deploying_web_process,
+            strategy: 'canary',
+            droplet: droplet,
+            state: state,
+            max_in_flight: max_in_flight,
+            original_web_process_instance_count: 10,
+            canary_steps: [{ instance_weight: 50 }, { instance_weight: 80 }],
+            canary_current_step: 1
+          )
+        end
+
+        context 'when the current step instance count has been reached' do
+          let(:current_deploying_instances) { 5 }
+          let(:all_instances_results) do
+            {
+              0 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              1 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              2 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              3 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              4 => { state: 'RUNNING', uptime: 50, since: 2, routable: true }
+            }
+          end
+
+          it 'transitions state to paused' do
+            subject.canary
+            expect(deployment.state).to eq(DeploymentModel::PAUSED_STATE)
+          end
+        end
+
+        context 'when the current step instance count has not been reached' do
+          let(:current_deploying_instances) { 4 }
+          let(:all_instances_results) do
+            {
+              0 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              1 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              2 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              3 => { state: 'RUNNING', uptime: 50, since: 2, routable: true }
+            }
+          end
+
+          it 'does not transition to paused' do
+            subject.canary
+            expect(deployment.state).not_to eq(DeploymentModel::PAUSED_STATE)
+          end
+        end
+
+        context 'when there is a single unhealthy instance' do
+          let(:current_deploying_instances) { 5 }
+          let(:all_instances_results) do
+            {
+              0 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              1 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              2 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              3 => { state: 'RUNNING', uptime: 50, since: 2, routable: false },
+              4 => { state: 'RUNNING', uptime: 50, since: 2, routable: true }
+            }
+          end
+
+          it 'does not transition to paused' do
+            subject.canary
+            expect(deployment.state).not_to eq(DeploymentModel::PAUSED_STATE)
+          end
+        end
+
+        context 'when there are not enough actual instances' do
+          let(:current_deploying_instances) { 5 }
+          let(:all_instances_results) do
+            {
+              0 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              1 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              2 => { state: 'RUNNING', uptime: 50, since: 2, routable: true },
+              3 => { state: 'RUNNING', uptime: 50, since: 2, routable: true }
+            }
+          end
+
+          it 'does not transition to paused' do
+            subject.canary
+            expect(deployment.state).not_to eq(DeploymentModel::PAUSED_STATE)
+          end
+        end
+      end
 
       context 'when the canary instance starts succesfully' do
         let(:all_instances_results) do
@@ -170,6 +268,11 @@ module VCAP::CloudController
           }
         end
 
+        it 'transitions state to paused' do
+          subject.canary
+          expect(deployment.state).to eq(DeploymentModel::PAUSED_STATE)
+        end
+
         it 'logs the canary is paused' do
           subject.canary
           expect(logger).to have_received(:info).with(
@@ -185,6 +288,59 @@ module VCAP::CloudController
         end
       end
 
+      context 'when the canary is not routable' do
+        let(:all_instances_results) do
+          {
+            0 => { state: 'RUNNING', uptime: 50, since: 2, routable: false }
+          }
+        end
+
+        it 'does not transition state to paused' do
+          subject.canary
+          expect(deployment.state).to eq(DeploymentModel::PREPAUSED_STATE)
+          expect(deployment.status_value).to eq(DeploymentModel::ACTIVE_STATUS_VALUE)
+          expect(deployment.status_reason).to eq(DeploymentModel::DEPLOYING_STATUS_REASON)
+        end
+      end
+
+      context 'while the canary instance is still starting' do
+        let(:all_instances_results) do
+          {
+            0 => { state: 'STARTING', uptime: 50, since: 2, routable: true }
+          }
+        end
+
+        it 'skips the deployment update' do
+          subject.canary
+          expect(deployment.state).to eq(DeploymentModel::PREPAUSED_STATE)
+          expect(deployment.status_value).to eq(DeploymentModel::ACTIVE_STATUS_VALUE)
+          expect(deployment.status_reason).to eq(DeploymentModel::DEPLOYING_STATUS_REASON)
+        end
+      end
+
+      context 'when the canary instance is failing' do
+        let(:all_instances_results) do
+          {
+            0 => { state: 'FAILING', uptime: 50, since: 2, routable: true }
+          }
+        end
+
+        it 'does not update the deployments last_healthy_at' do
+          Timecop.travel(Time.now + 1.minute) do
+            expect do
+              subject.canary
+            end.not_to(change { deployment.reload.last_healthy_at })
+          end
+        end
+
+        it 'skips the deployment update' do
+          subject.canary
+          expect(deployment.state).to eq(DeploymentModel::PREPAUSED_STATE)
+          expect(deployment.status_value).to eq(DeploymentModel::ACTIVE_STATUS_VALUE)
+          expect(deployment.status_reason).to eq(DeploymentModel::DEPLOYING_STATUS_REASON)
+        end
+      end
+
       context 'when an error occurs while canarying a deployment' do
         before do
           allow(deployment).to receive(:lock!).and_raise(StandardError.new('Something real bad happened'))