Skip to content

Commit 9cf1846

Browse files
authored
Restore service binding ordering in sys env presenter (#4665)
The ordering of multiple service bindings for the same service with VCAP service has changed. They are now sorted by service instance guid and not service binding id. This was implicit behaviour. While clients should be inspecting the binding object itslef and not relying on the index, unfortunately not all clients do that. This appears to have been introduced as part of this commit to support binding rotation 79b4da3#diff-403a1ff3052c3335b1d4f0db3569c895bf7ba4ca01539c562d009394c907d64bR148 and this migration f011651 and I believe the introduction of the index: ``` add_index %i[app_guid service_instance_guid], name: :service_bindings_app_guid_service_instance_guid_index ``` Means that the query to obtain the binding list now uses this index, and it uses the natural order of that index, which is by service_instance_guid rather than previously which would have been the natural order by id. This change restores the previous behaviour by explicitly specifying the ordering.
1 parent 2feeac5 commit 9cf1846

File tree

2 files changed

+20
-1
lines changed

2 files changed

+20
-1
lines changed

app/models/services/service_binding.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ def active_per_instance
148148
order: [Sequel.desc(:created_at), Sequel.desc(:id)]
149149
).as(:_rn)
150150
end.
151-
from_self.where(_rn: 1)
151+
from_self.where(_rn: 1).
152+
order_by(:id)
152153
end
153154
end
154155
end

spec/unit/presenters/system_environment/system_env_presenter_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,24 @@ module VCAP::CloudController
140140
end
141141
end
142142

143+
context 'when there are multiple service bindings for the same service offering and plan' do
144+
let!(:service_instance_2) { ManagedServiceInstance.make(space: space, service_plan: service_plan, name: 'instance-2') }
145+
let!(:service_instance_3) { ManagedServiceInstance.make(space: space, service_plan: service_plan, name: 'instance-3') }
146+
let!(:service_binding_2) { ServiceBinding.make(app: app, service_instance: service_instance_2) }
147+
let!(:service_binding_3) { ServiceBinding.make(app: app, service_instance: service_instance_3) }
148+
149+
it 'returns bindings in natural order by id' do
150+
bindings = system_env_presenter.system_env[:VCAP_SERVICES][service.label.to_sym]
151+
expect(bindings).to have(3).items
152+
153+
actual_binding_order = bindings.map { |b| b.to_hash[:binding_guid] }
154+
155+
expected_binding_order = [service_binding, service_binding_2, service_binding_3].map(&:guid)
156+
157+
expect(actual_binding_order).to eq(expected_binding_order)
158+
end
159+
end
160+
143161
describe 'volume mounts' do
144162
context 'when the service binding has volume mounts' do
145163
let!(:service_binding) do

0 commit comments

Comments
 (0)