Merge branch...

Merge branch '16303-persistent-volume-claims-should-be-regularly-cleaned-up-for-review-apps' into 'master'

ci: Automate the cleanup of stale PVC

See merge request gitlab-org/gitlab!51488

Guardfile

@@ -6,7 +6,7 @@ require "guard/rspec/dsl"
 
 cmd = ENV['GUARD_CMD'] || (ENV['SPRING'] ? 'spring rspec' : 'bundle exec rspec')
 
-directories %w(app ee lib spec)
+directories %w(app ee lib rubocop tooling spec)
 
 rspec_context_for = proc do |context_path|
   OpenStruct.new(to_s: "spec").tap do |rspec|
@@ -42,6 +42,8 @@ guard_setup = proc do |context_path|
 
   # Ruby files
   watch(%r{^#{context_path}(lib/.+)\.rb$}) { |m| rspec.spec.call(m[1]) }
+  watch(%r{^#{context_path}(rubocop/.+)\.rb$}) { |m| rspec.spec.call(m[1]) }
+  watch(%r{^#{context_path}(tooling/.+)\.rb$}) { |m| rspec.spec.call(m[1]) }
 
   # Rails files
   rails = rails_context_for.call(context_path, %w(erb haml slim))

scripts/review_apps/automated_cleanup.rb

@@ -115,6 +115,10 @@ class AutomatedCleanup
     delete_helm_releases(releases_to_delete)
   end
 
+  def perform_stale_pvc_cleanup!(days:)
+    kubernetes.cleanup_by_created_at(resource_type: 'pvc', created_before: threshold_time(days: days), wait: false)
+  end
+
   private
 
   def fetch_environment(environment)
@@ -155,7 +159,7 @@ class AutomatedCleanup
 
     releases_names = releases.map(&:name)
     helm.delete(release_name: releases_names)
-    kubernetes.cleanup(release_name: releases_names, wait: false)
+    kubernetes.cleanup_by_release(release_name: releases_names, wait: false)
 
   rescue Tooling::Helm3Client::CommandFailedError => ex
     raise ex unless ignore_exception?(ex.message, IGNORED_HELM_ERRORS)
@@ -198,4 +202,8 @@ timed('Helm releases cleanup') do
   automated_cleanup.perform_helm_releases_cleanup!(days: 7)
 end
 
+timed('Stale PVC cleanup') do
+  automated_cleanup.perform_stale_pvc_cleanup!(days: 30)
+end
+
 exit(0)

spec/tooling/lib/tooling/kubernetes_client_spec.rb

@@ -17,84 +17,111 @@ RSpec.describe Tooling::KubernetesClient do
     end
   end
 
-  describe '#cleanup' do
+  describe '#cleanup_by_release' do
     before do
       allow(subject).to receive(:raw_resource_names).and_return(raw_resource_names)
     end
 
+    shared_examples 'a kubectl command to delete resources' do
+      let(:wait) { true }
+      let(:release_names_in_command) { release_name.respond_to?(:join) ? %(-l 'release in (#{release_name.join(', ')})') : %(-l release="#{release_name}") }
+
+      specify do
+        expect(Gitlab::Popen).to receive(:popen_with_detail)
+          .with(["kubectl delete #{described_class::RESOURCE_LIST} " +
+            %(--namespace "#{namespace}" --now --ignore-not-found --include-uninitialized --wait=#{wait} #{release_names_in_command})])
+          .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
+
+        expect(Gitlab::Popen).to receive(:popen_with_detail)
+          .with([%(kubectl delete --namespace "#{namespace}" --ignore-not-found #{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_by_release(release_name: release_name) }.to output.to_stdout
+      end
+    end
+
     it 'raises an error if the Kubernetes command fails' do
       expect(Gitlab::Popen).to receive(:popen_with_detail)
         .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)
+      expect { subject.cleanup_by_release(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 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)))
+    it_behaves_like 'a kubectl command to delete resources'
 
-      expect(Gitlab::Popen).to receive(:popen_with_detail)
-        .with([%(kubectl delete --namespace "#{namespace}" --ignore-not-found #{pod_for_release})])
-        .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
+    context 'with multiple releases' do
+      let(:release_name) { %w[my-release my-release-2] }
 
-      # We're not verifying the output here, just silencing it
-      expect { subject.cleanup(release_name: release_name) }.to output.to_stdout
+      it_behaves_like 'a kubectl command to delete resources'
     end
 
-    context 'with multiple releases' do
-      let(:release_name) { %w[my-release my-release-2] }
+    context 'with `wait: false`' do
+      let(:wait) { false }
 
-      it 'raises an error if the Kubernetes command fails' do
-        expect(Gitlab::Popen).to receive(:popen_with_detail)
-          .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)))
+      it_behaves_like 'a kubectl command to delete resources'
+    end
+  end
 
-        expect { subject.cleanup(release_name: release_name) }.to raise_error(described_class::CommandFailedError)
-      end
+  describe '#cleanup_by_created_at' do
+    let(:two_days_ago) { Time.now - 3600 * 24 * 2 }
+    let(:resource_type) { 'pvc' }
+    let(:resource_names) { [pod_for_release] }
 
-      it 'calls kubectl with the correct arguments' do
-        expect(Gitlab::Popen).to receive(:popen_with_detail)
-          .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)))
+    before do
+      allow(subject).to receive(:resource_names_created_before).with(resource_type: resource_type, created_before: two_days_ago).and_return(resource_names)
+    end
+
+    shared_examples 'a kubectl command to delete resources by older than given creation time' do
+      let(:wait) { true }
+      let(:release_names_in_command) { resource_names.join(' ') }
 
+      specify do
         expect(Gitlab::Popen).to receive(:popen_with_detail)
-         .with([%(kubectl delete --namespace "#{namespace}" --ignore-not-found #{pod_for_release})])
-         .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
+          .with(["kubectl delete #{resource_type} ".squeeze(' ') +
+            %(--namespace "#{namespace}" --now --ignore-not-found --include-uninitialized --wait=#{wait} #{release_names_in_command})])
+          .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
+        expect { subject.cleanup_by_created_at(resource_type: resource_type, created_before: two_days_ago) }.to output.to_stdout
       end
     end
 
+    it 'raises an error if the Kubernetes command fails' do
+      expect(Gitlab::Popen).to receive(:popen_with_detail)
+        .with(["kubectl delete #{resource_type} " +
+          %(--namespace "#{namespace}" --now --ignore-not-found --include-uninitialized --wait=true #{pod_for_release})])
+        .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false)))
+
+      expect { subject.cleanup_by_created_at(resource_type: resource_type, created_before: two_days_ago) }.to raise_error(described_class::CommandFailedError)
+    end
+
+    it_behaves_like 'a kubectl command to delete resources by older than given creation time'
+
+    context 'with multiple resource names' do
+      let(:resource_names) { %w[pod-1 pod-2] }
+
+      it_behaves_like 'a kubectl command to delete resources by older than given creation time'
+    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 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)))
+      let(:wait) { false }
 
-        expect { subject.cleanup(release_name: release_name, wait: false) }.to raise_error(described_class::CommandFailedError)
-      end
+      it_behaves_like 'a kubectl command to delete resources by older than given creation time'
+    end
 
-      it 'calls kubectl with the correct arguments' do
-        expect(Gitlab::Popen).to receive(:popen_with_detail)
-          .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)))
+    context 'with no resource_type given' do
+      let(:resource_type) { nil }
 
-        expect(Gitlab::Popen).to receive(:popen_with_detail)
-          .with([%(kubectl delete --namespace "#{namespace}" --ignore-not-found #{pod_for_release})])
-          .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true)))
+      it_behaves_like 'a kubectl command to delete resources by older than given creation time'
+    end
 
-        # We're not verifying the output here, just silencing it
-        expect { subject.cleanup(release_name: release_name, wait: false) }.to output.to_stdout
-      end
+    context 'with multiple resource_type given' do
+      let(:resource_type) { 'pvc,service' }
+
+      it_behaves_like 'a kubectl command to delete resources by older than given creation time'
     end
   end
 
@@ -108,4 +135,59 @@ RSpec.describe Tooling::KubernetesClient do
       expect(subject.__send__(:raw_resource_names)).to eq(raw_resource_names)
     end
   end
+
+  describe '#resource_names_created_before' do
+    let(:three_days_ago) { Time.now - 3600 * 24 * 3 }
+    let(:two_days_ago) { Time.now - 3600 * 24 * 2 }
+    let(:pvc_created_three_days_ago) { 'pvc-created-three-days-ago' }
+    let(:resource_type) { 'pvc' }
+    let(:raw_resources) do
+      {
+        items: [
+          {
+            apiVersion: "v1",
+            kind: "PersistentVolumeClaim",
+            metadata: {
+                creationTimestamp: three_days_ago,
+                name: pvc_created_three_days_ago
+            }
+          },
+          {
+            apiVersion: "v1",
+            kind: "PersistentVolumeClaim",
+            metadata: {
+                creationTimestamp: Time.now,
+                name: 'another-pvc'
+            }
+          }
+        ]
+      }.to_json
+    end
+
+    shared_examples 'a kubectl command to retrieve resource names sorted by creationTimestamp' do
+      specify do
+        expect(Gitlab::Popen).to receive(:popen_with_detail)
+          .with(["kubectl get #{resource_type} ".squeeze(' ') +
+            %(--namespace "#{namespace}" ) +
+            "--sort-by='{.metadata.creationTimestamp}' -o json"])
+          .and_return(Gitlab::Popen::Result.new([], raw_resources, '', double(success?: true)))
+
+        expect(subject.__send__(:resource_names_created_before, resource_type: resource_type, created_before: two_days_ago)).to contain_exactly(pvc_created_three_days_ago)
+      end
+    end
+
+    it_behaves_like 'a kubectl command to retrieve resource names sorted by creationTimestamp'
+
+    context 'with no resource_type given' do
+      let(:resource_type) { nil }
+
+      it_behaves_like 'a kubectl command to retrieve resource names sorted by creationTimestamp'
+    end
+
+    context 'with multiple resource_type given' do
+      let(:resource_type) { 'pvc,service' }
+
+      it_behaves_like 'a kubectl command to retrieve resource names sorted by creationTimestamp'
+    end
+  end
 end

tooling/lib/tooling/helm3_client.rb

@@ -66,13 +66,15 @@ module Tooling
         %(--output json),
         *args
       ]
-      releases = JSON.parse(run_command(command)) # rubocop:disable Gitlab/Json
+
+      response = run_command(command)
+      releases = JSON.parse(response) # rubocop:disable Gitlab/Json
 
       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
+      puts "Ignoring this JSON parsing error: #{ex}\n\nResponse was:\n#{response}" # rubocop:disable Rails/Output
       []
     end
 

tooling/lib/tooling/kubernetes_client.rb

 # frozen_string_literal: true
 
+require 'json'
+require 'time'
 require_relative '../../../lib/gitlab/popen' unless defined?(Gitlab::Popen)
-require_relative '../../../lib/gitlab/json' unless defined?(Gitlab::JSON)
 
 module Tooling
   class KubernetesClient
@@ -14,11 +15,16 @@ module Tooling
       @namespace = namespace
     end
 
-    def cleanup(release_name:, wait: true)
+    def cleanup_by_release(release_name:, wait: true)
       delete_by_selector(release_name: release_name, wait: wait)
       delete_by_matching_name(release_name: release_name)
     end
 
+    def cleanup_by_created_at(resource_type:, created_before:, wait: true)
+      resource_names = resource_names_created_before(resource_type: resource_type, created_before: created_before)
+      delete_by_exact_names(resource_type: resource_type, resource_names: resource_names, wait: wait)
+    end
+
     private
 
     def delete_by_selector(release_name:, wait:)
@@ -45,6 +51,21 @@ module Tooling
       run_command(command)
     end
 
+    def delete_by_exact_names(resource_names:, wait:, resource_type: nil)
+      command = [
+        'delete',
+        resource_type,
+        %(--namespace "#{namespace}"),
+        '--now',
+        '--ignore-not-found',
+        '--include-uninitialized',
+        %(--wait=#{wait}),
+        resource_names.join(' ')
+      ]
+
+      run_command(command)
+    end
+
     def delete_by_matching_name(release_name:)
       resource_names = raw_resource_names
       command = [
@@ -70,8 +91,26 @@ module Tooling
       run_command(command).lines.map(&:strip)
     end
 
+    def resource_names_created_before(resource_type:, created_before:)
+      command = [
+        'get',
+        resource_type,
+        %(--namespace "#{namespace}"),
+        "--sort-by='{.metadata.creationTimestamp}'",
+        '-o json'
+      ]
+
+      response = run_command(command)
+      JSON.parse(response)['items'] # rubocop:disable Gitlab/Json
+        .map { |resource| resource.dig('metadata', 'name') if Time.parse(resource.dig('metadata', 'creationTimestamp')) < created_before }
+        .compact
+    rescue ::JSON::ParserError => ex
+      puts "Ignoring this JSON parsing error: #{ex}\n\nResponse was:\n#{response}" # rubocop:disable Rails/Output
+      []
+    end
+
     def run_command(command)
-      final_command = ['kubectl', *command].join(' ')
+      final_command = ['kubectl', *command.compact].join(' ')
       puts "Running command: `#{final_command}`" # rubocop:disable Rails/Output
 
       result = Gitlab::Popen.popen_with_detail([final_command])