Merge branch '34869-use-wait-false-in-scripts-review_apps-automated_cleanup-rb' into 'master'

Use `--wait=false` in `scripts/review_apps/automated_cleanup.rb`

Closes #34869

See merge request gitlab-org/gitlab!19132

lib/quality/kubernetes_client.rb

@@ -12,7 +12,7 @@ module Quality
       @namespace = namespace
     end
 
-    def cleanup(release_name:)
+    def cleanup(release_name:, wait: true)
       selector = case release_name
                  when String
                    %(-l release="#{release_name}")
@@ -29,6 +29,7 @@ module Quality
         '--now',
         '--ignore-not-found',
         '--include-uninitialized',
+        %(--wait=#{wait}),
         selector
       ]
 

scripts/review_apps/automated_cleanup.rb

@@ -138,7 +138,7 @@ class AutomatedCleanup
 
     releases_names = releases.map(&:name)
     helm.delete(release_name: releases_names)
-    kubernetes.cleanup(release_name: releases_names)
+    kubernetes.cleanup(release_name: releases_names, wait: false)
 
   rescue Quality::HelmClient::CommandFailedError => ex
     raise ex unless ignore_exception?(ex.message, IGNORED_HELM_ERRORS)

spec/lib/quality/kubernetes_client_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe Quality::KubernetesClient 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=\"#{release_name}\""])
+          "--now --ignore-not-found --include-uninitialized --wait=true -l release=\"#{release_name}\""])
         .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false)))
 
       expect { subject.cleanup(release_name: release_name) }.to raise_error(described_class::CommandFailedError)
@@ -23,7 +23,7 @@ RSpec.describe Quality::KubernetesClient 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=\"#{release_name}\""])
+          "--now --ignore-not-found --include-uninitialized --wait=true -l release=\"#{release_name}\""])
         .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
 
       # We're not verifying the output here, just silencing it
@@ -37,7 +37,7 @@ RSpec.describe Quality::KubernetesClient 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(', ')})'"])
+          "--now --ignore-not-found --include-uninitialized --wait=true -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)
@@ -47,12 +47,35 @@ RSpec.describe Quality::KubernetesClient 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(', ')})'"])
+          "--now --ignore-not-found --include-uninitialized --wait=true -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
+
+    context 'with `wait: false`' do
+      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 --wait=false -l release=\"#{release_name}\""])
+                                   .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false)))
+
+        expect { subject.cleanup(release_name: release_name, wait: false) }.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 --wait=false -l release=\"#{release_name}\""])
+                                   .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, wait: false) }.to output.to_stdout
+      end
+    end
   end
 end