Skip to content

Commit 35d8b72

Browse files
a18ea18e
committed
Adjust route options behaviour for manifest push
Overwrite behaviour for route options is now fixed and tested: Existing options are not modified if options is nil, {} or not provided A single option (e.g. loadbalancing-algorithm) can be removed by setting its value to nil
1 parent d5e369c commit 35d8b72

File tree

3 files changed

+148
-46
lines changed

3 files changed

+148
-46
lines changed

‎app/actions/route_update.rb‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,17 @@ def update(route:, message:)
44
Route.db.transaction do
55
if message.requested?(:options)
66
route.options = if message.options.nil?
7-
nil
7+
route.options
88
elsif route.options.nil?
99
message.options
1010
else
1111
route.options.merge(message.options)
1212
end
1313
end
14+
15+
# remove nil values from options
16+
route.options = route.options.compact if route.options
17+
1418
route.save
1519
MetadataUpdate.update(route, message)
1620
end

‎lib/cloud_controller/app_manifest/manifest_route.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def self.parse(route, options=nil)
1919

2020
if options
2121
attrs[:options] = {}
22-
attrs[:options][:lb_algo] = options[:'loadbalancing-algorithm'] if options[:'loadbalancing-algorithm']
22+
attrs[:options][:lb_algo] = options[:'loadbalancing-algorithm'] if options.key?(:'loadbalancing-algorithm')
2323
end
2424

2525
ManifestRoute.new(attrs)

‎spec/request/space_manifests_spec.rb‎

Lines changed: 142 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@
606606
before do
607607
app1_model.web_processes.first.update(state: VCAP::CloudController::ProcessModel::STARTED, instances: 4)
608608
space.update(space_quota_definition:
609-
VCAP::CloudController::SpaceQuotaDefinition.make(organization: space.organization, log_rate_limit: 0))
609+
VCAP::CloudController::SpaceQuotaDefinition.make(organization: space.organization, log_rate_limit: 0))
610610
end
611611

612612
it 'successfully applies the manifest' do
@@ -625,54 +625,177 @@
625625
end
626626

627627
describe 'route options' do
628-
context 'when an empty route options hash is provided' do
628+
context 'when an invalid route option is provided' do
629629
let(:yml_manifest) do
630630
{
631631
'applications' => [
632632
{
633633
'name' => app1_model.name,
634634
'routes' => [
635635
{ 'route' => "https://#{route.host}.#{route.domain.name}",
636-
'options' => {} }
636+
'options' => {
637+
'doesnt-exist' => 'doesnt-exist'
638+
} }
637639
]
638640
}
639641
]
640642
}.to_yaml
641643
end
642644

643-
it 'applies the manifest' do
645+
it 'returns a 422' do
644646
post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header)
645647

646-
expect(last_response.status).to eq(202)
648+
expect(last_response).to have_status_code(422)
649+
expect(last_response).to have_error_message('Routes contains invalid route options')
647650
end
648651
end
649652

650-
context 'when an invalid route option is provided' do
651-
let(:yml_manifest) do
652-
{
653+
context 'updating existing route options' do
654+
# using loadbalancing-algorithm as an example since it is the only route option currently supported
655+
before do
656+
yml_manifest = {
653657
'applications' => [
654-
{
655-
'name' => app1_model.name,
658+
{ 'name' => app1_model.name,
656659
'routes' => [
657-
{ 'route' => "https://#{route.host}.#{route.domain.name}",
660+
{ 'route' => "https://round-robin-app.#{shared_domain.name}",
658661
'options' => {
659-
'doesnt-exist' => 'doesnt-exist'
662+
'loadbalancing-algorithm' => 'round-robin'
660663
} }
661-
]
662-
}
664+
] }
663665
]
664666
}.to_yaml
667+
668+
# apply the manifest with the route option
669+
post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header)
670+
671+
expect(last_response.status).to eq(202)
672+
job_guid = VCAP::CloudController::PollableJobModel.last.guid
673+
674+
Delayed::Worker.new.work_off
675+
expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error
676+
app1_model.reload
677+
expect(app1_model.routes.first.options).to eq({ 'lb_algo' => 'round-robin' })
665678
end
666679

667-
it 'returns a 422' do
680+
it 'updates the route option when a new value is provided' do
681+
yml_manifest = {
682+
'applications' => [
683+
{ 'name' => app1_model.name,
684+
'routes' => [
685+
{ 'route' => "https://round-robin-app.#{shared_domain.name}",
686+
'options' => {
687+
'loadbalancing-algorithm' => 'least-connections'
688+
} }
689+
] }
690+
]
691+
}.to_yaml
692+
668693
post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header)
669694

670-
expect(last_response).to have_status_code(422)
671-
expect(last_response).to have_error_message('Routes contains invalid route options')
695+
expect(last_response.status).to eq(202)
696+
job_guid = VCAP::CloudController::PollableJobModel.last.guid
697+
698+
Delayed::Worker.new.work_off
699+
expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error
700+
app1_model.reload
701+
expect(app1_model.routes.first.options).to eq({ 'lb_algo' => 'least-connections' })
702+
end
703+
704+
it 'does not modify any route options when the options hash is not provided' do
705+
yml_manifest = {
706+
'applications' => [
707+
{ 'name' => app1_model.name,
708+
'routes' => [
709+
{ 'route' => "https://round-robin-app.#{shared_domain.name}" }
710+
] }
711+
]
712+
}.to_yaml
713+
714+
# apply the manifest with the route option
715+
post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header)
716+
717+
expect(last_response.status).to eq(202)
718+
job_guid = VCAP::CloudController::PollableJobModel.last.guid
719+
720+
Delayed::Worker.new.work_off
721+
expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error
722+
app1_model.reload
723+
expect(app1_model.routes.first.options).to eq({ 'lb_algo' => 'round-robin' })
724+
end
725+
726+
it 'does not modify any route options options: nil is provided' do
727+
yml_manifest = {
728+
'applications' => [
729+
{ 'name' => app1_model.name,
730+
'routes' => [
731+
{ 'route' => "https://round-robin-app.#{shared_domain.name}",
732+
'options' => nil }
733+
] }
734+
]
735+
}.to_yaml
736+
737+
# apply the manifest with the route option
738+
post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header)
739+
740+
expect(last_response.status).to eq(202)
741+
job_guid = VCAP::CloudController::PollableJobModel.last.guid
742+
743+
Delayed::Worker.new.work_off
744+
expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error
745+
app1_model.reload
746+
expect(app1_model.routes.first.options).to eq({ 'lb_algo' => 'round-robin' })
747+
end
748+
749+
it 'does not modify any route options if an empty options hash is provided' do
750+
yml_manifest = {
751+
'applications' => [
752+
{ 'name' => app1_model.name,
753+
'routes' => [
754+
{ 'route' => "https://round-robin-app.#{shared_domain.name}",
755+
'options' => {} }
756+
] }
757+
]
758+
}.to_yaml
759+
760+
# apply the manifest with the route option
761+
post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header)
762+
763+
expect(last_response.status).to eq(202)
764+
job_guid = VCAP::CloudController::PollableJobModel.last.guid
765+
766+
Delayed::Worker.new.work_off
767+
expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error
768+
app1_model.reload
769+
expect(app1_model.routes.first.options).to eq({ 'lb_algo' => 'round-robin' })
770+
end
771+
772+
it 'removes a specific route option when options: { key: nil } is provided' do
773+
yml_manifest = {
774+
'applications' => [
775+
{ 'name' => app1_model.name,
776+
'routes' => [
777+
{ 'route' => "https://round-robin-app.#{shared_domain.name}",
778+
'options' => {
779+
'loadbalancing-algorithm' => nil
780+
} }
781+
] }
782+
]
783+
}.to_yaml
784+
785+
# apply the manifest with the route option
786+
post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header)
787+
788+
expect(last_response.status).to eq(202)
789+
job_guid = VCAP::CloudController::PollableJobModel.last.guid
790+
791+
Delayed::Worker.new.work_off
792+
expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error
793+
app1_model.reload
794+
expect(app1_model.routes.first.options).to eq({})
672795
end
673796
end
674797

675-
context 'loadbalancing-algorithm' do
798+
context 'route-option: loadbalancing-algorithm' do
676799
context 'when the loadbalancing-algorithm is not supported' do
677800
let(:yml_manifest) do
678801
{
@@ -713,7 +836,7 @@
713836
}.to_yaml
714837
end
715838

716-
it 'adds and updates the loadbalancing-algorithm' do
839+
it 'adds the loadbalancing-algorithm' do
717840
post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header)
718841

719842
expect(last_response.status).to eq(202)
@@ -724,31 +847,6 @@
724847

725848
app1_model.reload
726849
expect(app1_model.routes.first.options).to eq({ 'lb_algo' => 'round-robin' })
727-
728-
### update the loadbalancing-algorithm from the route
729-
730-
yml_manifest = {
731-
'applications' => [
732-
{ 'name' => app1_model.name,
733-
'routes' => [
734-
{ 'route' => "https://round-robin-app.#{shared_domain.name}",
735-
'options' => {
736-
'loadbalancing-algorithm' => 'least-connections'
737-
} }
738-
] }
739-
]
740-
}.to_yaml
741-
742-
post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header)
743-
744-
expect(last_response.status).to eq(202)
745-
job_guid = VCAP::CloudController::PollableJobModel.last.guid
746-
747-
Delayed::Worker.new.work_off
748-
expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error
749-
750-
app1_model.reload
751-
expect(app1_model.routes.first.options).to eq({ 'lb_algo' => 'least-connections' })
752850
end
753851
end
754852
end

0 commit comments

Comments
 (0)