Merge branch '37330-improve-review-app-cleanup' into 'master'

Cleanup Review App components by name in addition to by label

Closes #37330

See merge request gitlab-org/gitlab!20775

lib/quality/kubernetes_client.rb

@@ -4,6 +4,7 @@ require_relative '../gitlab/popen' unless defined?(Gitlab::Popen)
 
 module Quality
   class KubernetesClient
+    RESOURCE_LIST = 'ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa,crd'
     CommandFailedError = Class.new(StandardError)
 
     attr_reader :namespace
@@ -13,6 +14,13 @@ module Quality
     end
 
     def cleanup(release_name:, wait: true)
+      delete_by_selector(release_name: release_name, wait: wait)
+      delete_by_matching_name(release_name: release_name)
+    end
+
+    private
+
+    def delete_by_selector(release_name:, wait:)
       selector = case release_name
                  when String
                    %(-l release="#{release_name}")
@@ -23,9 +31,9 @@ module Quality
                  end
 
       command = [
-        %(--namespace "#{namespace}"),
         'delete',
-        'ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa',
+        RESOURCE_LIST,
+        %(--namespace "#{namespace}"),
         '--now',
         '--ignore-not-found',
         '--include-uninitialized',
@@ -36,7 +44,29 @@ module Quality
       run_command(command)
     end
 
-    private
+    def delete_by_matching_name(release_name:)
+      resource_names = raw_resource_names
+      command = [
+        'delete',
+        %(--namespace "#{namespace}")
+      ]
+
+      Array(release_name).each do |release|
+        resource_names
+          .select { |resource_name| resource_name.include?(release) }
+          .each { |matching_resource| run_command(command + [matching_resource]) }
+      end
+    end
+
+    def raw_resource_names
+      command = [
+        'get',
+        RESOURCE_LIST,
+        %(--namespace "#{namespace}"),
+        '-o custom-columns=NAME:.metadata.name'
+      ]
+      run_command(command).lines.map(&:strip)
+    end
 
     def run_command(command)
       final_command = ['kubectl', *command].join(' ')

scripts/review_apps/review-apps.sh

@@ -48,11 +48,31 @@ function delete_release() {
     return
   fi
 
-  echoinfo "Deleting release '${release}'..." true
+  helm_delete_release "${namespace}" "${release}"
+  kubectl_cleanup_release "${namespace}" "${release}"
+}
+
+function helm_delete_release() {
+  local namespace="${1}"
+  local release="${2}"
+
+  echoinfo "Deleting Helm release '${release}'..." true
 
   helm delete --tiller-namespace "${namespace}" --purge "${release}"
 }
 
+function kubectl_cleanup_release() {
+  local namespace="${1}"
+  local release="${2}"
+
+  echoinfo "Deleting all K8s resources matching '${release}'..." true
+  kubectl --namespace "${namespace}" get ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa,crd 2>&1 \
+    | grep "${release}" \
+    | awk '{print $1}' \
+    | xargs kubectl --namespace "${namespace}" delete \
+    || true
+}
+
 function delete_failed_release() {
   local namespace="${KUBE_NAMESPACE}"
   local release="${CI_ENVIRONMENT_SLUG}"

spec/lib/quality/kubernetes_client_spec.rb

@@ -5,15 +5,27 @@ require 'fast_spec_helper'
 RSpec.describe Quality::KubernetesClient do
   let(:namespace) { 'review-apps-ee' }
   let(:release_name) { 'my-release' }
+  let(:pod_for_release) { "pod-my-release-abcd" }
+  let(:raw_resource_names_str) { "NAME\nfoo\n#{pod_for_release}\nbar" }
+  let(:raw_resource_names) { raw_resource_names_str.lines.map(&:strip) }
 
   subject { described_class.new(namespace: namespace) }
 
+  describe 'RESOURCE_LIST' do
+    it 'returns the correct list of resources separated by commas' do
+      expect(described_class::RESOURCE_LIST).to eq('ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,secret,clusterrole,clusterrolebinding,role,rolebinding,sa,crd')
+    end
+  end
+
   describe '#cleanup' do
+    before do
+      allow(subject).to receive(:raw_resource_names).and_return(raw_resource_names)
+    end
+
     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=true -l release=\"#{release_name}\""])
+        .with(["kubectl delete #{described_class::RESOURCE_LIST} " +
+          %(--namespace "#{namespace}" --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)
@@ -21,9 +33,12 @@ RSpec.describe Quality::KubernetesClient do
 
     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=true -l release=\"#{release_name}\""])
+        .with(["kubectl delete #{described_class::RESOURCE_LIST} " +
+          %(--namespace "#{namespace}" --now --ignore-not-found --include-uninitialized --wait=true -l release="#{release_name}")])
+        .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
+
+      expect(Gitlab::Popen).to receive(:popen_with_detail)
+        .with([%(kubectl delete --namespace "#{namespace}" #{pod_for_release})])
         .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
 
       # We're not verifying the output here, just silencing it
@@ -35,20 +50,22 @@ RSpec.describe Quality::KubernetesClient 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=true -l 'release in (#{release_name.join(', ')})'"])
-                                   .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false)))
+          .with(["kubectl delete #{described_class::RESOURCE_LIST} " +
+            %(--namespace "#{namespace}" --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)
       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=true -l 'release in (#{release_name.join(', ')})'"])
-                                   .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
+          .with(["kubectl delete #{described_class::RESOURCE_LIST} " +
+            %(--namespace "#{namespace}" --now --ignore-not-found --include-uninitialized --wait=true -l 'release in (#{release_name.join(', ')})')])
+          .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
+
+        expect(Gitlab::Popen).to receive(:popen_with_detail)
+         .with([%(kubectl delete --namespace "#{namespace}" #{pod_for_release})])
+         .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
@@ -58,24 +75,37 @@ RSpec.describe Quality::KubernetesClient do
     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)))
+          .with(["kubectl delete #{described_class::RESOURCE_LIST} " +
+            %(--namespace "#{namespace}" --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)))
+          .with(["kubectl delete #{described_class::RESOURCE_LIST} " +
+            %(--namespace "#{namespace}" --now --ignore-not-found --include-uninitialized --wait=false -l release="#{release_name}")])
+          .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
+
+        expect(Gitlab::Popen).to receive(:popen_with_detail)
+          .with([%(kubectl delete --namespace "#{namespace}" #{pod_for_release})])
+          .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
+
+  describe '#raw_resource_names' do
+    it 'calls kubectl to retrieve the resource names' do
+      expect(Gitlab::Popen).to receive(:popen_with_detail)
+        .with(["kubectl get #{described_class::RESOURCE_LIST} " +
+          %(--namespace "#{namespace}" -o custom-columns=NAME:.metadata.name)])
+        .and_return(Gitlab::Popen::Result.new([], raw_resource_names_str, '', double(success?: true)))
+
+      expect(subject.__send__(:raw_resource_names)).to eq(raw_resource_names)
+    end
+  end
 end