Skip to content

Commit

Permalink
Apply log rate limit to staging tasks
Browse files Browse the repository at this point in the history
- The log rate limit from the application web process is applied to
  the staging task
- The staging log rate limit can be customized when creating a build,
  for consistency with memory and disk limits

[#182311441](https://www.pivotaltracker.com/story/show/182311441)

Co-authored-by: Rebecca Roberts <[email protected]>
  • Loading branch information
acrmp and rroberts2222 committed Aug 19, 2022
1 parent fc22209 commit 6d3e2e8
Show file tree
Hide file tree
Showing 14 changed files with 188 additions and 5 deletions.
13 changes: 12 additions & 1 deletion app/actions/build_create.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
require 'cloud_controller/backends/quota_validating_staging_log_rate_limit_calculator'
require 'cloud_controller/backends/quota_validating_staging_memory_calculator'
require 'cloud_controller/backends/staging_disk_calculator'
require 'cloud_controller/backends/staging_environment_builder'
Expand Down Expand Up @@ -25,12 +26,14 @@ class StagingInProgress < BuildError
def initialize(user_audit_info: UserAuditInfo.from_context(SecurityContext),
memory_limit_calculator: QuotaValidatingStagingMemoryCalculator.new,
disk_limit_calculator: StagingDiskCalculator.new,
log_rate_limit_calculator: QuotaValidatingStagingLogRateLimitCalculator.new,
environment_presenter: StagingEnvironmentBuilder.new)

@user_audit_info = user_audit_info
@memory_limit_calculator = memory_limit_calculator
@disk_limit_calculator = disk_limit_calculator
@environment_builder = environment_presenter
@log_rate_limit_calculator = log_rate_limit_calculator
@environment_builder = environment_presenter
end

def create_and_stage(package:, lifecycle:, metadata: nil, start_after_staging: false)
Expand All @@ -49,6 +52,7 @@ def create_and_stage(package:, lifecycle:, metadata: nil, start_after_staging: f
app: package.app,
staging_memory_in_mb: staging_details.staging_memory_in_mb,
staging_disk_in_mb: staging_details.staging_disk_in_mb,
staging_log_rate_limit: staging_details.staging_log_rate_limit_bytes_per_second,
created_by_user_guid: @user_audit_info.user_guid,
created_by_user_name: @user_audit_info.user_name,
created_by_user_email: @user_audit_info.user_email
Expand Down Expand Up @@ -117,6 +121,7 @@ def get_staging_details(package, lifecycle)

memory_limit = get_memory_limit(lifecycle.staging_message.staging_memory_in_mb, app, space, org)
disk_limit = get_disk_limit(lifecycle.staging_message.staging_disk_in_mb, app)
log_rate_limit = get_log_rate_limit(lifecycle.staging_message.staging_log_rate_limit_bytes_per_second, app, space, org)
environment_variables = @environment_builder.build(app,
space,
lifecycle,
Expand All @@ -128,6 +133,7 @@ def get_staging_details(package, lifecycle)
staging_details.package = package
staging_details.staging_memory_in_mb = memory_limit
staging_details.staging_disk_in_mb = disk_limit
staging_details.staging_log_rate_limit_bytes_per_second = log_rate_limit
staging_details.environment_variables = environment_variables
staging_details.lifecycle = lifecycle
staging_details.isolation_segment = IsolationSegmentSelector.for_space(space)
Expand All @@ -151,6 +157,11 @@ def get_memory_limit(requested_limit, app, space, org)
raise OrgQuotaExceeded.new e.message
end

def get_log_rate_limit(requested_limit, app, space, org)
limit = requested_limit || app.newest_web_process&.log_rate_limit
@log_rate_limit_calculator.get_limit(limit, space, org)
end

def logger
@logger ||= Steno.logger('cc.action.build_create')
end
Expand Down
3 changes: 2 additions & 1 deletion app/messages/build_create_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@

module VCAP::CloudController
class BuildCreateMessage < MetadataBaseMessage
register_allowed_keys [:staging_memory_in_mb, :staging_disk_in_mb, :environment_variables, :lifecycle, :package]
register_allowed_keys [:staging_memory_in_mb, :staging_disk_in_mb, :staging_log_rate_limit_bytes_per_second, :environment_variables, :lifecycle, :package]

def self.lifecycle_requested?
@lifecycle_requested ||= proc { |a| a.requested?(:lifecycle) }
end

validates :staging_disk_in_mb, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true
validates :staging_memory_in_mb, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true
validates :staging_log_rate_limit_bytes_per_second, numericality: { only_integer: true, greater_than_or_equal_to: -1 }, allow_nil: true

validates_with NoAdditionalKeysValidator
validates_with LifecycleValidator, if: lifecycle_requested?
Expand Down
1 change: 1 addition & 0 deletions app/presenters/v3/build_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ def to_hash
state: build.state,
staging_memory_in_mb: build.staging_memory_in_mb,
staging_disk_in_mb: build.staging_disk_in_mb,
staging_log_rate_limit_bytes_per_second: build.staging_log_rate_limit,
error: error,
lifecycle: {
type: build.lifecycle_type,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Sequel.migration do
change do
alter_table :builds do
add_column :staging_log_rate_limit, :Bignum, null: false, default: -1
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
module VCAP::CloudController
class QuotaValidatingStagingLogRateLimitCalculator
class SpaceQuotaExceeded < StandardError; end
class OrgQuotaExceeded < StandardError; end

def get_limit(requested_limit, space, org)
if requested_limit.nil?
requested_limit = -1
end

requested_limit = requested_limit.to_i

space_quota_exceeded!(requested_limit) unless space.has_remaining_log_rate_limit(requested_limit)
org_quota_exceeded!(requested_limit) unless org.has_remaining_log_rate_limit(requested_limit)
requested_limit
end

private

def org_quota_exceeded!(staging_log_rate_limit)
raise OrgQuotaExceeded.new("staging requires #{staging_log_rate_limit} bytes per second")
end

def space_quota_exceeded!(staging_log_rate_limit)
raise SpaceQuotaExceeded.new("staging requires #{staging_log_rate_limit} bytes per second")
end
end
end
2 changes: 1 addition & 1 deletion lib/cloud_controller/diego/staging_details.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module VCAP::CloudController
module Diego
class StagingDetails
attr_accessor :staging_guid, :staging_memory_in_mb, :staging_disk_in_mb, :package,
attr_accessor :staging_guid, :staging_memory_in_mb, :staging_disk_in_mb, :staging_log_rate_limit_bytes_per_second, :package,
:environment_variables, :lifecycle, :start_after_staging, :isolation_segment
end
end
Expand Down
1 change: 1 addition & 0 deletions lib/cloud_controller/diego/task_recipe_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ def build_staging_task(config, staging_details)
log_guid: staging_details.package.app_guid,
log_source: STAGING_LOG_SOURCE,
memory_mb: staging_details.staging_memory_in_mb,
log_rate_limit_bytes_per_second: staging_details.staging_log_rate_limit_bytes_per_second,
network: generate_network(staging_details.package, Protocol::ContainerNetworkInfo::STAGING),
privileged: config.get(:diego, :use_privileged_containers_for_staging),
result_file: STAGING_RESULT_FILE,
Expand Down
4 changes: 4 additions & 0 deletions spec/request/apps_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1778,6 +1778,7 @@
app: app_model,
staging_memory_in_mb: 123,
staging_disk_in_mb: 456,
staging_log_rate_limit: 789,
created_by_user_name: 'bob the builder',
created_by_user_guid: user.guid,
created_by_user_email: '[email protected]'
Expand All @@ -1789,6 +1790,7 @@
app: app_model,
staging_memory_in_mb: 123,
staging_disk_in_mb: 456,
staging_log_rate_limit: 789,
created_at: build.created_at - 1.day,
created_by_user_name: 'bob the builder',
created_by_user_guid: user.guid,
Expand Down Expand Up @@ -1874,6 +1876,7 @@
'error' => nil,
'staging_memory_in_mb' => 123,
'staging_disk_in_mb' => 456,
'staging_log_rate_limit_bytes_per_second' => 789,
'lifecycle' => {
'type' => 'buildpack',
'data' => {
Expand Down Expand Up @@ -1902,6 +1905,7 @@
'error' => nil,
'staging_memory_in_mb' => 123,
'staging_disk_in_mb' => 456,
'staging_log_rate_limit_bytes_per_second' => 789,
'lifecycle' => {
'type' => 'buildpack',
'data' => {
Expand Down
9 changes: 8 additions & 1 deletion spec/request/builds_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
'state' => 'STAGING',
'staging_memory_in_mb' => 42,
'staging_disk_in_mb' => 42,
'staging_log_rate_limit_bytes_per_second' => -1,
'metadata' => { 'labels' => { 'release' => 'stable', 'seriouseats.com/potato' => 'mashed' }, 'annotations' => { 'potato' => 'idaho' } },
'error' => nil,
'lifecycle' => {
Expand Down Expand Up @@ -347,6 +348,7 @@
created_by_user_email: '[email protected]',
staging_memory_in_mb: 123,
staging_disk_in_mb: 456,
staging_log_rate_limit: 234
)
end
let!(:second_build) do
Expand All @@ -358,7 +360,8 @@
created_by_user_guid: developer.guid,
created_by_user_email: '[email protected]',
staging_memory_in_mb: 789,
staging_disk_in_mb: 12
staging_disk_in_mb: 12,
staging_log_rate_limit: 345
)
end
let(:package) { VCAP::CloudController::PackageModel.make(app_guid: app_model.guid) }
Expand Down Expand Up @@ -468,6 +471,7 @@
'state' => 'STAGED',
'staging_memory_in_mb' => 123,
'staging_disk_in_mb' => 456,
'staging_log_rate_limit_bytes_per_second' => 234,
'error' => nil,
'lifecycle' => {
'type' => 'buildpack',
Expand Down Expand Up @@ -496,6 +500,7 @@
'state' => 'STAGED',
'staging_memory_in_mb' => 789,
'staging_disk_in_mb' => 12,
'staging_log_rate_limit_bytes_per_second' => 345,
'error' => nil,
'lifecycle' => {
'type' => 'buildpack',
Expand Down Expand Up @@ -557,6 +562,7 @@
app: app_model,
staging_memory_in_mb: 123,
staging_disk_in_mb: 456,
staging_log_rate_limit: 789,
created_by_user_name: 'bob the builder',
created_by_user_guid: developer.guid,
created_by_user_email: '[email protected]'
Expand Down Expand Up @@ -593,6 +599,7 @@
'state' => 'STAGED',
'staging_memory_in_mb' => 123,
'staging_disk_in_mb' => 456,
'staging_log_rate_limit_bytes_per_second' => 789,
'error' => nil,
'lifecycle' => {
'type' => 'buildpack',
Expand Down
24 changes: 23 additions & 1 deletion spec/unit/actions/build_create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ module VCAP::CloudController
user_audit_info: user_audit_info,
memory_limit_calculator: memory_limit_calculator,
disk_limit_calculator: disk_limit_calculator,
log_rate_limit_calculator: log_rate_limit_calculator,
environment_presenter: environment_builder
)
end

let(:memory_limit_calculator) { double(:memory_limit_calculator) }
let(:disk_limit_calculator) { double(:disk_limit_calculator) }
let(:log_rate_limit_calculator) { double(:log_rate_limit_calculator) }
let(:environment_builder) { double(:environment_builder) }
let(:user_audit_info) { UserAuditInfo.new(user_email: '[email protected]', user_guid: '1234', user_name: 'charles') }

Expand All @@ -24,6 +26,7 @@ module VCAP::CloudController
{
staging_memory_in_mb: staging_memory_in_mb,
staging_disk_in_mb: staging_disk_in_mb,
staging_log_rate_limit_bytes_per_second: staging_log_rate_limit_bytes_per_second,
lifecycle: request_lifecycle,
}.deep_stringify_keys
end
Expand All @@ -50,7 +53,7 @@ module VCAP::CloudController
let(:space) { Space.make }
let(:org) { space.organization }
let(:app) { AppModel.make(space: space) }
let!(:process) { ProcessModel.make(app: app, memory: 8192, disk_quota: 512) }
let!(:process) { ProcessModel.make(app: app, memory: 8192, disk_quota: 512, log_rate_limit: 7168) }

let(:buildpack_git_url) { 'http://example.com/repo.git' }
let(:stack) { Stack.default }
Expand All @@ -65,9 +68,11 @@ module VCAP::CloudController
let(:stager) { instance_double(Diego::Stager) }
let(:calculated_mem_limit) { 32 }
let(:calculated_staging_disk_in_mb) { 64 }
let(:calculated_staging_log_rate_limit) { 96 }

let(:staging_memory_in_mb) { 1024 }
let(:staging_disk_in_mb) { 2048 }
let(:staging_log_rate_limit_bytes_per_second) { 3072 }
let(:environment_variables) { 'random string' }

before do
Expand All @@ -76,6 +81,7 @@ module VCAP::CloudController
allow(stager).to receive(:stage)
allow(memory_limit_calculator).to receive(:get_limit).with(staging_memory_in_mb, space, org).and_return(calculated_mem_limit)
allow(disk_limit_calculator).to receive(:get_limit).with(staging_disk_in_mb).and_return(calculated_staging_disk_in_mb)
allow(log_rate_limit_calculator).to receive(:get_limit).with(staging_log_rate_limit_bytes_per_second, space, org).and_return(calculated_staging_log_rate_limit)
allow(environment_builder).to receive(:build).and_return(environment_variables)
end

Expand All @@ -93,6 +99,7 @@ module VCAP::CloudController
expect(build.package_guid).to eq(package.guid)
expect(build.staging_memory_in_mb).to eq(calculated_mem_limit)
expect(build.staging_disk_in_mb).to eq(calculated_staging_disk_in_mb)
expect(build.staging_log_rate_limit).to eq(calculated_staging_log_rate_limit)
expect(build.lifecycle_data.to_hash).to eq(lifecycle_data)
expect(build.created_by_user_guid).to eq('1234')
expect(build.created_by_user_name).to eq('charles')
Expand Down Expand Up @@ -174,6 +181,7 @@ module VCAP::CloudController
expect(staging_details.staging_guid).to eq(build.guid)
expect(staging_details.staging_memory_in_mb).to eq(calculated_mem_limit)
expect(staging_details.staging_disk_in_mb).to eq(calculated_staging_disk_in_mb)
expect(staging_details.staging_log_rate_limit_bytes_per_second).to eq(calculated_staging_log_rate_limit)
expect(staging_details.environment_variables).to eq(environment_variables)
expect(staging_details.lifecycle).to eq(lifecycle)
expect(staging_details.isolation_segment).to be_nil
Expand Down Expand Up @@ -210,6 +218,20 @@ module VCAP::CloudController
end
end

context 'when staging log rate limit is not specified in the message' do
before do
allow(log_rate_limit_calculator).to receive(:get_limit).with(process.log_rate_limit, space, org).and_return(process.log_rate_limit)
end
let(:staging_log_rate_limit_bytes_per_second) { nil }

it 'uses the app web process log rate limit for staging log rate limit' do
expect(log_rate_limit_calculator).to receive(:get_limit).with(process.log_rate_limit, space, org)

build = action.create_and_stage(package: package, lifecycle: lifecycle)
expect(build.staging_log_rate_limit).to eq(process.log_rate_limit)
end
end

describe 'isolation segments' do
let(:assigner) { VCAP::CloudController::IsolationSegmentAssign.new }
let(:isolation_segment_model) { VCAP::CloudController::IsolationSegmentModel.make }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
require 'spec_helper'
require 'cloud_controller/backends/quota_validating_staging_log_rate_limit_calculator'

module VCAP::CloudController
RSpec.describe QuotaValidatingStagingLogRateLimitCalculator do
let(:calculator) { QuotaValidatingStagingLogRateLimitCalculator.new }

describe '#get_limit' do
let(:space_quota_limit) { 200 }
let(:org_quota_limit) { 200 }
let(:requested_limit) { 100 }
let(:space) { Space.make }
let(:org) { space.organization }
let(:space_quota_definition) { SpaceQuotaDefinition.make(organization: org, log_rate_limit: space_quota_limit) }
let(:quota_definition) { QuotaDefinition.make(log_rate_limit: org_quota_limit) }

before do
space.space_quota_definition = space_quota_definition
org.quota_definition = quota_definition
space.save
org.save
end

it 'uses the requested_limit' do
limit = calculator.get_limit(requested_limit, space, org)
expect(limit).to eq(requested_limit)
end

context 'when the requested_limit is passed as an integer string' do
let(:requested_limit) { '100' }

it 'uses the requested_limit' do
limit = calculator.get_limit(requested_limit, space, org)
expect(limit).to eq(100)
end
end

context 'when the requested_limit exceeds the space quota' do
let(:space_quota_limit) { requested_limit - 1 }

it 'raises a SpaceQuotaExceeded error' do
expect {
calculator.get_limit(requested_limit, space, org)
}.to raise_error(QuotaValidatingStagingLogRateLimitCalculator::SpaceQuotaExceeded, /staging requires 100 bytes per second/)
end
end

context 'when the requested_limit exceeds the org quota' do
let(:org_quota_limit) { requested_limit - 1 }

it 'raises a OrgQuotaExceeded error' do
expect {
calculator.get_limit(requested_limit, space, org)
}.to raise_error(QuotaValidatingStagingLogRateLimitCalculator::OrgQuotaExceeded, /staging requires 100 bytes per second/)
end
end

context 'when the requested_limit is nil' do
let(:requested_limit) { nil }

it 'uses no limit' do
limit = calculator.get_limit(requested_limit, space, org)
expect(limit).to eq(-1)
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module Diego
details.environment_variables = [::Diego::Bbs::Models::EnvironmentVariable.new(name: 'nightshade_fruit', value: 'potato')]
details.staging_memory_in_mb = 42
details.staging_disk_in_mb = 51
details.staging_log_rate_limit_bytes_per_second = 67
details.start_after_staging = true
details.lifecycle = lifecycle
details.isolation_segment = isolation_segment
Expand Down Expand Up @@ -152,6 +153,7 @@ module Diego

expect(result.memory_mb).to eq(42)
expect(result.disk_mb).to eq(51)
expect(result.log_rate_limit_bytes_per_second).to eq(67)
expect(result.image_layers).to eq(lifecycle_image_layers)
expect(result.cpu_weight).to eq(50)

Expand Down
Loading

0 comments on commit 6d3e2e8

Please sign in to comment.