From 457dca79ebcdbe6de589d5089d627ecede2a1710 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 11:20:31 +0300 Subject: [PATCH 01/16] spec get_resources happy case --- spec/fixtures/api/configmaps-bar.json | 15 +++++++ spec/fixtures/api/services-foo.json | 30 ++++++++++++++ spec/fixtures/resources/configmap-bar.yaml | 7 ++++ spec/fixtures/resources/service-foo.yaml | 11 +++++ spec/pharos/kube/client_spec.rb | 47 ++++++++++++++++++++++ 5 files changed, 110 insertions(+) create mode 100644 spec/fixtures/api/configmaps-bar.json create mode 100644 spec/fixtures/api/services-foo.json create mode 100644 spec/fixtures/resources/configmap-bar.yaml create mode 100644 spec/fixtures/resources/service-foo.yaml diff --git a/spec/fixtures/api/configmaps-bar.json b/spec/fixtures/api/configmaps-bar.json new file mode 100644 index 0000000..65048f9 --- /dev/null +++ b/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" + } +} diff --git a/spec/fixtures/api/services-foo.json b/spec/fixtures/api/services-foo.json new file mode 100644 index 0000000..45d56f7 --- /dev/null +++ b/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": {} + } +} diff --git a/spec/fixtures/resources/configmap-bar.yaml b/spec/fixtures/resources/configmap-bar.yaml new file mode 100644 index 0000000..d661036 --- /dev/null +++ b/spec/fixtures/resources/configmap-bar.yaml @@ -0,0 +1,7 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + namespace: default + name: bar +data: + foo: bar diff --git a/spec/fixtures/resources/service-foo.yaml b/spec/fixtures/resources/service-foo.yaml new file mode 100644 index 0000000..a426795 --- /dev/null +++ b/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 diff --git a/spec/pharos/kube/client_spec.rb b/spec/pharos/kube/client_spec.rb index 51745d5..ecab64e 100644 --- a/spec/pharos/kube/client_spec.rb +++ b/spec/pharos/kube/client_spec.rb @@ -237,5 +237,52 @@ 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'), 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 + end + end end end From 3bc322eb17a81846d6d93f0be33a145160f5b033 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 11:25:48 +0300 Subject: [PATCH 02/16] spec resource 404 handling --- spec/fixtures/api/configmaps-bar-404.json | 15 ++++++++++++ spec/pharos/kube/client_spec.rb | 28 +++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 spec/fixtures/api/configmaps-bar-404.json diff --git a/spec/fixtures/api/configmaps-bar-404.json b/spec/fixtures/api/configmaps-bar-404.json new file mode 100644 index 0000000..26be52f --- /dev/null +++ b/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 +} \ No newline at end of file diff --git a/spec/pharos/kube/client_spec.rb b/spec/pharos/kube/client_spec.rb index ecab64e..71c935f 100644 --- a/spec/pharos/kube/client_spec.rb +++ b/spec/pharos/kube/client_spec.rb @@ -282,6 +282,34 @@ ) 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 end end From e9289afb8e9fb29a5eebfcace249cefa28d826ae Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 11:35:04 +0300 Subject: [PATCH 03/16] spec broken crd case --- spec/fixtures/resources/crd-test.yaml | 11 +++++++++++ spec/fixtures/resources/test.yaml | 7 +++++++ spec/pharos/kube/client_spec.rb | 28 +++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 spec/fixtures/resources/crd-test.yaml create mode 100644 spec/fixtures/resources/test.yaml diff --git a/spec/fixtures/resources/crd-test.yaml b/spec/fixtures/resources/crd-test.yaml new file mode 100644 index 0000000..ecb4a52 --- /dev/null +++ b/spec/fixtures/resources/crd-test.yaml @@ -0,0 +1,11 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: test.pharos-test.k8s.io +spec: + group: pharos-test.k8s.io + version: v0 + names: + kind: Test + plural: tests + scope: Namespaced diff --git a/spec/fixtures/resources/test.yaml b/spec/fixtures/resources/test.yaml new file mode 100644 index 0000000..2255142 --- /dev/null +++ b/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 diff --git a/spec/pharos/kube/client_spec.rb b/spec/pharos/kube/client_spec.rb index 71c935f..d7dce8a 100644 --- a/spec/pharos/kube/client_spec.rb +++ b/spec/pharos/kube/client_spec.rb @@ -311,6 +311,34 @@ 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/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 From bc3ce79ffb722d45c14020f0e042931603cb33af Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 11:59:51 +0300 Subject: [PATCH 04/16] transport: fix gets options handling to allow skip_missing: true --- lib/k8s/transport.rb | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/k8s/transport.rb b/lib/k8s/transport.rb index 7950051..82567a6 100644 --- a/lib/k8s/transport.rb +++ b/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] - 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 From ef9eb117d5104b1155a0550105033d9077e0e9cd Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 12:00:22 +0300 Subject: [PATCH 05/16] specs: conditional DEBUG from env --- spec/spec_helper.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2c0a374..f6696c1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,6 +5,11 @@ require_relative 'helpers/fixture_helpers' +if ENV['DEBUG'] + K8s::Logging.debug! + K8s::Transport.verbose! +end + RSpec.configure do |config| # Enable flags like --only-failures and --next-failure config.example_status_persistence_file_path = ".rspec_status" From cf2cf4527c312edacd2dbad881f588988ef8e784 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 12:02:28 +0300 Subject: [PATCH 06/16] Client#apis: support skip_missing: true --- lib/k8s/client.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/k8s/client.rb b/lib/k8s/client.rb index c9b9407..5b3fe85 100644 --- a/lib/k8s/client.rb +++ b/lib/k8s/client.rb @@ -58,19 +58,22 @@ def api_groups end # @param api_versions [Array] 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] - 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 From 90eb68da55d321863d19bb0906ce934d8173ca40 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 12:15:46 +0300 Subject: [PATCH 07/16] Client#get_resources: skip requests for custom resources that are not yet defined --- lib/k8s/client.rb | 32 +++++++++++++++++++++----------- spec/pharos/kube/client_spec.rb | 12 +++++++++++- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/lib/k8s/client.rb b/lib/k8s/client.rb index 5b3fe85..d2f6e70 100644 --- a/lib/k8s/client.rb +++ b/lib/k8s/client.rb @@ -119,21 +119,31 @@ def get_resource(resource) client_for_resource(resource).get_resource(resource) end + # Returns nils for any resources that do not exist. + # # @param resources [Array] # @return [Array] def get_resources(resources) - # prefetch api resources - apis(resources.map{|resource| resource.apiVersion }.uniq, prefetch_resources: true) - - 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, + # prefetch api resources, skip missing APIs + resource_apis = apis(resources.map{|resource| resource.apiVersion }, prefetch_resources: true, skip_missing: true) + + request_map = Hash[resources.zip(resource_apis) + .select{|resource, api_client| api_client.api_resources? } # skip missing APIs + .map{|resource, api_client| [resource, api_client.client_for_resource(resource)] } + .map{|resource, resource_client| + [resource, { + 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) + ] + + # sparse array, may omit elements in resources + responses = @transport.requests(*request_map.values, skip_missing: true) + response_map = Hash[request_map.keys.zip(responses)] + + resources.map{|resource| response_map[resource] } end # @param resource [K8s::Resource] diff --git a/spec/pharos/kube/client_spec.rb b/spec/pharos/kube/client_spec.rb index d7dce8a..598577c 100644 --- a/spec/pharos/kube/client_spec.rb +++ b/spec/pharos/kube/client_spec.rb @@ -262,7 +262,11 @@ end it "prefetches API resources and pipelines the requests" do - expect(transport).to receive(:requests).once.with(hash_including(path: '/api/v1'), response_class: anything).and_call_original + 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'), @@ -320,6 +324,12 @@ context "which do not yet exist" do before do + stub_request(:get, 'localhost:8080/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/test.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, From 09be3f14f22053e532cce909ccf6dd4b29fec297 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 12:26:51 +0300 Subject: [PATCH 08/16] clarify comment --- lib/k8s/client.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/k8s/client.rb b/lib/k8s/client.rb index d2f6e70..89bdd7e 100644 --- a/lib/k8s/client.rb +++ b/lib/k8s/client.rb @@ -120,6 +120,7 @@ def 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] # @return [Array] From bfdddee9a95e2bca77719353dfefeb8067c07418 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 12:40:11 +0300 Subject: [PATCH 09/16] comment on CRD delete during stack prune --- lib/k8s/stack.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/k8s/stack.rb b/lib/k8s/stack.rb index a6849e3..3b13f3d 100644 --- a/lib/k8s/stack.rb +++ b/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 From 75041d3eb38f746df8fba3501886a01cb5d70b64 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 13:54:33 +0300 Subject: [PATCH 10/16] fix test crd to be valid --- spec/fixtures/resources/crd-test.yaml | 2 +- spec/pharos/kube/client_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/fixtures/resources/crd-test.yaml b/spec/fixtures/resources/crd-test.yaml index ecb4a52..8f30317 100644 --- a/spec/fixtures/resources/crd-test.yaml +++ b/spec/fixtures/resources/crd-test.yaml @@ -1,7 +1,7 @@ apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: - name: test.pharos-test.k8s.io + name: tests.pharos-test.k8s.io spec: group: pharos-test.k8s.io version: v0 diff --git a/spec/pharos/kube/client_spec.rb b/spec/pharos/kube/client_spec.rb index 598577c..1ddc66d 100644 --- a/spec/pharos/kube/client_spec.rb +++ b/spec/pharos/kube/client_spec.rb @@ -324,7 +324,7 @@ context "which do not yet exist" do before do - stub_request(:get, 'localhost:8080/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/test.pharos-test.k8s.io') + 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' }, From 3b0a4af8cbd2d9ff4aacb94b50ed4a64cd439e15 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 14:06:46 +0300 Subject: [PATCH 11/16] refactor using Util.compact_yield helper --- lib/k8s-client.rb | 1 + lib/k8s/client.rb | 34 +++++++++++++++++----------------- lib/k8s/util.rb | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 17 deletions(-) create mode 100644 lib/k8s/util.rb diff --git a/lib/k8s-client.rb b/lib/k8s-client.rb index fb59d2a..a0008de 100644 --- a/lib/k8s-client.rb +++ b/lib/k8s-client.rb @@ -11,3 +11,4 @@ require 'k8s/resource_client' require 'k8s/stack' require 'k8s/transport' +require 'k8s/util' diff --git a/lib/k8s/client.rb b/lib/k8s/client.rb index 89bdd7e..b455938 100644 --- a/lib/k8s/client.rb +++ b/lib/k8s/client.rb @@ -126,25 +126,25 @@ def get_resource(resource) # @return [Array] def get_resources(resources) # prefetch api resources, skip missing APIs - resource_apis = apis(resources.map{|resource| resource.apiVersion }, prefetch_resources: true, skip_missing: true) - - request_map = Hash[resources.zip(resource_apis) - .select{|resource, api_client| api_client.api_resources? } # skip missing APIs - .map{|resource, api_client| [resource, api_client.client_for_resource(resource)] } - .map{|resource, resource_client| - [resource, { - method: 'GET', - path: resource_client.path(resource.metadata.name, namespace: resource.metadata.namespace), - response_class: resource_client.resource_class, - }] - } - ] + 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? - # sparse array, may omit elements in resources - responses = @transport.requests(*request_map.values, skip_missing: true) - response_map = Hash[request_map.keys.zip(responses)] + resource_client = api_client.client_for_resource(resource) + + { + method: 'GET', + path: resource_client.path(resource.metadata.name, namespace: resource.metadata.namespace), + response_class: resource_client.resource_class, + } + } - resources.map{|resource| response_map[resource] } + # map requests to response objects, or nil for nil request options + Util.compact_yield(*requests) { |*requests| + @transport.requests(*requests, skip_missing: true) + } end # @param resource [K8s::Resource] diff --git a/lib/k8s/util.rb b/lib/k8s/util.rb new file mode 100644 index 0000000..79bc374 --- /dev/null +++ b/lib/k8s/util.rb @@ -0,0 +1,18 @@ +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. + # + # @return [Array] matching args + def self.compact_yield(*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 From 94250993a676db380abbdd55d3b3366d7241737c Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 15:11:18 +0300 Subject: [PATCH 12/16] rename Util.sparse_map, less argument unpacking --- lib/k8s/client.rb | 6 +++--- lib/k8s/util.rb | 9 ++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/k8s/client.rb b/lib/k8s/client.rb index b455938..48a59f2 100644 --- a/lib/k8s/client.rb +++ b/lib/k8s/client.rb @@ -123,7 +123,7 @@ def get_resource(resource) # This includes custom resources that were not yet defined. # # @param resources [Array] - # @return [Array] + # @return [Array] matching resources array 1:1 def get_resources(resources) # prefetch api resources, skip missing APIs resource_apis = apis(resources.map{ |resource| resource.apiVersion }, prefetch_resources: true, skip_missing: true) @@ -141,8 +141,8 @@ def get_resources(resources) } } - # map requests to response objects, or nil for nil request options - Util.compact_yield(*requests) { |*requests| + # map non-nil requests to response objects, or nil for nil request options + Util.sparse_map(requests) { |requests| @transport.requests(*requests, skip_missing: true) } end diff --git a/lib/k8s/util.rb b/lib/k8s/util.rb index 79bc374..545eae9 100644 --- a/lib/k8s/util.rb +++ b/lib/k8s/util.rb @@ -3,11 +3,14 @@ module Util # Yield with all non-nil args, returning matching array with corresponding return values or nils. # Args must be usable as hash keys. # - # @return [Array] matching args - def self.compact_yield(*args) + # @param args [Array] + # @yield args + # @yieldparam args [Array] omitting any nil values + # @return [Array] matching args array 1:1, containing yielded values for non-nil args + def self.sparse_map(args) func_args = args.compact - values = yield *func_args + values = yield func_args # Hash{arg => value} value_map = Hash[func_args.zip(values)] From 0b771dee3d49bbd24c3a01920f9b3c62dadf7648 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 15:16:04 +0300 Subject: [PATCH 13/16] compromise on compact_map, because sparse isn't ruby terminology --- lib/k8s/client.rb | 2 +- lib/k8s/util.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/k8s/client.rb b/lib/k8s/client.rb index 48a59f2..36da7ee 100644 --- a/lib/k8s/client.rb +++ b/lib/k8s/client.rb @@ -142,7 +142,7 @@ def get_resources(resources) } # map non-nil requests to response objects, or nil for nil request options - Util.sparse_map(requests) { |requests| + Util.compact_map(requests) { |requests| @transport.requests(*requests, skip_missing: true) } end diff --git a/lib/k8s/util.rb b/lib/k8s/util.rb index 545eae9..24416d5 100644 --- a/lib/k8s/util.rb +++ b/lib/k8s/util.rb @@ -7,7 +7,7 @@ module Util # @yield args # @yieldparam args [Array] omitting any nil values # @return [Array] matching args array 1:1, containing yielded values for non-nil args - def self.sparse_map(args) + def self.compact_map(args) func_args = args.compact values = yield func_args From affa8295945f22cee79aba3f8f26ddd11a4a85ca Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 15:20:31 +0300 Subject: [PATCH 14/16] trivial specs for compact_map --- spec/pharos/kube/util_spec.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 spec/pharos/kube/util_spec.rb diff --git a/spec/pharos/kube/util_spec.rb b/spec/pharos/kube/util_spec.rb new file mode 100644 index 0000000..d67a9fd --- /dev/null +++ b/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 From 6d2f7aeb0dba25ed64a693743844d7c7c534085a Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 15:24:38 +0300 Subject: [PATCH 15/16] add note on duplicate args --- lib/k8s/util.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/k8s/util.rb b/lib/k8s/util.rb index 24416d5..89529d7 100644 --- a/lib/k8s/util.rb +++ b/lib/k8s/util.rb @@ -1,7 +1,8 @@ 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. + # + # Args must be usable as hash keys. Duplicate args will all map to the same return value. # # @param args [Array] # @yield args From c35c448bf4744a3b6da9639ac98038a487502bfc Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 13 Aug 2018 15:28:53 +0300 Subject: [PATCH 16/16] abbreviate block arg to not shadow outer local --- lib/k8s/client.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/k8s/client.rb b/lib/k8s/client.rb index 36da7ee..8e1059a 100644 --- a/lib/k8s/client.rb +++ b/lib/k8s/client.rb @@ -142,8 +142,8 @@ def get_resources(resources) } # map non-nil requests to response objects, or nil for nil request options - Util.compact_map(requests) { |requests| - @transport.requests(*requests, skip_missing: true) + Util.compact_map(requests) { |reqs| + @transport.requests(*reqs, skip_missing: true) } end