diff --git a/app/actions/deployment_create.rb b/app/actions/deployment_create.rb index 6a15a2ec4ba..3a1bce39f99 100644 --- a/app/actions/deployment_create.rb +++ b/app/actions/deployment_create.rb @@ -13,6 +13,9 @@ def create(app:, user_audit_info:, message:) DeploymentModel.db.transaction do app.lock! + # Stopped apps will have quota validated since we scale their process up immediately later + validate_quota!(message, app) unless app.stopped? + message.strategy ||= DeploymentModel::ROLLING_STRATEGY target_state = DeploymentTargetState.new(app, message) @@ -29,45 +32,45 @@ def create(app:, user_audit_info:, message:) previous_deployment = DeploymentModel.find(app: app, status_value: DeploymentModel::ACTIVE_STATUS_VALUE) + deployment = create_deployment( + app, + message, + previous_deployment, + previous_droplet, + revision, + target_state, + user_audit_info + ) + if app.stopped? - return deployment_for_stopped_app( - app, - message, - previous_deployment, - previous_droplet, - revision, - target_state, - user_audit_info - ) + process = app.newest_web_process + else + process_instances = starting_process_instances(deployment, desired_instances(app.oldest_web_process, previous_deployment)) + process = create_deployment_process(app, deployment.guid, revision, process_instances) end - desired_instances = desired_instances(app.oldest_web_process, previous_deployment) - - validate_quota!(message, app) - - deployment = DeploymentModel.create( - app: app, - state: starting_state(message), - status_value: DeploymentModel::ACTIVE_STATUS_VALUE, - status_reason: DeploymentModel::DEPLOYING_STATUS_REASON, - droplet: target_state.droplet, - previous_droplet: previous_droplet, - original_web_process_instance_count: desired_instances, - revision_guid: revision&.guid, - revision_version: revision&.version, - strategy: message.strategy, - max_in_flight: message.max_in_flight, - canary_steps: message.options&.dig(:canary, :steps), - web_instances: message.web_instances || desired_instances - ) + process.memory = message.memory_in_mb if message.memory_in_mb + process.disk_quota = message.disk_in_mb if message.disk_in_mb + process.log_rate_limit = message.log_rate_limit_in_bytes_per_second if message.log_rate_limit_in_bytes_per_second - MetadataUpdate.update(deployment, message) + if app.stopped? + process.instances = message.web_instances if message.web_instances - supersede_deployment(previous_deployment) + process.save_changes + + # 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) + deployment.update(state: DeploymentModel::DEPLOYED_STATE, + status_value: DeploymentModel::FINALIZED_STATUS_VALUE, + status_reason: DeploymentModel::DEPLOYED_STATUS_REASON) + record_audit_event(deployment, target_state.droplet, user_audit_info, message) + return deployment + end - process_instances = starting_process_instances(deployment, desired_instances) + process.save + + supersede_deployment(previous_deployment) - process = create_deployment_process(app, deployment.guid, revision, process_instances) # Need to transition from STOPPED to STARTED to engage the ProcessObserver to desire the LRP. # It'd be better to do this via Diego::Runner.new(process, config).start, # but it is nontrivial to get that working in test. @@ -104,7 +107,6 @@ def clone_existing_web_process(app, revision, process_instances) else web_process.command end - ProcessModel.create( app: app, type: ProcessTypes::WEB, @@ -167,11 +169,15 @@ def record_audit_event(deployment, droplet, user_audit_info, message) private def validate_quota!(message, app) - return if message.web_instances.blank? + return if message.web_instances.blank? && message.memory_in_mb.blank? && message.log_rate_limit_in_bytes_per_second.blank? current_web_process = app.newest_web_process - current_web_process.instances = message.web_instances - + current_web_process.instances = message.web_instances if message.web_instances + current_web_process.memory = message.memory_in_mb if message.memory_in_mb + current_web_process.disk_quota = message.disk_in_mb if message.disk_in_mb + current_web_process.log_rate_limit = message.log_rate_limit_in_bytes_per_second if message.log_rate_limit_in_bytes_per_second + # Quotas wont get checked unless the process is started + current_web_process.state = ProcessModel::STARTED current_web_process.validate raise Sequel::ValidationFailed.new(current_web_process) unless current_web_process.valid? @@ -179,16 +185,12 @@ def validate_quota!(message, app) 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) - + def create_deployment(app, message, previous_deployment, previous_droplet, revision, target_state, _user_audit_info) deployment = DeploymentModel.create( app: app, - state: DeploymentModel::DEPLOYED_STATE, - status_value: DeploymentModel::FINALIZED_STATUS_VALUE, - status_reason: DeploymentModel::DEPLOYED_STATUS_REASON, + state: starting_state(message), + status_value: DeploymentModel::ACTIVE_STATUS_VALUE, + status_reason: DeploymentModel::DEPLOYING_STATUS_REASON, droplet: target_state.droplet, previous_droplet: previous_droplet, original_web_process_instance_count: desired_instances(app.oldest_web_process, previous_deployment), @@ -196,14 +198,13 @@ 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, + memory_in_mb: message.memory_in_mb, + disk_in_mb: message.disk_in_mb, + log_rate_limit_in_bytes_per_second: message.log_rate_limit_in_bytes_per_second, canary_steps: message.options&.dig(:canary, :steps), - web_instances: message.web_instances || desired_instances(app.oldest_web_process, previous_deployment) + web_instances: message.web_instances ) - MetadataUpdate.update(deployment, message) - - record_audit_event(deployment, target_state.droplet, user_audit_info, message) - deployment end diff --git a/app/messages/deployment_create_message.rb b/app/messages/deployment_create_message.rb index e260d88766b..f1ee39cde03 100644 --- a/app/messages/deployment_create_message.rb +++ b/app/messages/deployment_create_message.rb @@ -1,4 +1,5 @@ require 'messages/metadata_base_message' +require 'messages/process_scale_message' module VCAP::CloudController class DeploymentCreateMessage < MetadataBaseMessage @@ -14,6 +15,9 @@ class DeploymentCreateMessage < MetadataBaseMessage canary max_in_flight web_instances + memory_in_mb + disk_in_mb + log_rate_limit_in_bytes_per_second ].freeze ALLOWED_STEP_KEYS = [ @@ -48,6 +52,18 @@ def web_instances options&.dig(:web_instances) end + def memory_in_mb + options&.dig(:memory_in_mb) + end + + def disk_in_mb + options&.dig(:disk_in_mb) + end + + def log_rate_limit_in_bytes_per_second + options&.dig(:log_rate_limit_in_bytes_per_second) + end + private def mutually_exclusive_droplet_sources @@ -66,12 +82,31 @@ 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_scaling_options validate_max_in_flight if options[:max_in_flight] validate_canary if options[:canary] end + def validate_scaling_options + scaling_options = { + instances: options[:web_instances], + memory_in_mb: options[:memory_in_mb], + disk_in_mb: options[:disk_in_mb], + log_rate_limit_in_bytes_per_second: options[:log_rate_limit_in_bytes_per_second] + } + + message = ProcessScaleMessage.new(scaling_options) + message.valid? + if message.errors[:instances].present? + message.errors.select { |e| e.attribute == :instances }.each do |error| + errors.import(error, { attribute: :web_instances }) + end + message.errors.delete(:instances) + end + + errors.merge!(message.errors) + end + def validate_max_in_flight max_in_flight = options[:max_in_flight] diff --git a/app/models/runtime/constraints/max_log_rate_limit_policy.rb b/app/models/runtime/constraints/max_log_rate_limit_policy.rb index 65e24ce84fc..f4b979cb89a 100644 --- a/app/models/runtime/constraints/max_log_rate_limit_policy.rb +++ b/app/models/runtime/constraints/max_log_rate_limit_policy.rb @@ -50,7 +50,7 @@ class AppMaxLogRateLimitPolicy < BaseMaxLogRateLimitPolicy def additional_checks resource.started? && - (resource.column_changed?(:state) || resource.column_changed?(:instances)) + (resource.column_changed?(:state) || resource.column_changed?(:instances) || resource.column_changed?(:log_rate_limit)) end def requested_log_rate_limit diff --git a/app/models/runtime/deployment_model.rb b/app/models/runtime/deployment_model.rb index c8fb8cf0935..941c3190ff1 100644 --- a/app/models/runtime/deployment_model.rb +++ b/app/models/runtime/deployment_model.rb @@ -114,10 +114,6 @@ def continuable? 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 diff --git a/app/presenters/v3/deployment_presenter.rb b/app/presenters/v3/deployment_presenter.rb index f4cd240d1d9..dff7ebe9978 100644 --- a/app/presenters/v3/deployment_presenter.rb +++ b/app/presenters/v3/deployment_presenter.rb @@ -57,9 +57,12 @@ def new_processes def options(deployment) options = { - max_in_flight: deployment.max_in_flight, - web_instances: deployment.desired_web_instances + max_in_flight: deployment.max_in_flight } + options[:web_instances] = deployment.web_instances if deployment.web_instances + options[:memory_in_mb] = deployment.memory_in_mb if deployment.memory_in_mb + options[:disk_in_mb] = deployment.disk_in_mb if deployment.disk_in_mb + options[:log_rate_limit_in_bytes_per_second] = deployment.log_rate_limit_in_bytes_per_second if deployment.log_rate_limit_in_bytes_per_second if deployment.strategy == VCAP::CloudController::DeploymentModel::CANARY_STRATEGY && deployment.canary_steps options[:canary] = { diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index 51b471df184..649a71d6f04 100644 --- a/config/cloud_controller.yml +++ b/config/cloud_controller.yml @@ -11,7 +11,7 @@ readiness_port: external_protocol: http external_domain: api2.vcap.me -temporary_disable_deployments: true +temporary_disable_deployments: false deployment_updater: update_frequency_in_seconds: 30 lock_key: 'cf-deployment-updater' diff --git a/db/migrations/20250312233355_add_scaling_columns_to_deployments.rb b/db/migrations/20250312233355_add_scaling_columns_to_deployments.rb new file mode 100644 index 00000000000..8c3c445d4a9 --- /dev/null +++ b/db/migrations/20250312233355_add_scaling_columns_to_deployments.rb @@ -0,0 +1,16 @@ +Sequel.migration do + up do + alter_table(:deployments) do + add_column :memory_in_mb, :integer, null: true + add_column :disk_in_mb, :integer, null: true + add_column :log_rate_limit_in_bytes_per_second, :integer, null: true + end + end + down do + alter_table(:deployments) do + drop_column :memory_in_mb + drop_column :disk_in_mb + drop_column :log_rate_limit_in_bytes_per_second + end + end +end diff --git a/docs/v3/source/includes/api_resources/_deployments.erb b/docs/v3/source/includes/api_resources/_deployments.erb index 0d4b442858c..abdbd864aba 100644 --- a/docs/v3/source/includes/api_resources/_deployments.erb +++ b/docs/v3/source/includes/api_resources/_deployments.erb @@ -20,6 +20,9 @@ "options" : { "max_in_flight": 3, "web_instances": 5, + "memory_in_mb": 1024, + "disk_in_mb": 1024, + "log_rate_limit_in_bytes_per_second": -1, "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 90af1d46c37..1f9e4f9fd05 100644 --- a/docs/v3/source/includes/resources/deployments/_create.md.erb +++ b/docs/v3/source/includes/resources/deployments/_create.md.erb @@ -79,6 +79,9 @@ Name | Type | Description | Default **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.memory_in_mb** | _integer_ | The amount of memory in megabytes to allocate per web process instance. If `null`, the amount allocated will be taken from the previous web process. | `null` +**options.disk_in_mb** | _integer_ | The amount of disk in megabytes to allocate per web process instance. If `null`, the amount allocated will be taken from the previous web process. | `null` +**options.log_rate_limit_in_bytes_per_second** | _integer_ | Log rate limit in bytes per second to allocate per web process instance. If `null`, the amount allocated will be taken from the previous web process. | `null` **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 b8fd2179770..cb9458ab2d7 100644 --- a/docs/v3/source/includes/resources/deployments/_object.md.erb +++ b/docs/v3/source/includes/resources/deployments/_object.md.erb @@ -21,6 +21,9 @@ Name | Type | Description **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.memory_in_mb** | _integer_ | The amount of memory in megabytes to allocate per web process instance. If `null`, the amount allocated will be taken from the previous web process. +**options.disk_in_mb** | _integer_ | The amount of disk in megabytes to allocate per web process instance. If `null`, the amount allocated will be taken from the previous web process. +**options.log_rate_limit_in_bytes_per_second** | _integer_ | Log rate limit in bytes per second to allocate per web process instance. If `null`, the amount allocated will be taken from the previous web process. **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/spec/request/deployments_spec.rb b/spec/request/deployments_spec.rb index 3c2fa2f82dd..4ec66e94845 100644 --- a/spec/request/deployments_spec.rb +++ b/spec/request/deployments_spec.rb @@ -45,8 +45,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -147,8 +146,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => other_droplet.guid @@ -234,8 +232,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => other_droplet.guid @@ -357,8 +354,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -440,8 +436,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => other_droplet.guid @@ -525,8 +520,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => other_droplet.guid @@ -698,8 +692,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -763,8 +756,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -849,8 +841,7 @@ 'type' => deployment.deploying_web_process.type }], 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'created_at' => iso8601, 'updated_at' => iso8601, @@ -924,8 +915,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -1001,8 +991,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 5, - 'web_instances' => 1 + 'max_in_flight' => 5 }, 'droplet' => { 'guid' => droplet.guid @@ -1110,7 +1099,6 @@ 'strategy' => 'canary', 'options' => { 'max_in_flight' => 1, - 'web_instances' => 1, 'canary' => { 'steps' => [ { 'instance_weight' => 20 }, @@ -1161,6 +1149,202 @@ end end + context 'memory_in_mb' do + let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, instances: 10) } + let(:user) { make_developer_for_space(space) } + let(:memory_in_mb) { 500 } + let(:create_request) do + { + options: { + memory_in_mb: + }, + relationships: { + app: { + data: { + guid: app_model.guid + } + } + } + } + end + + context 'when memory_in_mb 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']['memory_in_mb']).to eq(500) + end + end + + context 'when memory_in_mb violates a quota' do + let(:memory_in_mb) { 1001 } + let(:quota) { VCAP::CloudController::QuotaDefinition.make(memory_limit: 10_000) } + + 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 memory_in_mb is not provided' do + let(:create_request) do + { + relationships: { + app: { + data: { + guid: app_model.guid + } + } + } + } + end + + it 'is not returned as part of the deployment options' do + post '/v3/deployments', create_request.to_json, user_header + expect(last_response.status).to eq(201), last_response.body + + expect(parsed_response['options'].key?('memory_in_mb')).to be false + end + end + end + + context 'disk_in_mb' do + let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, instances: 10) } + let(:user) { make_developer_for_space(space) } + let(:disk_in_mb) { 500 } + let(:create_request) do + { + options: { + disk_in_mb: + }, + relationships: { + app: { + data: { + guid: app_model.guid + } + } + } + } + end + + context 'when disk_in_mb 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']['disk_in_mb']).to eq(500) + end + end + + context 'when disk_in_mb violates a quota' do + let(:disk_in_mb) { 1001 } + + before do + TestConfig.override(maximum_app_disk_in_mb: 1000) + 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('disk_quota too much disk requested (requested 1024 MB - must be less than 1000 MB)') + end + end + + context 'when disk_in_mb is not provided' do + let(:create_request) do + { + relationships: { + app: { + data: { + guid: app_model.guid + } + } + } + } + end + + it 'is not returned as part of the deployment options' do + post '/v3/deployments', create_request.to_json, user_header + expect(last_response.status).to eq(201), last_response.body + + expect(parsed_response['options'].key?('disk_in_mb')).to be false + end + end + end + + context 'log_rate_limit_in_bytes_per_second' do + let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, instances: 10) } + let(:user) { make_developer_for_space(space) } + let(:log_rate_limit_in_bytes_per_second) { 500 } + let(:create_request) do + { + options: { + log_rate_limit_in_bytes_per_second: + }, + relationships: { + app: { + data: { + guid: app_model.guid + } + } + } + } + end + + context 'when log_rate_limit_in_bytes_per_second 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']['log_rate_limit_in_bytes_per_second']).to eq(500) + end + end + + context 'when log_rate_limit_in_bytes_per_second violates a quota' do + let(:log_rate_limit_in_bytes_per_second) { 1001 } + let(:quota) { VCAP::CloudController::QuotaDefinition.make(log_rate_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('log_rate_limit exceeds organization log rate quota') + end + end + + context 'when log_rate_limit_in_bytes_per_second is not provided' do + let(:create_request) do + { + relationships: { + app: { + data: { + guid: app_model.guid + } + } + } + } + end + + it 'is not returned as part of the deployment options' do + post '/v3/deployments', create_request.to_json, user_header + expect(last_response.status).to eq(201), last_response.body + + expect(parsed_response['options'].key?('log_rate_limit_in_bytes_per_second')).to be false + end + 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) } @@ -1218,11 +1402,11 @@ } end - it 'defaults to original_web_process_instance_count' do + it 'is not set' 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) + expect(parsed_response['options'].key?('web_instances')).to be false end end end @@ -1294,8 +1478,7 @@ }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => droplet.guid @@ -1403,8 +1586,7 @@ 'metadata' => metadata, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'relationships' => { 'app' => { @@ -1616,9 +1798,9 @@ def json_for_status(deployment, status_value, status_reason) def json_for_options(deployment) options = { - max_in_flight: deployment.max_in_flight, - web_instances: deployment.desired_web_instances + max_in_flight: deployment.max_in_flight } + options[:web_instances] = deployment.web_instances if deployment.web_instances if deployment.canary_steps options[:canary] = { steps: deployment.canary_steps @@ -1883,8 +2065,7 @@ def json_for_options(deployment) }, 'strategy' => 'rolling', 'options' => { - 'max_in_flight' => 1, - 'web_instances' => 1 + 'max_in_flight' => 1 }, 'droplet' => { 'guid' => droplet.guid diff --git a/spec/unit/actions/deployment_create_spec.rb b/spec/unit/actions/deployment_create_spec.rb index 6ab39220d19..c19b98fa147 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: original_instances, log_rate_limit: 101, state: process_state) } + let!(:web_process) { ProcessModel.make(app: app, instances: original_instances, memory: 500, disk_quota: 1024, 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) } @@ -22,13 +22,16 @@ module VCAP::CloudController let(:max_in_flight) { 1 } let(:original_instances) { 3 } let(:web_instances) { nil } + let(:memory_in_mb) { nil } + let(:disk_in_mb) { nil } + let(:log_rate_limit_in_bytes_per_second) { nil } let(:message) do DeploymentCreateMessage.new({ relationships: { app: { data: { guid: app.guid } } }, droplet: { guid: next_droplet.guid }, strategy: strategy, - options: { max_in_flight:, web_instances: } + options: { max_in_flight:, web_instances:, memory_in_mb:, disk_in_mb:, log_rate_limit_in_bytes_per_second: } }) end @@ -614,36 +617,83 @@ module VCAP::CloudController 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 + it 'sets web_instances to nil' do deployment = DeploymentCreate.create(app:, message:, user_audit_info:) - expect(deployment.web_instances).to eq(3) + expect(deployment.web_instances).to be_nil expect(app.reload.newest_web_process.instances).to eq(3) 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 end - context 'quota validations' do - let!(:web_process) { ProcessModel.make(app: app, instances: 3, memory: 999) } + context 'uses the memory_in_mb from the message' do + let(:memory_in_mb) { 1000 } - before do - app.organization.quota_definition = QuotaDefinition.make(app.organization, memory_limit: 10_000) - app.organization.save + it 'saves the memory_in_mb' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.memory_in_mb).to eq(1000) + expect(app.reload.newest_web_process.memory).to eq(1000) end - context 'when the final state of the resulting web_process will violate a quota' do - let(:web_instances) { 11 } + context 'when memory_in_mb is not set' do + let(:memory_in_mb) { nil } - it 'throws an error' do - expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, 'memory quota_exceeded') + it 'sets web_instances to the original web process\'s instance count' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.memory_in_mb).to be_nil + expect(app.reload.newest_web_process.memory).to eq(500) end end - context 'when the final state of the resulting web_process does not violate a quota' do - let(:web_instances) { 10 } + context 'quota validations' do + let!(:web_process) { ProcessModel.make(app: app, instances: 3, memory: 999) } - it 'does not throw an error' do - deployment = DeploymentCreate.create(app:, message:, user_audit_info:) - expect(deployment.web_instances).to eq(10) + 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(:memory_in_mb) { 4000 } + + 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(:memory_in_mb) { 3000 } + + it 'does not throw an error' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.memory_in_mb).to eq(3000) + expect(app.reload.newest_web_process.memory).to eq(3000) + end end end end @@ -789,9 +839,9 @@ module VCAP::CloudController 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 + it 'sets web_instances to nil' do deployment = DeploymentCreate.create(app:, message:, user_audit_info:) - expect(deployment.web_instances).to eq(3) + expect(deployment.web_instances).to be_nil end end @@ -816,39 +866,182 @@ module VCAP::CloudController expect(deployment.deploying_web_process.instances).to eq(5) end end + + context 'quota validations' do + let!(:web_process) { ProcessModel.make(app: app, instances: 3, memory: 999, state: 'STOPPED') } + + 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 end - context 'quota validations' do - let!(:web_process) { ProcessModel.make(app: app, instances: 3, memory: 999, state: process_state) } + context 'uses the memory_in_mb from the message' do + let(:memory_in_mb) { 1000 } - before do - app.organization.quota_definition = QuotaDefinition.make(app.organization, memory_limit: 10_000) - app.organization.save + it 'saves the memory_in_mb' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.memory_in_mb).to eq(1000) + expect(app.reload.newest_web_process.memory).to eq(1000) end - context 'when the final state of the resulting web_process will violate a quota' do - let(:web_instances) { 11 } + context 'when memory_in_mb is not set' do + let(:memory_in_mb) { nil } - it 'throws an error' do - expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, 'memory quota_exceeded') + it 'sets web_instances to the original web process\'s instance count' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.memory_in_mb).to be_nil + expect(app.reload.newest_web_process.memory).to eq(500) end end - context 'when the final state of the resulting web_process does not violate a quota' do - let(:web_instances) { 10 } + 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(:memory_in_mb) { 4000 } + + 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(:memory_in_mb) { 3000 } + + it 'does not throw an error' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.memory_in_mb).to eq(3000) + expect(app.reload.newest_web_process.memory).to eq(3000) + end + end + end + end - it 'does not throw an error' do + context 'uses the disk_in_mb from the message' do + let(:disk_in_mb) { 1000 } + + it 'saves the disk_in_mb' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.disk_in_mb).to eq(1000) + expect(app.reload.newest_web_process.disk_quota).to eq(1000) + end + + context 'when disk_in_mb is not set' do + let(:disk_in_mb) { 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(10) + expect(deployment.disk_in_mb).to be_nil + expect(app.reload.newest_web_process.disk_quota).to eq(1024) end end - context 'when checking quota validations' do - let(:web_instances) { 10 } + context 'quota validations' do + let!(:web_process) { ProcessModel.make(app: app, instances: 3, disk_quota: 999, state: process_state) } - 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) + before do + TestConfig.override(maximum_app_disk_in_mb: 1000) + end + + context 'when the final state of the resulting web_process will violate a quota' do + let(:disk_in_mb) { 4000 } + + it 'throws an error' do + expect do + DeploymentCreate.create(app:, message:, + user_audit_info:) + end.to raise_error(DeploymentCreate::Error, 'disk_quota too much disk requested (requested 4000 MB - must be less than 1000 MB)') + end + end + + context 'when the final state of the resulting web_process does not violate a quota' do + let(:disk_in_mb) { 999 } + + it 'does not throw an error' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.disk_in_mb).to eq(999) + expect(app.reload.newest_web_process.disk_quota).to eq(999) + end + end + end + end + + context 'uses the log_rate_limit_in_bytes_per_second from the message' do + let(:log_rate_limit_in_bytes_per_second) { 1000 } + + it 'saves the log_rate_limit_in_bytes_per_second' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.log_rate_limit_in_bytes_per_second).to eq(1000) + expect(app.reload.newest_web_process.log_rate_limit).to eq(1000) + end + + context 'when log_rate_limit_in_bytes_per_second is not set' do + let(:log_rate_limit_in_bytes_per_second) { nil } + + it 'sets web_instances to the original web process\'s instance count' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.log_rate_limit_in_bytes_per_second).to be_nil + expect(app.reload.newest_web_process.log_rate_limit).to eq(101) + end + end + + context 'quota validations' do + let!(:web_process) { ProcessModel.make(app: app, instances: 3, log_rate_limit: 999, memory: 999, state: process_state) } + + before do + app.organization.quota_definition = QuotaDefinition.make(app.organization, log_rate_limit: 10_000) + app.organization.save + end + + context 'when the final state of the resulting web_process will violate a quota' do + let(:log_rate_limit_in_bytes_per_second) { 4000 } + + it 'throws an error' do + expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, 'log_rate_limit exceeds organization log rate quota') + end + end + + context 'when the final state of the resulting web_process does not violate a quota' do + let(:log_rate_limit_in_bytes_per_second) { 3000 } + + it 'does not throw an error' do + deployment = DeploymentCreate.create(app:, message:, user_audit_info:) + expect(deployment.log_rate_limit_in_bytes_per_second).to eq(3000) + expect(app.reload.newest_web_process.log_rate_limit).to eq(3000) + end end end end diff --git a/spec/unit/messages/deployment_create_message_spec.rb b/spec/unit/messages/deployment_create_message_spec.rb index 909c32ead18..b8292ef12e1 100644 --- a/spec/unit/messages/deployment_create_message_spec.rb +++ b/spec/unit/messages/deployment_create_message_spec.rb @@ -136,7 +136,7 @@ module VCAP::CloudController 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') + expect(message.errors.full_messages).to include('Web instances is not a number') end end @@ -148,7 +148,7 @@ module VCAP::CloudController 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') + expect(message.errors.full_messages).to include('Web instances must be greater than or equal to 0') end end @@ -157,16 +157,172 @@ module VCAP::CloudController body['options'] = { web_instances: 0 } end + it 'is valid' do + message = DeploymentCreateMessage.new(body) + expect(message).to be_valid + 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 'memory_in_mb' do + context 'when set to a non-integer' do + before do + body['options'] = { memory_in_mb: 'two' } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Memory in mb is not a number') + end + end + + context 'when set to a negative integer' do + before do + body['options'] = { memory_in_mb: -2 } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Memory in mb must be greater than 0') + end + end + + context 'when set to zero' do + before do + body['options'] = { memory_in_mb: 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') + expect(message.errors.full_messages).to include('Memory in mb must be greater than 0') end end context 'when set to positive integer' do before do - body['options'] = { web_instances: 2 } + body['options'] = { memory_in_mb: 2 } + end + + it 'succeeds' do + message = DeploymentCreateMessage.new(body) + expect(message).to be_valid + end + end + end + + describe 'disk_in_mb' do + context 'when set to a non-integer' do + before do + body['options'] = { disk_in_mb: 'two' } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Disk in mb is not a number') + end + end + + context 'when set to a negative integer' do + before do + body['options'] = { disk_in_mb: -2 } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Disk in mb must be greater than 0') + end + end + + context 'when set to zero' do + before do + body['options'] = { disk_in_mb: 0 } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Disk in mb must be greater than 0') + end + end + + context 'when set to positive integer' do + before do + body['options'] = { disk_in_mb: 2 } + end + + it 'succeeds' do + message = DeploymentCreateMessage.new(body) + expect(message).to be_valid + end + end + end + + describe 'log_rate_limit_in_bytes_per_second' do + context 'when set to a non-integer' do + before do + body['options'] = { log_rate_limit_in_bytes_per_second: 'two' } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Log rate limit in bytes per second is not a number') + end + end + + context 'when set to a negative integer below -1' do + before do + body['options'] = { log_rate_limit_in_bytes_per_second: -2 } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Log rate limit in bytes per second must be greater than or equal to -1') + end + end + + context 'when set negative 1' do + before do + body['options'] = { log_rate_limit_in_bytes_per_second: -1 } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).to be_valid + end + end + + context 'when set to zero' do + before do + body['options'] = { log_rate_limit_in_bytes_per_second: 0 } + end + + it 'is not valid' do + message = DeploymentCreateMessage.new(body) + expect(message).to be_valid + end + end + + context 'when set to positive integer' do + before do + body['options'] = { log_rate_limit_in_bytes_per_second: 2 } end it 'succeeds' do diff --git a/spec/unit/models/runtime/constraints/max_log_rate_limit_policy_spec.rb b/spec/unit/models/runtime/constraints/max_log_rate_limit_policy_spec.rb index decc70e95e8..cf7ca107044 100644 --- a/spec/unit/models/runtime/constraints/max_log_rate_limit_policy_spec.rb +++ b/spec/unit/models/runtime/constraints/max_log_rate_limit_policy_spec.rb @@ -50,6 +50,17 @@ expect(validator).to validate_with_error(process, :log_rate_limit, error_name) end end + + context 'when the log_rate_limit has changed and would exceed the quota' do + before do + process.log_rate_limit = 500 + end + + it 'registers an error' do + expect(org_or_space).to receive(:has_remaining_log_rate_limit).with(400).and_return(false) + expect(validator).to validate_with_error(process, :log_rate_limit, error_name) + end + end end context 'when the app is being stopped' do diff --git a/spec/unit/presenters/v3/deployment_presenter_spec.rb b/spec/unit/presenters/v3/deployment_presenter_spec.rb index 3cae24ec55a..a037e7a927b 100644 --- a/spec/unit/presenters/v3/deployment_presenter_spec.rb +++ b/spec/unit/presenters/v3/deployment_presenter_spec.rb @@ -19,7 +19,8 @@ module VCAP::CloudController::Presenters::V3 state: deployment_state, status_value: VCAP::CloudController::DeploymentModel::ACTIVE_STATUS_VALUE, status_reason: VCAP::CloudController::DeploymentModel::DEPLOYING_STATUS_REASON, - web_instances: 20 + web_instances: 20, + memory_in_mb: 1000 ) end @@ -188,6 +189,11 @@ module VCAP::CloudController::Presenters::V3 expect(result[:options][:web_instances]).to eq(20) end + it 'sets memory_in_mb' do + result = DeploymentPresenter.new(deployment).to_hash + expect(result[:options][:memory_in_mb]).to eq(1000) + end + context 'when the strategy is not canary' do it 'does not present the canary steps' do result = DeploymentPresenter.new(deployment).to_hash