Skip to content

Improve error messaging for creating a Deployment that will violate quotas with high max-in-flight #4402

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7314715
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 11, 2025
ba1b5c9
Improve error messaging for creating a Deployment that will violate …
nookala Jun 11, 2025
cecb214
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 17, 2025
4e1f1ca
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 17, 2025
56c3daf
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 17, 2025
b016e37
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 19, 2025
5e9b4b7
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 19, 2025
aa23032
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 19, 2025
4e45544
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 19, 2025
aa08736
Merge branch 'main' into fix-error-message-inflight
nookala Jun 20, 2025
aa44c5d
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 20, 2025
b22f394
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 20, 2025
a9231eb
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 27, 2025
2b33179
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 27, 2025
6821192
Improve error messaging for creating a Deployment that will violate q…
nookala Jun 27, 2025
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
20 changes: 19 additions & 1 deletion app/actions/deployment_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,25 @@ def create(app:, user_audit_info:, message:)

deployment
rescue RevisionResolver::NoUpdateRollback, Sequel::ValidationFailed, AppStart::InvalidApp => e
error = DeploymentCreate::Error.new(e.message)
raise handle_deployment_create_error(e, app)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't there a raise inside the method already? I think it's clearer what's going on if you leave it out, but then maybe remove the raise on L106 and rename the method to something like enhanced_deployment_create_error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It think it's also fine to leave it in the method, but probably not in both places

end

def handle_deployment_create_error(e, app)
space_quota_errors = %w[space_quota_exceeded space_app_instance_limit_exceeded]
org_quota_errors = %w[quota_exceeded app_instance_limit_exceeded]
space_error_msg = " for space #{app.space.name}. This space's quota may not be large enough to support rolling deployments or your configured max-in-flight."
org_error_msg_1 = " for organization #{app.organization.name}. "
org_error_msg_2 = "This organization's quota may not be large enough to support rolling deployments or your configured max-in-flight."
org_error_msg = org_error_msg_1 + org_error_msg_2
error_message = e.message

if space_quota_errors.any? { |substring| e.message.include?(substring) }
error_message += space_error_msg
elsif org_quota_errors.any? { |substring| e.message.include?(substring) }
error_message += org_error_msg
end

error = DeploymentCreate::Error.new(error_message)
error.set_backtrace(e.backtrace)
raise error
end
Expand Down
38 changes: 24 additions & 14 deletions spec/unit/actions/deployment_create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,18 @@ module VCAP::CloudController
end
end

context 'when the app fails to start due to space errors' do
before do
allow(VCAP::CloudController::AppStart).to receive(:start).and_raise(VCAP::CloudController::AppStart::InvalidApp.new('memory space_quota_exceeded'))
end

it 'raises a DeploymentCreate::Error' do
error_msg_1 = "memory space_quota_exceeded for space #{app.space.name}. "
error_msg_2 = "This space's quota may not be large enough to support rolling deployments or your configured max-in-flight."
expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, error_msg_1 + error_msg_2)
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 }
Expand Down Expand Up @@ -637,7 +649,9 @@ module VCAP::CloudController
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')
error_msg_1 = "memory quota_exceeded for organization #{app.organization.name}. "
error_msg_2 = "This organization's quota may not be large enough to support rolling deployments or your configured max-in-flight."
expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, error_msg_1 + error_msg_2)
end
end

Expand Down Expand Up @@ -683,7 +697,9 @@ module VCAP::CloudController
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')
error_msg_1 = "memory quota_exceeded for organization #{app.organization.name}. "
error_msg_2 = "This organization's quota may not be large enough to support rolling deployments or your configured max-in-flight."
expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, error_msg_1 + error_msg_2)
end
end

Expand Down Expand Up @@ -717,16 +733,6 @@ module VCAP::CloudController
expect(deployment.canary_steps).to eq([{ 'instance_weight' => 40 }, { 'instance_weight' => 80 }])
end
end

context 'when the app fails to start' do
before do
allow(VCAP::CloudController::AppStart).to receive(:start).and_raise(VCAP::CloudController::AppStart::InvalidApp.new('memory quota_exceeded'))
end

it 'raises a DeploymentCreate::Error' do
expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, 'memory quota_exceeded')
end
end
end
end

Expand Down Expand Up @@ -880,7 +886,9 @@ module VCAP::CloudController
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')
error_msg_1 = "memory quota_exceeded for organization #{app.organization.name}. "
error_msg_2 = "This organization's quota may not be large enough to support rolling deployments or your configured max-in-flight."
expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, error_msg_1 + error_msg_2)
end
end

Expand Down Expand Up @@ -935,7 +943,9 @@ module VCAP::CloudController
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')
error_msg_1 = "memory quota_exceeded for organization #{app.organization.name}. "
error_msg_2 = "This organization's quota may not be large enough to support rolling deployments or your configured max-in-flight."
expect { DeploymentCreate.create(app:, message:, user_audit_info:) }.to raise_error(DeploymentCreate::Error, error_msg_1 + error_msg_2)
end
end

Expand Down