Merge branch '34838-perform-multiple-helm-and-kubectl-clean-up-in-one-call' into 'master'

Resolve "Perform multiple helm and kubectl clean up in one call"

Closes #34838

See merge request gitlab-org/gitlab!19058

lib/quality/kubernetes_client.rb

@@ -13,6 +13,15 @@ module Quality
     end
 
     def cleanup(release_name:)
+      selector = case release_name
+                 when String
+                   %(-l release="#{release_name}")
+                 when Array
+                   %(-l 'release in (#{release_name.join(', ')})')
+                 else
+                   raise ArgumentError, 'release_name must be a string or an array'
+                 end
+
       command = [
         %(--namespace "#{namespace}"),
         'delete',
@@ -20,7 +29,7 @@ module Quality
         '--now',
         '--ignore-not-found',
         '--include-uninitialized',
-        %(-l release="#{release_name}")
+        selector
       ]
 
       run_command(command)

scripts/review_apps/automated_cleanup.rb

@@ -60,6 +60,8 @@ class AutomatedCleanup
     stop_threshold = threshold_time(days: days_for_stop)
     deployments_look_back_threshold = threshold_time(days: days_for_delete * 5)
 
+    releases_to_delete = []
+
     gitlab.deployments(project_path, per_page: DEPLOYMENTS_PER_PAGE, sort: 'desc').auto_paginate do |deployment|
       break if Time.parse(deployment.created_at) < deployments_look_back_threshold
 
@@ -75,7 +77,7 @@ class AutomatedCleanup
       if deployed_at < delete_threshold
         delete_environment(environment, deployment)
         release = Quality::HelmClient::Release.new(environment.slug, 1, deployed_at.to_s, nil, nil, review_apps_namespace)
-        delete_helm_release(release)
+        releases_to_delete << release
       elsif deployed_at < stop_threshold
         stop_environment(environment, deployment)
       else
@@ -84,6 +86,8 @@ class AutomatedCleanup
 
       checked_environments << environment.slug
     end
+
+    delete_helm_releases(releases_to_delete)
   end
 
   def perform_helm_releases_cleanup!(days:)
@@ -91,16 +95,20 @@ class AutomatedCleanup
 
     threshold_day = threshold_time(days: days)
 
+    releases_to_delete = []
+
     helm_releases.each do |release|
       # 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_day
-        delete_helm_release(release)
+        releases_to_delete << release
       else
         print_release_state(subject: 'Release', release_name: release.name, release_date: release.last_update, action: 'leaving')
       end
     end
+
+    delete_helm_releases(releases_to_delete)
   end
 
   private
@@ -121,10 +129,17 @@ class AutomatedCleanup
     helm.releases(args: args)
   end
 
-  def delete_helm_release(release)
-    print_release_state(subject: 'Release', release_name: release.name, release_status: release.status, release_date: release.last_update, action: 'cleaning')
-    helm.delete(release_name: release.name)
-    kubernetes.cleanup(release_name: release.name)
+  def delete_helm_releases(releases)
+    return if releases.empty?
+
+    releases.each do |release|
+      print_release_state(subject: 'Release', release_name: release.name, release_status: release.status, release_date: release.last_update, action: 'cleaning')
+    end
+
+    releases_names = releases.map(&:name)
+    helm.delete(release_name: releases_names)
+    kubernetes.cleanup(release_name: releases_names)
+
   rescue Quality::HelmClient::CommandFailedError => ex
     raise ex unless ignore_exception?(ex.message, IGNORED_HELM_ERRORS)
 

spec/lib/quality/helm_client_spec.rb

@@ -107,5 +107,25 @@ RSpec.describe Quality::HelmClient do
 
       expect(subject.delete(release_name: release_name)).to eq('')
     end
+
+    context 'with multiple release names' do
+      let(:release_name) { ['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 "#{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 "#{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

spec/lib/quality/kubernetes_client_spec.rb

@@ -29,5 +29,30 @@ RSpec.describe Quality::KubernetesClient do
       # We're not verifying the output here, just silencing it
       expect { subject.cleanup(release_name: release_name) }.to output.to_stdout
     end
+
+    context 'with multiple releases' do
+      let(:release_name) { ['my-release', 'my-release-2'] }
+
+      it 'raises an error if the Kubernetes command fails' do
+        expect(Gitlab::Popen).to receive(:popen_with_detail)
+                                   .with([%(kubectl --namespace "#{namespace}" delete ) \
+          'ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa ' \
+          "--now --ignore-not-found --include-uninitialized -l 'release in (#{release_name.join(', ')})'"])
+                                   .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false)))
+
+        expect { subject.cleanup(release_name: release_name) }.to raise_error(described_class::CommandFailedError)
+      end
+
+      it 'calls kubectl with the correct arguments' do
+        expect(Gitlab::Popen).to receive(:popen_with_detail)
+                                   .with([%(kubectl --namespace "#{namespace}" delete ) \
+          'ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa ' \
+          "--now --ignore-not-found --include-uninitialized -l 'release in (#{release_name.join(', ')})'"])
+                                   .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
+
+        # We're not verifying the output here, just silencing it
+        expect { subject.cleanup(release_name: release_name) }.to output.to_stdout
+      end
+    end
   end
 end