Modify service so that it can be re-run

If the service fails mid-point, then we should be able to re-run this
service. So, detect presence of any previously created Kubernetes
resource and update or create accordingly.

Fix specs accordingly. In the case of finalize_creation_service_spec.rb,
I decided to stub out the async worker rather than maintaining
individual stubs for various kubeclient calls for that worker.

Also add test cases for group clusters

app/services/clusters/gcp/kubernetes/create_service_account_service.rb

@@ -38,8 +38,9 @@ module Clusters
 
         def execute
           ensure_project_namespace_exists if namespace_creator
-          kubeclient.create_service_account(service_account_resource)
-          kubeclient.create_secret(service_account_token_resource)
+
+          kubeclient.create_or_update_service_account(service_account_resource)
+          kubeclient.create_or_update_secret(service_account_token_resource)
           create_role_or_cluster_role_binding if rbac
         end
 
@@ -56,9 +57,9 @@ module Clusters
 
         def create_role_or_cluster_role_binding
           if namespace_creator
-            kubeclient.create_role_binding(role_binding_resource)
+            kubeclient.create_or_update_role_binding(role_binding_resource)
           else
-            kubeclient.create_cluster_role_binding(cluster_role_binding_resource)
+            kubeclient.create_or_update_cluster_role_binding(cluster_role_binding_resource)
           end
         end
 

changelogs/unreleased/retryable_create_or_update_kubernetes_namespace.yml

+---
+title: Updates service to update Kubernetes project namespaces and restricted service
+  account if present
+merge_request: 23525
+author:
+type: changed

lib/gitlab/kubernetes/kube_client.rb

@@ -46,6 +46,7 @@ module Gitlab
         :create_secret,
         :create_service_account,
         :update_config_map,
+        :update_secret,
         :update_service_account,
         to: :core_client
 
@@ -80,8 +81,64 @@ module Gitlab
         @kubeclient_options = kubeclient_options
       end
 
+      def create_or_update_cluster_role_binding(resource)
+        if cluster_role_binding_exists?(resource)
+          update_cluster_role_binding(resource)
+        else
+          create_cluster_role_binding(resource)
+        end
+      end
+
+      def create_or_update_role_binding(resource)
+        if role_binding_exists?(resource)
+          update_role_binding(resource)
+        else
+          create_role_binding(resource)
+        end
+      end
+
+      def create_or_update_service_account(resource)
+        if service_account_exists?(resource)
+          update_service_account(resource)
+        else
+          create_service_account(resource)
+        end
+      end
+
+      def create_or_update_secret(resource)
+        if secret_exists?(resource)
+          update_secret(resource)
+        else
+          create_secret(resource)
+        end
+      end
+
       private
 
+      def cluster_role_binding_exists?(resource)
+        get_cluster_role_binding(resource.metadata.name)
+      rescue ::Kubeclient::ResourceNotFoundError
+        false
+      end
+
+      def role_binding_exists?(resource)
+        get_role_binding(resource.metadata.name, resource.metadata.namespace)
+      rescue ::Kubeclient::ResourceNotFoundError
+        false
+      end
+
+      def service_account_exists?(resource)
+        get_service_account(resource.metadata.name, resource.metadata.namespace)
+      rescue ::Kubeclient::ResourceNotFoundError
+        false
+      end
+
+      def secret_exists?(resource)
+        get_secret(resource.metadata.name, resource.metadata.namespace)
+      rescue ::Kubeclient::ResourceNotFoundError
+        false
+      end
+
       def build_kubeclient(api_group, api_version)
         ::Kubeclient::Client.new(
           join_api_url(api_prefix, api_group),

spec/factories/clusters/kubernetes_namespaces.rb

@@ -5,10 +5,12 @@ FactoryBot.define do
     association :cluster, :project, :provided_by_gcp
 
     after(:build) do |kubernetes_namespace|
-      cluster_project = kubernetes_namespace.cluster.cluster_project
+      if kubernetes_namespace.cluster.project_type?
+        cluster_project = kubernetes_namespace.cluster.cluster_project
 
-      kubernetes_namespace.project = cluster_project.project
-      kubernetes_namespace.cluster_project = cluster_project
+        kubernetes_namespace.project = cluster_project.project
+        kubernetes_namespace.cluster_project = cluster_project
+      end
     end
 
     trait :with_token do

spec/lib/gitlab/kubernetes/kube_client_spec.rb

@@ -99,6 +99,7 @@ describe Gitlab::Kubernetes::KubeClient do
       :create_secret,
       :create_service_account,
       :update_config_map,
+      :update_secret,
       :update_service_account
     ].each do |method|
       describe "##{method}" do
@@ -174,6 +175,84 @@ describe Gitlab::Kubernetes::KubeClient do
     end
   end
 
+  shared_examples 'create_or_update method' do
+    let(:get_method) { "get_#{resource_type}" }
+    let(:update_method) { "update_#{resource_type}" }
+    let(:create_method) { "create_#{resource_type}" }
+
+    context 'resource exists' do
+      before do
+        expect(client).to receive(get_method).and_return(resource)
+      end
+
+      it 'calls the update method' do
+        expect(client).to receive(update_method).with(resource)
+
+        subject
+      end
+    end
+
+    context 'resource does not exist' do
+      before do
+        expect(client).to receive(get_method).and_raise(Kubeclient::ResourceNotFoundError.new(404, 'Not found', nil))
+      end
+
+      it 'calls the create method' do
+        expect(client).to receive(create_method).with(resource)
+
+        subject
+      end
+    end
+  end
+
+  describe '#create_or_update_cluster_role_binding' do
+    let(:resource_type) { 'cluster_role_binding' }
+
+    let(:resource) do
+      ::Kubeclient::Resource.new(metadata: { name: 'name', namespace: 'namespace' })
+    end
+
+    subject { client.create_or_update_cluster_role_binding(resource) }
+
+    it_behaves_like 'create_or_update method'
+  end
+
+  describe '#create_or_update_role_binding' do
+    let(:resource_type) { 'role_binding' }
+
+    let(:resource) do
+      ::Kubeclient::Resource.new(metadata: { name: 'name', namespace: 'namespace' })
+    end
+
+    subject { client.create_or_update_role_binding(resource) }
+
+    it_behaves_like 'create_or_update method'
+  end
+
+  describe '#create_or_update_service_account' do
+    let(:resource_type) { 'service_account' }
+
+    let(:resource) do
+      ::Kubeclient::Resource.new(metadata: { name: 'name', namespace: 'namespace' })
+    end
+
+    subject { client.create_or_update_service_account(resource) }
+
+    it_behaves_like 'create_or_update method'
+  end
+
+  describe '#create_or_update_secret' do
+    let(:resource_type) { 'secret' }
+
+    let(:resource) do
+      ::Kubeclient::Resource.new(metadata: { name: 'name', namespace: 'namespace' })
+    end
+
+    subject { client.create_or_update_secret(resource) }
+
+    it_behaves_like 'create_or_update method'
+  end
+
   describe 'methods that do not exist on any client' do
     it 'throws an error' do
       expect { client.non_existent_method }.to raise_error(NoMethodError)

spec/services/clusters/gcp/finalize_creation_service_spec.rb

@@ -19,6 +19,10 @@ describe Clusters::Gcp::FinalizeCreationService, '#execute' do
 
   subject { described_class.new.execute(provider) }
 
+  before do
+    allow(ClusterPlatformConfigureWorker).to receive(:perform_async)
+  end
+
   shared_examples 'success' do
     it 'configures provider and kubernetes' do
       subject
@@ -39,16 +43,6 @@ describe Clusters::Gcp::FinalizeCreationService, '#execute' do
       expect(platform.token).to eq(token)
     end
 
-    it 'creates kubernetes namespace model' do
-      subject
-
-      kubernetes_namespace = cluster.reload.kubernetes_namespace
-      expect(kubernetes_namespace).to be_persisted
-      expect(kubernetes_namespace.namespace).to eq(namespace)
-      expect(kubernetes_namespace.service_account_name).to eq("#{namespace}-service-account")
-      expect(kubernetes_namespace.service_account_token).to be_present
-    end
-
     it 'calls ClusterPlatformConfigureWorker in a ascync fashion' do
       expect(ClusterPlatformConfigureWorker).to receive(:perform_async).with(cluster.id)
 
@@ -110,8 +104,10 @@ describe Clusters::Gcp::FinalizeCreationService, '#execute' do
       stub_kubeclient_discover(api_url)
       stub_kubeclient_get_namespace(api_url)
       stub_kubeclient_create_namespace(api_url)
+      stub_kubeclient_get_service_account_error(api_url, 'gitlab')
       stub_kubeclient_create_service_account(api_url)
       stub_kubeclient_create_secret(api_url)
+      stub_kubeclient_put_secret(api_url, 'gitlab-token')
 
       stub_kubeclient_get_secret(
         api_url,
@@ -121,19 +117,6 @@ describe Clusters::Gcp::FinalizeCreationService, '#execute' do
           namespace: 'default'
         }
       )
-
-      stub_kubeclient_get_namespace(api_url, namespace: namespace)
-      stub_kubeclient_create_service_account(api_url, namespace: namespace)
-      stub_kubeclient_create_secret(api_url, namespace: namespace)
-
-      stub_kubeclient_get_secret(
-        api_url,
-        {
-          metadata_name: "#{namespace}-token",
-          token: Base64.encode64(token),
-          namespace: namespace
-        }
-      )
     end
   end
 
@@ -161,8 +144,8 @@ describe Clusters::Gcp::FinalizeCreationService, '#execute' do
     before do
       provider.legacy_abac = false
 
+      stub_kubeclient_get_cluster_role_binding_error(api_url, 'gitlab-admin')
       stub_kubeclient_create_cluster_role_binding(api_url)
-      stub_kubeclient_create_role_binding(api_url, namespace: namespace)
     end
 
     include_context 'kubernetes information successfully fetched'

spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb

@@ -10,6 +10,7 @@ describe Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService, '#execute' d
   let(:api_url) { 'https://kubernetes.example.com' }
   let(:project) { cluster.project }
   let(:cluster_project) { cluster.cluster_project }
+  let(:namespace) { "#{project.path}-#{project.id}" }
 
   subject do
     described_class.new(
@@ -18,40 +19,31 @@ describe Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService, '#execute' d
     ).execute
   end
 
-  shared_context 'kubernetes requests' do
-    before do
-      stub_kubeclient_discover(api_url)
-      stub_kubeclient_get_namespace(api_url)
-      stub_kubeclient_create_service_account(api_url)
-      stub_kubeclient_create_secret(api_url)
-
-      stub_kubeclient_get_namespace(api_url, namespace: namespace)
-      stub_kubeclient_create_service_account(api_url, namespace: namespace)
-      stub_kubeclient_create_secret(api_url, namespace: namespace)
-
-      stub_kubeclient_get_secret(
-        api_url,
-        {
-          metadata_name: "#{namespace}-token",
-          token: Base64.encode64('sample-token'),
-          namespace: namespace
-        }
-      )
-    end
+  before do
+    stub_kubeclient_discover(api_url)
+    stub_kubeclient_get_namespace(api_url)
+    stub_kubeclient_get_service_account_error(api_url, 'gitlab')
+    stub_kubeclient_create_service_account(api_url)
+    stub_kubeclient_get_secret_error(api_url, 'gitlab-token')
+    stub_kubeclient_create_secret(api_url)
+
+    stub_kubeclient_get_namespace(api_url, namespace: namespace)
+    stub_kubeclient_get_service_account_error(api_url, "#{namespace}-service-account", namespace: namespace)
+    stub_kubeclient_create_service_account(api_url, namespace: namespace)
+    stub_kubeclient_create_secret(api_url, namespace: namespace)
+    stub_kubeclient_put_secret(api_url, "#{namespace}-token", namespace: namespace)
+
+    stub_kubeclient_get_secret(
+      api_url,
+      {
+        metadata_name: "#{namespace}-token",
+        token: Base64.encode64('sample-token'),
+        namespace: namespace
+      }
+    )
   end
 
-  context 'when kubernetes namespace is not persisted' do
-    let(:namespace) { "#{project.path}-#{project.id}" }
-
-    let(:kubernetes_namespace) do
-      create(:cluster_kubernetes_namespace,
-            cluster: cluster,
-            project: cluster_project.project,
-            cluster_project: cluster_project)
-    end
-
-    include_context 'kubernetes requests'
-
+  shared_examples 'successful creation of kubernetes namespace' do
     it 'creates a Clusters::KubernetesNamespace' do
       expect do
         subject
@@ -74,42 +66,69 @@ describe Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService, '#execute' d
     end
   end
 
-  context 'when there is a Kubernetes Namespace associated' do
-    let(:namespace) { 'new-namespace' }
+  context 'group clusters' do
+    let(:cluster) { create(:cluster, :group, :provided_by_gcp) }
+    let(:group) { cluster.group }
+    let(:project) { create(:project, group: group) }
+
+    context 'when kubernetes namespace is not persisted' do
+      let(:kubernetes_namespace) do
+        build(:cluster_kubernetes_namespace,
+              cluster: cluster,
+              project: project)
+      end
 
-    let(:kubernetes_namespace) do
-      create(:cluster_kubernetes_namespace,
-             cluster: cluster,
-             project: cluster_project.project,
-             cluster_project: cluster_project)
+      it_behaves_like 'successful creation of kubernetes namespace'
     end
+  end
 
-    include_context 'kubernetes requests'
+  context 'project clusters' do
+    context 'when kubernetes namespace is not persisted' do
+      let(:kubernetes_namespace) do
+        build(:cluster_kubernetes_namespace,
+              cluster: cluster,
+              project: cluster_project.project,
+              cluster_project: cluster_project)
+      end
 
-    before do
-      platform.update_column(:namespace, 'new-namespace')
+      it_behaves_like 'successful creation of kubernetes namespace'
     end
 
-    it 'does not create any Clusters::KubernetesNamespace' do
-      subject
+    context 'when there is a Kubernetes Namespace associated' do
+      let(:namespace) { 'new-namespace' }
 
-      expect(cluster.kubernetes_namespace).to eq(kubernetes_namespace)
-    end
+      let(:kubernetes_namespace) do
+        create(:cluster_kubernetes_namespace,
+               cluster: cluster,
+               project: cluster_project.project,
+               cluster_project: cluster_project)
+      end
 
-    it 'creates project service account' do
-      expect_any_instance_of(Clusters::Gcp::Kubernetes::CreateServiceAccountService).to receive(:execute).once
+      before do
+        platform.update_column(:namespace, 'new-namespace')
+      end
 
-      subject
-    end
+      it 'does not create any Clusters::KubernetesNamespace' do
+        subject
 
-    it 'updates Clusters::KubernetesNamespace' do
-      subject
+        expect(cluster.kubernetes_namespace).to eq(kubernetes_namespace)
+      end
 
-      kubernetes_namespace.reload
+      it 'creates project service account' do
+        expect_any_instance_of(Clusters::Gcp::Kubernetes::CreateServiceAccountService).to receive(:execute).once
 
-      expect(kubernetes_namespace.namespace).to eq(namespace)
-      expect(kubernetes_namespace.service_account_name).to eq("#{namespace}-service-account")
-      expect(kubernetes_namespace.encrypted_service_account_token).to be_present
+        subject
+      end
+
+      it 'updates Clusters::KubernetesNamespace' do
+        subject
+
+        kubernetes_namespace.reload
+
+        expect(kubernetes_namespace.namespace).to eq(namespace)
+        expect(kubernetes_namespace.service_account_name).to eq("#{namespace}-service-account")
+        expect(kubernetes_namespace.encrypted_service_account_token).to be_present
+      end
     end
   end
 end

spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb

@@ -55,7 +55,11 @@ describe Clusters::Gcp::Kubernetes::CreateServiceAccountService do
   before do
     stub_kubeclient_discover(api_url)
     stub_kubeclient_get_namespace(api_url, namespace: namespace)
-    stub_kubeclient_create_service_account(api_url, namespace: namespace )
+
+    stub_kubeclient_get_service_account_error(api_url, service_account_name, namespace: namespace)
+    stub_kubeclient_create_service_account(api_url, namespace: namespace)
+
+    stub_kubeclient_get_secret_error(api_url, token_name, namespace: namespace)
     stub_kubeclient_create_secret(api_url, namespace: namespace)
   end
 
@@ -74,10 +78,12 @@ describe Clusters::Gcp::Kubernetes::CreateServiceAccountService do
 
     context 'with RBAC cluster' do
       let(:rbac) { true }
+      let(:cluster_role_binding_name) { 'gitlab-admin' }
 
       before do
         cluster.platform_kubernetes.rbac!
 
+        stub_kubeclient_get_cluster_role_binding_error(api_url, cluster_role_binding_name)
         stub_kubeclient_create_cluster_role_binding(api_url)
       end
 
@@ -130,10 +136,12 @@ describe Clusters::Gcp::Kubernetes::CreateServiceAccountService do
 
     context 'With RBAC enabled cluster' do
       let(:rbac) { true }
+      let(:role_binding_name) { "gitlab-#{namespace}"}
 
       before do
         cluster.platform_kubernetes.rbac!
 
+        stub_kubeclient_get_role_binding_error(api_url, role_binding_name, namespace: namespace)
         stub_kubeclient_create_role_binding(api_url, namespace: namespace)
       end
 

spec/support/helpers/kubernetes_helpers.rb

@@ -47,6 +47,11 @@ module KubernetesHelpers
       .to_return(status: [status, "Internal Server Error"])
   end
 
+  def stub_kubeclient_get_service_account_error(api_url, name, namespace: 'default', status: 404)
+    WebMock.stub_request(:get, api_url + "/api/v1/namespaces/#{namespace}/serviceaccounts/#{name}")
+      .to_return(status: [status, "Internal Server Error"])
+  end
+
   def stub_kubeclient_create_service_account(api_url, namespace: 'default')
     WebMock.stub_request(:post, api_url + "/api/v1/namespaces/#{namespace}/serviceaccounts")
       .to_return(kube_response({}))
@@ -62,11 +67,26 @@ module KubernetesHelpers
       .to_return(kube_response({}))
   end
 
+  def stub_kubeclient_put_secret(api_url, name, namespace: 'default')
+    WebMock.stub_request(:put, api_url + "/api/v1/namespaces/#{namespace}/secrets/#{name}")
+      .to_return(kube_response({}))
+  end
+
+  def stub_kubeclient_get_cluster_role_binding_error(api_url, name, status: 404)
+    WebMock.stub_request(:get, api_url + "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings/#{name}")
+      .to_return(status: [status, "Internal Server Error"])
+  end
+
   def stub_kubeclient_create_cluster_role_binding(api_url)
     WebMock.stub_request(:post, api_url + '/apis/rbac.authorization.k8s.io/v1/clusterrolebindings')
       .to_return(kube_response({}))
   end
 
+  def stub_kubeclient_get_role_binding_error(api_url, name, namespace: 'default', status: 404)
+    WebMock.stub_request(:get, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/rolebindings/#{name}")
+      .to_return(status: [status, "Internal Server Error"])
+  end
+
   def stub_kubeclient_create_role_binding(api_url, namespace: 'default')
     WebMock.stub_request(:post, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/rolebindings")
       .to_return(kube_response({}))