From 8296ff580c2bb1b2a9ee07193f79f66fd6444de0 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Wed, 19 Dec 2018 21:50:12 +1300 Subject: Port generic upgrade functionality to CE Port from EE generic upgrade related functionality used to upgrade Helm applications Remove memoization which could be incorrect It looks like we are memoizing without regard to the method's argument so this could result in an incorrect upgrade_command Remove `const_get` indirection now we are no longer in EE --- app/models/clusters/applications/prometheus.rb | 28 ++++++- .../clusters/applications/base_helm_service.rb | 4 + .../clusters/applications/prometheus_spec.rb | 97 ++++++++++++++++++++++ 3 files changed, 128 insertions(+), 1 deletion(-) diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index e25be522d68..7a97760536a 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -25,13 +25,21 @@ module Clusters end def ready_status - [:installed] + [:installed, :updating, :updated, :update_errored] end def ready? ready_status.include?(status_name) end + def update_in_progress? + status_name == :updating + end + + def update_errored? + status_name == :update_errored + end + def chart 'stable/prometheus' end @@ -55,6 +63,24 @@ module Clusters ) end + def upgrade_command(values) + ::Gitlab::Kubernetes::Helm::UpgradeCommand.new( + name, + version: VERSION, + chart: chart, + rbac: cluster.platform_kubernetes_rbac?, + files: files_with_replaced_values(values) + ) + end + + # Returns a copy of files where the values of 'values.yaml' + # are replaced by the argument. + # + # See #values for the data format required + def files_with_replaced_values(replaced_values) + files.merge('values.yaml': replaced_values) + end + def prometheus_client return unless kube_client diff --git a/app/services/clusters/applications/base_helm_service.rb b/app/services/clusters/applications/base_helm_service.rb index e86ca8cf1d0..8a71730d5ec 100644 --- a/app/services/clusters/applications/base_helm_service.rb +++ b/app/services/clusters/applications/base_helm_service.rb @@ -45,6 +45,10 @@ module Clusters def install_command @install_command ||= app.install_command end + + def upgrade_command(new_values = "") + app.upgrade_command(new_values) + end end end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index de6b844023a..e50ba67c493 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -90,6 +90,24 @@ describe Clusters::Applications::Prometheus do expect(application).not_to be_ready end + + it 'returns true when updating' do + application = build(:clusters_applications_prometheus, :updating, cluster: cluster) + + expect(application).to be_ready + end + + it 'returns true when updated' do + application = build(:clusters_applications_prometheus, :updated, cluster: cluster) + + expect(application).to be_ready + end + + it 'returns true when errored' do + application = build(:clusters_applications_prometheus, :update_errored, cluster: cluster) + + expect(application).to be_ready + end end describe '#prometheus_client' do @@ -197,6 +215,46 @@ describe Clusters::Applications::Prometheus do end end + describe '#upgrade_command' do + let(:prometheus) { build(:clusters_applications_prometheus) } + let(:values) { prometheus.values } + + it 'returns an instance of Gitlab::Kubernetes::Helm::GetCommand' do + expect(prometheus.upgrade_command(values)).to be_an_instance_of(::Gitlab::Kubernetes::Helm::UpgradeCommand) + end + + it 'should be initialized with 3 arguments' do + command = prometheus.upgrade_command(values) + + expect(command.name).to eq('prometheus') + expect(command.chart).to eq('stable/prometheus') + expect(command.version).to eq('6.7.3') + expect(command.files).to eq(prometheus.files) + end + end + + describe '#update_in_progress?' do + context 'when app is updating' do + it 'returns true' do + cluster = create(:cluster) + prometheus_app = build(:clusters_applications_prometheus, :updating, cluster: cluster) + + expect(prometheus_app.update_in_progress?).to be true + end + end + end + + describe '#update_errored?' do + context 'when app errored' do + it 'returns true' do + cluster = create(:cluster) + prometheus_app = build(:clusters_applications_prometheus, :update_errored, cluster: cluster) + + expect(prometheus_app.update_errored?).to be true + end + end + end + describe '#files' do let(:application) { create(:clusters_applications_prometheus) } let(:values) { subject[:'values.yaml'] } @@ -211,4 +269,43 @@ describe Clusters::Applications::Prometheus do expect(values).to include('serverFiles') end end + + describe '#files_with_replaced_values' do + let(:application) { build(:clusters_applications_prometheus) } + let(:files) { application.files } + + subject { application.files_with_replaced_values({ hello: :world }) } + + it 'does not modify #files' do + expect(subject[:'values.yaml']).not_to eq(files) + expect(files[:'values.yaml']).to eq(application.values) + end + + it 'returns values.yaml with replaced values' do + expect(subject[:'values.yaml']).to eq({ hello: :world }) + end + + it 'should include cert files' do + expect(subject[:'ca.pem']).to be_present + expect(subject[:'ca.pem']).to eq(application.cluster.application_helm.ca_cert) + + expect(subject[:'cert.pem']).to be_present + expect(subject[:'key.pem']).to be_present + + cert = OpenSSL::X509::Certificate.new(subject[:'cert.pem']) + expect(cert.not_after).to be < 60.minutes.from_now + end + + context 'when the helm application does not have a ca_cert' do + before do + application.cluster.application_helm.ca_cert = nil + end + + it 'should not include cert files' do + expect(subject[:'ca.pem']).not_to be_present + expect(subject[:'cert.pem']).not_to be_present + expect(subject[:'key.pem']).not_to be_present + end + end + end end -- cgit v1.2.1 From d87486e678cd2d957e504109c3665ab84de1a6cc Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Fri, 18 Jan 2019 17:38:28 +1300 Subject: Refactor some methods in Prometheus model - #update_errored? method def is redundant, already defined from state machine - #update_in_progress? moved to ApplicationStatus - #ready_status becomes a constant. Previously it was extended in ee/ so maybe that was why it's a instance method --- app/models/clusters/applications/prometheus.rb | 17 +++-------------- app/models/clusters/concerns/application_status.rb | 4 ++++ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index 7a97760536a..26bf73f4dd8 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -5,7 +5,8 @@ module Clusters class Prometheus < ActiveRecord::Base include PrometheusAdapter - VERSION = '6.7.3'.freeze + VERSION = '6.7.3' + READY_STATUS = [:installed, :updating, :updated, :update_errored].freeze self.table_name = 'clusters_applications_prometheus' @@ -24,20 +25,8 @@ module Clusters end end - def ready_status - [:installed, :updating, :updated, :update_errored] - end - def ready? - ready_status.include?(status_name) - end - - def update_in_progress? - status_name == :updating - end - - def update_errored? - status_name == :update_errored + READY_STATUS.include?(status_name) end def chart diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index 0e74cce29b7..a556dd5ad8b 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -77,6 +77,10 @@ module Clusters def available? installed? || updated? end + + def update_in_progress? + updating? + end end end end -- cgit v1.2.1