Skip to content

Commit

Permalink
Remove poll interval from client responses
Browse files Browse the repository at this point in the history
- Poll interval for fetch job now only comes from bosh configuration in the
  fetch job

[#91740148]
  • Loading branch information
rainmaker authored and Greg Cobb and Raina Masand committed Apr 14, 2015
1 parent 133c31e commit 739d596
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 104 deletions.
7 changes: 3 additions & 4 deletions app/actions/service_instance_delete.rb
Expand Up @@ -36,15 +36,15 @@ def delete_service_instance(service_instance)
lock = DeleterLock.new(service_instance)
lock.lock!

attributes_to_update, poll_interval = service_instance.client.deprovision(
attributes_to_update = service_instance.client.deprovision(
service_instance,
accepts_incomplete: @accepts_incomplete
)

if attributes_to_update[:last_operation][:state] == 'succeeded'
lock.unlock_and_destroy!
else
lock.enqueue_unlock!(attributes_to_update, build_fetch_job(poll_interval, service_instance))
lock.enqueue_unlock!(attributes_to_update, build_fetch_job(service_instance))
end
rescue => e
errors << e
Expand All @@ -59,14 +59,13 @@ def delete_service_bindings(service_instance)
ServiceBindingDelete.new.delete(service_instance.service_bindings_dataset)
end

def build_fetch_job(poll_interval, service_instance)
def build_fetch_job(service_instance)
VCAP::CloudController::Jobs::Services::ServiceInstanceStateFetch.new(
'service-instance-state-fetch',
service_instance.client.attrs,
service_instance.guid,
@event_repository_opts,
{},
poll_interval,
)
end
end
Expand Down
Expand Up @@ -19,7 +19,7 @@ def create_service_instance(request_attrs, params)
validate_create_action(request_attrs, params)

service_instance = ManagedServiceInstance.new(request_attrs.except('parameters'))
attributes_to_update, poll_interval_seconds = service_instance.client.provision(
attributes_to_update = service_instance.client.provision(
service_instance,
accepts_incomplete: accepts_incomplete?(params),
request_attrs: request_attrs,
Expand All @@ -38,8 +38,7 @@ def create_service_instance(request_attrs, params)
service_instance.client.attrs,
service_instance.guid,
event_repository_opts,
request_attrs,
poll_interval_seconds,
request_attrs
)
job.enqueue
end
Expand Down
Expand Up @@ -26,10 +26,10 @@ def update_service_instance(service_instance, request_attrs, params)
lock.lock!

begin
attributes_to_update, poll_interval_seconds, err = get_attributes_to_update(params, request_attrs, service_instance)
attributes_to_update, err = get_attributes_to_update(params, request_attrs, service_instance)

if attributes_to_update[:last_operation][:state] == 'in progress'
job = build_fetch_job(poll_interval_seconds, request_attrs, service_instance)
job = build_fetch_job(request_attrs, service_instance)
lock.enqueue_unlock!(attributes_to_update, job)
else
lock.synchronous_unlock!(attributes_to_update)
Expand All @@ -46,14 +46,13 @@ def update_service_instance(service_instance, request_attrs, params)
end
end

def build_fetch_job(poll_interval_seconds, request_attrs, service_instance)
def build_fetch_job(request_attrs, service_instance)
VCAP::CloudController::Jobs::Services::ServiceInstanceStateFetch.new(
'service-instance-state-fetch',
service_instance.client.attrs,
service_instance.guid,
event_repository_opts,
request_attrs,
poll_interval_seconds,
request_attrs
)
end

Expand Down
5 changes: 2 additions & 3 deletions app/jobs/services/service_instance_state_fetch.rb
Expand Up @@ -6,7 +6,7 @@ module Services
class ServiceInstanceStateFetch < VCAP::CloudController::Jobs::CCJob
attr_accessor :name, :client_attrs, :service_instance_guid, :services_event_repository_opts, :request_attrs, :poll_interval, :attempts_remaining

def initialize(name, client_attrs, service_instance_guid, services_event_repository_opts, request_attrs, poll_interval=nil, attempts_remaining=nil)
def initialize(name, client_attrs, service_instance_guid, services_event_repository_opts, request_attrs, attempts_remaining=nil)
@name = name
@client_attrs = client_attrs
@service_instance_guid = service_instance_guid
Expand All @@ -16,8 +16,7 @@ def initialize(name, client_attrs, service_instance_guid, services_event_reposit
@attempts_remaining = attempts_remaining || VCAP::CloudController::Config.config[:broker_client_max_async_poll_duration_minutes]

default_poll_interval = VCAP::CloudController::Config.config[:broker_client_default_async_poll_interval_seconds]
poll_interval ||= default_poll_interval
poll_interval = [[default_poll_interval, poll_interval].max, 24.hours].min
poll_interval = [default_poll_interval, 24.hours].min
@poll_interval = poll_interval
end

Expand Down
13 changes: 4 additions & 9 deletions lib/services/service_brokers/v2/client.rb
Expand Up @@ -35,7 +35,6 @@ def provision(instance, request_attrs: {}, accepts_incomplete: false)

parsed_response = @response_parser.parse_provision_or_bind(path, response)
last_operation_hash = parsed_response['last_operation'] || {}
poll_interval_seconds = last_operation_hash['async_poll_interval_seconds'].try(:to_i) || 60
attributes = {
# DEPRECATED, but needed because of not null constraint
credentials: {},
Expand All @@ -53,7 +52,7 @@ def provision(instance, request_attrs: {}, accepts_incomplete: false)
attributes[:last_operation][:state] = 'succeeded'
end

[attributes, poll_interval_seconds]
attributes
rescue Errors::ServiceBrokerApiTimeout, Errors::ServiceBrokerBadResponse => e
@orphan_mitigator.cleanup_failed_provision(@attrs, instance)
raise e
Expand Down Expand Up @@ -138,18 +137,15 @@ def deprovision(instance, accepts_incomplete: false)

parsed_response = @response_parser.parse_deprovision_or_unbind(path, response) || {}
last_operation_hash = parsed_response['last_operation'] || {}
poll_interval_seconds = last_operation_hash['async_poll_interval_seconds'].try(:to_i)
state = last_operation_hash['state']

attributes_to_update = {
{
last_operation: {
type: 'delete',
description: last_operation_hash['description'] || '',
state: state || 'succeeded'
}
}

[attributes_to_update, poll_interval_seconds]
rescue VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerConflict => e
raise VCAP::Errors::ApiError.new_from_details('ServiceInstanceDeprovisionFailed', e.message)
end
Expand All @@ -171,7 +167,6 @@ def update_service_plan(instance, plan, accepts_incomplete: false, parameters: n

parsed_response = @response_parser.parse_update(path, response)
last_operation_hash = parsed_response['last_operation'] || {}
poll_interval_seconds = last_operation_hash['async_poll_interval_seconds'].try(:to_i) || 60
state = last_operation_hash['state'] || 'succeeded'

attributes = {
Expand All @@ -188,7 +183,7 @@ def update_service_plan(instance, plan, accepts_incomplete: false, parameters: n
attributes[:last_operation][:proposed_changes] = { service_plan_guid: plan.guid }
end

return attributes, poll_interval_seconds, nil
return attributes, nil
rescue Errors::ServiceBrokerBadResponse,
Errors::ServiceBrokerApiTimeout,
Errors::ServiceBrokerResponseMalformed,
Expand All @@ -202,7 +197,7 @@ def update_service_plan(instance, plan, accepts_incomplete: false, parameters: n
description: e.message
}
}
return attributes, nil, e
return attributes, e
end

private
Expand Down
3 changes: 1 addition & 2 deletions spec/unit/actions/service_instance_delete_spec.rb
Expand Up @@ -70,7 +70,6 @@ module VCAP::CloudController
context 'when accepts_incomplete is true' do
let(:service_instance) { ManagedServiceInstance.make }
let(:event_repository_opts) { { some_opt: 'some value' } }
let(:polling_interval) { 60 }
let(:error_when_in_progress) { false }

subject(:service_instance_delete) do
Expand Down Expand Up @@ -108,7 +107,7 @@ module VCAP::CloudController
expect(inner_job.service_instance_guid).to eq service_instance.guid
expect(inner_job.services_event_repository_opts).to eq event_repository_opts
expect(inner_job.request_attrs).to eq({})
expect(inner_job.poll_interval).to eq(polling_interval)
expect(inner_job.poll_interval).to eq(60)
end

context 'and the caller wants to treat accepts_incomplete deprovisioning as a failure' do
Expand Down
37 changes: 4 additions & 33 deletions spec/unit/jobs/services/service_instance_state_fetch_spec.rb
Expand Up @@ -50,7 +50,6 @@ module Services
},
}
end
let(:poll_interval) { 60.second }
let(:max_duration) { 10088 }
let(:deprecated_attempts_remaining) { nil }
let(:request_attrs) do
Expand All @@ -66,7 +65,6 @@ module Services
service_instance.guid,
service_event_repository_opts,
request_attrs,
poll_interval,
deprecated_attempts_remaining,
)
end
Expand Down Expand Up @@ -94,38 +92,11 @@ def run_job(job)
end
end

context 'when the caller provides a polling interval' do
context 'and the value is less than the default value' do
let(:poll_interval) { 60 }
context 'when the default poll interval is greater than the max value (24 hours)' do
let(:default_polling_interval) { 24.hours + 1.minute }

it 'sets polling_interval to default polling interval' do
expect(job.poll_interval).to eq default_polling_interval
end
end

context 'and the value is greater than the max value (24 hours)' do
let(:poll_interval) { 24.hours + 1.minute }

it 'enqueues the job using the maximum polling interval' do
expect(job.poll_interval).to eq 24.hours
end
end

context 'and the value is between the default value and max value (24 hours)' do
let(:poll_interval) { 200 }

it 'enqueues the job using the broker provided polling interval' do
expect(job.poll_interval).to eq poll_interval
end
end

context 'when the default is greater than the max value (24 hours)' do
let(:default_polling_interval) { 24.hours + 1.minute }
let(:poll_interval) { 120 }

it 'enqueues the job using the maximum polling interval' do
expect(job.poll_interval).to eq 24.hours
end
it 'enqueues the job using the maximum polling interval' do
expect(job.poll_interval).to eq 24.hours
end
end
end
Expand Down
50 changes: 5 additions & 45 deletions spec/unit/lib/services/service_brokers/v2/client_spec.rb
Expand Up @@ -202,11 +202,6 @@ module VCAP::Services::ServiceBrokers::V2
expect(attributes[:last_operation][:type]).to eq('create')
expect(attributes[:last_operation][:state]).to eq('in progress')
end

it 'returns the interval for polling the operation state' do
_, polling_interval = client.provision(instance)
expect(polling_interval).to eq 60
end
end

context 'when the broker returns the state as failed' do
Expand Down Expand Up @@ -526,11 +521,6 @@ module VCAP::Services::ServiceBrokers::V2
expect(attributes[:last_operation][:description]).to eq('')
expect(error).to be_nil
end

it 'sets an interval for polling the operation state' do
_, polling_interval, _ = client.update_service_plan(instance, new_plan, accepts_incomplete: true)
expect(polling_interval).to eq(60)
end
end
end

Expand All @@ -546,13 +536,8 @@ module VCAP::Services::ServiceBrokers::V2
allow(http_client).to receive(:patch).and_raise(error)
end

it 'returns no polling_interval' do
_, polling_interval, _ = client.update_service_plan(instance, new_plan, accepts_incomplete: true)
expect(polling_interval).to be_nil
end

it 'returns an array containing the update attributes and the error' do
attrs, _, err = client.update_service_plan(instance, new_plan, accepts_incomplete: true)
attrs, err = client.update_service_plan(instance, new_plan, accepts_incomplete: true)

expect(err).to eq error
expect(attrs).to eq({
Expand All @@ -572,13 +557,8 @@ module VCAP::Services::ServiceBrokers::V2
allow(response_parser).to receive(:parse_update).and_raise(error)
end

it 'returns no polling_interval' do
_, polling_interval, _ = client.update_service_plan(instance, new_plan, accepts_incomplete: true)
expect(polling_interval).to be_nil
end

it 'returns an array containing the update attributes and the error' do
attrs, _, err = client.update_service_plan(instance, new_plan, accepts_incomplete: true)
attrs, err = client.update_service_plan(instance, new_plan, accepts_incomplete: true)

expect(err).to eq error
expect(attrs).to eq({
Expand All @@ -598,13 +578,8 @@ module VCAP::Services::ServiceBrokers::V2
allow(response_parser).to receive(:parse_update).and_raise(error)
end

it 'returns no polling_interval' do
_, polling_interval, _ = client.update_service_plan(instance, new_plan, accepts_incomplete: true)
expect(polling_interval).to be_nil
end

it 'returns an array containing the update attributes and the error' do
attrs, _, err = client.update_service_plan(instance, new_plan, accepts_incomplete: true)
attrs, err = client.update_service_plan(instance, new_plan, accepts_incomplete: true)

expect(err).to eq error
expect(attrs).to eq({
Expand All @@ -624,13 +599,8 @@ module VCAP::Services::ServiceBrokers::V2
allow(response_parser).to receive(:parse_update).and_raise(error)
end

it 'returns no polling_interval' do
_, polling_interval, _ = client.update_service_plan(instance, new_plan, accepts_incomplete: true)
expect(polling_interval).to be_nil
end

it 'returns an array containing the update attributes and the error' do
attrs, _, err = client.update_service_plan(instance, new_plan, accepts_incomplete: 'true')
attrs, err = client.update_service_plan(instance, new_plan, accepts_incomplete: 'true')

expect(err).to eq error
expect(attrs).to eq({
Expand All @@ -650,13 +620,8 @@ module VCAP::Services::ServiceBrokers::V2
allow(response_parser).to receive(:parse_update).and_raise(error)
end

it 'returns no polling_interval' do
_, polling_interval, _ = client.update_service_plan(instance, new_plan, accepts_incomplete: true)
expect(polling_interval).to be_nil
end

it 'returns an array containing the update attributes and the error' do
attrs, _, err = client.update_service_plan(instance, new_plan, accepts_incomplete: 'true')
attrs, err = client.update_service_plan(instance, new_plan, accepts_incomplete: 'true')

expect(err).to eq error
expect(attrs).to eq({
Expand Down Expand Up @@ -979,11 +944,6 @@ module VCAP::Services::ServiceBrokers::V2
}
})
end

it 'returns no polling_interval' do
_, polling_interval, _ = client.deprovision(instance, accepts_incomplete: true)
expect(polling_interval).to be_nil
end
end
end
end
Expand Down

0 comments on commit 739d596

Please sign in to comment.