diff options
author | Dylan Griffith <dyl.griffith@gmail.com> | 2018-07-23 16:35:19 +0200 |
---|---|---|
committer | Dylan Griffith <dyl.griffith@gmail.com> | 2018-07-28 09:50:31 +0000 |
commit | ce897f11a0650b0d6938cb506a030ef00160ab7a (patch) | |
tree | 05544fc0fb23c1c7066d832a6eb024d0e82b4999 | |
parent | 87f03f01735fb4b6dbef2e4bf625cf2546523a4e (diff) | |
download | gitlab-ce-ce897f11a0650b0d6938cb506a030ef00160ab7a.tar.gz |
Refactor Cluster Application classes to pass through a has of config files
This is refactoring in the lead up to passing mutual TLS certs for helm applications. As such we expect all applications to need config files so we can remove the logic about which applications need and do not need this (ie `#config_map?`).
22 files changed, 122 insertions, 128 deletions
diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index 58de3448577..06d85a69b29 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -15,7 +15,10 @@ module Clusters end def install_command - Gitlab::Kubernetes::Helm::InitCommand.new(name) + Gitlab::Kubernetes::Helm::InitCommand.new( + name: name, + files: {} + ) end end end diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index 27fc3b85465..64810812531 100644 --- a/app/models/clusters/applications/ingress.rb +++ b/app/models/clusters/applications/ingress.rb @@ -32,9 +32,9 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name, + name: name, chart: chart, - values: values + files: files ) end diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb index 975d434e1a4..cff5a423acb 100644 --- a/app/models/clusters/applications/jupyter.rb +++ b/app/models/clusters/applications/jupyter.rb @@ -35,9 +35,9 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name, + name: name, chart: chart, - values: values, + files: files, repository: repository ) end diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index ea6ec4d6b03..22815cc1219 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -43,10 +43,10 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name, + name: name, chart: chart, version: version, - values: values + files: files ) end diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index e6f795f3e0b..4c894b5376d 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -28,9 +28,9 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name, + name: name, chart: chart, - values: values, + files: files, repository: repository ) end diff --git a/app/models/clusters/concerns/application_data.rb b/app/models/clusters/concerns/application_data.rb index 96ac757e99e..215a299dd03 100644 --- a/app/models/clusters/concerns/application_data.rb +++ b/app/models/clusters/concerns/application_data.rb @@ -12,6 +12,12 @@ module Clusters File.read(chart_values_file) end + def files + { + 'values.yaml': values + } + end + private def chart_values_file diff --git a/lib/gitlab/kubernetes/config_map.rb b/lib/gitlab/kubernetes/config_map.rb index 8a8a59a9cd4..9e55dae137c 100644 --- a/lib/gitlab/kubernetes/config_map.rb +++ b/lib/gitlab/kubernetes/config_map.rb @@ -1,15 +1,15 @@ module Gitlab module Kubernetes class ConfigMap - def initialize(name, values = "") + def initialize(name, files) @name = name - @values = values + @files = files end def generate resource = ::Kubeclient::Resource.new resource.metadata = metadata - resource.data = { values: values } + resource.data = files resource end @@ -19,7 +19,7 @@ module Gitlab private - attr_reader :name, :values + attr_reader :name, :files def metadata { diff --git a/lib/gitlab/kubernetes/helm/api.rb b/lib/gitlab/kubernetes/helm/api.rb index c4de9a398cc..d65374cc23b 100644 --- a/lib/gitlab/kubernetes/helm/api.rb +++ b/lib/gitlab/kubernetes/helm/api.rb @@ -9,7 +9,7 @@ module Gitlab def install(command) namespace.ensure_exists! - create_config_map(command) if command.config_map? + create_config_map(command) kubeclient.create_pod(command.pod_resource) end diff --git a/lib/gitlab/kubernetes/helm/base_command.rb b/lib/gitlab/kubernetes/helm/base_command.rb index f9ebe53d6af..8decddcd92f 100644 --- a/lib/gitlab/kubernetes/helm/base_command.rb +++ b/lib/gitlab/kubernetes/helm/base_command.rb @@ -1,13 +1,7 @@ module Gitlab module Kubernetes module Helm - class BaseCommand - attr_reader :name - - def initialize(name) - @name = name - end - + module BaseCommand def pod_resource Gitlab::Kubernetes::Helm::Pod.new(self, namespace).generate end @@ -24,14 +18,22 @@ module Gitlab HEREDOC end - def config_map? - false - end - def pod_name "install-#{name}" end + def config_map_resource + Gitlab::Kubernetes::ConfigMap.new(name, files).generate + end + + def name + raise "Not implemented" + end + + def files + raise "Not implemented" + end + private def namespace diff --git a/lib/gitlab/kubernetes/helm/init_command.rb b/lib/gitlab/kubernetes/helm/init_command.rb index a02e64561f6..2f2b3e930ae 100644 --- a/lib/gitlab/kubernetes/helm/init_command.rb +++ b/lib/gitlab/kubernetes/helm/init_command.rb @@ -1,7 +1,16 @@ module Gitlab module Kubernetes module Helm - class InitCommand < BaseCommand + class InitCommand + include BaseCommand + + attr_reader :name, :files + + def initialize(name:, files:) + @name = name + @files = files + end + def generate_script super + [ init_helm_command diff --git a/lib/gitlab/kubernetes/helm/install_command.rb b/lib/gitlab/kubernetes/helm/install_command.rb index d2133a6d65b..5008724f17d 100644 --- a/lib/gitlab/kubernetes/helm/install_command.rb +++ b/lib/gitlab/kubernetes/helm/install_command.rb @@ -1,14 +1,17 @@ module Gitlab module Kubernetes module Helm - class InstallCommand < BaseCommand - attr_reader :name, :chart, :version, :repository, :values + class InstallCommand + include BaseCommand - def initialize(name, chart:, values:, version: nil, repository: nil) + attr_reader :name, :files + attr_reader :chart, :version, :repository + + def initialize(name:, chart:, files:, version: nil, repository: nil) @name = name @chart = chart @version = version - @values = values + @files = files @repository = repository end @@ -20,14 +23,6 @@ module Gitlab ].compact.join("\n") end - def config_map? - true - end - - def config_map_resource - Gitlab::Kubernetes::ConfigMap.new(name, values).generate - end - private def init_command diff --git a/lib/gitlab/kubernetes/helm/pod.rb b/lib/gitlab/kubernetes/helm/pod.rb index 1e12299eefd..862664ff543 100644 --- a/lib/gitlab/kubernetes/helm/pod.rb +++ b/lib/gitlab/kubernetes/helm/pod.rb @@ -10,10 +10,8 @@ module Gitlab def generate spec = { containers: [container_specification], restartPolicy: 'Never' } - if command.config_map? - spec[:volumes] = volumes_specification - spec[:containers][0][:volumeMounts] = volume_mounts_specification - end + spec[:volumes] = volumes_specification + spec[:containers][0][:volumeMounts] = volume_mounts_specification ::Kubeclient::Resource.new(metadata: metadata, spec: spec) end @@ -61,7 +59,7 @@ module Gitlab name: 'configuration-volume', configMap: { name: "values-content-configuration-#{command.name}", - items: [{ key: 'values', path: 'values.yaml' }] + items: command.files.map { |name, _| { key: name, path: name } } } } ] diff --git a/spec/lib/gitlab/kubernetes/config_map_spec.rb b/spec/lib/gitlab/kubernetes/config_map_spec.rb index e253b291277..fe65d03875f 100644 --- a/spec/lib/gitlab/kubernetes/config_map_spec.rb +++ b/spec/lib/gitlab/kubernetes/config_map_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::Kubernetes::ConfigMap do let(:kubeclient) { double('kubernetes client') } let(:application) { create(:clusters_applications_prometheus) } - let(:config_map) { described_class.new(application.name, application.values) } + let(:config_map) { described_class.new(application.name, application.files) } let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } let(:metadata) do @@ -15,7 +15,7 @@ describe Gitlab::Kubernetes::ConfigMap do end describe '#generate' do - let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: { values: application.values }) } + let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: application.files) } subject { config_map.generate } it 'should build a Kubeclient Resource' do diff --git a/spec/lib/gitlab/kubernetes/helm/api_spec.rb b/spec/lib/gitlab/kubernetes/helm/api_spec.rb index 6e9b4ca0869..341f71a3e49 100644 --- a/spec/lib/gitlab/kubernetes/helm/api_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/api_spec.rb @@ -39,7 +39,7 @@ describe Gitlab::Kubernetes::Helm::Api do end context 'with a ConfigMap' do - let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application.name, application.values).generate } + let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application.name, application.files).generate } it 'creates a ConfigMap on kubeclient' do expect(client).to receive(:create_config_map).with(resource).once diff --git a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb index 7be8be54d5e..3dcb48f4869 100644 --- a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb @@ -1,9 +1,21 @@ require 'spec_helper' +class TestClass + include Gitlab::Kubernetes::Helm::BaseCommand + def name + "test-class-name" + end + + def files + { + some: 'value' + } + end +end + describe Gitlab::Kubernetes::Helm::BaseCommand do let(:application) { create(:clusters_applications_helm) } - let(:base_command) { described_class.new(application.name) } - + let(:base_command) { TestClass.new } subject { base_command } it_behaves_like 'helm commands' do @@ -18,15 +30,9 @@ describe Gitlab::Kubernetes::Helm::BaseCommand do end end - describe '#config_map?' do - subject { base_command.config_map? } - - it { is_expected.to be_falsy } - end - describe '#pod_name' do subject { base_command.pod_name } - it { is_expected.to eq('install-helm') } + it { is_expected.to eq('install-test-class-name') } end end diff --git a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb index 89e36a298f8..7550e23259b 100644 --- a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::Kubernetes::Helm::InitCommand do let(:application) { create(:clusters_applications_helm) } let(:commands) { 'helm init >/dev/null' } - subject { described_class.new(application.name) } + subject { described_class.new(name: application.name, files: {}) } it_behaves_like 'helm commands' end diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb index 25c6fa3b9a3..1e8407c8dc3 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -62,12 +62,6 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do end end - describe '#config_map?' do - subject { install_command.config_map? } - - it { is_expected.to be_truthy } - end - describe '#config_map_resource' do let(:metadata) do { @@ -77,7 +71,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do } end - let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: { values: application.values }) } + let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: application.files) } subject { install_command.config_map_resource } diff --git a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb index 43adc80d576..c25978e96da 100644 --- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::Kubernetes::Helm::Pod do subject { described_class.new(command, namespace) } - shared_examples 'helm pod' do + context 'with a command' do it 'should generate a Kubeclient::Resource' do expect(subject.generate).to be_a_kind_of(Kubeclient::Resource) end @@ -41,10 +41,6 @@ describe Gitlab::Kubernetes::Helm::Pod do spec = subject.generate.spec expect(spec.restartPolicy).to eq('Never') end - end - - context 'with a install command' do - it_behaves_like 'helm pod' it 'should include volumes for the container' do container = subject.generate.spec.containers.first @@ -60,24 +56,8 @@ describe Gitlab::Kubernetes::Helm::Pod do it 'should mount configMap specification in the volume' do volume = subject.generate.spec.volumes.first expect(volume.configMap['name']).to eq("values-content-configuration-#{app.name}") - expect(volume.configMap['items'].first['key']).to eq('values') - expect(volume.configMap['items'].first['path']).to eq('values.yaml') - end - end - - context 'with a init command' do - let(:app) { create(:clusters_applications_helm, cluster: cluster) } - - it_behaves_like 'helm pod' - - it 'should not include volumeMounts inside the container' do - container = subject.generate.spec.containers.first - expect(container.volumeMounts).to be_nil - end - - it 'should not a volume inside the specification' do - spec = subject.generate.spec - expect(spec.volumes).to be_nil + expect(volume.configMap['items'].first['key']).to eq(:'values.yaml') + expect(volume.configMap['items'].first['path']).to eq(:'values.yaml') end end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index bb5b2ef3a47..fbb3c18319f 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -74,18 +74,18 @@ describe Clusters::Applications::Ingress do expect(subject.name).to eq('ingress') expect(subject.chart).to eq('stable/nginx-ingress') expect(subject.version).to be_nil - expect(subject.values).to eq(ingress.values) + expect(subject.files).to eq(ingress.files) end end - describe '#values' do - subject { ingress.values } + describe '#files' do + let(:values) { ingress.files[:'values.yaml'] } - it 'should include ingress valid keys' do - is_expected.to include('image') - is_expected.to include('repository') - is_expected.to include('stats') - is_expected.to include('podAnnotations') + it 'should include ingress valid keys in values' do + expect(values).to include('image') + expect(values).to include('repository') + expect(values).to include('stats') + expect(values).to include('podAnnotations') end end end diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index 65750141e65..0e2847592fc 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -38,23 +38,23 @@ describe Clusters::Applications::Jupyter do expect(subject.chart).to eq('jupyter/jupyterhub') expect(subject.version).to be_nil expect(subject.repository).to eq('https://jupyterhub.github.io/helm-chart/') - expect(subject.values).to eq(jupyter.values) + expect(subject.files).to eq(jupyter.files) end end - describe '#values' do + describe '#files' do let(:jupyter) { create(:clusters_applications_jupyter) } - subject { jupyter.values } + let(:values) { jupyter.files[:'values.yaml'] } it 'should include valid values' do - is_expected.to include('ingress') - is_expected.to include('hub') - is_expected.to include('rbac') - is_expected.to include('proxy') - is_expected.to include('auth') - is_expected.to include("clientId: #{jupyter.oauth_application.uid}") - is_expected.to include("callbackUrl: #{jupyter.callback_url}") + expect(values).to include('ingress') + expect(values).to include('hub') + expect(values).to include('rbac') + expect(values).to include('proxy') + expect(values).to include('auth') + expect(values).to match(/clientId: '?#{jupyter.oauth_application.uid}/) + expect(values).to match(/callbackUrl: '?#{jupyter.callback_url}/) end end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index e4b61552033..013cb8da22b 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -153,21 +153,21 @@ describe Clusters::Applications::Prometheus do expect(command.name).to eq('prometheus') expect(command.chart).to eq('stable/prometheus') expect(command.version).to eq('6.7.3') - expect(command.values).to eq(prometheus.values) + expect(command.files).to eq(prometheus.files) end end - describe '#values' do + describe '#files' do let(:prometheus) { create(:clusters_applications_prometheus) } - subject { prometheus.values } + let(:values) { prometheus.files[:'values.yaml'] } it 'should include prometheus valid values' do - is_expected.to include('alertmanager') - is_expected.to include('kubeStateMetrics') - is_expected.to include('nodeExporter') - is_expected.to include('pushgateway') - is_expected.to include('serverFiles') + expect(values).to include('alertmanager') + expect(values).to include('kubeStateMetrics') + expect(values).to include('nodeExporter') + expect(values).to include('pushgateway') + expect(values).to include('serverFiles') end end end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index b12500d0acd..4ac136a6274 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -33,25 +33,26 @@ describe Clusters::Applications::Runner do expect(subject.chart).to eq('runner/gitlab-runner') expect(subject.version).to be_nil expect(subject.repository).to eq('https://charts.gitlab.io') - expect(subject.values).to eq(gitlab_runner.values) + expect(subject.files).to eq(gitlab_runner.files) end end - describe '#values' do + describe '#files' do let(:gitlab_runner) { create(:clusters_applications_runner, runner: ci_runner) } - subject { gitlab_runner.values } + subject { gitlab_runner.files } + let(:values) { subject[:'values.yaml'] } it 'should include runner valid values' do - is_expected.to include('concurrent') - is_expected.to include('checkInterval') - is_expected.to include('rbac') - is_expected.to include('runners') - is_expected.to include('privileged: true') - is_expected.to include('image: ubuntu:16.04') - is_expected.to include('resources') - is_expected.to include("runnerToken: #{ci_runner.token}") - is_expected.to include("gitlabUrl: #{Gitlab::Routing.url_helpers.root_url}") + expect(values).to include('concurrent') + expect(values).to include('checkInterval') + expect(values).to include('rbac') + expect(values).to include('runners') + expect(values).to include('privileged: true') + expect(values).to include('image: ubuntu:16.04') + expect(values).to include('resources') + expect(values).to match(/runnerToken: '?#{ci_runner.token}/) + expect(values).to match(/gitlabUrl: '?#{Gitlab::Routing.url_helpers.root_url}/) end context 'without a runner' do @@ -66,7 +67,7 @@ describe Clusters::Applications::Runner do end it 'uses the new runner token' do - expect(subject).to include("runnerToken: #{gitlab_runner.reload.runner.token}") + expect(values).to match(/runnerToken: '?#{gitlab_runner.reload.runner.token}/) end it 'assigns the new runner to runner' do @@ -77,7 +78,7 @@ describe Clusters::Applications::Runner do end context 'with duplicated values on vendor/runner/values.yaml' do - let(:values) do + let(:stub_values) do { "concurrent" => 4, "checkInterval" => 3, @@ -96,11 +97,11 @@ describe Clusters::Applications::Runner do end before do - allow(gitlab_runner).to receive(:chart_values).and_return(values) + allow(gitlab_runner).to receive(:chart_values).and_return(stub_values) end it 'should overwrite values.yaml' do - is_expected.to include("privileged: #{gitlab_runner.privileged}") + expect(values).to match(/privileged: '?#{gitlab_runner.privileged}/) end end end |