Handle errors better in pipelined Client requests (#89)

* safeguard pipelined Client requests * rubocop * specs * synchronize #api_groups! * specs * use flat_map Co-Authored-By: jakolehm <[email protected]>
kontena · Jan 7, 2019 · c07038e · c07038e
1 parent e94fd9b
commit c07038e
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 7 deletions.
diff --git a/lib/k8s/client.rb b/lib/k8s/client.rb
@@ -3,6 +3,7 @@
 require 'openssl'
 require 'base64'
 require 'yajl'
+require 'monitor'
 
 require 'k8s/util'
 
@@ -81,6 +82,8 @@ def self.autoconfig(namespace: nil, **options)
       end
     end
 
+    include MonitorMixin
+
     attr_reader :transport
 
     # @param transport [K8s::Transport]
@@ -90,6 +93,7 @@ def initialize(transport, namespace: nil)
       @namespace = namespace
 
       @api_clients = {}
+      super()
     end
 
     # @raise [K8s::Error]
@@ -109,10 +113,16 @@ def api(api_version = 'v1')
     #
     # @return [Array<String>]
     def api_groups!
-      @api_groups = @transport.get(
-        '/apis',
-        response_class: K8s::API::MetaV1::APIGroupList
-      ).groups.map{ |api_group| api_group.versions.map(&:groupVersion) }.flatten
+      synchronize do
+        @api_groups = @transport.get(
+          '/apis',
+          response_class: K8s::API::MetaV1::APIGroupList
+        ).groups.flat_map{ |api_group| api_group.versions.map(&:groupVersion) }
+
+        @api_clients.clear
+      end
+
+      @api_groups
     end
 
     # Cached /apis preferred group apiVersions
@@ -136,8 +146,13 @@ def apis(api_versions = nil, prefetch_resources: false, skip_missing: false)
                     .map{ |api_version| APIClient.path(api_version) }
 
         # load into APIClient.api_resources=
-        @transport.gets(*api_paths, response_class: K8s::API::MetaV1::APIResourceList, skip_missing: skip_missing).each do |api_resource_list|
-          api(api_resource_list.groupVersion).api_resources = api_resource_list.resources if api_resource_list
+        begin
+          @transport.gets(*api_paths, response_class: K8s::API::MetaV1::APIResourceList, skip_missing: skip_missing).each do |api_resource_list|
+            api(api_resource_list.groupVersion).api_resources = api_resource_list.resources if api_resource_list
+          end
+        rescue K8s::Error::NotFound, K8s::Error::ServiceUnavailable # rubocop:disable Lint/HandleExceptions
+          # kubernetes api is in unstable state
+          # because this is only performance optimization, better to skip prefetch and move on
         end
       end
 
@@ -158,9 +173,18 @@ def resources(namespace: nil)
     # @param options @see K8s::ResourceClient#list
     # @return [Array<K8s::Resource>]
     def list_resources(resources = nil, **options)
+      cached_clients = @api_clients.size.positive?
       resources ||= self.resources.select(&:list?)
 
-      ResourceClient.list(resources, @transport, **options)
+      begin
+        ResourceClient.list(resources, @transport, **options)
+      rescue K8s::Error::NotFound
+        raise unless cached_clients
+
+        cached_clients = false
+        api_groups!
+        retry
+      end
     end
 
     # @param resource [K8s::Resource]

diff --git a/spec/k8s/client_spec.rb b/spec/k8s/client_spec.rb
@@ -108,6 +108,21 @@
           expect(apis.map{|api| api.api_resources}.flatten.map{|r| r.name}).to include 'nodes', 'pods', 'deployments', 'jobs'
         end
       end
+
+      context "stale api cache" do
+        before do
+          subject.api.api_resources
+        end
+
+        it "ignores errors in prefetch" do
+          expect(transport).to receive(:get).once.with('/apis', hash_including(:response_class)).and_call_original
+          expect(transport).to receive(:gets).once.and_raise(K8s::Error::NotFound.new('GET', '/foo', 404, 'NotFound'))
+
+          expect {
+            subject.apis(prefetch_resources: true)
+          }.not_to raise_error
+        end
+      end
     end
 
     describe '#create_resource' do
@@ -420,6 +435,18 @@
           end
         end
       end
+
+      context "with partial 404 errors" do
+        it "raises NotFound" do
+          allow(transport).to receive(:gets).and_raise(K8s::Error::NotFound.new('GET', '/foo', 404, 'NotFound'))
+          expect{
+            subject.list_resources([
+              subject.api('v1').resource('services'),
+              subject.api('v1').resource('configmaps'),
+            ])
+          }.to raise_error(K8s::Error::NotFound)
+        end
+      end
     end
   end
 end