Surface deployment updater errors to the API (#4295)

Author: sethboyles

app/presenters/v3/deployment_presenter.rb

@@ -83,6 +83,8 @@ def status(deployment)
         }
       }
 
+      status[:details][:error] = deployment.error if deployment.error
+
       if deployment.strategy == VCAP::CloudController::DeploymentModel::CANARY_STRATEGY
         status[:canary] = {
           steps: {

db/migrations/20250401233407_add_error_to_deployments.rb

@@ -0,0 +1,7 @@
+Sequel.migration do
+  change do
+    alter_table(:deployments) do
+      add_column :error, String, null: true, default: nil, size: 255
+    end
+  end
+end

docs/v3/source/includes/resources/deployments/_object.md.erb

@@ -16,6 +16,7 @@ Name | Type | Description
 **status.reason** | _string_ | The reason for the status of the deployment;<br>following list represents valid values:<br>1. If **status.value** is `ACTIVE`<br>- `DEPLOYING`<br>- `PAUSED` (only valid for canary deployments) <br>- `CANCELING`<br>2. If **status.value** is `FINALIZED`<br>- `DEPLOYED`<br>- `CANCELED`<br>- `SUPERSEDED` (another deployment created for app before completion)<br>
 **status.details.last_successful_healthcheck** | _[timestamp](#timestamps)_ | Timestamp of the last successful healthcheck
 **status.details.last_status_change** | _[timestamp](#timestamps)_ | Timestamp of last change to status.value or status.reason**status.details.last_status_change** | _[timestamp](#timestamps)_ | Timestamp of last change to status.value or status.reason
+**status.details.error** | _string_ | | Brief description of error encountered while deploying, if any. This field is cleared once the deployment progresses successfully.
 **status.canary.steps.current** | _integer_ | The current canary step. Only available for deployments with strategy 'canary'. (experimental)
 **status.canary.steps.total** | _integer_ |  The total number of canary steps. Only available for deployments with strategy 'canary'. (experimental)
 **strategy** | _string_ | Strategy used for the deployment; supported strategies are `rolling` and `canary` (experimental)

lib/cloud_controller/deployment_updater/actions/cancel.rb

@@ -28,7 +28,8 @@ def call
             deployment.update(
               state: DeploymentModel::CANCELED_STATE,
               status_value: DeploymentModel::FINALIZED_STATUS_VALUE,
-              status_reason: DeploymentModel::CANCELED_STATUS_REASON
+              status_reason: DeploymentModel::CANCELED_STATUS_REASON,
+              error: nil
             )
           end
         end

lib/cloud_controller/deployment_updater/actions/scale.rb

@@ -32,7 +32,8 @@ def call
 
             deployment.update(
               status_value: DeploymentModel::ACTIVE_STATUS_VALUE,
-              status_reason: DeploymentModel::DEPLOYING_STATUS_REASON
+              status_reason: DeploymentModel::DEPLOYING_STATUS_REASON,
+              error: nil
             )
 
             ScaleDownCanceledProcesses.new(deployment).call

lib/cloud_controller/deployment_updater/updater.rb

@@ -45,10 +45,32 @@ def cancel
 
       private
 
+      APPROVED_ERRORS = [
+        Sequel::ValidationFailed,
+        CloudController::Errors::ApiError
+      ].freeze
+
       def with_error_logging(error_message)
         yield
       rescue StandardError => e
         error_name = e.is_a?(CloudController::Errors::ApiError) ? e.name : e.class.name
+
+        begin
+          if APPROVED_ERRORS.include?(e.class)
+            deployment.update(error: e.message)
+          else
+            deployment.update(error: 'An unexpected error has occurred.')
+          end
+        rescue StandardError => new_error
+          logger.error(
+            'error-saving-deployment-error',
+            deployment_guid: deployment.guid,
+            error: new_error.class.name,
+            error_message: new_error.message,
+            backtrace: new_error.backtrace.join("\n")
+          )
+        end
+
         logger.error(
           error_message,
           deployment_guid: deployment.guid,

spec/request/deployments_spec.rb

@@ -1665,6 +1665,24 @@
       end
     end
 
+    context 'when a deployment has had an error' do
+      let(:user) { make_developer_for_space(space) }
+      let(:deployment) do
+        VCAP::CloudController::DeploymentModelTestFactory.make(
+          app: app_model,
+          droplet: droplet,
+          previous_droplet: old_droplet,
+          error: 'Quota error'
+        )
+      end
+
+      it 'includes the error as part of the status details' do
+        get "/v3/deployments/#{deployment.guid}", nil, user_header
+        parsed_response = Oj.load(last_response.body)
+        expect(parsed_response['status']['details']['error']).to eq('Quota error')
+      end
+    end
+
     it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
   end
 

spec/unit/lib/cloud_controller/deployment_updater/updater_spec.rb

@@ -24,7 +24,7 @@ module VCAP::CloudController
         instances: current_deploying_instances,
         guid: 'guid-final',
         revision: revision,
-        state: ProcessModel::STOPPED
+        state: ProcessModel::STARTED
       )
     end
     let(:revision) { RevisionModel.make(app: app, droplet: droplet, version: 300) }
@@ -183,6 +183,8 @@ module VCAP::CloudController
           expect do
             subject.scale
           end.not_to raise_error
+
+          expect(deployment.error).to eq 'An unexpected error has occurred.'
         end
       end
     end
@@ -393,6 +395,8 @@ module VCAP::CloudController
           expect do
             subject.canary
           end.not_to raise_error
+
+          expect(deployment.error).to eq 'An unexpected error has occurred.'
         end
       end
     end
@@ -424,6 +428,118 @@ module VCAP::CloudController
           expect do
             subject.cancel
           end.not_to raise_error
+
+          expect(deployment.error).to eq 'An unexpected error has occurred.'
+        end
+      end
+    end
+
+    describe 'error handling' do
+      context 'when saving the error to the deployment model' do
+        context 'when the error is a quota error' do
+          let(:web_instances) { nil }
+          let(:max_in_flight) { 100 }
+          let!(:quota_definition) { QuotaDefinition.make(memory_limit: 1) }
+
+          before do
+            org = app.organization
+            org.quota_definition = quota_definition
+            org.save
+          end
+
+          it 'saves the quota error to the model' do
+            subject.scale
+            expect(deployment.error).to eq 'memory quota_exceeded'
+          end
+        end
+
+        context 'when the error is not an approved error' do
+          before do
+            allow(deployment).to receive(:app).and_raise(StandardError.new('unrecognized error'))
+          end
+
+          it 'provides a helpful error on the deployment model' do
+            subject.scale
+            expect(deployment.error).to eq 'An unexpected error has occurred.'
+          end
+        end
+
+        context 'when the error is a SequelValidation error' do
+          before do
+            allow(deployment).to receive(:app).and_raise(Sequel::ValidationFailed.new('Something bad happened with model validation'))
+          end
+
+          it 'provides a helpful error on the deployment model' do
+            subject.scale
+            expect(deployment.error).to eq 'Something bad happened with model validation'
+          end
+        end
+
+        context 'when the error is a CloudController::Errors::ApiError error' do
+          before do
+            allow(deployment).to receive(:app).and_raise(CloudController::Errors::ApiError.new_from_details('RunnerError', 'some error'))
+          end
+
+          it 'provides a helpful error on the deployment model' do
+            subject.scale
+            expect(deployment.error).to eq 'Runner error: some error'
+          end
+        end
+
+        context 'when there is a problem saving the error column' do
+          before do
+            # too long for db column
+            allow(deployment).to receive(:app).and_raise(CloudController::Errors::ApiError.new_from_details('RunnerError', 'a' * 5000))
+          end
+
+          it 'logs the issue and continues' do
+            subject.scale
+
+            expect(logger).to have_received(:error).with(
+              'error-saving-deployment-error',
+              deployment_guid: deployment.guid,
+              error: 'Sequel::DatabaseError',
+              error_message: anything,
+              backtrace: anything
+            )
+          end
+        end
+
+        context 'when there was a prior error, but the error no longer occurs' do
+          describe '#scale' do
+            before do
+              deployment.error = 'old error'
+              deployment.save
+            end
+
+            it 'clears the error' do
+              subject.scale
+              expect(deployment.error).to be_nil
+            end
+          end
+
+          describe '#cancel' do
+            before do
+              deployment.update(state: 'CANCELING', error: 'old error')
+              allow_any_instance_of(VCAP::CloudController::Diego::Runner).to receive(:stop)
+            end
+
+            it 'clears the error' do
+              subject.cancel
+              expect(deployment.error).to be_nil
+            end
+          end
+
+          describe '#canary' do
+            before do
+              deployment.update(strategy: 'canary', error: 'old error')
+            end
+
+            it 'clears the error' do
+              subject.canary
+              expect(deployment.error).to be_nil
+            end
+          end
         end
       end
     end

spec/unit/presenters/v3/deployment_presenter_spec.rb

@@ -131,6 +131,30 @@ module VCAP::CloudController::Presenters::V3
         end
       end
 
+      describe 'error' do
+        context 'when the deployment has no error' do
+          before do
+            deployment.error = nil
+          end
+
+          it 'does not have the error key' do
+            result = DeploymentPresenter.new(deployment).to_hash
+            expect(result[:status][:details].key?(:error)).to be false
+          end
+        end
+
+        context 'when the deployment has an error' do
+          before do
+            deployment.error = 'Quota error'
+          end
+
+          it 'renders the error' do
+            result = DeploymentPresenter.new(deployment).to_hash
+            expect(result[:status][:details][:error]).to eq 'Quota error'
+          end
+        end
+      end
+
       describe 'status' do
         context 'when the strategy is rolling' do
           before do