From a99ad59e655d66fda8af7f2b89aced79b8bc1060 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 6 Nov 2017 23:06:10 +0900 Subject: [PATCH] Remove 10.3 comments (Tracked by a tech debts issue). Refactor spec factory name. Use ArgumentError --- app/models/clusters/providers/gcp.rb | 2 +- .../projects/clusters_controller_spec.rb | 81 ------------------- spec/factories/clusters/cluster.rb | 8 +- .../clusters/platforms/kubernetes.rb | 2 +- spec/factories/clusters/providers/gcp.rb | 2 +- .../clusters/platforms/kubernetes_spec.rb | 20 ++--- spec/models/clusters/providers/gcp_spec.rb | 24 +++--- spec/serializers/cluster_entity_spec.rb | 4 +- spec/serializers/cluster_serializer_spec.rb | 2 +- spec/services/clusters/create_service_spec.rb | 53 ------------ .../gcp/fetch_operation_service_spec.rb | 2 +- .../clusters/gcp/provision_service_spec.rb | 2 +- .../verify_provision_status_service_spec.rb | 2 +- .../google_api/cloud_platform_helpers.rb | 36 --------- spec/workers/cluster_provision_worker_spec.rb | 2 +- .../wait_for_cluster_creation_worker_spec.rb | 2 +- 16 files changed, 37 insertions(+), 207 deletions(-) diff --git a/app/models/clusters/providers/gcp.rb b/app/models/clusters/providers/gcp.rb index 7700ba86f1a..c4391729dd7 100644 --- a/app/models/clusters/providers/gcp.rb +++ b/app/models/clusters/providers/gcp.rb @@ -55,7 +55,7 @@ module Clusters before_transition any => [:creating] do |provider, transition| operation_id = transition.args.first - raise 'operation_id is required' unless operation_id + raise ArgumentError.new('operation_id is required') unless operation_id.present? provider.operation_id = operation_id end diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index bfa5b5a91dc..de4cf40b492 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -200,51 +200,6 @@ describe Projects::ClustersController do expect(response).to redirect_to(project_cluster_path(project, project.cluster)) end end - - # TODO: Activate in 10.3 - # context 'when adds a cluster manually' do - # let(:params) do - # { - # cluster: { - # name: 'new-cluster', - # platform_type: :kubernetes, - # provider_type: :user, - # platform_kubernetes_attributes: { - # namespace: 'custom-namespace', - # api_url: 'https://111.111.111.111', - # token: 'token' - # } - # } - # } - # end - - # it 'creates a new cluster' do - # expect(ClusterProvisionWorker).to receive(:perform_async) - # expect { go }.to change { Clusters::Cluster.count } - # expect(response).to redirect_to(project_cluster_path(project, project.cluster)) - # end - # end - - # TODO: We should fix this in 10.2 - # Maybe - # - validates :provider_gcp, presence: true, if: :gcp? - # - validates :provider_type, presence: true - # are required in Clusters::Cluster - # context 'when not all required parameters are set' do - # let(:params) do - # { - # cluster: { - # name: 'new-cluster' - # } - # } - # end - - # it 'shows an error message' do - # expect { go }.not_to change { Clusters::Cluster.count } - # expect(assigns(:cluster).errors).not_to be_empty - # expect(response).to render_template(:new) - # end - # end end context 'when access token is expired' do @@ -397,42 +352,6 @@ describe Projects::ClustersController do end end end - - # TODO: Activate in 10.3 - # context 'when update namespace' do - # let(:namespace) { 'namespace-123' } - - # let(:params) do - # { - # cluster: { - # platform_kubernetes_attributes: { - # namespace: namespace - # } - # } - # } - # end - - # it "updates and redirects back to show page" do - # go - - # cluster.reload - # expect(response).to redirect_to(project_cluster_path(project, project.cluster)) - # expect(flash[:notice]).to eq('Cluster was successfully updated.') - # expect(cluster.platform.namespace).to eq(namespace) - # end - - # context 'when namespace is invalid' do - # let(:namespace) { 'my Namespace 321321321 #' } - - # it "rejects changes" do - # go - - # expect(response).to have_gitlab_http_status(:ok) - # expect(response).to render_template(:show) - # expect(cluster.platform.namespace).not_to eq(namespace) - # end - # end - # end end describe 'security' do diff --git a/spec/factories/clusters/cluster.rb b/spec/factories/clusters/cluster.rb index 802981d47a0..c4261178f2d 100644 --- a/spec/factories/clusters/cluster.rb +++ b/spec/factories/clusters/cluster.rb @@ -14,7 +14,7 @@ FactoryGirl.define do platform_type :kubernetes platform_kubernetes do - create(:platform_kubernetes, :configured) + create(:cluster_platform_kubernetes, :configured) end end @@ -23,8 +23,8 @@ FactoryGirl.define do platform_type :kubernetes before(:create) do |cluster, evaluator| - cluster.platform_kubernetes = build(:platform_kubernetes, :configured) - cluster.provider_gcp = build(:provider_gcp, :created) + cluster.platform_kubernetes = build(:cluster_platform_kubernetes, :configured) + cluster.provider_gcp = build(:cluster_provider_gcp, :created) end end @@ -32,7 +32,7 @@ FactoryGirl.define do provider_type :gcp provider_gcp do - create(:provider_gcp, :creating) + create(:cluster_provider_gcp, :creating) end end end diff --git a/spec/factories/clusters/platforms/kubernetes.rb b/spec/factories/clusters/platforms/kubernetes.rb index 5d4e3e5cf5d..8b3e6ff35fa 100644 --- a/spec/factories/clusters/platforms/kubernetes.rb +++ b/spec/factories/clusters/platforms/kubernetes.rb @@ -1,5 +1,5 @@ FactoryGirl.define do - factory :platform_kubernetes, class: Clusters::Platforms::Kubernetes do + factory :cluster_platform_kubernetes, class: Clusters::Platforms::Kubernetes do cluster namespace nil api_url 'https://kubernetes.example.com' diff --git a/spec/factories/clusters/providers/gcp.rb b/spec/factories/clusters/providers/gcp.rb index 13bf50d7b7f..a815410512a 100644 --- a/spec/factories/clusters/providers/gcp.rb +++ b/spec/factories/clusters/providers/gcp.rb @@ -1,5 +1,5 @@ FactoryGirl.define do - factory :provider_gcp, class: Clusters::Providers::Gcp do + factory :cluster_provider_gcp, class: Clusters::Providers::Gcp do cluster gcp_project_id 'test-gcp-project' diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index e6ebe079ceb..ed76be703a5 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -9,7 +9,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching describe 'before_validation' do context 'when namespace includes upper case' do - let(:kubernetes) { create(:platform_kubernetes, :configured, namespace: namespace) } + let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) } let(:namespace) { 'ABC' } it 'converts to lower case' do @@ -22,7 +22,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching subject { kubernetes.valid? } context 'when validates namespace' do - let(:kubernetes) { build(:platform_kubernetes, :configured, namespace: namespace) } + let(:kubernetes) { build(:cluster_platform_kubernetes, :configured, namespace: namespace) } context 'when namespace is blank' do let(:namespace) { '' } @@ -50,7 +50,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end context 'when validates api_url' do - let(:kubernetes) { build(:platform_kubernetes, :configured) } + let(:kubernetes) { build(:cluster_platform_kubernetes, :configured) } before do kubernetes.api_url = api_url @@ -76,7 +76,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end context 'when validates token' do - let(:kubernetes) { build(:platform_kubernetes, :configured) } + let(:kubernetes) { build(:cluster_platform_kubernetes, :configured) } before do kubernetes.token = token @@ -95,8 +95,8 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching let(:enabled) { true } let(:project) { create(:project) } let(:cluster) { build(:cluster, provider_type: :gcp, platform_type: :kubernetes, platform_kubernetes: platform, provider_gcp: provider, enabled: enabled, projects: [project]) } - let(:platform) { build(:platform_kubernetes, :configured) } - let(:provider) { build(:provider_gcp) } + let(:platform) { build(:cluster_platform_kubernetes, :configured) } + let(:provider) { build(:cluster_provider_gcp) } let(:kubernetes_service) { project.kubernetes_service } it 'updates KubernetesService' do @@ -126,8 +126,8 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching context 'when kubernetes_service has been configured without cluster integration' do let!(:project) { create(:project) } let(:cluster) { build(:cluster, provider_type: :gcp, platform_type: :kubernetes, platform_kubernetes: platform, provider_gcp: provider, projects: [project]) } - let(:platform) { build(:platform_kubernetes, :configured, api_url: 'https://111.111.111.111') } - let(:provider) { build(:provider_gcp) } + let(:platform) { build(:cluster_platform_kubernetes, :configured, api_url: 'https://111.111.111.111') } + let(:provider) { build(:cluster_provider_gcp) } before do create(:kubernetes_service, project: project) @@ -144,7 +144,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) } let(:project) { cluster.project } - let(:kubernetes) { create(:platform_kubernetes, :configured, namespace: namespace) } + let(:kubernetes) { create(:cluster_platform_kubernetes, :configured, namespace: namespace) } context 'when namespace is present' do let(:namespace) { 'namespace-123' } @@ -170,7 +170,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching describe '#default_namespace' do subject { kubernetes.default_namespace } - let(:kubernetes) { create(:platform_kubernetes, :configured) } + let(:kubernetes) { create(:cluster_platform_kubernetes, :configured) } context 'when cluster belongs to a project' do let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) } diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb index 99eb8c46e9a..ecd0a08c953 100644 --- a/spec/models/clusters/providers/gcp_spec.rb +++ b/spec/models/clusters/providers/gcp_spec.rb @@ -5,7 +5,7 @@ describe Clusters::Providers::Gcp do it { is_expected.to validate_presence_of(:zone) } describe 'default_value_for' do - let(:gcp) { build(:provider_gcp) } + let(:gcp) { build(:cluster_provider_gcp) } it "has default value" do expect(gcp.zone).to eq('us-central1-a') @@ -18,7 +18,7 @@ describe Clusters::Providers::Gcp do subject { gcp.valid? } context 'when validates gcp_project_id' do - let(:gcp) { build(:provider_gcp, gcp_project_id: gcp_project_id) } + let(:gcp) { build(:cluster_provider_gcp, gcp_project_id: gcp_project_id) } context 'when gcp_project_id is shorter than 1' do let(:gcp_project_id) { '' } @@ -46,7 +46,7 @@ describe Clusters::Providers::Gcp do end context 'when validates num_nodes' do - let(:gcp) { build(:provider_gcp, num_nodes: num_nodes) } + let(:gcp) { build(:cluster_provider_gcp, num_nodes: num_nodes) } context 'when num_nodes is string' do let(:num_nodes) { 'A3' } @@ -76,7 +76,7 @@ describe Clusters::Providers::Gcp do describe '#state_machine' do context 'when any => [:created]' do - let(:gcp) { build(:provider_gcp, :creating) } + let(:gcp) { build(:cluster_provider_gcp, :creating) } before do gcp.make_created @@ -90,7 +90,7 @@ describe Clusters::Providers::Gcp do end context 'when any => [:creating]' do - let(:gcp) { build(:provider_gcp) } + let(:gcp) { build(:cluster_provider_gcp) } context 'when operation_id is present' do let(:operation_id) { 'operation-xxx' } @@ -116,7 +116,7 @@ describe Clusters::Providers::Gcp do end context 'when any => [:errored]' do - let(:gcp) { build(:provider_gcp, :creating) } + let(:gcp) { build(:cluster_provider_gcp, :creating) } let(:status_reason) { 'err msg' } it 'nullify access_token and operation_id' do @@ -129,7 +129,7 @@ describe Clusters::Providers::Gcp do end context 'when status_reason is nil' do - let(:gcp) { build(:provider_gcp, :errored) } + let(:gcp) { build(:cluster_provider_gcp, :errored) } it 'does not set status_reason' do gcp.make_errored(nil) @@ -144,13 +144,13 @@ describe Clusters::Providers::Gcp do subject { gcp.on_creation? } context 'when status is creating' do - let(:gcp) { create(:provider_gcp, :creating) } + let(:gcp) { create(:cluster_provider_gcp, :creating) } it { is_expected.to be_truthy } end context 'when status is created' do - let(:gcp) { create(:provider_gcp, :created) } + let(:gcp) { create(:cluster_provider_gcp, :created) } it { is_expected.to be_falsey } end @@ -160,7 +160,7 @@ describe Clusters::Providers::Gcp do subject { gcp.api_client } context 'when status is creating' do - let(:gcp) { build(:provider_gcp, :creating) } + let(:gcp) { build(:cluster_provider_gcp, :creating) } it 'returns Cloud Platform API clinet' do expect(subject).to be_an_instance_of(GoogleApi::CloudPlatform::Client) @@ -169,13 +169,13 @@ describe Clusters::Providers::Gcp do end context 'when status is created' do - let(:gcp) { build(:provider_gcp, :created) } + let(:gcp) { build(:cluster_provider_gcp, :created) } it { is_expected.to be_nil } end context 'when status is errored' do - let(:gcp) { build(:provider_gcp, :errored) } + let(:gcp) { build(:cluster_provider_gcp, :errored) } it { is_expected.to be_nil } end diff --git a/spec/serializers/cluster_entity_spec.rb b/spec/serializers/cluster_entity_spec.rb index 7b132a1b84d..72f02131211 100644 --- a/spec/serializers/cluster_entity_spec.rb +++ b/spec/serializers/cluster_entity_spec.rb @@ -8,7 +8,7 @@ describe ClusterEntity do let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } context 'when status is creating' do - let(:provider) { create(:provider_gcp, :creating) } + let(:provider) { create(:cluster_provider_gcp, :creating) } it 'has corresponded data' do expect(subject[:status]).to eq(:creating) @@ -17,7 +17,7 @@ describe ClusterEntity do end context 'when status is errored' do - let(:provider) { create(:provider_gcp, :errored) } + let(:provider) { create(:cluster_provider_gcp, :errored) } it 'has corresponded data' do expect(subject[:status]).to eq(:errored) diff --git a/spec/serializers/cluster_serializer_spec.rb b/spec/serializers/cluster_serializer_spec.rb index e5da92a451e..ff7d1789149 100644 --- a/spec/serializers/cluster_serializer_spec.rb +++ b/spec/serializers/cluster_serializer_spec.rb @@ -6,7 +6,7 @@ describe ClusterSerializer do context 'when provider type is gcp' do let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } - let(:provider) { create(:provider_gcp, :errored) } + let(:provider) { create(:cluster_provider_gcp, :errored) } it 'serializes only status' do expect(subject.keys).to contain_exactly(:status, :status_reason) diff --git a/spec/services/clusters/create_service_spec.rb b/spec/services/clusters/create_service_spec.rb index 9c2c288a2fa..5b6edb73beb 100644 --- a/spec/services/clusters/create_service_spec.rb +++ b/spec/services/clusters/create_service_spec.rb @@ -61,57 +61,4 @@ describe Clusters::CreateService do end end end - - # TODO: This will be active in 10.3 - # context 'when provider is user' do - # context 'when correct params' do - # let(:params) do - # { - # name: 'test-cluster', - # platform_type: :kubernetes, - # provider_type: :user, - # platform_kubernetes_attributes: { - # namespace: 'custom-namespace', - # api_url: 'https://111.111.111.111', - # token: 'token' - # } - # } - # end - - # it 'creates a cluster object and performs a worker' do - # expect(ClusterProvisionWorker).to receive(:perform_async) - - # expect { result } - # .to change { Clusters::Cluster.count }.by(1) - # .and change { Clusters::Platforms::Kubernetes.count }.by(1) - - # expect(result.name).to eq('test-cluster') - # expect(result.user).to eq(user) - # expect(result.project).to eq(project) - # expect(result.provider).to be_nil - # expect(result.platform.namespace).to eq('custom-namespace') - # end - # end - - # context 'when invalid params' do - # let(:params) do - # { - # name: 'test-cluster', - # platform_type: :kubernetes, - # provider_type: :user, - # platform_kubernetes_attributes: { - # namespace: 'custom-namespace', - # api_url: '!!!!!', - # token: 'token' - # } - # } - # end - - # it 'returns an error' do - # # expect(ClusterProvisionWorker).not_to receive(:perform_async) - # expect { result }.to change { Clusters::Cluster.count }.by(0) - # expect(result.errors[:"platform_kubernetes.api_url"]).to be_present - # end - # end - # end end diff --git a/spec/services/clusters/gcp/fetch_operation_service_spec.rb b/spec/services/clusters/gcp/fetch_operation_service_spec.rb index 20d46608033..e2fa93904c5 100644 --- a/spec/services/clusters/gcp/fetch_operation_service_spec.rb +++ b/spec/services/clusters/gcp/fetch_operation_service_spec.rb @@ -4,7 +4,7 @@ describe Clusters::Gcp::FetchOperationService do include GoogleApi::CloudPlatformHelpers describe '#execute' do - let(:provider) { create(:provider_gcp, :creating) } + let(:provider) { create(:cluster_provider_gcp, :creating) } let(:gcp_project_id) { provider.gcp_project_id } let(:zone) { provider.zone } let(:operation_id) { provider.operation_id } diff --git a/spec/services/clusters/gcp/provision_service_spec.rb b/spec/services/clusters/gcp/provision_service_spec.rb index f5f9d4800fd..f48afdc83b2 100644 --- a/spec/services/clusters/gcp/provision_service_spec.rb +++ b/spec/services/clusters/gcp/provision_service_spec.rb @@ -4,7 +4,7 @@ describe Clusters::Gcp::ProvisionService do include GoogleApi::CloudPlatformHelpers describe '#execute' do - let(:provider) { create(:provider_gcp, :scheduled) } + let(:provider) { create(:cluster_provider_gcp, :scheduled) } let(:gcp_project_id) { provider.gcp_project_id } let(:zone) { provider.zone } diff --git a/spec/services/clusters/gcp/verify_provision_status_service_spec.rb b/spec/services/clusters/gcp/verify_provision_status_service_spec.rb index 666fcf13cac..2ee2fa51f63 100644 --- a/spec/services/clusters/gcp/verify_provision_status_service_spec.rb +++ b/spec/services/clusters/gcp/verify_provision_status_service_spec.rb @@ -4,7 +4,7 @@ describe Clusters::Gcp::VerifyProvisionStatusService do include GoogleApi::CloudPlatformHelpers describe '#execute' do - let(:provider) { create(:provider_gcp, :creating) } + let(:provider) { create(:cluster_provider_gcp, :creating) } let(:gcp_project_id) { provider.gcp_project_id } let(:zone) { provider.zone } let(:operation_id) { provider.operation_id } diff --git a/spec/support/google_api/cloud_platform_helpers.rb b/spec/support/google_api/cloud_platform_helpers.rb index 4b785611ab5..dabf0db7666 100644 --- a/spec/support/google_api/cloud_platform_helpers.rb +++ b/spec/support/google_api/cloud_platform_helpers.rb @@ -71,15 +71,9 @@ module GoogleApi "name": options[:name] || 'string', "description": options[:description] || 'string', "initialNodeCount": options[:initialNodeCount] || 'number', - # "nodeConfig": {, - # object(NodeConfig), - # },, "masterAuth": { "username": options[:username] || 'string', "password": options[:password] || 'string', - # "clientCertificateConfig": { - # object(ClientCertificateConfig) - # }, "clusterCaCertificate": options[:clusterCaCertificate] || load_sample_cert, "clientCertificate": options[:clientCertificate] || 'string', "clientKey": options[:clientKey] || 'string' @@ -88,36 +82,9 @@ module GoogleApi "monitoringService": options[:monitoringService] || 'string', "network": options[:network] || 'string', "clusterIpv4Cidr": options[:clusterIpv4Cidr] || 'string', - # "addonsConfig": {, - # object(AddonsConfig), - # },, "subnetwork": options[:subnetwork] || 'string', - # "nodePools": [, - # {, - # object(NodePool), - # }, - # ],, - # "locations": [, - # string, - # ],, "enableKubernetesAlpha": options[:enableKubernetesAlpha] || 'boolean', - # "resourceLabels": {, - # string: string,, - # ..., - # },, "labelFingerprint": options[:labelFingerprint] || 'string', - # "legacyAbac": {, - # object(LegacyAbac), - # }, - # "networkPolicy": {, - # object(NetworkPolicy), - # }, - # "ipAllocationPolicy": {, - # object(IPAllocationPolicy), - # }, - # "masterAuthorizedNetworksConfig": {, - # object(MasterAuthorizedNetworksConfig), - # }, "selfLink": options[:selfLink] || 'string', "zone": options[:zone] || 'string', "endpoint": options[:endpoint] || 'string', @@ -129,9 +96,6 @@ module GoogleApi "statusMessage": options[:statusMessage] || 'string', "nodeIpv4CidrSize": options[:nodeIpv4CidrSize] || 'number', "servicesIpv4Cidr": options[:servicesIpv4Cidr] || 'string', - # "instanceGroupUrls": [, - # string, - # ],, "currentNodeCount": options[:currentNodeCount] || 'number', "expireTime": options[:expireTime] || 'string' } diff --git a/spec/workers/cluster_provision_worker_spec.rb b/spec/workers/cluster_provision_worker_spec.rb index 85c7dc20ede..8054ec11a48 100644 --- a/spec/workers/cluster_provision_worker_spec.rb +++ b/spec/workers/cluster_provision_worker_spec.rb @@ -4,7 +4,7 @@ describe ClusterProvisionWorker do describe '#perform' do context 'when provider type is gcp' do let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } - let(:provider) { create(:provider_gcp, :scheduled) } + let(:provider) { create(:cluster_provider_gcp, :scheduled) } it 'provision a cluster' do expect_any_instance_of(Clusters::Gcp::ProvisionService).to receive(:execute) diff --git a/spec/workers/wait_for_cluster_creation_worker_spec.rb b/spec/workers/wait_for_cluster_creation_worker_spec.rb index 29812408396..0e92b298178 100644 --- a/spec/workers/wait_for_cluster_creation_worker_spec.rb +++ b/spec/workers/wait_for_cluster_creation_worker_spec.rb @@ -4,7 +4,7 @@ describe WaitForClusterCreationWorker do describe '#perform' do context 'when provider type is gcp' do let(:cluster) { create(:cluster, provider_type: :gcp, provider_gcp: provider) } - let(:provider) { create(:provider_gcp, :creating) } + let(:provider) { create(:cluster_provider_gcp, :creating) } it 'provision a cluster' do expect_any_instance_of(Clusters::Gcp::VerifyProvisionStatusService).to receive(:execute)