diff options
author | Stan Hu <stanhu@gmail.com> | 2019-06-27 23:01:08 +0000 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-06-27 23:01:08 +0000 |
commit | 36451a753ac442250c1ed5a6427383817434d1ec (patch) | |
tree | fa19d4a05b104c516961dd8bd0289846ccbc213f | |
parent | 62a40c5170cbecfc77dcb5fc97a23f3e93898a53 (diff) | |
parent | 8152efbe2fc486520ec5cd11d54a49fbf7e554bf (diff) | |
download | gitlab-ce-36451a753ac442250c1ed5a6427383817434d1ec.tar.gz |
Merge branch 'remove_group_and_instance_clusters_feature_flag' into 'master'
Remove group and instance clusters feature flag
Closes #63383
See merge request gitlab-org/gitlab-ce!30124
-rw-r--r-- | app/controllers/groups/clusters_controller.rb | 9 | ||||
-rw-r--r-- | app/helpers/groups_helper.rb | 2 | ||||
-rw-r--r-- | app/models/clusters/instance.rb | 4 | ||||
-rw-r--r-- | app/models/concerns/deployment_platform.rb | 16 | ||||
-rw-r--r-- | app/models/group.rb | 4 | ||||
-rw-r--r-- | app/models/project.rb | 1 | ||||
-rw-r--r-- | app/policies/clusters/instance_policy.rb | 3 | ||||
-rw-r--r-- | changelogs/unreleased/remove_group_and_instance_clusters_feature_flag.yml | 5 | ||||
-rw-r--r-- | spec/controllers/admin/clusters/applications_controller_spec.rb | 10 | ||||
-rw-r--r-- | spec/controllers/admin/clusters_controller_spec.rb | 76 | ||||
-rw-r--r-- | spec/controllers/groups/clusters_controller_spec.rb | 80 | ||||
-rw-r--r-- | spec/models/concerns/deployment_platform_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/group_spec.rb | 29 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 1 | ||||
-rw-r--r-- | spec/policies/clusters/instance_policy_spec.rb | 18 |
15 files changed, 72 insertions, 196 deletions
diff --git a/app/controllers/groups/clusters_controller.rb b/app/controllers/groups/clusters_controller.rb index b846fb21266..92602fd8096 100644 --- a/app/controllers/groups/clusters_controller.rb +++ b/app/controllers/groups/clusters_controller.rb @@ -4,7 +4,6 @@ class Groups::ClustersController < Clusters::ClustersController include ControllerWithCrossProjectAccessCheck prepend_before_action :group - prepend_before_action :check_group_clusters_feature_flag! requires_cross_project_access layout 'group' @@ -18,12 +17,4 @@ class Groups::ClustersController < Clusters::ClustersController def group @group ||= find_routable!(Group, params[:group_id] || params[:id]) end - - def check_group_clusters_feature_flag! - render_404 unless group_clusters_enabled? - end - - def group_clusters_enabled? - group.group_clusters_enabled? - end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index a3f53ca8dd6..5aed7e313e6 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -142,7 +142,7 @@ module GroupsHelper can?(current_user, "read_group_#{resource}".to_sym, @group) end - if can?(current_user, :read_cluster, @group) && @group.group_clusters_enabled? + if can?(current_user, :read_cluster, @group) links << :kubernetes end diff --git a/app/models/clusters/instance.rb b/app/models/clusters/instance.rb index d8a888d53ba..f21dbdf7f26 100644 --- a/app/models/clusters/instance.rb +++ b/app/models/clusters/instance.rb @@ -9,9 +9,5 @@ module Clusters def feature_available?(feature) ::Feature.enabled?(feature, default_enabled: true) end - - def self.enabled? - ::Feature.enabled?(:instance_clusters, default_enabled: true) - end end end diff --git a/app/models/concerns/deployment_platform.rb b/app/models/concerns/deployment_platform.rb index 1bd8a799f0d..5a358ae2ef6 100644 --- a/app/models/concerns/deployment_platform.rb +++ b/app/models/concerns/deployment_platform.rb @@ -13,8 +13,8 @@ module DeploymentPlatform def find_deployment_platform(environment) find_cluster_platform_kubernetes(environment: environment) || - find_group_cluster_platform_kubernetes_with_feature_guard(environment: environment) || - find_instance_cluster_platform_kubernetes_with_feature_guard(environment: environment) + find_group_cluster_platform_kubernetes(environment: environment) || + find_instance_cluster_platform_kubernetes(environment: environment) end # EE would override this and utilize environment argument @@ -23,24 +23,12 @@ module DeploymentPlatform .last&.platform_kubernetes end - def find_group_cluster_platform_kubernetes_with_feature_guard(environment: nil) - return unless group_clusters_enabled? - - find_group_cluster_platform_kubernetes(environment: environment) - end - # EE would override this and utilize environment argument def find_group_cluster_platform_kubernetes(environment: nil) Clusters::Cluster.enabled.default_environment.ancestor_clusters_for_clusterable(self) .first&.platform_kubernetes end - def find_instance_cluster_platform_kubernetes_with_feature_guard(environment: nil) - return unless Clusters::Instance.enabled? - - find_instance_cluster_platform_kubernetes(environment: environment) - end - # EE would override this and utilize environment argument def find_instance_cluster_platform_kubernetes(environment: nil) Clusters::Instance.new.clusters.enabled.default_environment diff --git a/app/models/group.rb b/app/models/group.rb index dbec211935d..8e89c7ecfb1 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -410,10 +410,6 @@ class Group < Namespace ensure_runners_token! end - def group_clusters_enabled? - Feature.enabled?(:group_clusters, root_ancestor, default_enabled: true) - end - def project_creation_level super || ::Gitlab::CurrentSettings.default_project_creation end diff --git a/app/models/project.rb b/app/models/project.rb index c642b65f674..b102e0580e7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -306,7 +306,6 @@ class Project < ApplicationRecord delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_role, to: :team delegate :add_master, to: :team # @deprecated delegate :group_runners_enabled, :group_runners_enabled=, :group_runners_enabled?, to: :ci_cd_settings - delegate :group_clusters_enabled?, to: :group, allow_nil: true delegate :root_ancestor, to: :namespace, allow_nil: true delegate :last_pipeline, to: :commit, allow_nil: true delegate :external_dashboard_url, to: :metrics_setting, allow_nil: true, prefix: true diff --git a/app/policies/clusters/instance_policy.rb b/app/policies/clusters/instance_policy.rb index e1045c85e6d..f72096e8fc6 100644 --- a/app/policies/clusters/instance_policy.rb +++ b/app/policies/clusters/instance_policy.rb @@ -6,9 +6,8 @@ module Clusters condition(:has_clusters, scope: :subject) { clusterable_has_clusters? } condition(:can_have_multiple_clusters) { multiple_clusters_available? } - condition(:instance_clusters_enabled) { Instance.enabled? } - rule { admin & instance_clusters_enabled }.policy do + rule { admin }.policy do enable :read_cluster enable :add_cluster enable :create_cluster diff --git a/changelogs/unreleased/remove_group_and_instance_clusters_feature_flag.yml b/changelogs/unreleased/remove_group_and_instance_clusters_feature_flag.yml new file mode 100644 index 00000000000..fcc6c564345 --- /dev/null +++ b/changelogs/unreleased/remove_group_and_instance_clusters_feature_flag.yml @@ -0,0 +1,5 @@ +--- +title: Remove group and instance clusters feature flag +merge_request: 30124 +author: +type: changed diff --git a/spec/controllers/admin/clusters/applications_controller_spec.rb b/spec/controllers/admin/clusters/applications_controller_spec.rb index 76f261e7d3f..cf202d88acc 100644 --- a/spec/controllers/admin/clusters/applications_controller_spec.rb +++ b/spec/controllers/admin/clusters/applications_controller_spec.rb @@ -13,16 +13,6 @@ describe Admin::Clusters::ApplicationsController do it { expect { subject }.to be_allowed_for(:admin) } it { expect { subject }.to be_denied_for(:user) } it { expect { subject }.to be_denied_for(:external) } - - context 'when instance clusters are disabled' do - before do - stub_feature_flags(instance_clusters: false) - end - - it 'returns 404' do - is_expected.to have_http_status(:not_found) - end - end end let(:cluster) { create(:cluster, :instance, :provided_by_gcp) } diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index 7709f525119..e5501535875 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -17,66 +17,48 @@ describe Admin::ClustersController do get :index, params: params end - context 'when feature flag is not enabled' do - before do - stub_feature_flags(instance_clusters: false) - end + describe 'functionality' do + context 'when instance has one or more clusters' do + let!(:enabled_cluster) do + create(:cluster, :provided_by_gcp, :instance) + end - it 'responds with not found' do - get_index + let!(:disabled_cluster) do + create(:cluster, :disabled, :provided_by_gcp, :production_environment, :instance) + end - expect(response).to have_gitlab_http_status(404) - end - end + it 'lists available clusters' do + get_index - context 'when feature flag is enabled' do - before do - stub_feature_flags(instance_clusters: true) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index) + expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster]) + end - describe 'functionality' do - context 'when instance has one or more clusters' do - let!(:enabled_cluster) do - create(:cluster, :provided_by_gcp, :instance) - end + context 'when page is specified' do + let(:last_page) { Clusters::Cluster.instance_type.page.total_pages } - let!(:disabled_cluster) do - create(:cluster, :disabled, :provided_by_gcp, :production_environment, :instance) + before do + allow(Clusters::Cluster).to receive(:paginates_per).and_return(1) + create_list(:cluster, 2, :provided_by_gcp, :production_environment, :instance) end - it 'lists available clusters' do - get_index + it 'redirects to the page' do + get_index(page: last_page) expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index) - expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster]) - end - - context 'when page is specified' do - let(:last_page) { Clusters::Cluster.instance_type.page.total_pages } - - before do - allow(Clusters::Cluster).to receive(:paginates_per).and_return(1) - create_list(:cluster, 2, :provided_by_gcp, :production_environment, :instance) - end - - it 'redirects to the page' do - get_index(page: last_page) - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:clusters).current_page).to eq(last_page) - end + expect(assigns(:clusters).current_page).to eq(last_page) end end + end - context 'when instance does not have a cluster' do - it 'returns an empty state page' do - get_index + context 'when instance does not have a cluster' do + it 'returns an empty state page' do + get_index - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index, partial: :empty_state) - expect(assigns(:clusters)).to eq([]) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index, partial: :empty_state) + expect(assigns(:clusters)).to eq([]) end end end diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 2f64c7f3460..09677b42887 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -20,70 +20,52 @@ describe Groups::ClustersController do get :index, params: params.reverse_merge(group_id: group) end - context 'when feature flag is not enabled' do - before do - stub_feature_flags(group_clusters: false) - end + describe 'functionality' do + context 'when group has one or more clusters' do + let(:group) { create(:group) } - it 'renders 404' do - go + let!(:enabled_cluster) do + create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group]) + end - expect(response).to have_gitlab_http_status(404) - end - end + let!(:disabled_cluster) do + create(:cluster, :disabled, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group]) + end - context 'when feature flag is enabled' do - before do - stub_feature_flags(group_clusters: true) - end + it 'lists available clusters' do + go - describe 'functionality' do - context 'when group has one or more clusters' do - let(:group) { create(:group) } + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index) + expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster]) + end - let!(:enabled_cluster) do - create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group]) - end + context 'when page is specified' do + let(:last_page) { group.clusters.page.total_pages } - let!(:disabled_cluster) do - create(:cluster, :disabled, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group]) + before do + allow(Clusters::Cluster).to receive(:paginates_per).and_return(1) + create_list(:cluster, 2, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group]) end - it 'lists available clusters' do - go + it 'redirects to the page' do + go(page: last_page) expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index) - expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster]) - end - - context 'when page is specified' do - let(:last_page) { group.clusters.page.total_pages } - - before do - allow(Clusters::Cluster).to receive(:paginates_per).and_return(1) - create_list(:cluster, 2, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group]) - end - - it 'redirects to the page' do - go(page: last_page) - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:clusters).current_page).to eq(last_page) - end + expect(assigns(:clusters).current_page).to eq(last_page) end end + end - context 'when group does not have a cluster' do - let(:group) { create(:group) } + context 'when group does not have a cluster' do + let(:group) { create(:group) } - it 'returns an empty state page' do - go + it 'returns an empty state page' do + go - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index, partial: :empty_state) - expect(assigns(:clusters)).to eq([]) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index, partial: :empty_state) + expect(assigns(:clusters)).to eq([]) end end end diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index 96465a51db2..2378f400540 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -82,16 +82,6 @@ describe DeploymentPlatform do end end end - - context 'feature flag disabled' do - before do - stub_feature_flags(group_clusters: false) - end - - it 'returns nil' do - is_expected.to be_nil - end - end end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d7accbef6bd..470ce65707d 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -866,35 +866,6 @@ describe Group do end end - describe '#group_clusters_enabled?' do - before do - # Override global stub in spec/spec_helper.rb - expect(Feature).to receive(:enabled?).and_call_original - end - - subject { group.group_clusters_enabled? } - - it { is_expected.to be_truthy } - - context 'explicitly disabled for root ancestor' do - before do - feature = Feature.get(:group_clusters) - feature.disable(group.root_ancestor) - end - - it { is_expected.to be_falsey } - end - - context 'explicitly disabled for root ancestor' do - before do - feature = Feature.get(:group_clusters) - feature.enable(group.root_ancestor) - end - - it { is_expected.to be_truthy } - end - end - describe '#first_auto_devops_config' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index cc0f5002a1e..1bc092fa41a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -497,7 +497,6 @@ describe Project do it { is_expected.to delegate_method(:members).to(:team).with_prefix(true) } it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).with_arguments(allow_nil: true) } - it { is_expected.to delegate_method(:group_clusters_enabled?).to(:group).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:last_pipeline).to(:commit).with_arguments(allow_nil: true) } end diff --git a/spec/policies/clusters/instance_policy_spec.rb b/spec/policies/clusters/instance_policy_spec.rb index 9d755c6d29d..7b61819e079 100644 --- a/spec/policies/clusters/instance_policy_spec.rb +++ b/spec/policies/clusters/instance_policy_spec.rb @@ -16,21 +16,9 @@ describe Clusters::InstancePolicy do context 'when admin' do let(:user) { create(:admin) } - context 'with instance_level_clusters enabled' do - it { expect(policy).to be_allowed :read_cluster } - it { expect(policy).to be_allowed :update_cluster } - it { expect(policy).to be_allowed :admin_cluster } - end - - context 'with instance_level_clusters disabled' do - before do - stub_feature_flags(instance_clusters: false) - end - - it { expect(policy).to be_disallowed :read_cluster } - it { expect(policy).to be_disallowed :update_cluster } - it { expect(policy).to be_disallowed :admin_cluster } - end + it { expect(policy).to be_allowed :read_cluster } + it { expect(policy).to be_allowed :update_cluster } + it { expect(policy).to be_allowed :admin_cluster } end end end |