Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Commit 9ea993c

Browse files
cwlbraamatt-royal
andcommitted
retry 3 times on k8s route update 409s
[finishes #173583970] fixes cloudfoundry/capi-k8s-release#48 Co-authored-by: Connor Braa <cbraa@pivotal.io> Co-authored-by: Matt Royal <mroyal@pivotal.io>
1 parent b4134db commit 9ea993c

4 files changed

Lines changed: 57 additions & 9 deletions

File tree

lib/kubernetes/api_client.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
module Kubernetes
44
class ApiClient
5+
class Error < StandardError; end
6+
class ConflictError < Error; end
7+
58
def initialize(build_kube_client:, kpack_kube_client:, route_kube_client:)
69
@build_kube_client = build_kube_client
710
@kpack_kube_client = kpack_kube_client
@@ -64,6 +67,9 @@ def update_route(resource_config)
6467
@route_kube_client.update_route(resource_config)
6568
rescue Kubeclient::HttpError => e
6669
logger.error('update_route', error: e.inspect, response: e.response, backtrace: e.backtrace)
70+
71+
raise ConflictError.new("Conflict on update of #{resource_name(resource_config)}") if e.error_code == 409
72+
6773
error = CloudController::Errors::ApiError.new_from_details('KubernetesRouteResourceError', resource_name(resource_config), e.message, e.response)
6874
error.set_backtrace(e.backtrace)
6975
raise error

lib/kubernetes/route_resource_manager.rb

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
module Kubernetes
44
class RouteResourceManager
5+
UPDATE_DESTINATION_CONFLICT_RETRIES = 3
6+
57
def initialize(kube_client)
68
@client = kube_client
79
end
@@ -37,29 +39,40 @@ def create_route(route)
3739

3840
@client.create_route(Kubeclient::Resource.new(route_resource_hash))
3941
rescue => e
40-
Steno.logger('cc.action.route_create').info("Failed to Create Route CRD: #{e}")
42+
logger.info("Failed to Create Route CRD: #{e}")
4143
raise
4244
end
4345

4446
def update_destinations(route)
45-
route_resource = @client.get_route(route.guid, 'cf-workloads')
46-
route_resource.spec.destinations = get_destinations(route)
47-
48-
@client.update_route(route_resource)
49-
rescue => e
50-
Steno.logger('cc.action.route_update').info("Failed to Update Route CRD: #{e}")
51-
raise
47+
remaining_retries = UPDATE_DESTINATION_CONFLICT_RETRIES
48+
begin
49+
route_resource = @client.get_route(route.guid, 'cf-workloads')
50+
route_resource.spec.destinations = get_destinations(route)
51+
@client.update_route(route_resource)
52+
rescue ApiClient::ConflictError
53+
remaining_retries -= 1
54+
retry if remaining_retries.positive?
55+
logger.info("Failed to Update Route CRD after #{UPDATE_DESTINATION_CONFLICT_RETRIES}: #{e}")
56+
raise
57+
rescue => e
58+
logger.info("Failed to Update Route CRD: #{e}")
59+
raise
60+
end
5261
end
5362

5463
def delete_route(route)
5564
@client.delete_route(route.guid, VCAP::CloudController::Config.config.kubernetes_workloads_namespace)
5665
rescue => e
57-
Steno.logger('cc.action.route_create').info("Failed to Delete Route CRD: #{e}")
66+
logger.info("Failed to Delete Route CRD: #{e}")
5867
raise
5968
end
6069

6170
private
6271

72+
def logger
73+
Steno.logger('kubernetes.route_resource_manager')
74+
end
75+
6376
def get_destinations(route)
6477
route.route_mappings.map do |route_mapping|
6578
destination = {

spec/unit/lib/kubernetes/api_client_spec.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,17 @@
248248
end
249249
end
250250

251+
context 'when the error is a 409' do
252+
let(:error) { Kubeclient::HttpError.new(409, 'foo', 'bar') }
253+
254+
it 'raises as an ApiError that includes the resource name' do
255+
expect {
256+
subject.update_route(resource_config)
257+
}.to raise_error(Kubernetes::ApiClient::ConflictError)
258+
expect(logger).to have_received(:error).with('update_route', error: /status code/, response: error.response, backtrace: error.backtrace)
259+
end
260+
end
261+
251262
context 'when the config is a hash with symbol keys' do
252263
let(:resource_config) { config_hash.symbolize_keys }
253264

spec/unit/lib/kubernetes/route_resource_manager_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,24 @@
224224
}.to raise_error(error)
225225
end
226226
end
227+
228+
context 'when there is a k8s conflict error' do
229+
let(:error) { ::Kubernetes::ApiClient::ConflictError.new('boom') }
230+
231+
before do
232+
# raise a 409 twice, then succeed
233+
expect(k8s_client).to receive(:update_route).once.with(any_args).and_raise(error)
234+
expect(k8s_client).to receive(:update_route).once.with(any_args).and_raise(error)
235+
expect(k8s_client).to receive(:update_route).once.with(any_args)
236+
end
237+
238+
it 'retries 3 times, fetching the route to patch each time' do
239+
expect {
240+
subject.update_destinations(route)
241+
}.not_to raise_error
242+
expect(k8s_client).to have_received(:get_route).exactly(3).times
243+
end
244+
end
227245
end
228246
end
229247
end

0 commit comments

Comments
 (0)