Merge pull request #3538 from sap-contributions/clean-up-degenerate-d…

…eployments

Clean up degenerate deployments

app/models/runtime/deployment_model.rb

@@ -17,8 +17,7 @@ class DeploymentModel < Sequel::Model(:deployments)
       DEPLOYING_STATUS_REASON = 'DEPLOYING'.freeze,
       CANCELED_STATUS_REASON = 'CANCELED'.freeze,
       CANCELING_STATUS_REASON = 'CANCELING'.freeze,
-      SUPERSEDED_STATUS_REASON = 'SUPERSEDED'.freeze,
-      DEGENERATE_STATUS_REASON = 'DEGENERATE'.freeze
+      SUPERSEDED_STATUS_REASON = 'SUPERSEDED'.freeze
     ].freeze
 
     DEPLOYMENT_STRATEGIES = [

db/migrations/20231205143526_remove_deployments_with_degenerate.rb

@@ -0,0 +1,10 @@
+Sequel.migration do
+  up do
+    degenerate_records = self[:deployments].where(status_reason: 'DEGENERATE')
+    degenerate_records.delete
+  end
+
+  down do
+    # It is not possible to recover the deleted rows during rollback
+  end
+end

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

@@ -32,7 +32,7 @@ Name | Type | Description
 ---- | ---- | ------------
 **app_guids** | _list of strings_ | Comma-delimited list of app guids to filter by
 **states** | _list of strings_ | Comma-delimited list of states to filter by
-**status_reasons** | _list of strings_ | Comma-delimited list of status reasons to filter by;<br>valid values include `DEPLOYING`, `CANCELING`, `DEPLOYED`, `CANCELED`, `SUPERSEDED`, and `DEGENERATE`
+**status_reasons** | _list of strings_ | Comma-delimited list of status reasons to filter by;<br>valid values include `DEPLOYING`, `CANCELING`, `DEPLOYED`, `CANCELED`, `SUPERSEDED`
 **status_values** | _list of strings_ | Comma-delimited list of status values to filter by;<br>valid values include `ACTIVE` and `FINALIZED`
 **page** | _integer_ | Page to display; valid values are integers >= 1
 **per_page** | _integer_ | Number of results per page; <br>valid values are 1 through 5000

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

@@ -13,7 +13,7 @@ Name | Type | Description
 **created_at** | _[timestamp](#timestamps)_ | The time with zone when the object was created
 **updated_at** | _[timestamp](#timestamps)_ | The time with zone when the object was last updated
 **status.value** | _string_ | The current status of the deployment; valid values are `ACTIVE` (meaning in progress) and `FINALIZED` (meaning finished, either successfully or not)
-**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>- `CANCELING`<br>2. If **status.value** is `FINALIZED`<br>- `DEPLOYED`<br>- `CANCELED`<br>- `SUPERSEDED` (another deployment created for app before completion)<br>- `DEGENERATE` (the deployment was created incorrectly by the system)
+**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>- `CANCELING`<br>2. If **status.value** is `FINALIZED`<br>- `DEPLOYED`<br>- `CANCELED`<br>- `SUPERSEDED` (another deployment created for app before completion)<br>
 **status.details** | _object_ | The details for the status of the deployment shows a timestamp of the last successful healthcheck
 **strategy** | _string_ | Strategy used for the deployment; supported strategies are `rolling` only
 **droplet.guid** | _string_ | The droplet guid that the deployment is transitioning the app to

lib/cloud_controller/deployment_updater/dispatcher.rb

@@ -9,8 +9,6 @@ def dispatch
           logger = Steno.logger('cc.deployment_updater.update')
           logger.info('run-deployment-update')
 
-          finalize_degenerate_deployments(logger)
-
           deployments_to_scale = DeploymentModel.where(state: DeploymentModel::DEPLOYING_STATE).all
           deployments_to_cancel = DeploymentModel.where(state: DeploymentModel::CANCELING_STATE).all
 
@@ -35,22 +33,6 @@ def dispatch
             workpool.drain
           end
         end
-
-        def finalize_degenerate_deployments(logger)
-          deployments_without_deploying_web_process = DeploymentModel.qualify.
-                                                      left_join(:processes, guid: :deploying_web_process_guid).
-                                                      where(processes__id: nil).
-                                                      where(status_value: DeploymentModel::ACTIVE_STATUS_VALUE)
-          deployments_without_deploying_web_process.each do |d|
-            d.update(
-              state: DeploymentModel::DEPLOYED_STATE,
-              status_value: DeploymentModel::FINALIZED_STATUS_VALUE,
-              status_reason: DeploymentModel::DEGENERATE_STATUS_REASON
-            )
-
-            logger.warn('finalized-degenerate-deployment', { deployment: d.guid, app: d.app_guid })
-          end
-        end
       end
     end
   end

spec/migrations/20231205143526_remove_deployments_with_degenerate_spec.rb

@@ -0,0 +1,22 @@
+require 'spec_helper'
+require 'migrations/helpers/migration_shared_context'
+
+RSpec.describe 'migration to clean up degenerate records from deployments records', isolation: :truncation do
+  include_context 'migration' do
+    let(:migration_filename) { '20231205143526_remove_deployments_with_degenerate.rb' }
+  end
+
+  describe 'deployments table' do
+    it 'degenerate record is removed from deployments' do
+      db[:deployments].insert(
+        guid: 'bommel',
+        original_web_process_instance_count: 1,
+        status_reason: 'DEGENERATE'
+      )
+
+      expect { Sequel::Migrator.run(db, migration_to_test, allow_missing_migration_files: true) }.not_to raise_error
+      deployment = db[:deployments].first(status_reason: 'DEGENERATE')
+      expect(deployment).to be_nil
+    end
+  end
+end

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

@@ -2,38 +2,6 @@
 require 'cloud_controller/deployment_updater/dispatcher'
 
 module VCAP::CloudController
-  RSpec.shared_examples 'a degenerated deployment' do
-    let!(:deployment_process_model) { DeploymentProcessModel.make(deployment: scaling_deployment, process_guid: 'some_guid') }
-
-    before do
-      allow(DeploymentUpdater::Updater).to receive(:new).with(scaling_deployment, logger).and_return(updater)
-    end
-
-    it 'finalizes the deployment, sets the status, and logs' do
-      subject.dispatch
-
-      deployment = scaling_deployment.reload
-      expect(logger).to have_received(:warn).with(
-        'finalized-degenerate-deployment',
-        deployment: deployment.guid,
-        app: deployment.app.guid
-      )
-      expect(deployment.status_value).to eq(DeploymentModel::FINALIZED_STATUS_VALUE)
-      expect(deployment.status_reason).to eq(DeploymentModel::DEGENERATE_STATUS_REASON)
-    end
-
-    it 'does not scale the deployment' do
-      subject.dispatch
-      expect(updater).not_to have_received(:scale)
-    end
-
-    it 'processes the deployment only once' do
-      subject.dispatch
-      subject.dispatch
-      expect(logger).to have_received(:warn).with('finalized-degenerate-deployment', anything).once
-    end
-  end
-
   RSpec.describe DeploymentUpdater::Dispatcher do
     subject(:dispatcher) { DeploymentUpdater::Dispatcher }
 
@@ -82,42 +50,6 @@ module VCAP::CloudController
           expect(updater).to have_received(:cancel)
         end
       end
-
-      context 'when the deploying_web_process_guid is nil' do
-        let!(:scaling_deployment) { DeploymentModel.make(state: DeploymentModel::DEPLOYING_STATE, deploying_web_process: nil) }
-
-        it_behaves_like 'a degenerated deployment'
-      end
-
-      context 'when the deploying_web_process_guid is nil and state is DEPLOYED' do
-        let!(:scaling_deployment) do
-          DeploymentModel.make(state: DeploymentModel::DEPLOYED_STATE,
-                               status_value: DeploymentModel::FINALIZED_STATUS_VALUE,
-                               status_reason: DeploymentModel::DEPLOYED_STATUS_REASON,
-                               deploying_web_process: nil)
-        end
-
-        before do
-          allow(DeploymentUpdater::Updater).to receive(:new).with(scaling_deployment, logger).and_return(updater)
-        end
-
-        it 'does not change the status_reason to DEGENERATE' do
-          subject.dispatch
-          deployment = scaling_deployment.reload
-
-          expect(deployment.status_reason).to eq(DeploymentModel::DEPLOYED_STATUS_REASON)
-        end
-      end
-
-      context 'when the deploying_web_process_guid references a non-existent process' do
-        let!(:scaling_deployment) do
-          deployment = DeploymentModel.make(state: DeploymentModel::DEPLOYING_STATE)
-          deployment.deploying_web_process.destroy
-          deployment
-        end
-
-        it_behaves_like 'a degenerated deployment'
-      end
     end
   end
 end