From 0cd76190deb01db8ff080186f3daa5b6067a27f6 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Sun, 22 Jul 2018 22:48:53 +1200 Subject: Lock helm charts to the VERSION already specified for each application. Fix up VERSION for each of the applications * There is no 0.0.1 helm version for jupyterhub. Use the latest version instead * `:nginx` is not a valid chart version. Lock the ingress application GitLab installs to the latest chart version. * Use the latest gitlab-runner chart to prevent GitLab installing older versions when users have been installing the lastest version Always install from the VERSION and not the database `version` column. This should fix cases like https://gitlab.com/gitlab-org/gitlab-ee/issues/6795 in the instances where an install command failed previously, which locked the version in the database to an older version. Also, ensure that the version column is updated to the version we are installing. Add specs to show how previously failed appplications will be handled when the helm installation is run again Add changelog entry --- app/models/clusters/applications/ingress.rb | 6 +++++- app/models/clusters/applications/jupyter.rb | 4 +++- app/models/clusters/applications/prometheus.rb | 3 ++- app/models/clusters/applications/runner.rb | 4 +++- .../clusters/concerns/application_version.rb | 17 +++++++++++++++ ...ck-install-buttons-should-be-version-locked.yml | 6 ++++++ .../gitlab/kubernetes/helm/install_command_spec.rb | 6 +++--- spec/models/clusters/applications/ingress_spec.rb | 24 +++++++++++++++++++++- spec/models/clusters/applications/jupyter_spec.rb | 24 +++++++++++++++++++++- .../clusters/applications/prometheus_spec.rb | 22 ++++++++++++++++++++ spec/models/clusters/applications/runner_spec.rb | 24 +++++++++++++++++++++- 11 files changed, 130 insertions(+), 10 deletions(-) create mode 100644 app/models/clusters/concerns/application_version.rb create mode 100644 changelogs/unreleased/48834-chart-versions-for-applications-installed-by-one-click-install-buttons-should-be-version-locked.yml diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index 27fc3b85465..4a8fd9a0b8c 100644 --- a/app/models/clusters/applications/ingress.rb +++ b/app/models/clusters/applications/ingress.rb @@ -1,15 +1,18 @@ module Clusters module Applications class Ingress < ActiveRecord::Base + VERSION = '0.23.0'.freeze + self.table_name = 'clusters_applications_ingress' include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus + include ::Clusters::Concerns::ApplicationVersion include ::Clusters::Concerns::ApplicationData include AfterCommitQueue default_value_for :ingress_type, :nginx - default_value_for :version, :nginx + default_value_for :version, VERSION enum ingress_type: { nginx: 1 @@ -33,6 +36,7 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( name, + version: VERSION, chart: chart, values: values ) diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb index 975d434e1a4..72dd734246b 100644 --- a/app/models/clusters/applications/jupyter.rb +++ b/app/models/clusters/applications/jupyter.rb @@ -1,12 +1,13 @@ module Clusters module Applications class Jupyter < ActiveRecord::Base - VERSION = '0.0.1'.freeze + VERSION = 'v0.6'.freeze self.table_name = 'clusters_applications_jupyter' include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus + include ::Clusters::Concerns::ApplicationVersion include ::Clusters::Concerns::ApplicationData belongs_to :oauth_application, class_name: 'Doorkeeper::Application' @@ -36,6 +37,7 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( name, + version: VERSION, chart: chart, values: values, repository: repository diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index ea6ec4d6b03..53ac9199ae2 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -9,6 +9,7 @@ module Clusters include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus + include ::Clusters::Concerns::ApplicationVersion include ::Clusters::Concerns::ApplicationData default_value_for :version, VERSION @@ -44,8 +45,8 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( name, + version: VERSION, chart: chart, - version: version, values: values ) end diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index e6f795f3e0b..6d97dd1448a 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -1,12 +1,13 @@ module Clusters module Applications class Runner < ActiveRecord::Base - VERSION = '0.1.13'.freeze + VERSION = '0.1.31'.freeze self.table_name = 'clusters_applications_runners' include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus + include ::Clusters::Concerns::ApplicationVersion include ::Clusters::Concerns::ApplicationData belongs_to :runner, class_name: 'Ci::Runner', foreign_key: :runner_id @@ -29,6 +30,7 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( name, + version: VERSION, chart: chart, values: values, repository: repository diff --git a/app/models/clusters/concerns/application_version.rb b/app/models/clusters/concerns/application_version.rb new file mode 100644 index 00000000000..ccad74dc35a --- /dev/null +++ b/app/models/clusters/concerns/application_version.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Clusters + module Concerns + module ApplicationVersion + extend ActiveSupport::Concern + + included do + state_machine :status do + after_transition any => [:installing] do |application| + application.update(version: application.class.const_get(:VERSION)) + end + end + end + end + end +end diff --git a/changelogs/unreleased/48834-chart-versions-for-applications-installed-by-one-click-install-buttons-should-be-version-locked.yml b/changelogs/unreleased/48834-chart-versions-for-applications-installed-by-one-click-install-buttons-should-be-version-locked.yml new file mode 100644 index 00000000000..d79b95411aa --- /dev/null +++ b/changelogs/unreleased/48834-chart-versions-for-applications-installed-by-one-click-install-buttons-should-be-version-locked.yml @@ -0,0 +1,6 @@ +--- +title: Chart versions for applications installed by one click install buttons should + be version locked +merge_request: 20765 +author: +type: fixed diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb index 25c6fa3b9a3..cd456a45287 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -14,7 +14,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do let(:commands) do <<~EOS helm init --client-only >/dev/null - helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end @@ -42,7 +42,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do <<~EOS helm init --client-only >/dev/null helm repo add #{application.name} #{application.repository} - helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end @@ -56,7 +56,7 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do <<~EOS helm init --client-only >/dev/null helm repo add #{application.name} #{application.repository} - helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index bb5b2ef3a47..d378248d5d6 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -23,6 +23,20 @@ describe Clusters::Applications::Ingress do it { is_expected.to contain_exactly(cluster) } end + describe '#make_installing!' do + before do + application.make_installing! + end + + context 'application install previously errored with older version' do + let(:application) { create(:clusters_applications_ingress, :scheduled, version: '0.22.0') } + + it 'updates the application version' do + expect(application.reload.version).to eq('0.23.0') + end + end + end + describe '#make_installed!' do before do application.make_installed! @@ -73,9 +87,17 @@ describe Clusters::Applications::Ingress do it 'should be initialized with ingress arguments' do expect(subject.name).to eq('ingress') expect(subject.chart).to eq('stable/nginx-ingress') - expect(subject.version).to be_nil + expect(subject.version).to eq('0.23.0') expect(subject.values).to eq(ingress.values) end + + context 'application failed to install previously' do + let(:ingress) { create(:clusters_applications_ingress, :errored, version: 'nginx') } + + it 'should be initialized with the locked version' do + expect(subject.version).to eq('0.23.0') + end + end end describe '#values' do diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index 65750141e65..e0d57ac65f7 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -25,6 +25,20 @@ describe Clusters::Applications::Jupyter do end end + describe '#make_installing!' do + before do + application.make_installing! + end + + context 'application install previously errored with older version' do + let(:application) { create(:clusters_applications_jupyter, :scheduled, version: 'v0.5') } + + it 'updates the application version' do + expect(application.reload.version).to eq('v0.6') + end + end + end + describe '#install_command' do let!(:ingress) { create(:clusters_applications_ingress, :installed, external_ip: '127.0.0.1') } let!(:jupyter) { create(:clusters_applications_jupyter, cluster: ingress.cluster) } @@ -36,10 +50,18 @@ describe Clusters::Applications::Jupyter do it 'should be initialized with 4 arguments' do expect(subject.name).to eq('jupyter') expect(subject.chart).to eq('jupyter/jupyterhub') - expect(subject.version).to be_nil + expect(subject.version).to eq('v0.6') expect(subject.repository).to eq('https://jupyterhub.github.io/helm-chart/') expect(subject.values).to eq(jupyter.values) end + + context 'application failed to install previously' do + let(:jupyter) { create(:clusters_applications_jupyter, :errored, version: '0.0.1') } + + it 'should be initialized with the locked version' do + expect(subject.version).to eq('v0.6') + end + end end describe '#values' do diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index e4b61552033..3812c65b3b6 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -16,6 +16,20 @@ describe Clusters::Applications::Prometheus do it { is_expected.to contain_exactly(cluster) } end + describe '#make_installing!' do + before do + application.make_installing! + end + + context 'application install previously errored with older version' do + let(:application) { create(:clusters_applications_prometheus, :scheduled, version: '6.7.2') } + + it 'updates the application version' do + expect(application.reload.version).to eq('6.7.3') + end + end + end + describe 'transition to installed' do let(:project) { create(:project) } let(:cluster) { create(:cluster, projects: [project]) } @@ -155,6 +169,14 @@ describe Clusters::Applications::Prometheus do expect(command.version).to eq('6.7.3') expect(command.values).to eq(prometheus.values) end + + context 'application failed to install previously' do + let(:prometheus) { create(:clusters_applications_prometheus, :errored, version: '2.0.0') } + + it 'should be initialized with the locked version' do + expect(subject.version).to eq('6.7.3') + end + end end describe '#values' do diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index b12500d0acd..526300755b5 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -8,6 +8,20 @@ describe Clusters::Applications::Runner do it { is_expected.to belong_to(:runner) } + describe '#make_installing!' do + before do + application.make_installing! + end + + context 'application install previously errored with older version' do + let(:application) { create(:clusters_applications_runner, :scheduled, version: '0.1.30') } + + it 'updates the application version' do + expect(application.reload.version).to eq('0.1.31') + end + end + end + describe '.installed' do subject { described_class.installed } @@ -31,10 +45,18 @@ describe Clusters::Applications::Runner do it 'should be initialized with 4 arguments' do expect(subject.name).to eq('runner') expect(subject.chart).to eq('runner/gitlab-runner') - expect(subject.version).to be_nil + expect(subject.version).to eq('0.1.31') expect(subject.repository).to eq('https://charts.gitlab.io') expect(subject.values).to eq(gitlab_runner.values) end + + context 'application failed to install previously' do + let(:gitlab_runner) { create(:clusters_applications_runner, :errored, runner: ci_runner, version: '0.1.13') } + + it 'should be initialized with the locked version' do + expect(subject.version).to eq('0.1.31') + end + end end describe '#values' do -- cgit v1.2.1