Skip to content

Commit c230855

Browse files
authored
Fix quota issue when cancelling deployment (#4246)
Ensure we do not hit a quota when cancelling a deployment by destroying the deployment web processes before restoring the originals. Since both these operations are handled by diego this reorder should not effect app availability. We may want to improve this logic in the future to cancel more gracefully by ensuring app availability.
1 parent d771052 commit c230855

File tree

2 files changed

+24
-6
lines changed
  • lib/cloud_controller/deployment_updater/actions
  • spec/unit/lib/cloud_controller/deployment_updater/actions

2 files changed

+24
-6
lines changed

lib/cloud_controller/deployment_updater/actions/cancel.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ def call
2121
prior_web_process = find_interim_web_process || app.oldest_web_process
2222
prior_web_process.lock!
2323

24-
prior_web_process.update(instances: deployment.original_web_process_instance_count)
25-
2624
app.web_processes.reject { |p| p.guid == prior_web_process.guid }.map(&:destroy)
2725

26+
prior_web_process.update(instances: deployment.original_web_process_instance_count)
27+
2828
deployment.update(
2929
state: DeploymentModel::CANCELED_STATE,
3030
status_value: DeploymentModel::FINALIZED_STATUS_VALUE,

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

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,21 @@ module VCAP::CloudController
66
subject(:cancel_action) { DeploymentUpdater::Actions::Cancel.new(deployment, logger) }
77
let(:a_day_ago) { Time.now - 1.day }
88
let(:an_hour_ago) { Time.now - 1.hour }
9-
let(:app) { AppModel.make(droplet: droplet, revisions_enabled: true) }
9+
let(:organization) { Organization.make }
10+
let(:space) { Space.make(organization: organization, space_quota_definition: quota) }
11+
let(:app) { AppModel.make(droplet: droplet, revisions_enabled: true, space: space) }
1012
let(:droplet) { DropletModel.make }
13+
let(:memory) { 1024 }
14+
let(:memory_limit) { memory * 1000 }
15+
let(:quota) { SpaceQuotaDefinition.make(organization:, memory_limit:) }
1116
let!(:web_process) do
1217
ProcessModel.make(
1318
instances: current_web_instances,
1419
created_at: a_day_ago,
1520
guid: 'guid-original',
16-
app: app
21+
app: app,
22+
memory: memory,
23+
state: ProcessModel::STARTED
1724
)
1825
end
1926
let!(:route_mapping) { RouteMappingModel.make(app: web_process.app, process_type: web_process.type) }
@@ -24,12 +31,12 @@ module VCAP::CloudController
2431
instances: current_deploying_instances,
2532
guid: 'guid-final',
2633
revision: revision,
27-
state: ProcessModel::STOPPED
34+
memory: memory,
35+
state: ProcessModel::STARTED
2836
)
2937
end
3038
let(:revision) { RevisionModel.make(app: app, droplet: droplet, version: 300) }
3139
let!(:deploying_route_mapping) { RouteMappingModel.make(app: web_process.app, process_type: deploying_web_process.type) }
32-
let(:space) { web_process.space }
3340
let(:original_web_process_instance_count) { 6 }
3441
let(:current_web_instances) { 2 }
3542

@@ -188,5 +195,16 @@ module VCAP::CloudController
188195
expect(deployment).not_to have_received(:update)
189196
end
190197
end
198+
199+
context 'when the app is at a quota limit' do
200+
let(:current_web_instances) { 1 }
201+
let(:current_deploying_instances) { original_web_process_instance_count }
202+
let(:memory_limit) { memory * (current_deploying_instances + current_web_instances) }
203+
204+
it 'can still be cancelled succesfully' do
205+
expect { subject.call }.not_to raise_error
206+
expect(ProcessModel.find(guid: deploying_web_process.guid)).to be_nil
207+
end
208+
end
191209
end
192210
end

0 commit comments

Comments
 (0)