Deprecate helm 2 from review-apps

- Merge `review-cleanup` and `review-gcp-cleanup` jobs
- Remove helm 2 MR migration script
- Use minutes in helm timeout
- Remove Helm 2 code

.gitlab/ci/review.gitlab-ci.yml

@@ -17,49 +17,24 @@ build-qa-image:
     - export QA_IMAGE="${CI_REGISTRY}/${CI_PROJECT_PATH}/gitlab-${GITLAB_EDITION}-qa:${CI_COMMIT_REF_SLUG}"
     - /kaniko/executor --context=${CI_PROJECT_DIR} --dockerfile=${CI_PROJECT_DIR}/qa/Dockerfile --destination=${QA_IMAGE} --cache=true
 
-.review-cleanup-base:
+review-cleanup:
   extends:
     - .default-retry
     - .review:rules:review-cleanup
+  image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-helm3-kubectl1.14
   stage: prepare
-  allow_failure: true
   environment:
     name: review/auto-cleanup
     action: stop
   before_script:
     - source scripts/utils.sh
+    - source scripts/review_apps/gcp_cleanup.sh
     - install_gitlab_gem
+    - setup_gcp_dependencies
   script:
     - ruby -rrubygems scripts/review_apps/automated_cleanup.rb
-
-review-cleanup:
-  extends:
-    - .review-cleanup-base
-  image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-charts-build-base
-
-review-cleanup-helm3:
-  extends:
-    - .review-cleanup-base
-  variables:
-    HELM_3: 1
-  image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-helm3-kubectl1.14
-
-review-gcp-cleanup:
-  extends:
-    - .review:rules:review-gcp-cleanup
-  stage: prepare
-  image: gcr.io/google.com/cloudsdktool/cloud-sdk:latest
-  allow_failure: true
-  environment:
-    name: review/auto-gcp-cleanup
-    action: stop
-  before_script:
-    - gcloud auth activate-service-account --key-file=$REVIEW_APPS_GCP_CREDENTIALS
-    - gcloud config set project $REVIEW_APPS_GCP_PROJECT
-    - apt-get install -y jq
-    - source scripts/review_apps/gcp_cleanup.sh
-  script:
     - gcp_cleanup
+  allow_failure: true
 
 review-build-cng:
   extends:
@@ -150,7 +125,6 @@ review-stop-failed-deployment:
   stage: prepare
   script:
     - delete_failed_release
-    - delete_helm2_release
 
 review-stop:
   extends:

lib/quality/helm3_client.rb

@@ -17,10 +17,6 @@ module Quality
         @revision ||= self[:revision].to_i
       end
 
-      def status
-        @status ||= self[:status].downcase
-      end
-
       def last_update
         @last_update ||= Time.parse(self[:last_update])
       end
@@ -29,7 +25,7 @@ module Quality
     # A single page of data and the corresponding page number.
     Page = Struct.new(:releases, :number)
 
-    def initialize(namespace:, tiller_namespace: nil)
+    def initialize(namespace:)
       @namespace = namespace
     end
 

lib/quality/helm_client.rb

-# frozen_string_literal: true
-
-require 'time'
-require_relative '../gitlab/popen' unless defined?(Gitlab::Popen)
-
-module Quality
-  class HelmClient
-    CommandFailedError = Class.new(StandardError)
-
-    attr_reader :tiller_namespace, :namespace
-
-    RELEASE_JSON_ATTRIBUTES = %w[Name Revision Updated Status Chart AppVersion Namespace].freeze
-
-    Release = Struct.new(:name, :revision, :last_update, :status, :chart, :app_version, :namespace) do
-      def revision
-        @revision ||= self[:revision].to_i
-      end
-
-      def last_update
-        @last_update ||= Time.parse(self[:last_update])
-      end
-    end
-
-    # A single page of data and the corresponding page number.
-    Page = Struct.new(:releases, :number)
-
-    def initialize(tiller_namespace:, namespace:)
-      @tiller_namespace = tiller_namespace
-      @namespace = namespace
-    end
-
-    def releases(args: [])
-      each_release(args)
-    end
-
-    def delete(release_name:)
-      run_command([
-        'delete',
-        %(--tiller-namespace "#{tiller_namespace}"),
-        '--purge',
-        release_name
-      ])
-    end
-
-    private
-
-    def run_command(command)
-      final_command = ['helm', *command].join(' ')
-      puts "Running command: `#{final_command}`" # rubocop:disable Rails/Output
-
-      result = Gitlab::Popen.popen_with_detail([final_command])
-
-      if result.status.success?
-        result.stdout.chomp.freeze
-      else
-        raise CommandFailedError, "The `#{final_command}` command failed (status: #{result.status}) with the following error:\n#{result.stderr}"
-      end
-    end
-
-    def raw_releases(args = [])
-      command = [
-        'list',
-        %(--namespace "#{namespace}"),
-        %(--tiller-namespace "#{tiller_namespace}" --output json),
-        *args
-      ]
-      json = JSON.parse(run_command(command))
-
-      releases = json['Releases'].map do |json_release|
-        Release.new(*json_release.values_at(*RELEASE_JSON_ATTRIBUTES))
-      end
-
-      [releases, json['Next']]
-    rescue JSON::ParserError => ex
-      puts "Ignoring this JSON parsing error: #{ex}" # rubocop:disable Rails/Output
-      [[], nil]
-    end
-
-    # Fetches data from Helm and yields a Page object for every page
-    # of data, without loading all of them into memory.
-    #
-    # method - The Octokit method to use for getting the data.
-    # args - Arguments to pass to the `helm list` command.
-    def each_releases_page(args, &block)
-      return to_enum(__method__, args) unless block_given?
-
-      page = 1
-      offset = ''
-
-      loop do
-        final_args = args.dup
-        final_args << "--offset #{offset}" unless offset.to_s.empty?
-        collection, offset = raw_releases(final_args)
-
-        yield Page.new(collection, page += 1)
-
-        break if offset.to_s.empty?
-      end
-    end
-
-    # Iterates over all of the releases.
-    #
-    # args - Any arguments to pass to the `helm list` command.
-    def each_release(args, &block)
-      return to_enum(__method__, args) unless block_given?
-
-      each_releases_page(args) do |page|
-        page.releases.each do |release|
-          yield release
-        end
-      end
-    end
-  end
-end

scripts/review_apps/automated_cleanup.rb

 # frozen_string_literal: true
 
 require 'gitlab'
-require_relative File.expand_path('../../lib/quality/helm_client.rb', __dir__)
 require_relative File.expand_path('../../lib/quality/helm3_client.rb', __dir__)
 require_relative File.expand_path('../../lib/quality/kubernetes_client.rb', __dir__)
 
@@ -9,7 +8,6 @@ class AutomatedCleanup
   attr_reader :project_path, :gitlab_token
 
   DEPLOYMENTS_PER_PAGE = 100
-  HELM_RELEASES_BATCH_SIZE = 5
   IGNORED_HELM_ERRORS = [
     'transport is closing',
     'error upgrading connection',
@@ -45,18 +43,8 @@ class AutomatedCleanup
     self.class.ee? ? 'review-apps-ee' : 'review-apps-ce'
   end
 
-  def helm3?
-    !ENV['HELM_3'].nil?
-  end
-
-  def helm_client_class
-    helm3? ? Quality::Helm3Client : Quality::HelmClient
-  end
-
   def helm
-    @helm ||= helm_client_class.new(
-      tiller_namespace: review_apps_namespace,
-      namespace: review_apps_namespace)
+    @helm ||= Quality::Helm3Client.new(namespace: review_apps_namespace)
   end
 
   def kubernetes
@@ -88,7 +76,7 @@ class AutomatedCleanup
       if deployed_at < delete_threshold
         deleted_environment = delete_environment(environment, deployment)
         if deleted_environment
-          release = helm_client_class::Release.new(environment.slug, 1, deployed_at.to_s, nil, nil, review_apps_namespace)
+          release = Quality::Helm3Client::Release.new(environment.slug, 1, deployed_at.to_s, nil, nil, review_apps_namespace)
           releases_to_delete << release
         end
       else
@@ -117,7 +105,7 @@ class AutomatedCleanup
       # Prevents deleting `dns-gitlab-review-app` releases or other unrelated releases
       next unless release.name.start_with?('review-')
 
-      if release.status.casecmp('failed') == 0 || release.last_update < threshold
+      if release.status == 'failed' || release.last_update < threshold
         releases_to_delete << release
       else
         print_release_state(subject: 'Release', release_name: release.name, release_date: release.last_update, action: 'leaving')
@@ -154,7 +142,6 @@ class AutomatedCleanup
 
   def helm_releases
     args = ['--all', '--date']
-    args << "--max #{HELM_RELEASES_BATCH_SIZE}" unless helm3?
 
     helm.releases(args: args)
   end
@@ -170,7 +157,7 @@ class AutomatedCleanup
     helm.delete(release_name: releases_names)
     kubernetes.cleanup(release_name: releases_names, wait: false)
 
-  rescue helm_client_class::CommandFailedError => ex
+  rescue Quality::Helm3Client::CommandFailedError => ex
     raise ex unless ignore_exception?(ex.message, IGNORED_HELM_ERRORS)
 
     puts "Ignoring the following Helm error:\n#{ex}\n"

scripts/review_apps/gcp_cleanup.sh

@@ -2,6 +2,13 @@
 
 source scripts/utils.sh
 
+function setup_gcp_dependencies() {
+  apk add jq
+
+  gcloud auth activate-service-account --key-file="${REVIEW_APPS_GCP_CREDENTIALS}"
+  gcloud config set project "${REVIEW_APPS_GCP_PROJECT}"
+}
+
 # These scripts require the following environment variables:
 # - REVIEW_APPS_GCP_REGION - e.g `us-central1`
 # - KUBE_NAMESPACE - e.g `review-apps-ee`

scripts/review_apps/review-apps.sh

@@ -95,37 +95,6 @@ function delete_failed_release() {
   fi
 }
 
-function helm2_deploy_exists() {
-  local namespace="${1}"
-  local release="${2}"
-  local deploy_exists
-
-  echoinfo "Checking if Helm 2 ${release} exists in the ${namespace} namespace..." true
-
-  kubectl get cm -l OWNER=TILLER -n ${namespace} | grep ${release} 2>&1
-  deploy_exists=$?
-
-  echoinfo "Helm 2 release for ${release} is ${deploy_exists}"
-  return $deploy_exists
-}
-
-function delete_helm2_release() {
-  local namespace="${KUBE_NAMESPACE}"
-  local release="${CI_ENVIRONMENT_SLUG}"
-
-  if [ -z "${release}" ]; then
-    echoerr "No release given, aborting the delete!"
-    return
-  fi
-
-  if ! helm2_deploy_exists "${namespace}" "${release}"; then
-    echoinfo "No Review App with ${release} is currently deployed by Helm 2."
-  else
-    echoinfo "Cleaning up ${release} installed by Helm 2"
-    kubectl_cleanup_release "${namespace}" "${release}"
-  fi
-}
-
 function get_pod() {
   local namespace="${KUBE_NAMESPACE}"
   local release="${CI_ENVIRONMENT_SLUG}"
@@ -290,7 +259,7 @@ HELM_CMD=$(cat << EOF
     --namespace="${namespace}" \
     --install \
     --wait \
-    --timeout 900s \
+    --timeout 15m \
     --set ci.branch="${CI_COMMIT_REF_NAME}" \
     --set ci.commit.sha="${CI_COMMIT_SHORT_SHA}" \
     --set ci.job.url="${CI_JOB_URL}" \

spec/lib/quality/helm_client_spec.rb

-# frozen_string_literal: true
-
-require 'fast_spec_helper'
-
-RSpec.describe Quality::HelmClient do
-  let(:tiller_namespace) { 'review-apps-ee' }
-  let(:namespace) { tiller_namespace }
-  let(:release_name) { 'my-release' }
-  let(:raw_helm_list_page1) do
-    <<~OUTPUT
-    {"Next":"review-6709-group-t40qbv",
-      "Releases":[
-        {"Name":"review-qa-60-reor-1mugd1", "Revision":1,"Updated":"Thu Oct  4 17:52:31 2018","Status":"FAILED", "Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"},
-        {"Name":"review-7846-fix-s-261vd6","Revision":1,"Updated":"Thu Oct  4 17:33:29 2018","Status":"FAILED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"},
-        {"Name":"review-7867-snowp-lzo3iy","Revision":1,"Updated":"Thu Oct  4 17:22:14 2018","Status":"DEPLOYED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"},
-        {"Name":"review-rename-geo-o4a780","Revision":1,"Updated":"Thu Oct  4 17:14:57 2018","Status":"DEPLOYED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"},
-        {"Name":"review-5781-opera-0k93fx","Revision":1,"Updated":"Thu Oct  4 17:06:15 2018","Status":"FAILED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"},
-        {"Name":"review-6709-group-2pzeec","Revision":1,"Updated":"Thu Oct  4 16:36:59 2018","Status":"FAILED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"},
-        {"Name":"review-ce-to-ee-2-l554mn","Revision":1,"Updated":"Thu Oct  4 16:27:02 2018","Status":"FAILED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"},
-        {"Name":"review-epics-e2e-m690eb","Revision":1,"Updated":"Thu Oct  4 16:08:26 2018","Status":"DEPLOYED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"},
-        {"Name":"review-7126-admin-06fae2","Revision":1,"Updated":"Thu Oct  4 15:56:35 2018","Status":"FAILED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"},
-        {"Name":"review-6983-promo-xyou11","Revision":1,"Updated":"Thu Oct  4 15:15:34 2018","Status":"FAILED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"}
-      ]}
-    OUTPUT
-  end
-  let(:raw_helm_list_page2) do
-    <<~OUTPUT
-    {"Releases":[
-      {"Name":"review-6709-group-t40qbv","Revision":1,"Updated":"Thu Oct  4 17:52:31 2018","Status":"FAILED","Chart":"gitlab-1.1.3","AppVersion":"master","Namespace":"#{namespace}"}
-    ]}
-    OUTPUT
-  end
-
-  subject { described_class.new(tiller_namespace: tiller_namespace, namespace: namespace) }
-
-  describe '#releases' do
-    it 'raises an error if the Helm command fails' do
-      expect(Gitlab::Popen).to receive(:popen_with_detail)
-        .with([%(helm list --namespace "#{namespace}" --tiller-namespace "#{tiller_namespace}" --output json)])
-        .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false)))
-
-      expect { subject.releases.to_a }.to raise_error(described_class::CommandFailedError)
-    end
-
-    it 'calls helm list with default arguments' do
-      expect(Gitlab::Popen).to receive(:popen_with_detail)
-        .with([%(helm list --namespace "#{namespace}" --tiller-namespace "#{tiller_namespace}" --output json)])
-        .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
-
-      subject.releases.to_a
-    end
-
-    it 'calls helm list with extra arguments' do
-      expect(Gitlab::Popen).to receive(:popen_with_detail)
-        .with([%(helm list --namespace "#{namespace}" --tiller-namespace "#{tiller_namespace}" --output json --deployed)])
-        .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
-
-      subject.releases(args: ['--deployed']).to_a
-    end
-
-    it 'returns a list of Release objects' do
-      expect(Gitlab::Popen).to receive(:popen_with_detail)
-        .with([%(helm list --namespace "#{namespace}" --tiller-namespace "#{tiller_namespace}" --output json --deployed)])
-        .and_return(Gitlab::Popen::Result.new([], raw_helm_list_page2, '', double(success?: true)))
-
-      releases = subject.releases(args: ['--deployed']).to_a
-
-      expect(releases.size).to eq(1)
-      expect(releases[0]).to have_attributes(
-        name: 'review-6709-group-t40qbv',
-        revision: 1,
-        last_update: Time.parse('Thu Oct 4 17:52:31 2018'),
-        status: 'FAILED',
-        chart: 'gitlab-1.1.3',
-        app_version: 'master',
-        namespace: namespace
-      )
-    end
-
-    it 'automatically paginates releases' do
-      expect(Gitlab::Popen).to receive(:popen_with_detail).ordered
-        .with([%(helm list --namespace "#{namespace}" --tiller-namespace "#{tiller_namespace}" --output json)])
-        .and_return(Gitlab::Popen::Result.new([], raw_helm_list_page1, '', double(success?: true)))
-      expect(Gitlab::Popen).to receive(:popen_with_detail).ordered
-        .with([%(helm list --namespace "#{namespace}" --tiller-namespace "#{tiller_namespace}" --output json --offset review-6709-group-t40qbv)])
-        .and_return(Gitlab::Popen::Result.new([], raw_helm_list_page2, '', double(success?: true)))
-
-      releases = subject.releases.to_a
-
-      expect(releases.size).to eq(11)
-      expect(releases.last.name).to eq('review-6709-group-t40qbv')
-    end
-  end
-
-  describe '#delete' do
-    it 'raises an error if the Helm command fails' do
-      expect(Gitlab::Popen).to receive(:popen_with_detail)
-        .with([%(helm delete --tiller-namespace "#{tiller_namespace}" --purge #{release_name})])
-        .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false)))
-
-      expect { subject.delete(release_name: release_name) }.to raise_error(described_class::CommandFailedError)
-    end
-
-    it 'calls helm delete with default arguments' do
-      expect(Gitlab::Popen).to receive(:popen_with_detail)
-        .with([%(helm delete --tiller-namespace "#{tiller_namespace}" --purge #{release_name})])
-        .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
-
-      expect(subject.delete(release_name: release_name)).to eq('')
-    end
-
-    context 'with multiple release names' do
-      let(:release_name) { %w[my-release my-release-2] }
-
-      it 'raises an error if the Helm command fails' do
-        expect(Gitlab::Popen).to receive(:popen_with_detail)
-                                   .with([%(helm delete --tiller-namespace "#{tiller_namespace}" --purge #{release_name.join(' ')})])
-                                   .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false)))
-
-        expect { subject.delete(release_name: release_name) }.to raise_error(described_class::CommandFailedError)
-      end
-
-      it 'calls helm delete with multiple release names' do
-        expect(Gitlab::Popen).to receive(:popen_with_detail)
-                                   .with([%(helm delete --tiller-namespace "#{tiller_namespace}" --purge #{release_name.join(' ')})])
-                                   .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
-
-        expect(subject.delete(release_name: release_name)).to eq('')
-      end
-    end
-  end
-end