Skip to content

Commit 943495e

Browse files
committed
wip scale memory with deployments
1 parent 01e87c7 commit 943495e

File tree

7 files changed

+313
-51
lines changed

7 files changed

+313
-51
lines changed

app/actions/deployment_create.rb

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ def create(app:, user_audit_info:, message:)
5656
revision_guid: revision&.guid,
5757
revision_version: revision&.version,
5858
strategy: message.strategy,
59+
memory_in_mb: message.memory_in_mb,
5960
max_in_flight: message.max_in_flight,
6061
canary_steps: message.options&.dig(:canary, :steps),
6162
web_instances: message.web_instances || desired_instances
@@ -67,7 +68,7 @@ def create(app:, user_audit_info:, message:)
6768

6869
process_instances = starting_process_instances(deployment, desired_instances)
6970

70-
process = create_deployment_process(app, deployment.guid, revision, process_instances)
71+
process = create_deployment_process(app, message, deployment.guid, revision, process_instances)
7172
# Need to transition from STOPPED to STARTED to engage the ProcessObserver to desire the LRP.
7273
# It'd be better to do this via Diego::Runner.new(process, config).start,
7374
# but it is nontrivial to get that working in test.
@@ -85,8 +86,8 @@ def create(app:, user_audit_info:, message:)
8586
raise error
8687
end
8788

88-
def create_deployment_process(app, deployment_guid, revision, process_instances)
89-
process = clone_existing_web_process(app, revision, process_instances)
89+
def create_deployment_process(app, message, deployment_guid, revision, process_instances)
90+
process = clone_existing_web_process(app, message, revision, process_instances)
9091

9192
DeploymentProcessModel.create(
9293
deployment_guid: deployment_guid,
@@ -97,21 +98,20 @@ def create_deployment_process(app, deployment_guid, revision, process_instances)
9798
process
9899
end
99100

100-
def clone_existing_web_process(app, revision, process_instances)
101+
def clone_existing_web_process(app, message, revision, process_instances)
101102
web_process = app.newest_web_process
102103
command = if revision
103104
revision.commands_by_process_type[ProcessTypes::WEB]
104105
else
105106
web_process.command
106107
end
107-
108108
ProcessModel.create(
109109
app: app,
110110
type: ProcessTypes::WEB,
111111
state: ProcessModel::STOPPED,
112112
instances: process_instances,
113113
command: command,
114-
memory: web_process.memory,
114+
memory: message.memory_in_mb || web_process.memory,
115115
file_descriptors: web_process.file_descriptors,
116116
disk_quota: web_process.disk_quota,
117117
log_rate_limit: web_process.log_rate_limit,
@@ -167,11 +167,13 @@ def record_audit_event(deployment, droplet, user_audit_info, message)
167167
private
168168

169169
def validate_quota!(message, app)
170-
return if message.web_instances.blank?
170+
return if message.web_instances.blank? && message.memory_in_mb.blank?
171171

172172
current_web_process = app.newest_web_process
173-
current_web_process.instances = message.web_instances
174-
173+
current_web_process.instances = message.web_instances if message.web_instances
174+
current_web_process.memory = message.memory_in_mb if message.memory_in_mb
175+
# Quotas wont get checked unless the process is started
176+
current_web_process.state = ProcessModel::STARTED
175177
current_web_process.validate
176178

177179
raise Sequel::ValidationFailed.new(current_web_process) unless current_web_process.valid?
@@ -181,6 +183,7 @@ def validate_quota!(message, app)
181183

182184
def deployment_for_stopped_app(app, message, previous_deployment, previous_droplet, revision, target_state, user_audit_info)
183185
app.newest_web_process.update(instances: message.web_instances) if message.web_instances
186+
app.newest_web_process.update(memory: message.memory_in_mb) if message.memory_in_mb
184187
# Do not create a revision here because AppStart will not handle the rollback case
185188
AppStart.start(app: app, user_audit_info: user_audit_info, create_revision: false)
186189

@@ -197,6 +200,7 @@ def deployment_for_stopped_app(app, message, previous_deployment, previous_dropl
197200
strategy: message.strategy,
198201
max_in_flight: message.max_in_flight,
199202
canary_steps: message.options&.dig(:canary, :steps),
203+
memory_in_mb: message.memory_in_mb,
200204
web_instances: message.web_instances || desired_instances(app.oldest_web_process, previous_deployment)
201205
)
202206

app/messages/deployment_create_message.rb

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
require 'messages/metadata_base_message'
2+
require 'messages/process_scale_message'
23

34
module VCAP::CloudController
45
class DeploymentCreateMessage < MetadataBaseMessage
@@ -14,6 +15,7 @@ class DeploymentCreateMessage < MetadataBaseMessage
1415
canary
1516
max_in_flight
1617
web_instances
18+
memory_in_mb
1719
].freeze
1820

1921
ALLOWED_STEP_KEYS = [
@@ -48,6 +50,10 @@ def web_instances
4850
options&.dig(:web_instances)
4951
end
5052

53+
def memory_in_mb
54+
options&.dig(:memory_in_mb)
55+
end
56+
5157
private
5258

5359
def mutually_exclusive_droplet_sources
@@ -66,12 +72,30 @@ def validate_options
6672

6773
disallowed_keys = options.keys - ALLOWED_OPTION_KEYS
6874
errors.add(:options, "has unsupported key(s): #{disallowed_keys.join(', ')}") if disallowed_keys.present?
69-
70-
validate_web_instances if options[:web_instances]
75+
validate_scaling_options
76+
# validate_web_instances if options[:web_instances]
7177
validate_max_in_flight if options[:max_in_flight]
7278
validate_canary if options[:canary]
7379
end
7480

81+
def validate_scaling_options
82+
scaling_options = {
83+
instances: options[:web_instances],
84+
memory_in_mb: options[:memory_in_mb]
85+
}
86+
87+
message = ProcessScaleMessage.new(scaling_options)
88+
message.valid?
89+
if message.errors[:instances].present?
90+
message.errors.select { |e| e.attribute == :instances }.each do |error|
91+
errors.import(error, { attribute: :web_instances })
92+
end
93+
message.errors.delete(:instances)
94+
end
95+
96+
errors.merge!(message.errors)
97+
end
98+
7599
def validate_max_in_flight
76100
max_in_flight = options[:max_in_flight]
77101

app/models/runtime/constraints/max_memory_policy.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ def field
3434
end
3535

3636
class AppMaxMemoryPolicy < BaseMaxMemoryPolicy
37+
def validate
38+
return unless policy_target
39+
return unless additional_checks
40+
41+
return if policy_target.has_remaining_memory(requested_memory)
42+
43+
resource.errors.add(field, error_name)
44+
end
45+
3746
private
3847

3948
def additional_checks
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
Sequel.migration do
2+
up do
3+
alter_table(:deployments) do
4+
add_column :memory_in_mb, :integer, null: true
5+
end
6+
end
7+
down do
8+
alter_table(:deployments) do
9+
drop_column :memory_in_mb
10+
end
11+
end
12+
end

spec/request/deployments_spec.rb

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,6 +1161,76 @@
11611161
end
11621162
end
11631163

1164+
context 'memory_in_mb' do
1165+
let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, instances: 10) }
1166+
let(:user) { make_developer_for_space(space) }
1167+
let(:memory_in_mb) { 5 }
1168+
let(:create_request) do
1169+
{
1170+
options: {
1171+
memory_in_mb:
1172+
},
1173+
relationships: {
1174+
app: {
1175+
data: {
1176+
guid: app_model.guid
1177+
}
1178+
}
1179+
}
1180+
}
1181+
end
1182+
1183+
context 'when memory_in_mb is provided' do
1184+
it 'is set on the resource' do
1185+
post '/v3/deployments', create_request.to_json, user_header
1186+
expect(last_response.status).to eq(201), last_response.body
1187+
1188+
expect(parsed_response['options']['memory_in_mb']).to eq(5)
1189+
end
1190+
end
1191+
1192+
context 'when memory_in_mb violates a quota' do
1193+
let(:memory_in_mb) { 100_000 }
1194+
let(:quota) { VCAP::CloudController::QuotaDefinition.make(memory_limit: 1_000_000) }
1195+
1196+
before do
1197+
org.quota_definition = quota
1198+
org.save
1199+
end
1200+
1201+
it 'is set on the resource' do
1202+
post '/v3/deployments', create_request.to_json, user_header
1203+
expect(last_response.status).to eq(422), last_response.body
1204+
expect(parsed_response['errors'][0]['detail']).to match('memory quota_exceeded')
1205+
end
1206+
end
1207+
1208+
context 'when memory_in_mb is not provided' do
1209+
let(:create_request) do
1210+
{
1211+
relationships: {
1212+
app: {
1213+
data: {
1214+
guid: app_model.guid
1215+
}
1216+
}
1217+
}
1218+
}
1219+
end
1220+
1221+
it 'is not returned as part of the deployment options' do
1222+
post '/v3/deployments', create_request.to_json, user_header
1223+
expect(last_response.status).to eq(201), last_response.body
1224+
1225+
expect(parsed_response['options'].key?('memory_in_mb')).to be false
1226+
end
1227+
end
1228+
end
1229+
1230+
context 'disk_in_mb'
1231+
1232+
context 'log_rate_limit_in_bytes_per_second'
1233+
11641234
context 'web_instances' do
11651235
let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, instances: 10) }
11661236
let(:user) { make_developer_for_space(space) }

0 commit comments

Comments
 (0)