From 6b65124a25d2bdb6b91aa0037c141e4d7c3068b9 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Tue, 12 Oct 2021 19:28:01 -0400 Subject: [PATCH 1/4] only use context when specified --- lib/ood_core/job/adapters/kubernetes/batch.rb | 34 +++++++----------- spec/job/adapters/kubernetes/batch_spec.rb | 36 ------------------- 2 files changed, 12 insertions(+), 58 deletions(-) diff --git a/lib/ood_core/job/adapters/kubernetes/batch.rb b/lib/ood_core/job/adapters/kubernetes/batch.rb index 597ea5a36..9d467a5bd 100644 --- a/lib/ood_core/job/adapters/kubernetes/batch.rb +++ b/lib/ood_core/job/adapters/kubernetes/batch.rb @@ -11,8 +11,8 @@ class OodCore::Job::Adapters::Kubernetes::Batch class Error < StandardError; end class NotFoundError < StandardError; end - attr_reader :config_file, :bin, :cluster, :mounts - attr_reader :all_namespaces, :using_context, :helper + attr_reader :config_file, :bin, :cluster, :conset-clustertext, :mounts + attr_reader :all_namespaces, :helper attr_reader :username_prefix, :namespace_prefix attr_reader :auto_supplemental_groups @@ -22,21 +22,14 @@ def initialize(options = {}) @config_file = options.fetch(:config_file, default_config_file) @bin = options.fetch(:bin, '/usr/bin/kubectl') @cluster = options.fetch(:cluster, 'open-ondemand') + @context = options.fetch(:context, nil) @mounts = options.fetch(:mounts, []).map { |m| m.to_h.symbolize_keys } @all_namespaces = options.fetch(:all_namespaces, false) @username_prefix = options.fetch(:username_prefix, '') @namespace_prefix = options.fetch(:namespace_prefix, '') @auto_supplemental_groups = options.fetch(:auto_supplemental_groups, false) - @using_context = false @helper = OodCore::Job::Adapters::Kubernetes::Helper.new - - begin - make_kubectl_config(options) - rescue - # FIXME could use a log here - # means you couldn't 'kubectl set config' - end end def resource_file(resource_type = 'pod') @@ -250,10 +243,6 @@ def namespace "#{namespace_prefix}#{username}" end - def context - cluster - end - def default_config_file (ENV['KUBECONFIG'] || "#{Dir.home}/.kube/config") end @@ -281,7 +270,7 @@ def namespaced_cmd def base_cmd base = "#{bin} --kubeconfig=#{config_file}" - base << " --context=#{context}" if using_context + base << " --context=#{context}" if context? base end @@ -309,9 +298,10 @@ def pod_info_from_json(pod) nil end - def make_kubectl_config(config) - set_cluster(config.fetch(:server, default_server).to_h.symbolize_keys) - configure_auth(config.fetch(:auth, default_auth).to_h.symbolize_keys) + def self.initialize!(config) + k = self.new(config) + k.set_cluster(config.fetch(:server, default_server).to_h.symbolize_keys) + k.configure_auth(config.fetch(:auth, default_auth).to_h.symbolize_keys) end def configure_auth(auth) @@ -326,8 +316,8 @@ def configure_auth(auth) end end - def use_context - @using_context = true + def context? + !@context.nil? end def managed?(type) @@ -363,12 +353,12 @@ def set_gke_credentials(auth) end def set_context - cmd = "#{base_cmd} config set-context #{cluster}" + # can't really use base_cmd, bc it may use --context flag + cmd = "#{bin} --kubeconfig=#{config_file} config set-context #{cluster}" cmd << " --cluster=#{cluster} --namespace=#{namespace}" cmd << " --user=#{k8s_username}" call(cmd) - use_context end def set_cluster(config) diff --git a/spec/job/adapters/kubernetes/batch_spec.rb b/spec/job/adapters/kubernetes/batch_spec.rb index 10574aec2..24e25ac79 100644 --- a/spec/job/adapters/kubernetes/batch_spec.rb +++ b/spec/job/adapters/kubernetes/batch_spec.rb @@ -87,27 +87,12 @@ } before(:each) do - allow(Open3).to receive(:capture3).with( - {}, - "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config " \ - "config set-cluster open-ondemand --server=https://localhost:8080", - stdin_data: "" - ).and_return(['', '', success]) - @basic_batch = described_class.new({}) allow(@basic_batch).to receive(:username).and_return('testuser') allow(@basic_batch).to receive(:helper).and_return(helper) end let(:configured_batch){ - allow(Open3).to receive(:capture3).with( - {}, - "/usr/bin/wontwork --kubeconfig=~/kube.config " \ - "config set-cluster test-cluster --server=https://some.k8s.host " \ - "--certificate-authority=/etc/some.cert", - stdin_data: "" - ).and_return(['', '', success]) - batch = described_class.new(config) allow(batch).to receive(:username).and_return('testuser') allow(batch).to receive(:helper).and_return(helper) @@ -942,13 +927,6 @@ def build_script(opts = {}) describe "#info" do def info_batch(id, file) - allow(Open3).to receive(:capture3).with( - {}, - "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config " \ - "config set-cluster open-ondemand --server=https://localhost:8080", - stdin_data: "" - ).and_return(['', '', success]) - batch = described_class.new({}) allow(batch).to receive(:username).and_return('testuser') allow(batch).to receive(:helper).and_return(helper) @@ -978,13 +956,6 @@ def info_batch(id, file) end def info_batch_full(id) - allow(Open3).to receive(:capture3).with( - {}, - "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config " \ - "config set-cluster open-ondemand --server=https://localhost:8080", - stdin_data: "" - ).and_return(['', '', success]) - batch = described_class.new({}) allow(batch).to receive(:username).and_return('testuser') allow(DateTime).to receive(:now).and_return(past) @@ -1012,13 +983,6 @@ def info_batch_full(id) end def not_found_batch(id) - allow(Open3).to receive(:capture3).with( - {}, - "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config " \ - "config set-cluster open-ondemand --server=https://localhost:8080", - stdin_data: "" - ).and_return(['', '', success]) - batch = described_class.new({}) allow(batch).to receive(:username).and_return('testuser') allow(batch).to receive(:helper).and_return(helper) From 0b2fb1c193c75123e88112e6cee33561068e473c Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Tue, 12 Oct 2021 19:55:43 -0400 Subject: [PATCH 2/4] static stuff actually works --- lib/ood_core/job/adapters/kubernetes/batch.rb | 53 ++++++++++--------- spec/job/adapters/kubernetes/batch_spec.rb | 20 ++++--- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/lib/ood_core/job/adapters/kubernetes/batch.rb b/lib/ood_core/job/adapters/kubernetes/batch.rb index 9d467a5bd..eaf6055db 100644 --- a/lib/ood_core/job/adapters/kubernetes/batch.rb +++ b/lib/ood_core/job/adapters/kubernetes/batch.rb @@ -11,7 +11,7 @@ class OodCore::Job::Adapters::Kubernetes::Batch class Error < StandardError; end class NotFoundError < StandardError; end - attr_reader :config_file, :bin, :cluster, :conset-clustertext, :mounts + attr_reader :config_file, :bin, :cluster, :context, :mounts attr_reader :all_namespaces, :helper attr_reader :username_prefix, :namespace_prefix attr_reader :auto_supplemental_groups @@ -19,7 +19,7 @@ class NotFoundError < StandardError; end def initialize(options = {}) options = options.to_h.symbolize_keys - @config_file = options.fetch(:config_file, default_config_file) + @config_file = options.fetch(:config_file, Batch.default_config_file) @bin = options.fetch(:bin, '/usr/bin/kubectl') @cluster = options.fetch(:cluster, 'open-ondemand') @context = options.fetch(:context, nil) @@ -110,6 +110,32 @@ def delete(id) safe_call("delete", "configmap", configmap_name(id)) end + class << self + def default_config_file + (ENV['KUBECONFIG'] || "#{Dir.home}/.kube/config") + end + + def default_auth + { + type: 'managaged' + }.symbolize_keys + end + + def default_server + { + endpoint: 'https://localhost:8080', + cert_authority_file: nil + }.symbolize_keys + end + + def configure_kube!(config) + k = self.new(config) + # TODO: probably shouldn't be using send here + k.send(:set_cluster, config.fetch(:server, default_server).to_h.symbolize_keys) + k.send(:configure_auth, config.fetch(:auth, default_auth).to_h.symbolize_keys) + end + end + private def safe_call(verb, resource, id) @@ -243,23 +269,6 @@ def namespace "#{namespace_prefix}#{username}" end - def default_config_file - (ENV['KUBECONFIG'] || "#{Dir.home}/.kube/config") - end - - def default_auth - { - type: 'managaged' - }.symbolize_keys - end - - def default_server - { - endpoint: 'https://localhost:8080', - cert_authority_file: nil - }.symbolize_keys - end - def formatted_ns_cmd "#{namespaced_cmd} -o json" end @@ -298,12 +307,6 @@ def pod_info_from_json(pod) nil end - def self.initialize!(config) - k = self.new(config) - k.set_cluster(config.fetch(:server, default_server).to_h.symbolize_keys) - k.configure_auth(config.fetch(:auth, default_auth).to_h.symbolize_keys) - end - def configure_auth(auth) type = auth.fetch(:type) return if managed?(type) diff --git a/spec/job/adapters/kubernetes/batch_spec.rb b/spec/job/adapters/kubernetes/batch_spec.rb index 24e25ac79..c65e86b5c 100644 --- a/spec/job/adapters/kubernetes/batch_spec.rb +++ b/spec/job/adapters/kubernetes/batch_spec.rb @@ -74,6 +74,7 @@ config_file: '~/kube.config', bin: '/usr/bin/wontwork', cluster: 'test-cluster', + context: 'ood-test-cluster', mounts: mounts, all_namespaces: true, namespace_prefix: 'user-', @@ -248,6 +249,7 @@ expect(@basic_batch.config_file).to eq("#{ENV['HOME']}/.kube/config") expect(@basic_batch.bin).to eq('/usr/bin/kubectl') expect(@basic_batch.mounts).to eq([]) + expect(@basic_batch.context).to eq(nil) end it "does not throw an error when it can't configure" do @@ -337,7 +339,7 @@ def build_script(opts = {}) # make sure template get's passed into command correctly allow(Open3).to receive(:capture3).with( {}, - "/usr/bin/wontwork --kubeconfig=~/kube.config " \ + "/usr/bin/wontwork --kubeconfig=~/kube.config --context=ood-test-cluster " \ "--namespace=user-testuser -o json create -f -", stdin_data: pod_yml_from_all_configs.to_s ).and_return(['', '', success]) @@ -904,7 +906,7 @@ def build_script(opts = {}) it "throws error up the stack with --all-namespaces" do allow(Open3).to receive(:capture3).with( {}, - "/usr/bin/wontwork --kubeconfig=~/kube.config get pods -o json --all-namespaces", + "/usr/bin/wontwork --kubeconfig=~/kube.config --context=ood-test-cluster get pods -o json --all-namespaces", stdin_data: "" ).and_return(['', errmsg, failure]) @@ -1061,11 +1063,11 @@ def not_found_batch(id) end end - describe '#set_context' do + describe '#configure_kube!' do it 'generates correct command' do - expected_cmd = "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config config set-context open-ondemand --cluster=open-ondemand --namespace=testuser --user=testuser" - expect(@basic_batch).to receive(:call).with(expected_cmd) - @basic_batch.send(:set_context) + expected_cmd = "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config config set-cluster open-ondemand --server=https://localhost:8080" + expect_any_instance_of(Batch).to receive(:call).with(expected_cmd) + Batch.configure_kube!({}) end it 'generates correct command when username prefix defined' do @@ -1074,5 +1076,11 @@ def not_found_batch(id) expect(@basic_batch).to receive(:call).with(expected_cmd) @basic_batch.send(:set_context) end + + it 'generates correct command' do + expected_cmd = "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config config set-context open-ondemand --cluster=open-ondemand --namespace=testuser --user=testuser" + expect(@basic_batch).to receive(:call).with(expected_cmd) + @basic_batch.send(:set_context) + end end end From c01a75ece41843dd0549800407a7c225cbf50c0e Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Wed, 13 Oct 2021 09:28:29 -0400 Subject: [PATCH 3/4] could bugs and some tests --- lib/ood_core/job/adapters/kubernetes/batch.rb | 7 +-- spec/job/adapters/kubernetes/batch_spec.rb | 52 +++++++++++++++---- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/lib/ood_core/job/adapters/kubernetes/batch.rb b/lib/ood_core/job/adapters/kubernetes/batch.rb index eaf6055db..d8ceba1f2 100644 --- a/lib/ood_core/job/adapters/kubernetes/batch.rb +++ b/lib/ood_core/job/adapters/kubernetes/batch.rb @@ -117,7 +117,7 @@ def default_config_file def default_auth { - type: 'managaged' + type: 'managed' }.symbolize_keys end @@ -357,7 +357,7 @@ def set_gke_credentials(auth) def set_context # can't really use base_cmd, bc it may use --context flag - cmd = "#{bin} --kubeconfig=#{config_file} config set-context #{cluster}" + cmd = "#{bin} --kubeconfig=#{config_file} config set-context #{context}" cmd << " --cluster=#{cluster} --namespace=#{namespace}" cmd << " --user=#{k8s_username}" @@ -368,7 +368,8 @@ def set_cluster(config) server = config.fetch(:endpoint) cert = config.fetch(:cert_authority_file, nil) - cmd = "#{base_cmd} config set-cluster #{cluster}" + # shouldn't use context here either + cmd = "#{bin} --kubeconfig=#{config_file} config set-cluster #{cluster}" cmd << " --server=#{server}" cmd << " --certificate-authority=#{cert}" unless cert.nil? diff --git a/spec/job/adapters/kubernetes/batch_spec.rb b/spec/job/adapters/kubernetes/batch_spec.rb index c65e86b5c..ebadaa741 100644 --- a/spec/job/adapters/kubernetes/batch_spec.rb +++ b/spec/job/adapters/kubernetes/batch_spec.rb @@ -78,11 +78,14 @@ mounts: mounts, all_namespaces: true, namespace_prefix: 'user-', - username_prefix: 'dev', + username_prefix: 'dev-', server: { endpoint: 'https://some.k8s.host', cert_authority_file: '/etc/some.cert' }, + auth: { + type: 'oidc' + }, auto_supplemental_groups: true } } @@ -1064,23 +1067,50 @@ def not_found_batch(id) end describe '#configure_kube!' do - it 'generates correct command' do + it 'generates correct default commands' do expected_cmd = "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config config set-cluster open-ondemand --server=https://localhost:8080" expect_any_instance_of(Batch).to receive(:call).with(expected_cmd) Batch.configure_kube!({}) end - it 'generates correct command when username prefix defined' do - allow(@basic_batch).to receive(:username_prefix).and_return('dev-') - expected_cmd = "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config config set-context open-ondemand --cluster=open-ondemand --namespace=testuser --user=dev-testuser" - expect(@basic_batch).to receive(:call).with(expected_cmd) - @basic_batch.send(:set_context) + it 'generates correct default auth commands' do + expected_cmd = "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config config set-cluster open-ondemand --server=https://localhost:8080" + expect_any_instance_of(Batch).to receive(:call).with(expected_cmd) + Batch.configure_kube!({ :auth => Batch.default_auth }) + end + + it 'generates sets oidc context' do + set_cluster = "/usr/bin/wontwork --kubeconfig=~/kube.config config set-cluster test-cluster " \ + "--server=https://some.k8s.host --certificate-authority=/etc/some.cert" + set_context = "/usr/bin/wontwork --kubeconfig=~/kube.config config set-context ood-test-cluster " \ + "--cluster=test-cluster --namespace=user-jessie --user=dev-jessie" + + expect_any_instance_of(Batch).to receive(:call).with(set_cluster) + expect_any_instance_of(Batch).to receive(:username).and_return('jessie') + expect_any_instance_of(Batch).to receive(:username).and_return('jessie') + expect_any_instance_of(Batch).to receive(:call).with(set_context) + Batch.configure_kube!(config) end - it 'generates correct command' do - expected_cmd = "/usr/bin/kubectl --kubeconfig=#{ENV['HOME']}/.kube/config config set-context open-ondemand --cluster=open-ondemand --namespace=testuser --user=testuser" - expect(@basic_batch).to receive(:call).with(expected_cmd) - @basic_batch.send(:set_context) + it 'runs the correct gke commans' do + cfg = { + :auth => { + :type => 'gke', + :region => 'ohio', + :svc_acct_file => '~/.gke/acct.key', + }, + :cluster => 'gke-cluster-oakley', + :config_file => '~/.gke/oakley.config', + } + + set_cluster = "/usr/bin/kubectl --kubeconfig=~/.gke/oakley.config config set-cluster gke-cluster-oakley " \ + "--server=https://localhost:8080" + gcloud = "gcloud auth activate-service-account --key-file=~/.gke/acct.key" + # env = {:env } + + # expect_any_instance_of(Batch).to receive(:call).with(set_cluster) + # expect_any_instance_of(Batch).to receive(:call).with(gcloud, { 'KUBECONFIG' => '~/.gke/oakley.config' }) + # Batch.configure_kube!(cfg) end end end From 2abc321fec5efe89e9de9dc8a1c53bbcb9d69371 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Wed, 13 Oct 2021 09:49:20 -0400 Subject: [PATCH 4/4] fix gke tests --- lib/ood_core/job/adapters/kubernetes/batch.rb | 2 +- spec/job/adapters/kubernetes/batch_spec.rb | 38 ++++++++++++++++--- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/lib/ood_core/job/adapters/kubernetes/batch.rb b/lib/ood_core/job/adapters/kubernetes/batch.rb index d8ceba1f2..f5d45ea0e 100644 --- a/lib/ood_core/job/adapters/kubernetes/batch.rb +++ b/lib/ood_core/job/adapters/kubernetes/batch.rb @@ -352,7 +352,7 @@ def set_gke_credentials(auth) # gke cluster name can probably can differ from what ood calls the cluster cmd = "gcloud container clusters get-credentials #{locale} #{cluster}" env = { 'KUBECONFIG' => config_file } - call(cmd, env) + call(cmd, env: env) end def set_context diff --git a/spec/job/adapters/kubernetes/batch_spec.rb b/spec/job/adapters/kubernetes/batch_spec.rb index ebadaa741..168c6b1a5 100644 --- a/spec/job/adapters/kubernetes/batch_spec.rb +++ b/spec/job/adapters/kubernetes/batch_spec.rb @@ -1092,7 +1092,7 @@ def not_found_batch(id) Batch.configure_kube!(config) end - it 'runs the correct gke commans' do + it 'runs the correct gke commands' do cfg = { :auth => { :type => 'gke', @@ -1105,12 +1105,38 @@ def not_found_batch(id) set_cluster = "/usr/bin/kubectl --kubeconfig=~/.gke/oakley.config config set-cluster gke-cluster-oakley " \ "--server=https://localhost:8080" - gcloud = "gcloud auth activate-service-account --key-file=~/.gke/acct.key" - # env = {:env } + auth_activate = "gcloud auth activate-service-account --key-file=~/.gke/acct.key" + ctr_cluster = "gcloud container clusters get-credentials --region=ohio gke-cluster-oakley" + env = env = { 'KUBECONFIG' => '~/.gke/oakley.config' } - # expect_any_instance_of(Batch).to receive(:call).with(set_cluster) - # expect_any_instance_of(Batch).to receive(:call).with(gcloud, { 'KUBECONFIG' => '~/.gke/oakley.config' }) - # Batch.configure_kube!(cfg) + expect_any_instance_of(Batch).to receive(:call).with(set_cluster) + expect_any_instance_of(Batch).to receive(:call).with(auth_activate) + expect_any_instance_of(Batch).to receive(:call).with(ctr_cluster, env: env) + Batch.configure_kube!(cfg) + end + + # same as the test above, only using --zone instead of --region + it 'zone works in gke' do + cfg = { + :auth => { + :type => 'gke', + :zone => 'ohio', + :svc_acct_file => '~/.gke/acct.key', + }, + :cluster => 'gke-cluster-oakley', + :config_file => '~/.gke/oakley.config', + } + + set_cluster = "/usr/bin/kubectl --kubeconfig=~/.gke/oakley.config config set-cluster gke-cluster-oakley " \ + "--server=https://localhost:8080" + auth_activate = "gcloud auth activate-service-account --key-file=~/.gke/acct.key" + ctr_cluster = "gcloud container clusters get-credentials --zone=ohio gke-cluster-oakley" + env = env = { 'KUBECONFIG' => '~/.gke/oakley.config' } + + expect_any_instance_of(Batch).to receive(:call).with(set_cluster) + expect_any_instance_of(Batch).to receive(:call).with(auth_activate) + expect_any_instance_of(Batch).to receive(:call).with(ctr_cluster, env: env) + Batch.configure_kube!(cfg) end end end