Merge branch 'create-kubectl-delete-method-65343' into 'master'

Resolve "Follow-up from !30458: create `kubectl_delete(args)`"

See merge request gitlab-org/gitlab-ce!31353

6 files changed, 83 insertions, 12 deletions

diff --git a/app/models/clusters/applications/cert_manager.rb b/app/models/clusters/applications/cert_manager.rb
index 2fc1b67dfd2..6bd7473c8ff 100644
--- a/app/models/clusters/applications/cert_manager.rb
+++ b/app/models/clusters/applications/cert_manager.rb
@@ -64,11 +64,15 @@ module Clusters
       end
 
       def delete_private_key
-        "kubectl delete secret -n #{Gitlab::Kubernetes::Helm::NAMESPACE} #{private_key_name} --ignore-not-found" if private_key_name.present?
+        return unless private_key_name.present?
+
+        args = %W(secret -n #{Gitlab::Kubernetes::Helm::NAMESPACE} #{private_key_name} --ignore-not-found)
+
+        Gitlab::Kubernetes::KubectlCmd.delete(*args)
       end
 
       def delete_crd(definition)
-        "kubectl delete crd #{definition} --ignore-not-found"
+        Gitlab::Kubernetes::KubectlCmd.delete("crd", definition, "--ignore-not-found")
       end
 
       def cluster_issuer_file
diff --git a/app/models/clusters/applications/knative.rb b/app/models/clusters/applications/knative.rb
index 5eae23659ae..244fe738396 100644
--- a/app/models/clusters/applications/knative.rb
+++ b/app/models/clusters/applications/knative.rb
@@ -89,7 +89,7 @@ module Clusters
 
       def delete_knative_services
         cluster.kubernetes_namespaces.map do |kubernetes_namespace|
-          "kubectl delete ksvc --all -n #{kubernetes_namespace.namespace}"
+          Gitlab::Kubernetes::KubectlCmd.delete("ksvc", "--all", "-n", kubernetes_namespace.namespace)
         end
       end
 
@@ -99,14 +99,14 @@ module Clusters
 
       def delete_knative_namespaces
         [
-          "kubectl delete --ignore-not-found ns knative-serving",
-          "kubectl delete --ignore-not-found ns knative-build"
+          Gitlab::Kubernetes::KubectlCmd.delete("--ignore-not-found", "ns", "knative-serving"),
+          Gitlab::Kubernetes::KubectlCmd.delete("--ignore-not-found", "ns", "knative-build")
         ]
       end
 
       def delete_knative_and_istio_crds
         api_resources.map do |crd|
-          "kubectl delete --ignore-not-found crd #{crd}"
+          Gitlab::Kubernetes::KubectlCmd.delete("--ignore-not-found", "crd", "#{crd}")
         end
       end
 
@@ -119,13 +119,13 @@ module Clusters
       def install_knative_metrics
         return [] unless cluster.application_prometheus_available?
 
-        ["kubectl apply -f #{METRICS_CONFIG}"]
+        [Gitlab::Kubernetes::KubectlCmd.apply_file(METRICS_CONFIG)]
       end
 
       def delete_knative_istio_metrics
         return [] unless cluster.application_prometheus_available?
 
-        ["kubectl delete --ignore-not-found -f #{METRICS_CONFIG}"]
+        [Gitlab::Kubernetes::KubectlCmd.delete("--ignore-not-found", "-f", METRICS_CONFIG)]
       end
 
       def verify_cluster?
diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb
index 08e52f32bb3..f31a6b8b50e 100644
--- a/app/models/clusters/applications/prometheus.rb
+++ b/app/models/clusters/applications/prometheus.rb
@@ -106,13 +106,13 @@ module Clusters
       def install_knative_metrics
         return [] unless cluster.application_knative_available?
 
-        ["kubectl apply -f #{Clusters::Applications::Knative::METRICS_CONFIG}"]
+        [Gitlab::Kubernetes::KubectlCmd.apply_file(Clusters::Applications::Knative::METRICS_CONFIG)]
       end
 
       def delete_knative_istio_metrics
         return [] unless cluster.application_knative_available?
 
-        ["kubectl delete -f #{Clusters::Applications::Knative::METRICS_CONFIG}"]
+        [Gitlab::Kubernetes::KubectlCmd.delete("-f", Clusters::Applications::Knative::METRICS_CONFIG)]
       end
     end
   end
diff --git a/lib/gitlab/kubernetes/helm/reset_command.rb b/lib/gitlab/kubernetes/helm/reset_command.rb
index 37e1d8573ab..a35ffa34c58 100644
--- a/lib/gitlab/kubernetes/helm/reset_command.rb
+++ b/lib/gitlab/kubernetes/helm/reset_command.rb
@@ -38,9 +38,9 @@ module Gitlab
         # Tracking this method to be removed here:
         # https://gitlab.com/gitlab-org/gitlab-ce/issues/52791#note_199374155
         def delete_tiller_replicaset
-          command = %w[kubectl delete replicaset -n gitlab-managed-apps -l name=tiller]
+          delete_args = %w[replicaset -n gitlab-managed-apps -l name=tiller]
 
-          command.shelljoin
+          Gitlab::Kubernetes::KubectlCmd.delete(*delete_args)
         end
 
         def reset_helm_command
diff --git a/lib/gitlab/kubernetes/kubectl_cmd.rb b/lib/gitlab/kubernetes/kubectl_cmd.rb
new file mode 100644
index 00000000000..981eb5681dc
--- /dev/null
+++ b/lib/gitlab/kubernetes/kubectl_cmd.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Kubernetes
+    module KubectlCmd
+      class << self
+        def delete(*args)
+          %w(kubectl delete).concat(args).shelljoin
+        end
+
+        def apply_file(filename, *args)
+          raise ArgumentError, "filename is not present" unless filename.present?
+
+          %w(kubectl apply -f).concat([filename], args).shelljoin
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/kubernetes/kubectl_cmd_spec.rb b/spec/lib/gitlab/kubernetes/kubectl_cmd_spec.rb
new file mode 100644
index 00000000000..f24ab5579df
--- /dev/null
+++ b/spec/lib/gitlab/kubernetes/kubectl_cmd_spec.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+describe Gitlab::Kubernetes::KubectlCmd do
+  describe '.delete' do
+    it 'constructs string properly' do
+      args = %w(resource_type type --flag-1 --flag-2)
+
+      expected_command = 'kubectl delete resource_type type --flag-1 --flag-2'
+
+      expect(described_class.delete(*args)).to eq expected_command
+    end
+  end
+
+  describe '.apply_file' do
+    context 'without optional args' do
+      it 'requires filename to be present' do
+        expect { described_class.apply_file(nil) }.to raise_error(ArgumentError, "filename is not present")
+        expect { described_class.apply_file("  ") }.to raise_error(ArgumentError, "filename is not present")
+      end
+
+      it 'constructs string properly' do
+        expected_command = 'kubectl apply -f filename'
+
+        expect(described_class.apply_file('filename')).to eq expected_command
+      end
+    end
+
+    context 'with optional args' do
+      it 'constructs command properly with many args' do
+        args = %w(arg-1 --flag-0-1 arg-2 --flag-0-2)
+
+        expected_command = 'kubectl apply -f filename arg-1 --flag-0-1 arg-2 --flag-0-2'
+
+        expect(described_class.apply_file('filename', *args)).to eq expected_command
+      end
+
+      it 'constructs command properly with single arg' do
+        args = "arg-1"
+
+        expected_command = 'kubectl apply -f filename arg-1'
+
+        expect(described_class.apply_file('filename', args)).to eq(expected_command)
+      end
+    end
+  end
+end