Ignore `not found` from `helm uninstall/delete`

This is because the command might be passed a release
that is tracked by a different helm version.

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

@@ -20,12 +20,11 @@ build-qa-image:
     - time docker build --cache-from "${QA_MASTER_IMAGE}" --tag ${QA_IMAGE} --file ./qa/Dockerfile ./
     - time docker push ${QA_IMAGE}
 
-review-cleanup:
+.review-cleanup-base:
   extends:
     - .default-retry
     - .review:rules:review-cleanup
   stage: prepare
-  image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-charts-build-base
   allow_failure: true
   environment:
     name: review/auto-cleanup
@@ -36,6 +35,18 @@ review-cleanup:
   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

lib/quality/helm3_client.rb

+# frozen_string_literal: true
+
+require 'time'
+require_relative '../gitlab/popen' unless defined?(Gitlab::Popen)
+
+module Quality
+  class Helm3Client
+    CommandFailedError = Class.new(StandardError)
+
+    attr_reader :namespace
+
+    RELEASE_JSON_ATTRIBUTES = %w[name revision updated status chart app_version namespace].freeze
+    PAGINATION_SIZE = 256 # Default helm list pagination size
+
+    Release = Struct.new(:name, :revision, :last_update, :status, :chart, :app_version, :namespace) do
+      def revision
+        @revision ||= self[:revision].to_i
+      end
+
+      def status
+        @status ||= self[:status].downcase
+      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(namespace:, tiller_namespace: nil)
+      @namespace = namespace
+    end
+
+    def releases(args: [])
+      each_release(args)
+    end
+
+    def delete(release_name:)
+      run_command([
+        'uninstall',
+        %(--namespace "#{namespace}"),
+        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(page, args = [])
+      command = [
+        'list',
+        %(--namespace "#{namespace}"),
+        %(--max #{PAGINATION_SIZE}),
+        %(--offset #{PAGINATION_SIZE * page}),
+        %(--output json),
+        *args
+      ]
+      releases = JSON.parse(run_command(command))
+
+      releases.map do |release|
+        Release.new(*release.values_at(*RELEASE_JSON_ATTRIBUTES))
+      end
+    rescue JSON::ParserError => ex
+      puts "Ignoring this JSON parsing error: #{ex}" # rubocop:disable Rails/Output
+      []
+    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 = 0
+      final_args = args.dup
+
+      begin
+        collection = raw_releases(page, final_args)
+
+        yield Page.new(collection, page += 1)
+      end while collection.any?
+    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

@@ -2,6 +2,7 @@
 
 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__)
 
 class AutomatedCleanup
@@ -11,7 +12,8 @@ class AutomatedCleanup
   HELM_RELEASES_BATCH_SIZE = 5
   IGNORED_HELM_ERRORS = [
     'transport is closing',
-    'error upgrading connection'
+    'error upgrading connection',
+    'not found'
   ].freeze
   IGNORED_KUBERNETES_ERRORS = [
     'NotFound'
@@ -43,8 +45,16 @@ 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 ||= Quality::HelmClient.new(
+    @helm ||= helm_client_class.new(
       tiller_namespace: review_apps_namespace,
       namespace: review_apps_namespace)
   end
@@ -78,7 +88,7 @@ class AutomatedCleanup
       if deployed_at < delete_threshold
         deleted_environment = delete_environment(environment, deployment)
         if deleted_environment
-          release = Quality::HelmClient::Release.new(environment.slug, 1, deployed_at.to_s, nil, nil, review_apps_namespace)
+          release = helm_client_class::Release.new(environment.slug, 1, deployed_at.to_s, nil, nil, review_apps_namespace)
           releases_to_delete << release
         end
       else
@@ -97,7 +107,7 @@ class AutomatedCleanup
   end
 
   def perform_helm_releases_cleanup!(days:)
-    puts "Checking for Helm releases that are FAILED or not updated in the last #{days} days..."
+    puts "Checking for Helm releases that are failed or not updated in the last #{days} days..."
 
     threshold = threshold_time(days: days)
 
@@ -107,7 +117,7 @@ class AutomatedCleanup
       # Prevents deleting `dns-gitlab-review-app` releases or other unrelated releases
       next unless release.name.start_with?('review-')
 
-      if release.status == 'FAILED' || release.last_update < threshold
+      if release.status.casecmp('failed') == 0 || 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')
@@ -143,7 +153,8 @@ class AutomatedCleanup
   end
 
   def helm_releases
-    args = ['--all', '--date', "--max #{HELM_RELEASES_BATCH_SIZE}"]
+    args = ['--all', '--date']
+    args << "--max #{HELM_RELEASES_BATCH_SIZE}" unless helm3?
 
     helm.releases(args: args)
   end
@@ -159,7 +170,7 @@ class AutomatedCleanup
     helm.delete(release_name: releases_names)
     kubernetes.cleanup(release_name: releases_names, wait: false)
 
-  rescue Quality::HelmClient::CommandFailedError => ex
+  rescue helm_client_class::CommandFailedError => ex
     raise ex unless ignore_exception?(ex.message, IGNORED_HELM_ERRORS)
 
     puts "Ignoring the following Helm error:\n#{ex}\n"

spec/lib/quality/helm3_client_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+RSpec.describe Quality::Helm3Client do
+  let(:namespace) { 'review-apps-ee' }
+  let(:release_name) { 'my-release' }
+  let(:raw_helm_list_page1) do
+    <<~OUTPUT
+    [
+      {"name":"review-qa-60-reor-1mugd1","namespace":"#{namespace}","revision":1,"updated":"2020-04-03 17:27:10.245952 +0800 +08","status":"failed","chart":"gitlab-1.1.3","app_version":"12.9.2"},
+      {"name":"review-7846-fix-s-261vd6","namespace":"#{namespace}","revision":2,"updated":"2020-04-02 17:27:12.245952 +0800 +08","status":"deployed","chart":"gitlab-1.1.3","app_version":"12.9.2"},
+      {"name":"review-7867-snowp-lzo3iy","namespace":"#{namespace}","revision":1,"updated":"2020-04-02 15:27:12.245952 +0800 +08","status":"deployed","chart":"gitlab-1.1.3","app_version":"12.9.1"},
+      {"name":"review-6709-group-2pzeec","namespace":"#{namespace}","revision":2,"updated":"2020-04-01 21:27:12.245952 +0800 +08","status":"failed","chart":"gitlab-1.1.3","app_version":"12.9.1"}
+    ]
+    OUTPUT
+  end
+  let(:raw_helm_list_page2) do
+    <<~OUTPUT
+    [
+      {"name":"review-6709-group-t40qbv","namespace":"#{namespace}","revision":2,"updated":"2020-04-01 11:27:12.245952 +0800 +08","status":"deployed","chart":"gitlab-1.1.3","app_version":"12.9.1"}
+    ]
+    OUTPUT
+  end
+  let(:raw_helm_list_empty) do
+    <<~OUTPUT
+    []
+    OUTPUT
+  end
+
+  subject { described_class.new(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}" --max 256 --offset 0 --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}" --max 256 --offset 0 --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}" --max 256 --offset 0 --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}" --max 256 --offset 0 --output json --deployed)])
+        .and_return(Gitlab::Popen::Result.new([], raw_helm_list_page2, '', double(success?: true)))
+      expect(Gitlab::Popen).to receive(:popen_with_detail).ordered
+        .and_return(Gitlab::Popen::Result.new([], raw_helm_list_empty, '', 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: 2,
+        last_update: Time.parse('2020-04-01 11:27:12.245952 +0800 +08'),
+        status: 'deployed',
+        chart: 'gitlab-1.1.3',
+        app_version: '12.9.1',
+        namespace: namespace
+      )
+    end
+
+    it 'automatically paginates releases' do
+      expect(Gitlab::Popen).to receive(:popen_with_detail).ordered
+        .with([%(helm list --namespace "#{namespace}" --max 256 --offset 0 --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}" --max 256 --offset 256 --output json)])
+        .and_return(Gitlab::Popen::Result.new([], raw_helm_list_page2, '', double(success?: true)))
+      expect(Gitlab::Popen).to receive(:popen_with_detail).ordered
+        .with([%(helm list --namespace "#{namespace}" --max 256 --offset 512 --output json)])
+        .and_return(Gitlab::Popen::Result.new([], raw_helm_list_empty, '', double(success?: true)))
+      releases = subject.releases.to_a
+
+      expect(releases.size).to eq(5)
+      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 uninstall --namespace "#{namespace}" #{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 uninstall with default arguments' do
+      expect(Gitlab::Popen).to receive(:popen_with_detail)
+        .with([%(helm uninstall --namespace "#{namespace}" #{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 uninstall --namespace "#{namespace}" #{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 uninstall with multiple release names' do
+        expect(Gitlab::Popen).to receive(:popen_with_detail)
+                                   .with([%(helm uninstall --namespace "#{namespace}" #{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