From fa5a6ae172584c5c33665a6efa4a6aa4efaea9ad Mon Sep 17 00:00:00 2001 From: Tiger Date: Thu, 11 Apr 2019 13:26:10 +1000 Subject: [PATCH] Stop configuring group clusters on creation Immediate configuration is not ideal for group and instance level clusters as projects that may never be deployed would still have Kubernetes namespaces and service accounts created for them. As of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25586 we now create only the resources that are required for the project being deployed, at the time of deployment. --- app/models/ci/build.rb | 2 -- app/services/clusters/refresh_service.rb | 6 +---- app/services/projects/create_service.rb | 6 ----- app/services/projects/transfer_service.rb | 5 ---- app/workers/cluster_configure_worker.rb | 2 +- ...remove-ci-preparing-state-feature-flag.yml | 5 ++++ spec/models/ci/build_spec.rb | 20 +++++--------- .../services/clusters/refresh_service_spec.rb | 26 +++--------------- spec/services/projects/create_service_spec.rb | 27 ------------------- .../projects/transfer_service_spec.rb | 27 ------------------- spec/workers/cluster_configure_worker_spec.rb | 21 ++------------- 11 files changed, 19 insertions(+), 128 deletions(-) create mode 100644 changelogs/unreleased/60379-remove-ci-preparing-state-feature-flag.yml diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d2f5ff13408..0a90995f689 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -378,8 +378,6 @@ module Ci end def any_unmet_prerequisites? - return false unless Feature.enabled?(:ci_preparing_state, default_enabled: true) - prerequisites.present? end diff --git a/app/services/clusters/refresh_service.rb b/app/services/clusters/refresh_service.rb index b02bb9c0247..3752a306793 100644 --- a/app/services/clusters/refresh_service.rb +++ b/app/services/clusters/refresh_service.rb @@ -21,11 +21,7 @@ module Clusters private_class_method :projects_with_missing_kubernetes_namespaces_for_cluster def self.clusters_with_missing_kubernetes_namespaces_for_project(project) - if Feature.enabled?(:ci_preparing_state, default_enabled: true) - project.clusters.managed.missing_kubernetes_namespace(project.kubernetes_namespaces) - else - project.all_clusters.managed.missing_kubernetes_namespace(project.kubernetes_namespaces) - end + project.clusters.managed.missing_kubernetes_namespace(project.kubernetes_namespaces) end private_class_method :clusters_with_missing_kubernetes_namespaces_for_project diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 4ea40e3c8ce..9f335cceb67 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -100,8 +100,6 @@ module Projects current_user.invalidate_personal_projects_count create_readme if @initialize_with_readme - - configure_group_clusters_for_project end # Refresh the current user's authorizations inline (so they can access the @@ -127,10 +125,6 @@ module Projects Files::CreateService.new(@project, current_user, commit_attrs).execute end - def configure_group_clusters_for_project - ClusterProjectConfigureWorker.perform_async(@project.id) - end - def skip_wiki? !@project.feature_available?(:wiki, current_user) || @skip_wiki end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 91c01eca75c..233dcf37e35 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -54,7 +54,6 @@ module Projects end attempt_transfer_transaction - configure_group_clusters_for_project end # rubocop: enable CodeReuse/ActiveRecord @@ -164,9 +163,5 @@ module Projects @new_namespace.full_path ) end - - def configure_group_clusters_for_project - ClusterProjectConfigureWorker.perform_async(project.id) - end end end diff --git a/app/workers/cluster_configure_worker.rb b/app/workers/cluster_configure_worker.rb index 37ea7dde7a1..6f64b7ea0ab 100644 --- a/app/workers/cluster_configure_worker.rb +++ b/app/workers/cluster_configure_worker.rb @@ -6,7 +6,7 @@ class ClusterConfigureWorker def perform(cluster_id) Clusters::Cluster.managed.find_by_id(cluster_id).try do |cluster| - if cluster.project_type? || Feature.disabled?(:ci_preparing_state, default_enabled: true) + if cluster.project_type? Clusters::RefreshService.create_or_update_namespaces_for_cluster(cluster) end end diff --git a/changelogs/unreleased/60379-remove-ci-preparing-state-feature-flag.yml b/changelogs/unreleased/60379-remove-ci-preparing-state-feature-flag.yml new file mode 100644 index 00000000000..a9b7aeb3024 --- /dev/null +++ b/changelogs/unreleased/60379-remove-ci-preparing-state-feature-flag.yml @@ -0,0 +1,5 @@ +--- +title: Remove ability for group clusters to be automatically configured on creation +merge_request: 27245 +author: +type: removed diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 9b489baf163..5f2e8aa0baa 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2925,26 +2925,18 @@ describe Ci::Build do subject { build.any_unmet_prerequisites? } + before do + allow(build).to receive(:prerequisites).and_return(prerequisites) + end + context 'build has prerequisites' do - before do - allow(build).to receive(:prerequisites).and_return([double]) - end + let(:prerequisites) { [double] } it { is_expected.to be_truthy } - - context 'and the ci_preparing_state feature is disabled' do - before do - stub_feature_flags(ci_preparing_state: false) - end - - it { is_expected.to be_falsey } - end end context 'build does not have prerequisites' do - before do - allow(build).to receive(:prerequisites).and_return([]) - end + let(:prerequisites) { [] } it { is_expected.to be_falsey } end diff --git a/spec/services/clusters/refresh_service_spec.rb b/spec/services/clusters/refresh_service_spec.rb index 94c35228955..5bc8a709941 100644 --- a/spec/services/clusters/refresh_service_spec.rb +++ b/spec/services/clusters/refresh_service_spec.rb @@ -93,32 +93,14 @@ describe Clusters::RefreshService do let(:group) { cluster.group } let(:project) { create(:project, group: group) } - context 'when ci_preparing_state feature flag is enabled' do - include_examples 'does not create a kubernetes namespace' + include_examples 'does not create a kubernetes namespace' - context 'when project already has kubernetes namespace' do - before do - create(:cluster_kubernetes_namespace, project: project, cluster: cluster) - end - - include_examples 'does not create a kubernetes namespace' - end - end - - context 'when ci_preparing_state feature flag is disabled' do + context 'when project already has kubernetes namespace' do before do - stub_feature_flags(ci_preparing_state: false) + create(:cluster_kubernetes_namespace, project: project, cluster: cluster) end - include_examples 'creates a kubernetes namespace' - - context 'when project already has kubernetes namespace' do - before do - create(:cluster_kubernetes_namespace, project: project, cluster: cluster) - end - - include_examples 'does not create a kubernetes namespace' - end + include_examples 'does not create a kubernetes namespace' end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index bd7a0c68766..f54f9200661 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -268,33 +268,6 @@ describe Projects::CreateService, '#execute' do end end - context 'when group has kubernetes cluster' do - let(:group_cluster) { create(:cluster, :group, :provided_by_gcp) } - let(:group) { group_cluster.group } - - let(:token) { 'aaaa' } - let(:service_account_creator) { double(Clusters::Gcp::Kubernetes::CreateOrUpdateServiceAccountService, execute: true) } - let(:secrets_fetcher) { double(Clusters::Gcp::Kubernetes::FetchKubernetesTokenService, execute: token) } - - before do - group.add_owner(user) - - stub_feature_flags(ci_preparing_state: false) - expect(Clusters::Gcp::Kubernetes::CreateOrUpdateServiceAccountService).to receive(:namespace_creator).and_return(service_account_creator) - expect(Clusters::Gcp::Kubernetes::FetchKubernetesTokenService).to receive(:new).and_return(secrets_fetcher) - end - - it 'creates kubernetes namespace for the project' do - project = create_project(user, opts.merge!(namespace_id: group.id)) - - expect(project).to be_valid - - kubernetes_namespace = group_cluster.kubernetes_namespaces.first - expect(kubernetes_namespace).to be_present - expect(kubernetes_namespace.project).to eq(project) - end - end - context 'when there is an active service template' do before do create(:service, project: nil, template: true, active: true) diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 9acc3657fa9..a47c10d991a 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -73,33 +73,6 @@ describe Projects::TransferService do shard_name: project.repository_storage ) end - - context 'new group has a kubernetes cluster' do - let(:group_cluster) { create(:cluster, :group, :provided_by_gcp) } - let(:group) { group_cluster.group } - - let(:token) { 'aaaa' } - let(:service_account_creator) { double(Clusters::Gcp::Kubernetes::CreateOrUpdateServiceAccountService, execute: true) } - let(:secrets_fetcher) { double(Clusters::Gcp::Kubernetes::FetchKubernetesTokenService, execute: token) } - - subject { transfer_project(project, user, group) } - - before do - stub_feature_flags(ci_preparing_state: false) - expect(Clusters::Gcp::Kubernetes::CreateOrUpdateServiceAccountService).to receive(:namespace_creator).and_return(service_account_creator) - expect(Clusters::Gcp::Kubernetes::FetchKubernetesTokenService).to receive(:new).and_return(secrets_fetcher) - end - - it 'creates kubernetes namespace for the project' do - subject - - expect(project.kubernetes_namespaces.count).to eq(1) - - kubernetes_namespace = group_cluster.kubernetes_namespaces.first - expect(kubernetes_namespace).to be_present - expect(kubernetes_namespace.project).to eq(project) - end - end end context 'when transfer fails' do diff --git a/spec/workers/cluster_configure_worker_spec.rb b/spec/workers/cluster_configure_worker_spec.rb index daf014ac574..975088f3ee6 100644 --- a/spec/workers/cluster_configure_worker_spec.rb +++ b/spec/workers/cluster_configure_worker_spec.rb @@ -4,11 +4,6 @@ require 'spec_helper' describe ClusterConfigureWorker, '#perform' do let(:worker) { described_class.new } - let(:ci_preparing_state_enabled) { false } - - before do - stub_feature_flags(ci_preparing_state: ci_preparing_state_enabled) - end shared_examples 'configured cluster' do it 'creates a namespace' do @@ -33,26 +28,14 @@ describe ClusterConfigureWorker, '#perform' do context 'when group has a project' do let!(:project) { create(:project, group: group) } - it_behaves_like 'configured cluster' - - context 'ci_preparing_state feature is enabled' do - let(:ci_preparing_state_enabled) { true } - - it_behaves_like 'unconfigured cluster' - end + it_behaves_like 'unconfigured cluster' end context 'when group has project in a sub-group' do let!(:subgroup) { create(:group, parent: group) } let!(:project) { create(:project, group: subgroup) } - it_behaves_like 'configured cluster' - - context 'ci_preparing_state feature is enabled' do - let(:ci_preparing_state_enabled) { true } - - it_behaves_like 'unconfigured cluster' - end + it_behaves_like 'unconfigured cluster' end end