Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent quota updates or assignment with finite log rates #2948

Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Prevent org and space quota log rate limit changes
- If an existing deployment is upgraded to gain log rate limiting
  support then existing application processes will default to -1
  (unlimited).
- If an org or space quota is subsequently updated to include a finite
  log rate limit then processes in the affected orgs or spaces will be
  unable to restart without having their log rate limit changed.
- Block org and space quota finite log rate limit updates until
  processes under the affected orgs or spaces have been updated with a
  finite log rate limit. Specify the names of the first two orgs or
  spaces and a total count in the error.
- Block assignment of org or space quotas with finite log rate limits to
  orgs or spaces that contain processes with unlimited log rate limits.

Co-authored-by: Carson Long <[email protected]>
  • Loading branch information
acrmp and ctlong committed Sep 12, 2022
commit a3356ff96ee3debace18d0f38dec69365951550f
14 changes: 13 additions & 1 deletion app/actions/organization_quota_apply.rb
Original file line number Diff line number Diff line change
@@ -4,8 +4,20 @@ class Error < ::StandardError
end

def apply(org_quota, message)
orgs = valid_orgs(message.organization_guids)

if org_quota.log_rate_limit != QuotaDefinition::UNLIMITED
affected_processes = Organization.where(Sequel[:organizations][:id] => orgs.map(&:id)).
join(:spaces, organization_id: :id).
join(:apps, space_guid: :guid).
join(:processes, app_guid: :guid)

unless affected_processes.where(log_rate_limit: ProcessModel::UNLIMITED_LOG_RATE).empty?
error!('Current usage exceeds new quota values. The org(s) being assigned this quota contain apps running with an unlimited log rate limit.')
end
end

QuotaDefinition.db.transaction do
orgs = valid_orgs(message.organization_guids)
orgs.each { |org| org_quota.add_organization(org) }
end
rescue Sequel::ValidationFailed => e
39 changes: 39 additions & 0 deletions app/actions/organization_quotas_update.rb
Original file line number Diff line number Diff line change
@@ -2,8 +2,18 @@ module VCAP::CloudController
class OrganizationQuotasUpdate
class Error < ::StandardError
end

MAX_ORGS_TO_LIST_ON_FAILURE = 2

# rubocop:disable Metrics/CyclomaticComplexity
def self.update(quota, message)
if log_rate_limit(message) != QuotaDefinition::UNLIMITED
orgs = orgs_with_unlimited_processes(quota)
if orgs.any?
unlimited_processes_exist_error!(orgs)
end
end

quota.db.transaction do
quota.lock!

@@ -84,5 +94,34 @@ def self.total_routes(message)
def self.total_private_domains(message)
default_if_nil(message.total_domains, QuotaDefinition::UNLIMITED)
end

def self.orgs_with_unlimited_processes(quota)
quota.organizations_dataset.
distinct.
select(Sequel[:organizations][:name]).
join(:spaces, organization_id: :id).
join(:apps, space_guid: :guid).
join(:processes, app_guid: :guid).
where(log_rate_limit: QuotaDefinition::UNLIMITED).
order(:name).
all
end

def self.unlimited_processes_exist_error!(orgs)
named_orgs = orgs.take(MAX_ORGS_TO_LIST_ON_FAILURE).map(&:name).join("', '")
message = if orgs.size == 1
"Org '#{named_orgs}' assigned this quota contains"
elsif orgs.size > MAX_ORGS_TO_LIST_ON_FAILURE
"Orgs '#{named_orgs}' and #{orgs.drop(MAX_ORGS_TO_LIST_ON_FAILURE).size}" \
' other orgs assigned this quota contain'
else
"Orgs '#{named_orgs}' assigned this quota contain"
end + ' apps running with an unlimited log rate limit.'

raise Error.new("Current usage exceeds new quota values. #{message}")
end

private_class_method :orgs_with_unlimited_processes,
:unlimited_processes_exist_error!
end
end
10 changes: 10 additions & 0 deletions app/actions/space_quota_apply.rb
Original file line number Diff line number Diff line change
@@ -6,6 +6,16 @@ class Error < ::StandardError
def apply(space_quota, message, visible_space_guids: [], all_spaces_visible: false)
spaces = valid_spaces(message.space_guids, visible_space_guids, all_spaces_visible, space_quota.organization_id)

if space_quota.log_rate_limit != QuotaDefinition::UNLIMITED
affected_processes = Space.where(Sequel[:spaces][:id] => spaces.map(&:id)).
join(:apps, space_guid: :guid).
join(:processes, app_guid: :guid)

unless affected_processes.where(log_rate_limit: ProcessModel::UNLIMITED_LOG_RATE).empty?
error!('Current usage exceeds new quota values. The space(s) being assigned this quota contain apps running with an unlimited log rate limit.')
end
end

SpaceQuotaDefinition.db.transaction do
spaces.each { |space| space_quota.add_space(space) }
end
37 changes: 37 additions & 0 deletions app/actions/space_quota_update.rb
Original file line number Diff line number Diff line change
@@ -3,8 +3,17 @@ class SpaceQuotaUpdate
class Error < ::StandardError
end

MAX_SPACES_TO_LIST_ON_FAILURE = 2

# rubocop:disable Metrics/CyclomaticComplexity
def self.update(quota, message)
if log_rate_limit(message) != QuotaDefinition::UNLIMITED
spaces = spaces_with_unlimited_processes(quota)
if spaces.any?
unlimited_processes_exist_error!(spaces)
end
end

quota.db.transaction do
quota.lock!

@@ -79,5 +88,33 @@ def self.total_reserved_route_ports(message)
def self.total_routes(message)
default_if_nil(message.total_routes, SpaceQuotaDefinition::UNLIMITED)
end

def self.spaces_with_unlimited_processes(quota)
quota.spaces_dataset.
distinct.
select(Sequel[:spaces][:name]).
join(:apps, space_guid: :guid).
join(:processes, app_guid: :guid).
where(log_rate_limit: QuotaDefinition::UNLIMITED).
order(:name).
all
end

def self.unlimited_processes_exist_error!(spaces)
named_spaces = spaces.take(MAX_SPACES_TO_LIST_ON_FAILURE).map(&:name).join("', '")
message = if spaces.size == 1
"Space '#{named_spaces}' assigned this quota contains"
elsif spaces.size > MAX_SPACES_TO_LIST_ON_FAILURE
"Spaces '#{named_spaces}' and #{spaces.drop(MAX_SPACES_TO_LIST_ON_FAILURE).size}" \
' other spaces assigned this quota contain'
else
"Spaces '#{named_spaces}' assigned this quota contain"
end + ' apps running with an unlimited log rate limit.'

raise Error.new("Current usage exceeds new quota values. #{message}")
end

private_class_method :spaces_with_unlimited_processes,
:unlimited_processes_exist_error!
end
end
1 change: 1 addition & 0 deletions app/models/runtime/process_model.rb
Original file line number Diff line number Diff line change
@@ -34,6 +34,7 @@ def after_initialize
NO_APP_PORT_SPECIFIED = -1
DEFAULT_HTTP_PORT = 8080
DEFAULT_PORTS = [DEFAULT_HTTP_PORT].freeze
UNLIMITED_LOG_RATE = -1

many_to_one :app, class: 'VCAP::CloudController::AppModel', key: :app_guid, primary_key: :guid, without_guid_generation: true
many_to_one :revision, class: 'VCAP::CloudController::RevisionModel', key: :revision_guid, primary_key: :guid, without_guid_generation: true
27 changes: 27 additions & 0 deletions spec/request/organization_quotas_spec.rb
Original file line number Diff line number Diff line change
@@ -462,6 +462,20 @@ module VCAP::CloudController
expect(last_response).to include_error_message("Organization Quota '#{organization_quota.name}' already exists.")
end
end

context 'when trying to set a log rate limit and there are apps with unlimited log rates' do
let!(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }

it 'returns 422' do
patch "/v3/organization_quotas/#{organization_quota.guid}", params.to_json, admin_header

expect(last_response).to have_status_code(422)
expect(last_response).to include_error_message(
'Current usage exceeds new quota values. ' \
"Org '#{org.name}' assigned this quota contains apps running with an unlimited log rate limit.")
acrmp marked this conversation as resolved.
Show resolved Hide resolved
end
end
end

describe 'POST /v3/organization_quotas/:guid/relationships/organizations' do
@@ -524,6 +538,19 @@ module VCAP::CloudController
expect(parsed_response['errors'][0]['detail']).to eq('Invalid data type: Data[0] guid should be a string.')
end
end

context 'when the quota has a finite log rate limit and there are apps with unlimited log rates' do
let(:org_quota) { VCAP::CloudController::QuotaDefinition.make(log_rate_limit: 100) }
let!(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }

it 'returns 422' do
post "/v3/organization_quotas/#{org_quota.guid}/relationships/organizations", params.to_json, admin_header
expect(last_response).to have_status_code(422)
expect(last_response).to include_error_message(
'Current usage exceeds new quota values. The org(s) being assigned this quota contain apps running with an unlimited log rate limit.')
end
end
end

describe 'DELETE /v3/organization_quotas/:guid/' do
25 changes: 25 additions & 0 deletions spec/request/space_quotas_spec.rb
Original file line number Diff line number Diff line change
@@ -260,6 +260,18 @@ module VCAP::CloudController
expect(last_response).to include_error_message("Unknown field(s): 'wat'")
end
end
context 'when trying to set a log rate limit and there are apps with unlimited log rates' do
let!(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }

it 'returns 422' do
patch "/v3/space_quotas/#{space_quota.guid}", params.to_json, admin_header

expect(last_response).to have_status_code(422)
expect(last_response).to include_error_message(
"Current usage exceeds new quota values. Space '#{space.name}' assigned this quota contains apps running with an unlimited log rate limit.")
end
end
end

describe 'GET /v3/space_quotas' do
@@ -810,6 +822,19 @@ module VCAP::CloudController
expect(parsed_response['errors'][0]['detail']).to eq('Invalid data type: Data[1] guid should be a string.')
end
end
context 'when the quota has a finite log rate limit and there are apps with unlimited log rates' do
let(:space_quota) { VCAP::CloudController::SpaceQuotaDefinition.make(guid: 'space-quota-guid', organization: org, log_rate_limit: 100) }
let!(:other_space) { VCAP::CloudController::Space.make(guid: 'other-space-guid', organization: org, space_quota_definition: space_quota) }
let!(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: other_space) }
let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }

it 'returns 422' do
post "/v3/space_quotas/#{space_quota.guid}/relationships/spaces", params.to_json, admin_header
expect(last_response).to have_status_code(422)
expect(last_response).to include_error_message(
'Current usage exceeds new quota values. The space(s) being assigned this quota contain apps running with an unlimited log rate limit.')
end
end
end

describe 'DELETE /v3/space_quotas/:guid/relationships/spaces' do
17 changes: 16 additions & 1 deletion spec/unit/actions/organization_quota_apply_spec.rb
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@

module VCAP::CloudController
RSpec.describe OrganizationQuotaApply do
describe '#create' do
describe '#apply' do
subject { OrganizationQuotaApply.new }

let(:org) { VCAP::CloudController::Organization.make }
@@ -53,6 +53,21 @@ module VCAP::CloudController
}.to raise_error(OrganizationQuotaApply::Error, "Organizations with guids [\"#{invalid_org_guid}\"] do not exist")
end
end

context 'when trying to set a log rate limit and there are apps with unlimited log rates' do
let(:space) { VCAP::CloudController::Space.make(guid: 'space-guid', organization: org) }
let(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
let(:org_quota) { VCAP::CloudController::QuotaDefinition.make(log_rate_limit: 2000) }

it 'raises an error' do
expect {
subject.apply(org_quota, message)
}.to raise_error(OrganizationQuotaApply::Error,
'Current usage exceeds new quota values. ' \
'The org(s) being assigned this quota contain apps running with an unlimited log rate limit.')
end
end
end
end
end
61 changes: 61 additions & 0 deletions spec/unit/actions/organization_quotas_update_spec.rb
Original file line number Diff line number Diff line change
@@ -103,6 +103,67 @@ module VCAP::CloudController
end
end
end

context 'when there are affected processes that have an unlimited log rate limit' do
def create_orgs_with_unlimited_log_rate_process(count)
count.downto(1) do |i|
org = VCAP::CloudController::Organization.make(guid: "org-guid-#{i}", name: "org-name-#{i}", quota_definition: org_quota)
space = VCAP::CloudController::Space.make(guid: "space-guid-#{i}", organization: org)
app_model = VCAP::CloudController::AppModel.make(name: "app-#{i}", space: space)
VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1)
end
end

context 'and they are only in a single org' do
before do
create_orgs_with_unlimited_log_rate_process(1)
end
it 'errors with a message telling the user the affected org' do
expect do
OrganizationQuotasUpdate.update(org_quota, message)
end.to raise_error(OrganizationQuotasUpdate::Error, "Current usage exceeds new quota values. Org 'org-name-1' " \
'assigned this quota contains apps running with an unlimited log rate limit.')
end
end
context 'and they are in two orgs' do
before do
create_orgs_with_unlimited_log_rate_process(2)
end
it 'errors with a message telling the user the affected orgs' do
expect do
OrganizationQuotasUpdate.update(org_quota, message)
end.to raise_error(OrganizationQuotasUpdate::Error, "Current usage exceeds new quota values. Orgs 'org-name-1', 'org-name-2' " \
'assigned this quota contain apps running with an unlimited log rate limit.')
end
end

context 'and they are spread across five orgs' do
before do
create_orgs_with_unlimited_log_rate_process(5)
end
it 'errors with a message telling the user some of the affected orgs and a total count' do
expect do
OrganizationQuotasUpdate.update(org_quota, message)
end.to raise_error(OrganizationQuotasUpdate::Error, "Current usage exceeds new quota values. Orgs 'org-name-1', 'org-name-2' and 3 other orgs " \
'assigned this quota contain apps running with an unlimited log rate limit.')
end
end

context 'and there is more than one affected process within an org' do
let(:org) { VCAP::CloudController::Organization.make(guid: 'org-guid', name: 'org-name', quota_definition: org_quota) }
let(:space) { VCAP::CloudController::Space.make(guid: 'space-guid', organization: org) }
let(:app_model) { VCAP::CloudController::AppModel.make(name: 'app', space: space) }
let!(:process_1) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
let!(:process_2) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }

it 'only names the org once in the error message' do
expect do
OrganizationQuotasUpdate.update(org_quota, message)
end.to raise_error(OrganizationQuotasUpdate::Error, "Current usage exceeds new quota values. Org 'org-name' assigned this quota contains apps " \
'running with an unlimited log rate limit.')
end
end
end
end
end
end
15 changes: 15 additions & 0 deletions spec/unit/actions/space_quota_apply_spec.rb
Original file line number Diff line number Diff line change
@@ -62,6 +62,21 @@ module VCAP::CloudController
end
end

context 'when trying to set a log rate limit and there are apps with unlimited log rates' do
let(:visible_space_guids) { [space.guid] }
let(:app_model) { VCAP::CloudController::AppModel.make(name: 'name1', space: space) }
let!(:process_model) { VCAP::CloudController::ProcessModel.make(app: app_model, log_rate_limit: -1) }
let(:space_quota) { VCAP::CloudController::SpaceQuotaDefinition.make(organization: org, log_rate_limit: 2000) }

it 'raises an error' do
expect {
subject.apply(space_quota, message, visible_space_guids: visible_space_guids)
}.to raise_error(SpaceQuotaApply::Error,
'Current usage exceeds new quota values. ' \
'The space(s) being assigned this quota contain apps running with an unlimited log rate limit.')
end
end

context "when the space is outside the space quota's org" do
let(:other_space) { VCAP::CloudController::Space.make }
let(:invalid_space_guid) { other_space.guid }
Loading