Fix Client#get_resources for custom resources that are not yet defined

lib/k8s-client.rb

@@ -11,3 +11,4 @@
 require 'k8s/resource_client'
 require 'k8s/stack'
 require 'k8s/transport'
+require 'k8s/util'

lib/k8s/client.rb

@@ -58,19 +58,22 @@ def api_groups
     end
 
     # @param api_versions [Array<String>] defaults to all APIs
+    # @param prefetch_resources [Boolean] prefetch any missing api_resources for each api_version
+    # @param skip_missing [Boolean] return APIClient without api_resources? if 404
     # @return [Array<APIClient>]
-    def apis(api_versions = nil, prefetch_resources: false)
+    def apis(api_versions = nil, prefetch_resources: false, skip_missing: false)
       api_versions ||= ['v1'] + self.api_groups
 
       if prefetch_resources
         # api groups that are missing their api_resources
         api_paths = api_versions
+          .uniq
           .select{|api_version| !api(api_version).api_resources? }
           .map{|api_version| APIClient.path(api_version) }
 
         # load into APIClient.api_resources=
-        @transport.gets(*api_paths, response_class: K8s::API::MetaV1::APIResourceList).each do |api_resource_list|
-          api(api_resource_list.groupVersion).api_resources = api_resource_list.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
         end
       end
 
@@ -116,21 +119,32 @@ def get_resource(resource)
       client_for_resource(resource).get_resource(resource)
     end
 
+    # Returns nils for any resources that do not exist.
+    # This includes custom resources that were not yet defined.
+    #
     # @param resources [Array<K8s::Resource>]
-    # @return [Array<K8s::Resource, nil>]
+    # @return [Array<K8s::Resource, nil>] matching resources array 1:1
     def get_resources(resources)
-      # prefetch api resources
-      apis(resources.map{|resource| resource.apiVersion }.uniq, prefetch_resources: true)
+      # prefetch api resources, skip missing APIs
+      resource_apis = apis(resources.map{ |resource| resource.apiVersion }, prefetch_resources: true, skip_missing: true)
+
+      # map each resource to excon request options, or nil if resource is not (yet) defined
+      requests = resources.zip(resource_apis).map{ |resource, api_client|
+        next nil unless api_client.api_resources?
+
+        resource_client = api_client.client_for_resource(resource)
 
-      resource_clients = resources.map{|resource| client_for_resource(resource) }
-      requests = resources.zip(resource_clients).map{|resource, resource_client|
         {
           method: 'GET',
           path: resource_client.path(resource.metadata.name, namespace: resource.metadata.namespace),
           response_class: resource_client.resource_class,
         }
       }
-      responses = @transport.requests(*requests, skip_missing: true)
+
+      # map non-nil requests to response objects, or nil for nil request options
+      Util.compact_map(requests) { |reqs|
+        @transport.requests(*reqs, skip_missing: true)
+      }
     end
 
     # @param resource [K8s::Resource]

lib/k8s/stack.rb

@@ -124,6 +124,7 @@ def prune(client, keep_resources: )
             client.delete_resource(resource)
           rescue K8s::Error::NotFound
             # assume aliased objects in multiple API groups, like for Deployments
+            # alternatively, a custom resource whose definition was already deleted earlier
           end
         end
       end

lib/k8s/transport.rb

@@ -201,19 +201,21 @@ def request(response_class: nil, **options)
     # @param skip_missing [Boolean] return nil for HTTP 404 responses
     # @param retry_errors [Boolean] retry with non-pipelined request for HTTP 503 responses
     # @return [Array<response_class, Hash, nil>]
-    def requests(*options, response_class: nil, skip_missing: false, retry_errors: true)
+    def requests(*options, skip_missing: false, retry_errors: true, **common_options)
       return [] if options.empty? # excon chokes
 
       start = Time.now
       responses = excon.requests(
-        options.map{|options| request_options(**options)}
+        options.map{|options| request_options(**common_options.merge(options))}
       )
       t = Time.now - start
 
       objects = responses.zip(options).map{|response, request_options|
+        response_class = request_options[:response_class] || common_options[:response_class]
+
         begin
           parse_response(response, request_options,
-            response_class: request_options[:response_class] || response_class,
+            response_class: response_class,
           )
         rescue K8s::Error::NotFound
           if skip_missing
@@ -226,7 +228,7 @@ def requests(*options, response_class: nil, skip_missing: false, retry_errors: t
             logger.warn { "Retry #{format_request(request_options)} => HTTP #{exc.code} #{exc.reason} in #{'%.3f' % t}s" }
 
             # only retry the failed request, not the entire pipeline
-            request(response_class: response_class, **request_options)
+            request(response_class: response_class, **common_options.merge(request_options))
           else
             raise
           end
@@ -250,13 +252,12 @@ def get(*path, **options)
     end
 
     # @param *paths [String]
-    def gets(*paths, response_class: nil, **options)
+    def gets(*paths, **options)
       requests(*paths.map{|path| {
           method: 'GET',
           path: self.path(path),
-          **options,
         } },
-        response_class: response_class,
+        **options
       )
     end
   end

lib/k8s/util.rb

@@ -0,0 +1,22 @@
+module K8s
+  module Util
+    # Yield with all non-nil args, returning matching array with corresponding return values or nils.
+    #
+    # Args must be usable as hash keys. Duplicate args will all map to the same return value.
+    #
+    # @param args [Array<nil, Object>]
+    # @yield args
+    # @yieldparam args [Array<Object>] omitting any nil values
+    # @return [Array<nil, Object>] matching args array 1:1, containing yielded values for non-nil args
+    def self.compact_map(args)
+      func_args = args.compact
+
+      values = yield func_args
+
+      # Hash{arg => value}
+      value_map = Hash[func_args.zip(values)]
+
+      args.map{|arg| value_map[arg] }
+    end
+  end
+end

spec/fixtures/api/configmaps-bar-404.json

@@ -0,0 +1,15 @@
+{
+  "kind": "Status",
+  "apiVersion": "v1",
+  "metadata": {
+
+  },
+  "status": "Failure",
+  "message": "configmaps \"bar\" not found",
+  "reason": "NotFound",
+  "details": {
+    "name": "bar",
+    "kind": "configmaps"
+  },
+  "code": 404
+}

spec/fixtures/api/configmaps-bar.json

@@ -0,0 +1,15 @@
+{
+    "apiVersion": "v1",
+    "data": {
+        "foo": "bar"
+    },
+    "kind": "ConfigMap",
+    "metadata": {
+        "creationTimestamp": "2018-08-13T08:07:22Z",
+        "name": "bar",
+        "namespace": "default",
+        "resourceVersion": "3718",
+        "selfLink": "/api/v1/namespaces/default/configmaps/bar",
+        "uid": "e8293208-9ecf-11e8-b464-76d458035df6"
+    }
+}

spec/fixtures/api/services-foo.json

@@ -0,0 +1,30 @@
+{
+    "apiVersion": "v1",
+    "kind": "Service",
+    "metadata": {
+        "creationTimestamp": "2018-08-13T08:06:38Z",
+        "name": "foo",
+        "namespace": "default",
+        "resourceVersion": "3646",
+        "selfLink": "/api/v1/namespaces/default/services/foo",
+        "uid": "cdc7144a-9ecf-11e8-b464-76d458035df6"
+    },
+    "spec": {
+        "clusterIP": "10.250.83.45",
+        "ports": [
+            {
+                "port": 80,
+                "protocol": "TCP",
+                "targetPort": 80
+            }
+        ],
+        "selector": {
+            "app": "foo"
+        },
+        "sessionAffinity": "None",
+        "type": "ClusterIP"
+    },
+    "status": {
+        "loadBalancer": {}
+    }
+}

spec/fixtures/resources/configmap-bar.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  namespace: default
+  name: bar
+data:
+  foo: bar

spec/fixtures/resources/crd-test.yaml

@@ -0,0 +1,11 @@
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: tests.pharos-test.k8s.io
+spec:
+  group: pharos-test.k8s.io
+  version: v0
+  names:
+    kind: Test
+    plural: tests
+  scope: Namespaced

spec/fixtures/resources/service-foo.yaml

@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Service
+metadata:
+  namespace: default
+  name: foo
+spec:
+  selector:
+    app: foo
+  type: ClusterIP
+  ports:
+    - port: 80

spec/fixtures/resources/test.yaml

@@ -0,0 +1,7 @@
+apiVersion: pharos-test.k8s.io/v0
+kind: Test
+metadata:
+  name: test
+  namespace: default
+spec:
+  test: true

spec/pharos/kube/client_spec.rb

@@ -237,5 +237,118 @@
         end
       end
     end
+
+    describe '#get_resources' do
+      context "for standard resources" do
+        let(:resources) {[
+          K8s::Resource.from_file(fixture_path('resources/service-foo.yaml')),
+          K8s::Resource.from_file(fixture_path('resources/configmap-bar.yaml')),
+        ]}
+
+        context "which already exist" do
+          before do
+            stub_request(:get, 'localhost:8080/api/v1/namespaces/default/services/foo')
+              .to_return(
+                status: 200,
+                headers: { 'Content-Type' => 'application/json' },
+                body: fixture('api/services-foo.json'),
+              )
+            stub_request(:get, 'localhost:8080/api/v1/namespaces/default/configmaps/bar')
+              .to_return(
+                status: 200,
+                headers: { 'Content-Type' => 'application/json' },
+                body: fixture('api/configmaps-bar.json'),
+              )
+          end
+
+          it "prefetches API resources and pipelines the requests" do
+            expect(transport).to receive(:requests).once.with(
+              hash_including(path: '/api/v1'),
+              skip_missing: true,
+              response_class: anything,
+            ).and_call_original
+            expect(transport).to receive(:requests).once.with(
+              hash_including(path: '/api/v1/namespaces/default/services/foo'),
+              hash_including(path: '/api/v1/namespaces/default/configmaps/bar'),
+              skip_missing: true,
+            ).and_call_original
+
+            r = subject.get_resources(resources)
+
+            expect(r).to match [K8s::Resource, K8s::Resource]
+            expect(r[0].to_hash).to match hash_including(
+              apiVersion: 'v1',
+              kind: 'Service',
+            )
+            expect(r[1].to_hash).to match hash_including(
+              apiVersion: 'v1',
+              kind: 'ConfigMap',
+            )
+          end
+        end
+
+        context "which may be missing" do
+          before do
+            stub_request(:get, 'localhost:8080/api/v1/namespaces/default/services/foo')
+              .to_return(
+                status: 200,
+                headers: { 'Content-Type' => 'application/json' },
+                body: fixture('api/services-foo.json'),
+              )
+            stub_request(:get, 'localhost:8080/api/v1/namespaces/default/configmaps/bar')
+              .to_return(
+                status: 404,
+                headers: { 'Content-Type' => 'application/json' },
+                body: fixture('api/configmaps-bar-404.json'),
+              )
+          end
+
+          it "returns mixed nils" do
+            r = subject.get_resources(resources)
+
+            expect(r).to match [K8s::Resource, nil]
+            expect(r[0].to_hash).to match hash_including(
+              apiVersion: 'v1',
+              kind: 'Service',
+            )
+            expect(r[1]).to be nil
+          end
+        end
+      end
+
+      context "for custom resources" do
+        let(:resources) {[
+          K8s::Resource.from_file(fixture_path('resources/crd-test.yaml')),
+          K8s::Resource.from_file(fixture_path('resources/test.yaml')),
+        ]}
+
+        context "which do not yet exist" do
+          before do
+            stub_request(:get, 'localhost:8080/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/tests.pharos-test.k8s.io')
+              .to_return(
+                status: 404,
+                headers: { 'Content-Type' => 'text/plain' },
+                body: '404 page not found',
+              )
+            stub_request(:get, 'localhost:8080/apis/pharos-test.k8s.io/v0')
+              .to_return(
+                status: 404,
+                headers: { 'Content-Type' => 'text/plain' },
+                body: '404 page not found',
+              )
+            stub_request(:get, 'localhost:8080/apis/pharos-test.k8s.io/v0/namespaces/default/tests/test')
+              .to_return(
+                status: 404,
+              )
+          end
+
+          it "returns nils" do
+            r = subject.get_resources(resources)
+
+            expect(r).to match [nil, nil]
+          end
+        end
+      end
+    end
   end
 end

spec/pharos/kube/util_spec.rb

@@ -0,0 +1,23 @@
+RSpec.describe K8s::Util do
+  describe '#compact_map' do
+    it "returns an empty array for an empty array" do
+      expect(described_class.compact_map([]) { |args| args.map(&:to_s) }).to eq []
+    end
+
+    it "returns an array of nil for an array of nil" do
+      expect(described_class.compact_map([nil]) { |args| args.map(&:to_s) }).to eq [nil]
+    end
+
+    it "translates non-nil values" do
+      expect(described_class.compact_map([1]) { |args| args.map(&:to_s) }).to eq ['1']
+    end
+
+    it "translates non-nil values and passes through nil values" do
+      expect(described_class.compact_map([nil, 1]) { |args| args.map(&:to_s) }).to eq [nil, '1']
+    end
+
+    it "handles duplicates" do
+      expect(described_class.compact_map([nil, 1, 2, 1]) { |args| args.map(&:to_s) }).to eq [nil, '1', '2', '1']
+    end
+  end
+end