From b4d3ca78c568d00546a76d528000d08e28cb1524 Mon Sep 17 00:00:00 2001 From: Seth Boyles Date: Wed, 5 Mar 2025 15:03:14 -0800 Subject: [PATCH] Allow scaling with Deployments directly --- app/actions/deployment_create.rb | 24 +++- app/messages/deployment_create_message.rb | 14 ++ app/models/runtime/deployment_model.rb | 18 ++- app/presenters/v3/deployment_presenter.rb | 3 +- ...215542_add_web_instances_to_deployments.rb | 12 ++ .../includes/api_resources/_deployments.erb | 1 + .../resources/deployments/_create.md.erb | 1 + .../resources/deployments/_object.md.erb | 1 + .../deployment_updater/updater.rb | 2 +- spec/request/deployments_spec.rb | 112 +++++++++++++-- spec/unit/actions/deployment_create_spec.rb | 131 +++++++++++++++++- .../deployment_updater/updater_spec.rb | 30 +++- .../deployment_create_message_spec.rb | 77 +++++++++- .../models/runtime/deployment_model_spec.rb | 59 +++++++- .../v3/deployment_presenter_spec.rb | 10 +- 15 files changed, 467 insertions(+), 28 deletions(-) create mode 100644 db/migrations/20250304215542_add_web_instances_to_deployments.rb diff --git a/app/actions/deployment_create.rb b/app/actions/deployment_create.rb index dc021520966..6a15a2ec4ba 100644 --- a/app/actions/deployment_create.rb +++ b/app/actions/deployment_create.rb @@ -43,6 +43,8 @@ def create(app:, user_audit_info:, message:) desired_instances = desired_instances(app.oldest_web_process, previous_deployment) + validate_quota!(message, app) + deployment = DeploymentModel.create( app: app, state: starting_state(message), @@ -55,7 +57,8 @@ def create(app:, user_audit_info:, message:) revision_version: revision&.version, strategy: message.strategy, max_in_flight: message.max_in_flight, - canary_steps: message.options&.dig(:canary, :steps) + canary_steps: message.options&.dig(:canary, :steps), + web_instances: message.web_instances || desired_instances ) MetadataUpdate.update(deployment, message) @@ -163,7 +166,21 @@ def record_audit_event(deployment, droplet, user_audit_info, message) private + def validate_quota!(message, app) + return if message.web_instances.blank? + + current_web_process = app.newest_web_process + current_web_process.instances = message.web_instances + + current_web_process.validate + + raise Sequel::ValidationFailed.new(current_web_process) unless current_web_process.valid? + + current_web_process.reload + end + def deployment_for_stopped_app(app, message, previous_deployment, previous_droplet, revision, target_state, user_audit_info) + app.newest_web_process.update(instances: message.web_instances) if message.web_instances # Do not create a revision here because AppStart will not handle the rollback case AppStart.start(app: app, user_audit_info: user_audit_info, create_revision: false) @@ -179,7 +196,8 @@ def deployment_for_stopped_app(app, message, previous_deployment, previous_dropl revision_version: revision&.version, strategy: message.strategy, max_in_flight: message.max_in_flight, - canary_steps: message.options&.dig(:canary, :steps) + canary_steps: message.options&.dig(:canary, :steps), + web_instances: message.web_instances || desired_instances(app.oldest_web_process, previous_deployment) ) MetadataUpdate.update(deployment, message) @@ -223,6 +241,8 @@ def starting_state(message) def starting_process_instances(deployment, desired_instances) starting_process_count = if deployment.strategy == DeploymentModel::CANARY_STRATEGY deployment.canary_step[:canary] + elsif deployment.web_instances + deployment.web_instances else desired_instances end diff --git a/app/messages/deployment_create_message.rb b/app/messages/deployment_create_message.rb index 683114e6941..e260d88766b 100644 --- a/app/messages/deployment_create_message.rb +++ b/app/messages/deployment_create_message.rb @@ -13,6 +13,7 @@ class DeploymentCreateMessage < MetadataBaseMessage ALLOWED_OPTION_KEYS = %i[ canary max_in_flight + web_instances ].freeze ALLOWED_STEP_KEYS = [ @@ -43,6 +44,10 @@ def max_in_flight options&.dig(:max_in_flight) || 1 end + def web_instances + options&.dig(:web_instances) + end + private def mutually_exclusive_droplet_sources @@ -62,6 +67,7 @@ def validate_options disallowed_keys = options.keys - ALLOWED_OPTION_KEYS errors.add(:options, "has unsupported key(s): #{disallowed_keys.join(', ')}") if disallowed_keys.present? + validate_web_instances if options[:web_instances] validate_max_in_flight if options[:max_in_flight] validate_canary if options[:canary] end @@ -74,6 +80,14 @@ def validate_max_in_flight errors.add(:max_in_flight, 'must be an integer greater than 0') end + def validate_web_instances + web_instances = options[:web_instances] + + return unless !web_instances.is_a?(Integer) || web_instances < 1 + + errors.add(:web_instances, 'must be an integer greater than 0') + end + def validate_canary canary_options = options[:canary] unless canary_options.is_a?(Hash) diff --git a/app/models/runtime/deployment_model.rb b/app/models/runtime/deployment_model.rb index 8d63b8aafc1..c8fb8cf0935 100644 --- a/app/models/runtime/deployment_model.rb +++ b/app/models/runtime/deployment_model.rb @@ -113,6 +113,14 @@ def continuable? state == DeploymentModel::PAUSED_STATE end + def desired_web_instances + # It seems redundant to have method since web_instances defaults to original_web_process_instance_count, + # (in deployment create action) + # but this should handle deployments created on old API vms mid bosh deployment + # we can probably clean this up in the future + web_instances || original_web_process_instance_count + end + def current_canary_instance_target canary_step[:canary] end @@ -133,11 +141,17 @@ def canary_step_plan return [{ canary: 1, original: original_web_process_instance_count }] if canary_steps.nil? + instances = if web_instances && web_instances < original_web_process_instance_count + web_instances + else + original_web_process_instance_count + end + canary_steps.map do |step| weight = step['instance_weight'] - target_canary = (original_web_process_instance_count * (weight.to_f / 100)).round.to_i + target_canary = (instances * (weight.to_f / 100)).round.to_i target_canary = 1 if target_canary.zero? - target_original = original_web_process_instance_count - target_canary + 1 + target_original = instances - target_canary + 1 target_original = 0 if weight == 100 { canary: target_canary, original: target_original } end diff --git a/app/presenters/v3/deployment_presenter.rb b/app/presenters/v3/deployment_presenter.rb index 581659695df..f4cd240d1d9 100644 --- a/app/presenters/v3/deployment_presenter.rb +++ b/app/presenters/v3/deployment_presenter.rb @@ -57,7 +57,8 @@ def new_processes def options(deployment) options = { - max_in_flight: deployment.max_in_flight + max_in_flight: deployment.max_in_flight, + web_instances: deployment.desired_web_instances } if deployment.strategy == VCAP::CloudController::DeploymentModel::CANARY_STRATEGY && deployment.canary_steps diff --git a/db/migrations/20250304215542_add_web_instances_to_deployments.rb b/db/migrations/20250304215542_add_web_instances_to_deployments.rb new file mode 100644 index 00000000000..f9a52c02b28 --- /dev/null +++ b/db/migrations/20250304215542_add_web_instances_to_deployments.rb @@ -0,0 +1,12 @@ +Sequel.migration do + up do + alter_table(:deployments) do + add_column :web_instances, :integer, null: true + end + end + down do + alter_table(:deployments) do + drop_column :web_instances + end + end +end diff --git a/docs/v3/source/includes/api_resources/_deployments.erb b/docs/v3/source/includes/api_resources/_deployments.erb index 5a1741cc039..0d4b442858c 100644 --- a/docs/v3/source/includes/api_resources/_deployments.erb +++ b/docs/v3/source/includes/api_resources/_deployments.erb @@ -19,6 +19,7 @@ "strategy": "canary", "options" : { "max_in_flight": 3, + "web_instances": 5, "canary": { "steps": [ { diff --git a/docs/v3/source/includes/resources/deployments/_create.md.erb b/docs/v3/source/includes/resources/deployments/_create.md.erb index 9a812ec0e9c..90af1d46c37 100644 --- a/docs/v3/source/includes/resources/deployments/_create.md.erb +++ b/docs/v3/source/includes/resources/deployments/_create.md.erb @@ -78,6 +78,7 @@ Name | Type | Description | Default **revision**[1] | _object_ | The [revision](#revisions) whose droplet to deploy for the app; this will update the app's [current droplet](#get-current-droplet-association-for-an-app) to this droplet | **strategy** | _string_ | The strategy to use for the deployment | `rolling` **options.max_in_flight** | _integer_ | The maximum number of new instances to deploy simultaneously | 1 +**options.web_instances** | _integer_ | The number of web instances the deployment will scale to | The current web process's instance count **options.canary.steps** | _array of [canary step objects](#canary-steps-object)_ | An array of canary steps to use for the deployment **metadata.labels** | [_label object_](#labels) | Labels applied to the deployment **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the deployment diff --git a/docs/v3/source/includes/resources/deployments/_object.md.erb b/docs/v3/source/includes/resources/deployments/_object.md.erb index bff77b90308..b8fd2179770 100644 --- a/docs/v3/source/includes/resources/deployments/_object.md.erb +++ b/docs/v3/source/includes/resources/deployments/_object.md.erb @@ -20,6 +20,7 @@ Name | Type | Description **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) **options.max_in_flight** | _integer_ | The maximum number of new instances to deploy simultaneously +**options.web_instances** | _integer_ | The number of web instances the deployment will scale to **options.canary.steps** | _array of [canary step objects](#canary-steps-object)_ | Canary steps to use for the deployment. Only available for deployments with strategy 'canary'. (experimental) **droplet.guid** | _string_ | The droplet guid that the deployment is transitioning the app to **previous_droplet.guid** | _string_ | The app's [current droplet guid](#get-current-droplet-association-for-an-app) before the deployment was created diff --git a/lib/cloud_controller/deployment_updater/updater.rb b/lib/cloud_controller/deployment_updater/updater.rb index a8872f8d42e..49645e05c88 100644 --- a/lib/cloud_controller/deployment_updater/updater.rb +++ b/lib/cloud_controller/deployment_updater/updater.rb @@ -14,7 +14,7 @@ def initialize(deployment, logger) def scale with_error_logging('error-scaling-deployment') do - finished = Actions::Scale.new(deployment, logger, deployment.original_web_process_instance_count).call + finished = Actions::Scale.new(deployment, logger, deployment.desired_web_instances).call Actions::Finalize.new(deployment).call if finished logger.info("ran-deployment-update-for-#{deployment.guid}") end diff --git a/spec/request/deployments_spec.rb b/spec/request/deployments_spec.rb index e041bd0eabf..3c2fa2f82dd 100644 --- a/spec/request/deployments_spec.rb +++ b/spec/request/deployments_spec.rb @@ -45,7 +45,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -146,7 +147,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => other_droplet.guid @@ -232,7 +234,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => other_droplet.guid @@ -354,7 +357,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -436,7 +440,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => other_droplet.guid @@ -520,7 +525,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => other_droplet.guid @@ -692,7 +698,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -756,7 +763,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -841,7 +849,8 @@ 'type' => deployment.deploying_web_process.type }], 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'created_at' => iso8601, 'updated_at' => iso8601, @@ -915,7 +924,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -991,7 +1001,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 5 + 'max_in_flight' => 5, + 'web_instances' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -1099,6 +1110,7 @@ 'strategy' => 'canary', 'options' => { 'max_in_flight' => 1, + 'web_instances' => 1, 'canary' => { 'steps' => [ { 'instance_weight' => 20 }, @@ -1149,6 +1161,72 @@ end end + context 'web_instances' do + let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, instances: 10) } + let(:user) { make_developer_for_space(space) } + let(:web_instances) { 5 } + let(:create_request) do + { + options: { + web_instances: + }, + relationships: { + app: { + data: { + guid: app_model.guid + } + } + } + } + end + + context 'when web_instances is provided' do + it 'is set on the resource' do + post '/v3/deployments', create_request.to_json, user_header + expect(last_response.status).to eq(201), last_response.body + + expect(parsed_response['options']['web_instances']).to eq(5) + end + end + + context 'when web_instances violates a quota' do + let(:web_instances) { 100_000 } + let(:quota) { VCAP::CloudController::QuotaDefinition.make(memory_limit: 1000) } + + before do + org.quota_definition = quota + org.save + end + + it 'is set on the resource' do + post '/v3/deployments', create_request.to_json, user_header + expect(last_response.status).to eq(422), last_response.body + expect(parsed_response['errors'][0]['detail']).to match('memory quota_exceeded') + end + end + + context 'when web_instances is not provided' do + let(:create_request) do + { + relationships: { + app: { + data: { + guid: app_model.guid + } + } + } + } + end + + it 'defaults to original_web_process_instance_count' do + post '/v3/deployments', create_request.to_json, user_header + expect(last_response.status).to eq(201), last_response.body + + expect(parsed_response['options']['web_instances']).to eq(10) + end + end + end + context 'validation failures' do let(:user) { make_developer_for_space(space) } let(:smol_quota) { VCAP::CloudController::QuotaDefinition.make(memory_limit: 1) } @@ -1216,7 +1294,8 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -1324,7 +1403,8 @@ 'metadata' => metadata, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'relationships' => { 'app' => { @@ -1536,7 +1616,8 @@ def json_for_status(deployment, status_value, status_reason) def json_for_options(deployment) options = { - max_in_flight: deployment.max_in_flight + max_in_flight: deployment.max_in_flight, + web_instances: deployment.desired_web_instances } if deployment.canary_steps options[:canary] = { @@ -1802,7 +1883,8 @@ def json_for_options(deployment) }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1 + 'max_in_flight' => 1, + 'web_instances' => 1 }, 'droplet' => { 'guid' => droplet.guid diff --git a/spec/unit/actions/deployment_create_spec.rb b/spec/unit/actions/deployment_create_spec.rb index b9dd1f4663b..6ab39220d19 100644 --- a/spec/unit/actions/deployment_create_spec.rb +++ b/spec/unit/actions/deployment_create_spec.rb @@ -6,7 +6,7 @@ module VCAP::CloudController RSpec.describe DeploymentCreate do let(:app) { AppModel.make(desired_state: ProcessModel::STARTED) } - let!(:web_process) { ProcessModel.make(app: app, instances: 3, log_rate_limit: 101) } + let!(:web_process) { ProcessModel.make(app: app, instances: original_instances, log_rate_limit: 101, state: process_state) } let(:original_droplet) { DropletModel.make(app: app, process_types: { 'web' => 'asdf' }) } let(:next_droplet) { DropletModel.make(app: app, process_types: { 'web' => '1234' }) } let!(:route1) { Route.make(space: app.space) } @@ -16,15 +16,19 @@ module VCAP::CloudController let(:user_audit_info) { UserAuditInfo.new(user_guid: '123', user_email: 'connor@example.com', user_name: 'braa') } let(:runner) { instance_double(Diego::Runner) } + let(:process_state) { ProcessModel::STARTED } + let(:strategy) { 'rolling' } let(:max_in_flight) { 1 } + let(:original_instances) { 3 } + let(:web_instances) { nil } let(:message) do DeploymentCreateMessage.new({ relationships: { app: { data: { guid: app.guid } } }, droplet: { guid: next_droplet.guid }, strategy: strategy, - options: { max_in_flight: } + options: { max_in_flight:, web_instances: } }) end @@ -495,6 +499,7 @@ module VCAP::CloudController context 'when the app is stopped' do let(:max_in_flight) { 3 } let(:strategy) { 'canary' } + let(:process_state) { ProcessModel::STOPPED } before do app.update(desired_state: ProcessModel::STOPPED) @@ -596,6 +601,53 @@ module VCAP::CloudController end end + context 'uses the web_instances from the message' do + # stopped apps come up immediately and don't go through the deployment updater + let(:web_instances) { 12 } + + it 'saves the web_instances' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.web_instances).to eq(12) + expect(app.reload.newest_web_process.instances).to eq(12) + end + + context 'when web_instances is not set' do + let(:web_instances) { nil } + + it 'sets web_instances to the original web process\'s instance count' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.web_instances).to eq(3) + expect(app.reload.newest_web_process.instances).to eq(3) + end + end + end + + context 'quota validations' do + let!(:web_process) { ProcessModel.make(app: app, instances: 3, memory: 999) } + + before do + app.organization.quota_definition = QuotaDefinition.make(app.organization, memory_limit: 10_000) + app.organization.save + end + + context 'when the final state of the resulting web_process will violate a quota' do + let(:web_instances) { 11 } + + it 'throws an error' do + expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, 'memory quota_exceeded') + end + end + + context 'when the final state of the resulting web_process does not violate a quota' do + let(:web_instances) { 10 } + + it 'does not throw an error' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.web_instances).to eq(10) + end + end + end + context 'uses canary steps from the message' do let(:strategy) { 'canary' } @@ -726,6 +778,81 @@ module VCAP::CloudController end end + context 'uses the web_instances from the message' do + let(:web_instances) { 12 } + + it 'saves the web_instances' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.web_instances).to eq(12) + end + + context 'when web_instances is not set' do + let(:web_instances) { nil } + + it 'sets web_instances to the original web process\'s instance count' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.web_instances).to eq(3) + end + end + + context 'when max_in_flight instances is higher than web_instances' do + let(:web_instances) { 10 } + let(:max_in_flight) { 200 } + let(:original_instances) { 5 } + + it 'creates a web process with web_instances, not original_instances or max_in_flight' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.deploying_web_process.instances).to eq(10) + end + end + + context 'web_instances is lower than original_instances' do + let(:web_instances) { 5 } + let(:max_in_flight) { 200 } + let(:original_instances) { 10 } + + it 'creates a web process with web_instances, not original_instances or max_in_flight' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.deploying_web_process.instances).to eq(5) + end + end + end + + context 'quota validations' do + let!(:web_process) { ProcessModel.make(app: app, instances: 3, memory: 999, state: process_state) } + + before do + app.organization.quota_definition = QuotaDefinition.make(app.organization, memory_limit: 10_000) + app.organization.save + end + + context 'when the final state of the resulting web_process will violate a quota' do + let(:web_instances) { 11 } + + it 'throws an error' do + expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, 'memory quota_exceeded') + end + end + + context 'when the final state of the resulting web_process does not violate a quota' do + let(:web_instances) { 10 } + + it 'does not throw an error' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.web_instances).to eq(10) + end + end + + context 'when checking quota validations' do + let(:web_instances) { 10 } + + it 'doesnt alter the state of the current web process' do + DeploymentCreate.create(app:, message:, user_audit_info:) + expect(app.newest_web_process.instances).to eq(3) + end + end + end + context 'when the app is stopped' do before do app.update(desired_state: ProcessModel::STOPPED) diff --git a/spec/unit/lib/cloud_controller/deployment_updater/updater_spec.rb b/spec/unit/lib/cloud_controller/deployment_updater/updater_spec.rb index 91f7141bd01..766445c41c4 100644 --- a/spec/unit/lib/cloud_controller/deployment_updater/updater_spec.rb +++ b/spec/unit/lib/cloud_controller/deployment_updater/updater_spec.rb @@ -42,10 +42,14 @@ module VCAP::CloudController deploying_web_process: deploying_web_process, state: state, original_web_process_instance_count: original_web_process_instance_count, - max_in_flight: 1 + max_in_flight: max_in_flight, + web_instances: web_instances ) end + let(:web_instances) { nil } + let(:max_in_flight) { 1 } + let(:all_instances_results) do { 0 => { state: 'RUNNING', uptime: 50, since: 2, routable: true }, @@ -127,6 +131,30 @@ module VCAP::CloudController end end + context 'when deployment has web_instances' do + let(:web_instances) { 10 } + let(:max_in_flight) { 100 } + + it 'scales to web_instances instead of original_web_process_instance_count' do + subject.scale + deployment.reload + + expect(deployment.deploying_web_process.instances).to eq(10) + end + end + + context 'when deployment does not have web_instances' do + let(:web_instances) { nil } + let(:max_in_flight) { 100 } + + it 'scales to original_web_process_instance_count' do + subject.scale + deployment.reload + + expect(deployment.deploying_web_process.instances).to eq(6) + end + end + context 'when an error occurs while scaling a deployment' do let(:failing_process) { ProcessModel.make(app: web_process.app, type: 'failing', instances: 5) } let(:deployment) { DeploymentModel.make(app: web_process.app, deploying_web_process: failing_process, state: 'DEPLOYING') } diff --git a/spec/unit/messages/deployment_create_message_spec.rb b/spec/unit/messages/deployment_create_message_spec.rb index b97bf331f1d..909c32ead18 100644 --- a/spec/unit/messages/deployment_create_message_spec.rb +++ b/spec/unit/messages/deployment_create_message_spec.rb @@ -127,6 +127,55 @@ module VCAP::CloudController end end + describe 'web_instances' do + context 'when set to a non-integer' do + before do + body['options'] = { web_instances: 'two' } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Web instances must be an integer greater than 0') + end + end + + context 'when set to a negative integer' do + before do + body['options'] = { web_instances: -2 } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Web instances must be an integer greater than 0') + end + end + + context 'when set to zero' do + before do + body['options'] = { web_instances: 0 } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Web instances must be an integer greater than 0') + end + end + + context 'when set to positive integer' do + before do + body['options'] = { web_instances: 2 } + end + + it 'succeeds' do + message = DeploymentCreateMessage.new(body) + expect(message).to be_valid + end + end + end + describe 'canary options' do before do body['strategy'] = 'canary' @@ -318,8 +367,34 @@ module VCAP::CloudController end end + describe 'web_instances' do + context 'when options is not specified' do + before do + body['options'] = nil + end + + it 'returns nil' do + message = DeploymentCreateMessage.new(body) + expect(message).to be_valid + expect(message.web_instances).to be_nil + end + end + + context 'when web_instances is specified' do + before do + body['options'] = { 'web_instances' => 10 } + end + + it 'returns the passed value' do + message = DeploymentCreateMessage.new(body) + expect(message).to be_valid + expect(message.web_instances).to eq 10 + end + end + end + describe 'max_in_flight' do - context 'when objects is not specified' do + context 'when options is not specified' do before do body['options'] = nil end diff --git a/spec/unit/models/runtime/deployment_model_spec.rb b/spec/unit/models/runtime/deployment_model_spec.rb index bac031187c0..21cc5c5fd9f 100644 --- a/spec/unit/models/runtime/deployment_model_spec.rb +++ b/spec/unit/models/runtime/deployment_model_spec.rb @@ -7,7 +7,8 @@ module VCAP::CloudController let(:deploying_web_process) { ProcessModel.make(health_check_timeout: 180) } let(:canary_steps) { [{ 'instance_weight' => 20 }, { 'instance_weight' => 40 }] } let(:strategy) { DeploymentModel::CANARY_STRATEGY } - let(:deployment) { DeploymentModel.make(app:, droplet:, deploying_web_process:, canary_steps:, strategy:) } + let(:deployment) { DeploymentModel.make(app:, droplet:, deploying_web_process:, canary_steps:, strategy:, web_instances:) } + let(:web_instances) { nil } it 'has an app' do expect(deployment.app.name).to eq('rolling-app') @@ -267,6 +268,22 @@ module VCAP::CloudController end end + describe '#desired_web_instances' do + context 'when web_instances is set' do + let(:web_instances) { 10 } + + it 'returns web_instances' do + expect(deployment.desired_web_instances).to eq(10) + end + end + + context 'when web_instances is not set' do + it 'returns original_web_process' do + expect(deployment.desired_web_instances).to eq(1) + end + end + end + describe '#canary_step_plan' do tests = [ { @@ -380,6 +397,46 @@ module VCAP::CloudController end end + context 'when web_instances is lower than original_web_process_instance_count' do + let(:deployment) do + DeploymentModel.make( + app: app, + strategy: 'canary', + droplet: droplet, + deploying_web_process: deploying_web_process, + canary_steps: [{ instance_weight: 1 }, { instance_weight: 25 }, { instance_weight: 50 }, { instance_weight: 99 }, { instance_weight: 100 }], + original_web_process_instance_count: 100, + web_instances: 10, + canary_current_step: 1 + ) + end + + it 'uses web_instances to calculate a plan' do + expect(deployment.canary_step_plan).to eq([{ canary: 1, original: 10 }, { canary: 3, original: 8 }, { canary: 5, original: 6 }, { canary: 10, original: 1 }, + { canary: 10, original: 0 }]) + end + end + + context 'when original_web_process_instance_count is lower than web_instances' do + let(:deployment) do + DeploymentModel.make( + app: app, + strategy: 'canary', + droplet: droplet, + deploying_web_process: deploying_web_process, + canary_steps: [{ instance_weight: 1 }, { instance_weight: 25 }, { instance_weight: 50 }, { instance_weight: 99 }, { instance_weight: 100 }], + original_web_process_instance_count: 10, + web_instances: 100, + canary_current_step: 1 + ) + end + + it 'uses original_web_process_instance_count to calculate a plan' do + expect(deployment.canary_step_plan).to eq([{ canary: 1, original: 10 }, { canary: 3, original: 8 }, { canary: 5, original: 6 }, { canary: 10, original: 1 }, + { canary: 10, original: 0 }]) + end + end + context 'when deployment is not canary' do let(:deployment) do DeploymentModel.make( diff --git a/spec/unit/presenters/v3/deployment_presenter_spec.rb b/spec/unit/presenters/v3/deployment_presenter_spec.rb index 7c5d9f0e2ac..3cae24ec55a 100644 --- a/spec/unit/presenters/v3/deployment_presenter_spec.rb +++ b/spec/unit/presenters/v3/deployment_presenter_spec.rb @@ -18,7 +18,8 @@ module VCAP::CloudController::Presenters::V3 status_updated_at: '2019-07-11 19:01:54', state: deployment_state, status_value: VCAP::CloudController::DeploymentModel::ACTIVE_STATUS_VALUE, - status_reason: VCAP::CloudController::DeploymentModel::DEPLOYING_STATUS_REASON + status_reason: VCAP::CloudController::DeploymentModel::DEPLOYING_STATUS_REASON, + web_instances: 20 ) end @@ -179,7 +180,12 @@ module VCAP::CloudController::Presenters::V3 describe 'options' do it 'sets max in flight' do result = DeploymentPresenter.new(deployment).to_hash - expect(result[:options][:max_in_flight]).to be(1) + expect(result[:options][:max_in_flight]).to eq(1) + end + + it 'sets web_instances' do + result = DeploymentPresenter.new(deployment).to_hash + expect(result[:options][:web_instances]).to eq(20) end context 'when the strategy is not canary' do