diff options
author | Thong Kuah <tkuah@gitlab.com> | 2018-11-28 13:31:28 +0300 |
---|---|---|
committer | Thong Kuah <tkuah@gitlab.com> | 2018-12-05 00:16:44 +0300 |
commit | d3866fb48cdf640cd16d48b9825dba2c261a78f3 (patch) | |
tree | c73d4923b32530b329926a08c2fa418cc0412097 | |
parent | 546aa0cae4999176bf914a00893f25e177886c53 (diff) |
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.
7 files changed, 252 insertions, 85 deletions
diff --git a/app/services/clusters/gcp/kubernetes/create_service_account_service.rb b/app/services/clusters/gcp/kubernetes/create_service_account_service.rb index dfc4bf7a358..f2d7cc05552 100644 --- a/app/services/clusters/gcp/kubernetes/create_service_account_service.rb +++ b/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 diff --git a/lib/gitlab/kubernetes/kube_client.rb b/lib/gitlab/kubernetes/kube_client.rb index b947f6b551e..fe839940f74 100644 --- a/lib/gitlab/kubernetes/kube_client.rb +++ b/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), diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb index 3979a43216c..8fc85301304 100644 --- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb +++ b/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) diff --git a/spec/services/clusters/gcp/finalize_creation_service_spec.rb b/spec/services/clusters/gcp/finalize_creation_service_spec.rb index cb8f4bd32c8..d69678c1277 100644 --- a/spec/services/clusters/gcp/finalize_creation_service_spec.rb +++ b/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' diff --git a/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb b/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb index 661364ac765..62a5c26d908 100644 --- a/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb +++ b/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 diff --git a/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb b/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb index 588edff85d4..647050f6ad1 100644 --- a/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb +++ b/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 diff --git a/spec/support/helpers/kubernetes_helpers.rb b/spec/support/helpers/kubernetes_helpers.rb index ccaf86aa3a6..bef951e1517 100644 --- a/spec/support/helpers/kubernetes_helpers.rb +++ b/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({})) |