From bd31c4be0d7cfcb0c2cc887a66c313c592ce8e88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cunha?= Date: Tue, 16 Jul 2019 16:11:10 +0000 Subject: [PATCH] Enable GitLabb runner to be uninstalled from cluster - Set as uninstallable app - Update docs - Adjust specs --- app/models/clusters/applications/runner.rb | 15 +++++---- .../clusters/concerns/application_core.rb | 10 ++++++ .../clusters/concerns/application_status.rb | 24 ++++++++------ .../check_uninstall_progress_service.rb | 1 + ...bernetes-applications-uninstall-runner.yml | 5 +++ doc/user/clusters/applications.md | 3 +- .../clusters/applications/runner_spec.rb | 33 ++++++++++++++++++- .../check_uninstall_progress_service_spec.rb | 8 ++++- 8 files changed, 79 insertions(+), 20 deletions(-) create mode 100644 changelogs/unreleased/60666-kubernetes-applications-uninstall-runner.yml diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index f0256ff4d41..6ae8c3bd7f3 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -29,13 +29,6 @@ module Clusters content_values.to_yaml end - # Need to investigate if pipelines run by this runner will stop upon the - # executor pod stopping - # I.e.run a pipeline, and uninstall runner while pipeline is running - def allowed_to_uninstall? - false - end - def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( name: name, @@ -47,6 +40,14 @@ module Clusters ) end + def prepare_uninstall + runner&.update!(active: false) + end + + def post_uninstall + runner.destroy! + end + private def ensure_runner diff --git a/app/models/clusters/concerns/application_core.rb b/app/models/clusters/concerns/application_core.rb index 4514498b84b..803a65726d3 100644 --- a/app/models/clusters/concerns/application_core.rb +++ b/app/models/clusters/concerns/application_core.rb @@ -46,6 +46,16 @@ module Clusters command.version = version end end + + def prepare_uninstall + # Override if your application needs any action before + # being uninstalled by Helm + end + + def post_uninstall + # Override if your application needs any action after + # being uninstalled by Helm + end end end end diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index 54a3dda6d75..342d766f723 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -59,29 +59,33 @@ module Clusters transition [:scheduled] => :uninstalling end - before_transition any => [:scheduled] do |app_status, _| - app_status.status_reason = nil + before_transition any => [:scheduled] do |application, _| + application.status_reason = nil end - before_transition any => [:errored] do |app_status, transition| + before_transition any => [:errored] do |application, transition| status_reason = transition.args.first - app_status.status_reason = status_reason if status_reason + application.status_reason = status_reason if status_reason end - before_transition any => [:updating] do |app_status, _| - app_status.status_reason = nil + before_transition any => [:updating] do |application, _| + application.status_reason = nil end - before_transition any => [:update_errored, :uninstall_errored] do |app_status, transition| + before_transition any => [:update_errored, :uninstall_errored] do |application, transition| status_reason = transition.args.first - app_status.status_reason = status_reason if status_reason + application.status_reason = status_reason if status_reason end - before_transition any => [:installed, :updated] do |app_status, _| + before_transition any => [:installed, :updated] do |application, _| # When installing any application we are also performing an update # of tiller (see Gitlab::Kubernetes::Helm::ClientCommand) so # therefore we need to reflect that in the database. - app_status.cluster.application_helm.update!(version: Gitlab::Kubernetes::Helm::HELM_VERSION) + application.cluster.application_helm.update!(version: Gitlab::Kubernetes::Helm::HELM_VERSION) + end + + after_transition any => [:uninstalling], :use_transactions => false do |application, _| + application.prepare_uninstall end end end diff --git a/app/services/clusters/applications/check_uninstall_progress_service.rb b/app/services/clusters/applications/check_uninstall_progress_service.rb index 8786d295d6a..e51d84ef052 100644 --- a/app/services/clusters/applications/check_uninstall_progress_service.rb +++ b/app/services/clusters/applications/check_uninstall_progress_service.rb @@ -23,6 +23,7 @@ module Clusters private def on_success + app.post_uninstall app.destroy! rescue StandardError => e app.make_errored!(_('Application uninstalled but failed to destroy: %{error_message}') % { error_message: e.message }) diff --git a/changelogs/unreleased/60666-kubernetes-applications-uninstall-runner.yml b/changelogs/unreleased/60666-kubernetes-applications-uninstall-runner.yml new file mode 100644 index 00000000000..3632c8eec20 --- /dev/null +++ b/changelogs/unreleased/60666-kubernetes-applications-uninstall-runner.yml @@ -0,0 +1,5 @@ +--- +title: Allow GitLab Runner to be uninstalled from the UI +merge_request: 30176 +author: +type: added diff --git a/doc/user/clusters/applications.md b/doc/user/clusters/applications.md index 2246ea8ed5a..67c21bd7639 100644 --- a/doc/user/clusters/applications.md +++ b/doc/user/clusters/applications.md @@ -251,6 +251,7 @@ The applications below can be uninstalled. | Application | GitLab version | Notes | | ----------- | -------------- | ----- | +| GitLab Runner | 12.2+ | Any running pipelines will be canceled. | | Ingress | 12.1+ | The associated load balancer and IP will be deleted and cannot be restored. Furthermore, it can only be uninstalled if JupyterHub is not installed. | | JupyterHub | 12.1+ | All data not committed to GitLab will be deleted and cannot be restored. | | Prometheus | 11.11+ | All data will be deleted and cannot be restored. | @@ -278,7 +279,7 @@ Error: remote error: tls: bad certificate To avoid installation errors: - Before starting the installation of applications, make sure that time is synchronized - between your GitLab server and your Kubernetes cluster. +between your GitLab server and your Kubernetes cluster. - Ensure certificates are not out of sync. When installing applications, GitLab expects a new cluster with no previous installation of Helm. You can confirm that the certificates match via `kubectl`: diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 4f0cd0efe9c..4abe45a2152 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -18,7 +18,7 @@ describe Clusters::Applications::Runner do subject { gitlab_runner.can_uninstall? } - it { is_expected.to be_falsey } + it { is_expected.to be_truthy } end describe '#install_command' do @@ -156,4 +156,35 @@ describe Clusters::Applications::Runner do end end end + + describe '#prepare_uninstall' do + it 'pauses associated runner' do + active_runner = create(:ci_runner, contacted_at: 1.second.ago) + + expect(active_runner.status).to eq(:online) + + application_runner = create(:clusters_applications_runner, :scheduled, runner: active_runner) + application_runner.prepare_uninstall + + expect(active_runner.status).to eq(:paused) + end + end + + describe '#make_uninstalling!' do + subject { create(:clusters_applications_runner, :scheduled, runner: ci_runner) } + + it 'calls prepare_uninstall' do + expect_any_instance_of(described_class).to receive(:prepare_uninstall).and_call_original + + subject.make_uninstalling! + end + end + + describe '#post_uninstall' do + it 'destroys its runner' do + application_runner = create(:clusters_applications_runner, :scheduled, runner: ci_runner) + + expect { application_runner.post_uninstall }.to change { Ci::Runner.count }.by(-1) + end + end end diff --git a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb index 9ab83d913f5..a948b442441 100644 --- a/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_uninstall_progress_service_spec.rb @@ -41,7 +41,7 @@ describe Clusters::Applications::CheckUninstallProgressService do end end - context 'when application is installing' do + context 'when application is uninstalling' do RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase } context 'when installation POD succeeded' do @@ -56,6 +56,12 @@ describe Clusters::Applications::CheckUninstallProgressService do service.execute end + it 'runs application post_uninstall' do + expect(application).to receive(:post_uninstall).and_call_original + + service.execute + end + it 'destroys the application' do expect(worker_class).not_to receive(:perform_in)