From 5b2ca1c66c3a69a1177c0d2f62c208bdce7a81c0 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 24 Nov 2017 18:31:13 +0900 Subject: Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes --- ...rnetes_service_to_new_clusters_architectures.rb | 104 +++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb new file mode 100644 index 00000000000..dde69058523 --- /dev/null +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -0,0 +1,104 @@ +class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migration + DOWNTIME = false + DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService' + + class Cluster < ActiveRecord::Base + self.table_name = 'clusters' + + has_many :cluster_projects, class_name: 'ClustersProject' + has_many :projects, through: :cluster_projects, class_name: 'Project' + has_one :provider_gcp, class_name: 'ProvidersGcp' + has_one :platform_kubernetes, class_name: 'PlatformsKubernetes' + + attr_encrypted :token, + mode: :per_attribute_iv, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' + + accepts_nested_attributes_for :provider_gcp + accepts_nested_attributes_for :platform_kubernetes + + enum platform_type: { + kubernetes: 1 + } + + enum provider_type: { + user: 0, + gcp: 1 + } + end + + class Project < ActiveRecord::Base + self.table_name = 'projects' + + has_one :cluster_project, class_name: 'ClustersProject' + has_one :cluster, through: :cluster_project, class_name: 'Cluster' + end + + class Service < ActiveRecord::Base + include EachBatch + + self.table_name = 'services' + + belongs_to :project, class_name: 'Project' + + # When users create a cluster, KubernetesService is automatically synchronized + # with Platforms::Kubernetes due to delegate Kubernetes specific logic. + # We only target unmanaged KubernetesService records. + scope :unmanaged_kubernetes_service, -> do + joins( + 'INNER JOIN projects ON projects.id = services.project_id' \ + 'INNER JOIN cluster_projects ON projects.id = cluster_projects.project_id' \ + 'INNER JOIN clusters ON cluster_projects.cluster_id = clusters.id' \ + 'INNER JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = clusters.id') + .where( + "services.category = 'deployment' AND services.type = 'KubernetesService'" \ + "AND (" \ + " cluster_projects.project_id IS NULL" \ + " OR" \ + " services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')" \ + ")") + end + end + + class ClustersProject < ActiveRecord::Base + self.table_name = 'cluster_projects' + + belongs_to :cluster, class_name: 'Cluster' + belongs_to :project, class_name: 'Project' + end + + class ProvidersGcp < ActiveRecord::Base + self.table_name = 'cluster_providers_gcp' + end + + class PlatformsKubernetes < ActiveRecord::Base + self.table_name = 'cluster_platforms_kubernetes' + end + + def up + Service.unmanaged_kubernetes_service + .find_each(batch_size: 1) do |kubernetes_service| + Cluster.create( + enabled: kubernetes_service.active, + user_id: nil, # KubernetesService doesn't have + name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, + provider_type: Cluster.provider_types[:user], + platform_type: Cluster.platform_types[:kubernetes], + projects: [kubernetes_service.project], + platform_kubernetes_attributes: { + api_url: kubernetes_service.api_url, + ca_cert: kubernetes_service.ca_pem, + namespace: kubernetes_service.namespace, + username: nil, # KubernetesService doesn't have + encrypted_password: nil, # KubernetesService doesn't have + encrypted_password_iv: nil, # KubernetesService doesn't have + token: kubernetes_service.token # encrypted_token and encrypted_token_iv + } ) + end + end + + def down + # noop + end +end -- cgit v1.2.1 From e4745492821440b47a48b75e8786d049fde50fca Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 29 Nov 2017 02:38:55 +0900 Subject: Add test. Disable KubernetesService when migrated --- ...rnetes_service_to_new_clusters_architectures.rb | 23 +-- ...s_service_to_new_clusters_architectures_spec.rb | 189 +++++++++++++++++++++ 2 files changed, 201 insertions(+), 11 deletions(-) create mode 100644 spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index dde69058523..e1e02c2852a 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -46,18 +46,16 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati # with Platforms::Kubernetes due to delegate Kubernetes specific logic. # We only target unmanaged KubernetesService records. scope :unmanaged_kubernetes_service, -> do - joins( - 'INNER JOIN projects ON projects.id = services.project_id' \ - 'INNER JOIN cluster_projects ON projects.id = cluster_projects.project_id' \ - 'INNER JOIN clusters ON cluster_projects.cluster_id = clusters.id' \ + joins('INNER JOIN projects ON projects.id = services.project_id ' \ + 'INNER JOIN cluster_projects ON projects.id = cluster_projects.project_id ' \ + 'INNER JOIN clusters ON cluster_projects.cluster_id = clusters.id ' \ 'INNER JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = clusters.id') - .where( - "services.category = 'deployment' AND services.type = 'KubernetesService'" \ - "AND (" \ - " cluster_projects.project_id IS NULL" \ - " OR" \ - " services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')" \ - ")") + .where("services.category = 'deployment' AND services.type = 'KubernetesService' " \ + "AND ( " \ + " cluster_projects.project_id IS NULL " \ + " OR " \ + " services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%') " \ + ") ") end end @@ -95,6 +93,9 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati encrypted_password_iv: nil, # KubernetesService doesn't have token: kubernetes_service.token # encrypted_token and encrypted_token_iv } ) + + # Disable the service, so that new cluster archetecture is going to be used + kubernetes_service.updated(active: false) end end diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb new file mode 100644 index 00000000000..5621f2689fa --- /dev/null +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -0,0 +1,189 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb') + +describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do + context 'when user configured kubernetes from CI/CD > Clusters' do + let(:project) { create(:project) } + let(:user) { create(:user) } + + # Platforms::Kubernetes (New archtecture) + let!(:cluster) do + create(:cluster, + projects: [project], + user: user, + provider_type: :gcp, + platform_type: :kubernetes, + provider_gcp: provider_gcp, + platform_kubernetes: platform_kubernetes) + end + + let(:provider_gcp) { create(:cluster_provider_gcp, :created) } + let(:platform_kubernetes) { create(:cluster_platform_kubernetes, :configured) } + + # KubernetesService (Automatically synchronized when Platforms::Kubernetes created) + let!(:kubernetes_service) { create(:kubernetes_service, project: project) } + + context 'when user is using the cluster' do + it 'migrates' do + expect{ migrate! }.not_to change { Clusters::Cluster.count } + expect(cluster).to be_active + expect(kubernetes_service).not_to be_active + end + end + + context 'when user disabled cluster' do + before do + disable_cluster! + end + + context 'when user configured kubernetes from Integration > Kubernetes' do + before do + kubernetes_service.update( + active: true, + api_url: 'http://new.kube.com', + ca_pem: nil, + token: 'z' * 40).reload + end + + context 'when user is using the kubernetes service' do + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).to be_active + expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + end + + expect(kubernetes_service).not_to be_active + end + end + + context 'when user stopped using the kubernetes service' do + before do + kubernetes_service.update(active: false) + end + + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).not_to be_active + expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + end + + expect(kubernetes_service).not_to be_active + end + end + end + end + + context 'when user deleted cluster' do + before do + destory_cluster! + end + + context 'when user configured kubernetes from Integration > Kubernetes' do + let!(:new_kubernetes_service) do + project.create_kubernetes_service( + active: true, + api_url: 'http://123.123.123.123', + ca_pem: nil, + token: 'a' * 40) + end + + context 'when user is using the kubernetes service' do + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).to be_active + expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) + end + + expect(new_kubernetes_service).not_to be_active + end + end + + context 'when user stopped using the kubernetes service' do + before do + new_kubernetes_service.update(active: false) + end + + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).not_to be_active + expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) + end + + expect(new_kubernetes_service).not_to be_active + end + end + end + end + end + + context 'when user configured kubernetes from Integration > Kubernetes' do + let(:project) { create(:project) } + let!(:kubernetes_service) { create(:kubernetes_service, project: project) } + + context 'when user is using the kubernetes service' do + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).to be_active + expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + end + + expect(kubernetes_service).not_to be_active + end + end + + context 'when user stopped using the kubernetes service' do + before do + kubernetes_service.update(active: false) + end + + it 'migrates' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + Clusters::Cluster.last.tap do |c| + expect(c).not_to be_active + expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + end + + expect(kubernetes_service).not_to be_active + end + end + end + + context 'when nothing is configured' do + it 'migrates' do + expect{ migrate! }.not_to change { Clusters::Cluster.count } + end + end + + def disable_cluster! + cluster.update!(enabled: false) + kubernetes_service.update!(active: false) + end + + def destory_cluster! + cluster.destroy! + kubernetes_service.destroy! + end +end -- cgit v1.2.1 From b9fbfe5a6b21bed39010224fd012366d3d39b117 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 14 Dec 2017 00:17:56 +0900 Subject: Fix unmanaged_kubernetes_service scope for multiple clusters --- ...rnetes_service_to_new_clusters_architectures.rb | 44 +++++++++++++--------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index e1e02c2852a..4b9e3094f94 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -1,13 +1,12 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migration DOWNTIME = false - DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService' + DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService'.freeze class Cluster < ActiveRecord::Base self.table_name = 'clusters' has_many :cluster_projects, class_name: 'ClustersProject' has_many :projects, through: :cluster_projects, class_name: 'Project' - has_one :provider_gcp, class_name: 'ProvidersGcp' has_one :platform_kubernetes, class_name: 'PlatformsKubernetes' attr_encrypted :token, @@ -15,7 +14,6 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati key: Gitlab::Application.secrets.db_key_base, algorithm: 'aes-256-cbc' - accepts_nested_attributes_for :provider_gcp accepts_nested_attributes_for :platform_kubernetes enum platform_type: { @@ -31,8 +29,8 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati class Project < ActiveRecord::Base self.table_name = 'projects' - has_one :cluster_project, class_name: 'ClustersProject' - has_one :cluster, through: :cluster_project, class_name: 'Cluster' + has_many :cluster_projects, class_name: 'ClustersProject' + has_many :clusters, through: :cluster_projects, class_name: 'Cluster' end class Service < ActiveRecord::Base @@ -42,20 +40,30 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'Project' + # 10.1 ~ 10.2 # When users create a cluster, KubernetesService is automatically synchronized # with Platforms::Kubernetes due to delegate Kubernetes specific logic. # We only target unmanaged KubernetesService records. + # + # 10.3 + # We no longer create KubernetesService because Platforms::Kubernetes has the specific logic. + # + # "unmanaged" means "unmanaged by Platforms::Kubernetes(New archetecture)" + # + # "cluster_projects.project_id IS NULL" -> it's not copied from KubernetesService + # "services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')" -> KubernetesService has unique configuration which is not included in Platforms::Kubernetes scope :unmanaged_kubernetes_service, -> do - joins('INNER JOIN projects ON projects.id = services.project_id ' \ - 'INNER JOIN cluster_projects ON projects.id = cluster_projects.project_id ' \ - 'INNER JOIN clusters ON cluster_projects.cluster_id = clusters.id ' \ - 'INNER JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = clusters.id') - .where("services.category = 'deployment' AND services.type = 'KubernetesService' " \ - "AND ( " \ - " cluster_projects.project_id IS NULL " \ - " OR " \ - " services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%') " \ - ") ") + joins('INNER JOIN projects ON projects.id = services.project_id') + .where("services.category = 'deployment'") + .where("services.type = 'KubernetesService'") + .where("services.template = FALSE") + .where("NOT EXISTS (?)", + PlatformsKubernetes + .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') + .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') + .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") + .select('1') ) + .order('services.project_id') end end @@ -94,8 +102,10 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati token: kubernetes_service.token # encrypted_token and encrypted_token_iv } ) - # Disable the service, so that new cluster archetecture is going to be used - kubernetes_service.updated(active: false) + # Disable the KubernetesService. Platforms::Kubernetes will be used from next time. + kubernetes_service.active = false + kubernetes_service.propaties.merge!( { migrated: true } ) + kubernetes_service.save! end end -- cgit v1.2.1 From 40c6af546e3a941f829bf91435e848c22bf2aed8 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 15 Dec 2017 02:56:05 +0900 Subject: Fix migration file typos and reorder Table definition --- ...rnetes_service_to_new_clusters_architectures.rb | 46 ++++++++++------------ 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 4b9e3094f94..7c7e5046ec8 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -2,6 +2,13 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati DOWNTIME = false DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService'.freeze + class Project < ActiveRecord::Base + self.table_name = 'projects' + + has_many :cluster_projects, class_name: 'ClustersProject' + has_many :clusters, through: :cluster_projects, class_name: 'Cluster' + end + class Cluster < ActiveRecord::Base self.table_name = 'clusters' @@ -9,11 +16,6 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati has_many :projects, through: :cluster_projects, class_name: 'Project' has_one :platform_kubernetes, class_name: 'PlatformsKubernetes' - attr_encrypted :token, - mode: :per_attribute_iv, - key: Gitlab::Application.secrets.db_key_base, - algorithm: 'aes-256-cbc' - accepts_nested_attributes_for :platform_kubernetes enum platform_type: { @@ -26,11 +28,20 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati } end - class Project < ActiveRecord::Base - self.table_name = 'projects' + class ClustersProject < ActiveRecord::Base + self.table_name = 'cluster_projects' - has_many :cluster_projects, class_name: 'ClustersProject' - has_many :clusters, through: :cluster_projects, class_name: 'Cluster' + belongs_to :cluster, class_name: 'Cluster' + belongs_to :project, class_name: 'Project' + end + + class PlatformsKubernetes < ActiveRecord::Base + self.table_name = 'cluster_platforms_kubernetes' + + attr_encrypted :token, + mode: :per_attribute_iv, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' end class Service < ActiveRecord::Base @@ -67,21 +78,6 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end end - class ClustersProject < ActiveRecord::Base - self.table_name = 'cluster_projects' - - belongs_to :cluster, class_name: 'Cluster' - belongs_to :project, class_name: 'Project' - end - - class ProvidersGcp < ActiveRecord::Base - self.table_name = 'cluster_providers_gcp' - end - - class PlatformsKubernetes < ActiveRecord::Base - self.table_name = 'cluster_platforms_kubernetes' - end - def up Service.unmanaged_kubernetes_service .find_each(batch_size: 1) do |kubernetes_service| @@ -104,7 +100,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati # Disable the KubernetesService. Platforms::Kubernetes will be used from next time. kubernetes_service.active = false - kubernetes_service.propaties.merge!( { migrated: true } ) + kubernetes_service.properties.merge!( { migrated: true } ) kubernetes_service.save! end end -- cgit v1.2.1 From 27111e2940115be9c7c97648a95b80d9fefbf722 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 15 Dec 2017 02:56:23 +0900 Subject: Restructure spec --- ...s_service_to_new_clusters_architectures_spec.rb | 389 ++++++++++++--------- 1 file changed, 229 insertions(+), 160 deletions(-) diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 5621f2689fa..0f270dd37da 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -2,188 +2,257 @@ require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb') describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do - context 'when user configured kubernetes from CI/CD > Clusters' do - let(:project) { create(:project) } - let(:user) { create(:user) } - - # Platforms::Kubernetes (New archtecture) - let!(:cluster) do - create(:cluster, - projects: [project], - user: user, - provider_type: :gcp, - platform_type: :kubernetes, - provider_gcp: provider_gcp, - platform_kubernetes: platform_kubernetes) - end - - let(:provider_gcp) { create(:cluster_provider_gcp, :created) } - let(:platform_kubernetes) { create(:cluster_platform_kubernetes, :configured) } - - # KubernetesService (Automatically synchronized when Platforms::Kubernetes created) - let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - context 'when user is using the cluster' do - it 'migrates' do - expect{ migrate! }.not_to change { Clusters::Cluster.count } - expect(cluster).to be_active - expect(kubernetes_service).not_to be_active - end - end - - context 'when user disabled cluster' do - before do - disable_cluster! - end - - context 'when user configured kubernetes from Integration > Kubernetes' do - before do - kubernetes_service.update( - active: true, - api_url: 'http://new.kube.com', - ca_pem: nil, - token: 'z' * 40).reload + context 'when unique KubernetesService exists' do + shared_examples 'KubernetesService migration' do + let(:sample_num) { 2 } + let(:projects) { create_list(:project, sample_num) } + + let!(:kubernetes_services) do + projects.map do |project| + create(:kubernetes_service, + project: project, + active: active, + api_url: "https://kubernetes#{project.id}.com", + token: defined?(token) ? token : "token#{project.id}", + ca_pem: "ca_pem#{project.id}") end + end - context 'when user is using the kubernetes service' do - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - Clusters::Cluster.last.tap do |c| - expect(c).to be_active - expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - end - - expect(kubernetes_service).not_to be_active - end - end - - context 'when user stopped using the kubernetes service' do - before do - kubernetes_service.update(active: false) - end - - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - Clusters::Cluster.last.tap do |c| - expect(c).not_to be_active - expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - end - - expect(kubernetes_service).not_to be_active + it 'migrates the KubernetesService to Platform::Kubernetes' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + + projects.each do |project| + project.clusters.last.tap do |cluster| + expect(cluster.enabled).to eq(active) + expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) + expect(project.kubernetes_service).not_to be_active + expect(project.kubernetes_service.properties['migrated']).to be_truthy end end end end - context 'when user deleted cluster' do - before do - destory_cluster! - end - - context 'when user configured kubernetes from Integration > Kubernetes' do - let!(:new_kubernetes_service) do - project.create_kubernetes_service( - active: true, - api_url: 'http://123.123.123.123', - ca_pem: nil, - token: 'a' * 40) - end - - context 'when user is using the kubernetes service' do - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + context 'when KubernetesService is active' do + let(:active) { true } - Clusters::Cluster.last.tap do |c| - expect(c).to be_active - expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) - end - - expect(new_kubernetes_service).not_to be_active - end - end - - context 'when user stopped using the kubernetes service' do - before do - new_kubernetes_service.update(active: false) - end + it_behaves_like 'KubernetesService migration' + end - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + context 'when KubernetesService is not active' do + let(:active) { false } - Clusters::Cluster.last.tap do |c| - expect(c).not_to be_active - expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) - end + # Platforms::Kubernetes validates `token` reagdless of the activeness + # KubernetesService validates `token` if only it's activated + # However, in this migration file, there are no validations because of the migration specific model class + # therefore, Validation Error will not happen in this case and just migrate data + let(:token) { '' } - expect(new_kubernetes_service).not_to be_active - end - end - end + it_behaves_like 'KubernetesService migration' end end - context 'when user configured kubernetes from Integration > Kubernetes' do - let(:project) { create(:project) } - let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - context 'when user is using the kubernetes service' do - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - Clusters::Cluster.last.tap do |c| - expect(c).to be_active - expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - end + context 'when unique KubernetesService spawned from Service Template' do + it 'migrates the KubernetesService to Platform::Kubernetes' do - expect(kubernetes_service).not_to be_active - end end + end - context 'when user stopped using the kubernetes service' do - before do - kubernetes_service.update(active: false) - end - - it 'migrates' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - Clusters::Cluster.last.tap do |c| - expect(c).not_to be_active - expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - end + context 'when synced KubernetesService exists' do + it 'does not migrate the KubernetesService' do # Because the corresponding Platform::Kubernetes already exists - expect(kubernetes_service).not_to be_active - end end end - context 'when nothing is configured' do - it 'migrates' do - expect{ migrate! }.not_to change { Clusters::Cluster.count } - end - end + context 'when KubernetesService does not exist' do + it 'does not migrate the KubernetesService' do - def disable_cluster! - cluster.update!(enabled: false) - kubernetes_service.update!(active: false) + end end - def destory_cluster! - cluster.destroy! - kubernetes_service.destroy! - end + # context 'when user configured kubernetes from CI/CD > Clusters' do + # let(:project) { create(:project) } + # let(:user) { create(:user) } + + # # Platforms::Kubernetes (New archtecture) + # let!(:cluster) do + # create(:cluster, + # projects: [project], + # user: user, + # provider_type: :gcp, + # platform_type: :kubernetes, + # provider_gcp: provider_gcp, + # platform_kubernetes: platform_kubernetes) + # end + + # let(:provider_gcp) { create(:cluster_provider_gcp, :created) } + # let(:platform_kubernetes) { create(:cluster_platform_kubernetes, :configured) } + + # # KubernetesService (Automatically synchronized when Platforms::Kubernetes created) + # let!(:kubernetes_service) { create(:kubernetes_service, project: project) } + + # context 'when user is using the cluster' do + # it 'migrates' do + # expect{ migrate! }.not_to change { Clusters::Cluster.count } + # expect(cluster).to be_active + # expect(kubernetes_service).not_to be_active + # end + # end + + # context 'when user disabled cluster' do + # before do + # disable_cluster! + # end + + # context 'when user configured kubernetes from Integration > Kubernetes' do + # before do + # kubernetes_service.update( + # active: true, + # api_url: 'http://new.kube.com', + # ca_pem: nil, + # token: 'z' * 40).reload + # end + + # context 'when user is using the kubernetes service' do + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).to be_active + # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + # end + + # expect(kubernetes_service).not_to be_active + # end + # end + + # context 'when user stopped using the kubernetes service' do + # before do + # kubernetes_service.update(active: false) + # end + + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).not_to be_active + # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + # end + + # expect(kubernetes_service).not_to be_active + # end + # end + # end + # end + + # context 'when user deleted cluster' do + # before do + # destory_cluster! + # end + + # context 'when user configured kubernetes from Integration > Kubernetes' do + # let!(:new_kubernetes_service) do + # project.create_kubernetes_service( + # active: true, + # api_url: 'http://123.123.123.123', + # ca_pem: nil, + # token: 'a' * 40) + # end + + # context 'when user is using the kubernetes service' do + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).to be_active + # expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) + # end + + # expect(new_kubernetes_service).not_to be_active + # end + # end + + # context 'when user stopped using the kubernetes service' do + # before do + # new_kubernetes_service.update(active: false) + # end + + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).not_to be_active + # expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) + # end + + # expect(new_kubernetes_service).not_to be_active + # end + # end + # end + # end + # end + + # context 'when user configured kubernetes from Integration > Kubernetes' do + # let(:project) { create(:project) } + # let!(:kubernetes_service) { create(:kubernetes_service, project: project) } + + # context 'when user is using the kubernetes service' do + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).to be_active + # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + # end + + # expect(kubernetes_service).not_to be_active + # end + # end + + # context 'when user stopped using the kubernetes service' do + # before do + # kubernetes_service.update(active: false) + # end + + # it 'migrates' do + # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + # Clusters::Cluster.last.tap do |c| + # expect(c).not_to be_active + # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) + # end + + # expect(kubernetes_service).not_to be_active + # end + # end + # end + + # context 'when nothing is configured' do + # it 'migrates' do + # expect{ migrate! }.not_to change { Clusters::Cluster.count } + # end + # end + + # def disable_cluster! + # cluster.update!(enabled: false) + # kubernetes_service.update!(active: false) + # end + + # def destory_cluster! + # cluster.destroy! + # kubernetes_service.destroy! + # end end -- cgit v1.2.1 From 4dc14576d506218c71debb3a0600acdf855afe09 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 15 Dec 2017 22:41:30 +0900 Subject: Fix comments --- ...bernetes_service_to_new_clusters_architectures.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 7c7e5046ec8..e05d206eb9c 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -52,24 +52,24 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'Project' # 10.1 ~ 10.2 - # When users create a cluster, KubernetesService is automatically synchronized - # with Platforms::Kubernetes due to delegate Kubernetes specific logic. - # We only target unmanaged KubernetesService records. + # When users created a cluster, KubernetesService was automatically configured + # by Platforms::Kubernetes parameters. + # Because Platforms::Kubernetes delegated some logic to KubernetesService. # # 10.3 - # We no longer create KubernetesService because Platforms::Kubernetes has the specific logic. + # When users create a cluster, KubernetesService is no longer synchronized. + # Because we copied delegated logic in Platforms::Kubernetes. # - # "unmanaged" means "unmanaged by Platforms::Kubernetes(New archetecture)" - # - # "cluster_projects.project_id IS NULL" -> it's not copied from KubernetesService - # "services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')" -> KubernetesService has unique configuration which is not included in Platforms::Kubernetes + # NOTE: + # - "unmanaged" means "unmanaged by Platforms::Kubernetes(New archetecture)" + # - We only want to migrate records which are not synchronized with Platforms::Kubernetes. scope :unmanaged_kubernetes_service, -> do - joins('INNER JOIN projects ON projects.id = services.project_id') - .where("services.category = 'deployment'") + where("services.category = 'deployment'") .where("services.type = 'KubernetesService'") .where("services.template = FALSE") .where("NOT EXISTS (?)", PlatformsKubernetes + .joins('INNER JOIN projects ON projects.id = services.project_id') .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") -- cgit v1.2.1 From 8e6ffe358873ff56eb1d2b6354fac39ce2dfa80e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 15 Dec 2017 22:43:25 +0900 Subject: Fix test --- ...s_service_to_new_clusters_architectures_spec.rb | 239 +++++---------------- 1 file changed, 52 insertions(+), 187 deletions(-) diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 0f270dd37da..d3fa1fe9054 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -54,205 +54,70 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when unique KubernetesService spawned from Service Template' do - it 'migrates the KubernetesService to Platform::Kubernetes' do + let(:sample_num) { 2 } + let(:projects) { create_list(:project, sample_num) } + + let!(:kubernetes_service_template) do + create(:kubernetes_service, + project: nil, + template: true, + api_url: "https://sample.kubernetes.com", + token: "token-sample", + ca_pem: "ca_pem-sample") + end + + let!(:kubernetes_services) do + projects.map do |project| + create(:kubernetes_service, + project: project, + api_url: kubernetes_service_template.api_url, + token: kubernetes_service_template.token, + ca_pem: kubernetes_service_template.ca_pem) + end + end + + it 'migrates the KubernetesService to Platform::Kubernetes without template' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + projects.each do |project| + project.clusters.last.tap do |cluster| + expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) + expect(project.kubernetes_service).not_to be_active + expect(project.kubernetes_service.properties['migrated']).to be_truthy + end + end end end context 'when synced KubernetesService exists' do + let(:project) { create(:project) } + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + let!(:platform_kubernetes) { cluster.platform_kubernetes } + + let!(:kubernetes_service) do + create(:kubernetes_service, + project: project, + active: cluster.enabled, + api_url: platform_kubernetes.api_url, + token: platform_kubernetes.token, + ca_pem: platform_kubernetes.ca_cert) + end + it 'does not migrate the KubernetesService' do # Because the corresponding Platform::Kubernetes already exists + expect{ migrate! }.not_to change { Clusters::Cluster.count } + expect(kubernetes_service).to be_active + expect(kubernetes_service.properties['migrated']).to be_falsy end end context 'when KubernetesService does not exist' do - it 'does not migrate the KubernetesService' do + let!(:project) { create(:project) } + it 'does not migrate the KubernetesService' do + expect{ migrate! }.not_to change { Clusters::Cluster.count } end end - - # context 'when user configured kubernetes from CI/CD > Clusters' do - # let(:project) { create(:project) } - # let(:user) { create(:user) } - - # # Platforms::Kubernetes (New archtecture) - # let!(:cluster) do - # create(:cluster, - # projects: [project], - # user: user, - # provider_type: :gcp, - # platform_type: :kubernetes, - # provider_gcp: provider_gcp, - # platform_kubernetes: platform_kubernetes) - # end - - # let(:provider_gcp) { create(:cluster_provider_gcp, :created) } - # let(:platform_kubernetes) { create(:cluster_platform_kubernetes, :configured) } - - # # KubernetesService (Automatically synchronized when Platforms::Kubernetes created) - # let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - # context 'when user is using the cluster' do - # it 'migrates' do - # expect{ migrate! }.not_to change { Clusters::Cluster.count } - # expect(cluster).to be_active - # expect(kubernetes_service).not_to be_active - # end - # end - - # context 'when user disabled cluster' do - # before do - # disable_cluster! - # end - - # context 'when user configured kubernetes from Integration > Kubernetes' do - # before do - # kubernetes_service.update( - # active: true, - # api_url: 'http://new.kube.com', - # ca_pem: nil, - # token: 'z' * 40).reload - # end - - # context 'when user is using the kubernetes service' do - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).to be_active - # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - # end - - # expect(kubernetes_service).not_to be_active - # end - # end - - # context 'when user stopped using the kubernetes service' do - # before do - # kubernetes_service.update(active: false) - # end - - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).not_to be_active - # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - # end - - # expect(kubernetes_service).not_to be_active - # end - # end - # end - # end - - # context 'when user deleted cluster' do - # before do - # destory_cluster! - # end - - # context 'when user configured kubernetes from Integration > Kubernetes' do - # let!(:new_kubernetes_service) do - # project.create_kubernetes_service( - # active: true, - # api_url: 'http://123.123.123.123', - # ca_pem: nil, - # token: 'a' * 40) - # end - - # context 'when user is using the kubernetes service' do - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).to be_active - # expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) - # end - - # expect(new_kubernetes_service).not_to be_active - # end - # end - - # context 'when user stopped using the kubernetes service' do - # before do - # new_kubernetes_service.update(active: false) - # end - - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).not_to be_active - # expect(c.platform_kubernetes.api_url).to eq(new_kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(new_kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(new_kubernetes_service.token) - # end - - # expect(new_kubernetes_service).not_to be_active - # end - # end - # end - # end - # end - - # context 'when user configured kubernetes from Integration > Kubernetes' do - # let(:project) { create(:project) } - # let!(:kubernetes_service) { create(:kubernetes_service, project: project) } - - # context 'when user is using the kubernetes service' do - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).to be_active - # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - # end - - # expect(kubernetes_service).not_to be_active - # end - # end - - # context 'when user stopped using the kubernetes service' do - # before do - # kubernetes_service.update(active: false) - # end - - # it 'migrates' do - # expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) - - # Clusters::Cluster.last.tap do |c| - # expect(c).not_to be_active - # expect(c.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - # expect(c.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) - # expect(c.platform_kubernetes.token).to eq(kubernetes_service.token) - # end - - # expect(kubernetes_service).not_to be_active - # end - # end - # end - - # context 'when nothing is configured' do - # it 'migrates' do - # expect{ migrate! }.not_to change { Clusters::Cluster.count } - # end - # end - - # def disable_cluster! - # cluster.update!(enabled: false) - # kubernetes_service.update!(active: false) - # end - - # def destory_cluster! - # cluster.destroy! - # kubernetes_service.destroy! - # end end -- cgit v1.2.1 From 7eeada80d5c03025c8c37bc1c1567f2500acc8d1 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 18 Dec 2017 16:32:41 +0900 Subject: Add env_scope tests --- ...rnetes_service_to_new_clusters_architectures.rb | 1 + ...s_service_to_new_clusters_architectures_spec.rb | 32 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index e05d206eb9c..5dee8cb8c5f 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -88,6 +88,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati provider_type: Cluster.provider_types[:user], platform_type: Cluster.platform_types[:kubernetes], projects: [kubernetes_service.project], + environment_scope: '*', # KubernetesService is considered as a default cluster platform_kubernetes_attributes: { api_url: kubernetes_service.api_url, ca_cert: kubernetes_service.ca_pem, diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index d3fa1fe9054..bf41b2df359 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -113,6 +113,38 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end + context 'when production cluster has already been existsed' do + let(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'production/*', projects: [project]) } + let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + + it 'migrates the KubernetesService to Platform::Kubernetes' do + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + kubernetes_service.reload + project.clusters.last.tap do |cluster| + expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) + expect(kubernetes_service).not_to be_active + expect(kubernetes_service.properties['migrated']).to be_truthy + end + end + end + + context 'when default cluster has already been existsed' do + let(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } + let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + + it 'does not migrate the KubernetesService' do # Because environment_scope is duplicated + expect{ migrate! }.not_to change { Clusters::Cluster.count } + + expect(kubernetes_service).to be_active + expect(kubernetes_service.properties['migrated']).to be_falsy + end + end + context 'when KubernetesService does not exist' do let!(:project) { create(:project) } -- cgit v1.2.1 From f083739e9935f774984f4c8ba2ce4663c29c7564 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 3 Jan 2018 20:58:54 +0900 Subject: Add logic to swtich environment_scope by the situation --- ...rnetes_service_to_new_clusters_architectures.rb | 52 ++++++++++++---------- ...s_service_to_new_clusters_architectures_spec.rb | 50 ++++++++++++++++----- 2 files changed, 69 insertions(+), 33 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 5dee8cb8c5f..46b40ea7a44 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -51,36 +51,41 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'Project' - # 10.1 ~ 10.2 - # When users created a cluster, KubernetesService was automatically configured - # by Platforms::Kubernetes parameters. - # Because Platforms::Kubernetes delegated some logic to KubernetesService. - # - # 10.3 - # When users create a cluster, KubernetesService is no longer synchronized. - # Because we copied delegated logic in Platforms::Kubernetes. - # - # NOTE: - # - "unmanaged" means "unmanaged by Platforms::Kubernetes(New archetecture)" - # - We only want to migrate records which are not synchronized with Platforms::Kubernetes. - scope :unmanaged_kubernetes_service, -> do + scope :kubernetes_service, -> do where("services.category = 'deployment'") .where("services.type = 'KubernetesService'") .where("services.template = FALSE") - .where("NOT EXISTS (?)", - PlatformsKubernetes - .joins('INNER JOIN projects ON projects.id = services.project_id') - .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') - .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') - .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") - .select('1') ) .order('services.project_id') end end + def find_dedicated_environement_scope(project) + environment_scopes = project.clusters.map(&:environment_scope) + + return '*' if environment_scopes.exclude?('*') # KubernetesService should be added as a default cluster (environment_scope: '*') at first place + return 'migrated/*' if environment_scopes.exclude?('migrated/*') # If it's conflicted, the KubernetesService added as a migrated cluster + + unique_iid = 0 + + # If it's still conflicted, finding an unique environment scope incrementaly + while true + candidate = "migrated#{unique_iid}/*" + return candidate if environment_scopes.exclude?(candidate) + unique_iid += 1 + end + end + + # KubernetesService might be already managed by clusters + def managed_by_clusters?(kubernetes_service) + kubernetes_service.project.clusters + .joins('INNER JOIN cluster_platforms_kubernetes ON clusters.id = cluster_platforms_kubernetes.cluster_id') + .where('cluster_platforms_kubernetes.api_url = ?', kubernetes_service.api_url) + .exists? + end + def up - Service.unmanaged_kubernetes_service - .find_each(batch_size: 1) do |kubernetes_service| + Service.kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| + unless managed_by_clusters?(kubernetes_service) Cluster.create( enabled: kubernetes_service.active, user_id: nil, # KubernetesService doesn't have @@ -88,7 +93,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati provider_type: Cluster.provider_types[:user], platform_type: Cluster.platform_types[:kubernetes], projects: [kubernetes_service.project], - environment_scope: '*', # KubernetesService is considered as a default cluster + environment_scope: find_dedicated_environement_scope(kubernetes_service.project), platform_kubernetes_attributes: { api_url: kubernetes_service.api_url, ca_cert: kubernetes_service.ca_pem, @@ -98,6 +103,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati encrypted_password_iv: nil, # KubernetesService doesn't have token: kubernetes_service.token # encrypted_token and encrypted_token_iv } ) + end # Disable the KubernetesService. Platforms::Kubernetes will be used from next time. kubernetes_service.active = false diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index bf41b2df359..5ce1c43d92a 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -91,7 +91,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end - context 'when synced KubernetesService exists' do + context 'when managed KubernetesService exists' do let(:project) { create(:project) } let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } let!(:platform_kubernetes) { cluster.platform_kubernetes } @@ -105,15 +105,16 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do ca_pem: platform_kubernetes.ca_cert) end - it 'does not migrate the KubernetesService' do # Because the corresponding Platform::Kubernetes already exists + it 'does not migrate the KubernetesService and disables the kubernetes_service' do # Because the corresponding Platform::Kubernetes already exists expect{ migrate! }.not_to change { Clusters::Cluster.count } - expect(kubernetes_service).to be_active - expect(kubernetes_service.properties['migrated']).to be_falsy + kubernetes_service.reload + expect(kubernetes_service).not_to be_active + expect(kubernetes_service.properties['migrated']).to be_truthy end end - context 'when production cluster has already been existsed' do + context 'when production cluster has already been existed' do # i.e. There are no environment_scope conflicts let(:project) { create(:project) } let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'production/*', projects: [project]) } let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } @@ -123,6 +124,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do kubernetes_service.reload project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('*') expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) @@ -132,16 +134,44 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end - context 'when default cluster has already been existsed' do + context 'when default cluster has already been existed' do let(:project) { create(:project) } let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } - it 'does not migrate the KubernetesService' do # Because environment_scope is duplicated - expect{ migrate! }.not_to change { Clusters::Cluster.count } + it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + kubernetes_service.reload + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('migrated/*') + expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) + expect(kubernetes_service).not_to be_active + expect(kubernetes_service.properties['migrated']).to be_truthy + end + end + end - expect(kubernetes_service).to be_active - expect(kubernetes_service.properties['migrated']).to be_falsy + context 'when default cluster and migrated cluster has already been existed' do + let(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } + let!(:migrated_cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'migrated/*', projects: [project]) } + let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + + it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated + expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + + kubernetes_service.reload + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('migrated0/*') + expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) + expect(kubernetes_service).not_to be_active + expect(kubernetes_service.properties['migrated']).to be_truthy + end end end -- cgit v1.2.1 From 9b7719b6c60f2ba9a2e8e6ded543bb4fb87cd11b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 3 Jan 2018 23:44:08 +0900 Subject: Use explicit namespace for avoiding reference from application code --- ...rnetes_service_to_new_clusters_architectures.rb | 29 ++++++++++++---------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 46b40ea7a44..da34880b4e1 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -5,16 +5,17 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati class Project < ActiveRecord::Base self.table_name = 'projects' - has_many :cluster_projects, class_name: 'ClustersProject' - has_many :clusters, through: :cluster_projects, class_name: 'Cluster' + has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' + has_many :clusters, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' + has_many :services, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service' end class Cluster < ActiveRecord::Base self.table_name = 'clusters' - has_many :cluster_projects, class_name: 'ClustersProject' - has_many :projects, through: :cluster_projects, class_name: 'Project' - has_one :platform_kubernetes, class_name: 'PlatformsKubernetes' + has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' + has_many :projects, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' + has_one :platform_kubernetes, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes' accepts_nested_attributes_for :platform_kubernetes @@ -31,13 +32,15 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati class ClustersProject < ActiveRecord::Base self.table_name = 'cluster_projects' - belongs_to :cluster, class_name: 'Cluster' - belongs_to :project, class_name: 'Project' + belongs_to :cluster, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' + belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' end class PlatformsKubernetes < ActiveRecord::Base self.table_name = 'cluster_platforms_kubernetes' + belongs_to :cluster, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' + attr_encrypted :token, mode: :per_attribute_iv, key: Gitlab::Application.secrets.db_key_base, @@ -49,7 +52,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati self.table_name = 'services' - belongs_to :project, class_name: 'Project' + belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' scope :kubernetes_service, -> do where("services.category = 'deployment'") @@ -84,15 +87,15 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end def up - Service.kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| + MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| unless managed_by_clusters?(kubernetes_service) - Cluster.create( + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( enabled: kubernetes_service.active, user_id: nil, # KubernetesService doesn't have name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, - provider_type: Cluster.provider_types[:user], - platform_type: Cluster.platform_types[:kubernetes], - projects: [kubernetes_service.project], + provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], + platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], + projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], environment_scope: find_dedicated_environement_scope(kubernetes_service.project), platform_kubernetes_attributes: { api_url: kubernetes_service.api_url, -- cgit v1.2.1 From 665972e2f51ae8a72a307dc4926d379bff816336 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 3 Jan 2018 23:49:34 +0900 Subject: Avoid quotes in ActiveRecord query --- ...27_migrate_kubernetes_service_to_new_clusters_architectures.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index da34880b4e1..e6c7ea58bd2 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -55,10 +55,10 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' scope :kubernetes_service, -> do - where("services.category = 'deployment'") - .where("services.type = 'KubernetesService'") - .where("services.template = FALSE") - .order('services.project_id') + where(category: 'deployment') + .where(type: 'KubernetesService') + .where(template: false) + .order(project_id: :asc) end end -- cgit v1.2.1 From c80598816c4cb79c25255ecd53906c84fc215a87 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 4 Jan 2018 23:49:42 +0900 Subject: Opitmize migration process by using both unmanaged_kubernetes_service and kubernetes_service_without_template --- ...rnetes_service_to_new_clusters_architectures.rb | 70 +++++++++++----------- ...s_service_to_new_clusters_architectures_spec.rb | 6 -- 2 files changed, 36 insertions(+), 40 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index e6c7ea58bd2..32f8ef3ff02 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -54,7 +54,21 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' - scope :kubernetes_service, -> do + scope :unmanaged_kubernetes_service, -> do + where(category: 'deployment') + .where(type: 'KubernetesService') + .where(template: false) + .where("NOT EXISTS (?)", + MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes + .joins('INNER JOIN projects ON projects.id = services.project_id') + .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') + .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') + .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") + .select('1') ) + .order(project_id: :asc) + end + + scope :kubernetes_service_without_template, -> do where(category: 'deployment') .where(type: 'KubernetesService') .where(template: false) @@ -78,41 +92,29 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end end - # KubernetesService might be already managed by clusters - def managed_by_clusters?(kubernetes_service) - kubernetes_service.project.clusters - .joins('INNER JOIN cluster_platforms_kubernetes ON clusters.id = cluster_platforms_kubernetes.cluster_id') - .where('cluster_platforms_kubernetes.api_url = ?', kubernetes_service.api_url) - .exists? - end - def up - MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| - unless managed_by_clusters?(kubernetes_service) - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( - enabled: kubernetes_service.active, - user_id: nil, # KubernetesService doesn't have - name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, - provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], - platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], - projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], - environment_scope: find_dedicated_environement_scope(kubernetes_service.project), - platform_kubernetes_attributes: { - api_url: kubernetes_service.api_url, - ca_cert: kubernetes_service.ca_pem, - namespace: kubernetes_service.namespace, - username: nil, # KubernetesService doesn't have - encrypted_password: nil, # KubernetesService doesn't have - encrypted_password_iv: nil, # KubernetesService doesn't have - token: kubernetes_service.token # encrypted_token and encrypted_token_iv - } ) - end - - # Disable the KubernetesService. Platforms::Kubernetes will be used from next time. - kubernetes_service.active = false - kubernetes_service.properties.merge!( { migrated: true } ) - kubernetes_service.save! + MigrateKubernetesServiceToNewClustersArchitectures::Service + .unmanaged_kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( + enabled: kubernetes_service.active, + user_id: nil, # KubernetesService doesn't have + name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, + provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], + platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], + projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], + environment_scope: find_dedicated_environement_scope(kubernetes_service.project), + platform_kubernetes_attributes: { + api_url: kubernetes_service.api_url, + ca_cert: kubernetes_service.ca_pem, + namespace: kubernetes_service.namespace, + username: nil, # KubernetesService doesn't have + encrypted_password: nil, # KubernetesService doesn't have + encrypted_password_iv: nil, # KubernetesService doesn't have + token: kubernetes_service.token # encrypted_token and encrypted_token_iv + } ) end + + MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service_without_template.update_all(active: false) end def down diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 5ce1c43d92a..d6643a63f0d 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -28,7 +28,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) expect(project.kubernetes_service).not_to be_active - expect(project.kubernetes_service.properties['migrated']).to be_truthy end end end @@ -85,7 +84,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) expect(project.kubernetes_service).not_to be_active - expect(project.kubernetes_service.properties['migrated']).to be_truthy end end end @@ -110,7 +108,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do kubernetes_service.reload expect(kubernetes_service).not_to be_active - expect(kubernetes_service.properties['migrated']).to be_truthy end end @@ -129,7 +126,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active - expect(kubernetes_service.properties['migrated']).to be_truthy end end end @@ -149,7 +145,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active - expect(kubernetes_service.properties['migrated']).to be_truthy end end end @@ -170,7 +165,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active - expect(kubernetes_service.properties['migrated']).to be_truthy end end end -- cgit v1.2.1 From b8a275d3e46a4204505ed5a4b7a9b3a6d49c9b4f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 5 Jan 2018 00:53:50 +0900 Subject: Use bulk_insert instead of AR create --- ...rnetes_service_to_new_clusters_architectures.rb | 57 +++++++++++++++++----- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 32f8ef3ff02..f2455a3d8b0 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -94,24 +94,57 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati def up MigrateKubernetesServiceToNewClustersArchitectures::Service - .unmanaged_kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| - MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( - enabled: kubernetes_service.active, - user_id: nil, # KubernetesService doesn't have - name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, - provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], - platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], - projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], - environment_scope: find_dedicated_environement_scope(kubernetes_service.project), - platform_kubernetes_attributes: { + .unmanaged_kubernetes_service.each_batch(of: 100) do |kubernetes_services| + + rows_for_clusters = kubernetes_services.map do |kubernetes_service| + { + enabled: kubernetes_service.active, + user_id: nil, # KubernetesService doesn't have + name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, + provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], + platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], + environment_scope: find_dedicated_environement_scope(kubernetes_service.project), + created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), + updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) + } + end + + inserted_cluster_ids = Gitlab::Database.bulk_insert('clusters', rows_for_clusters, return_ids: true) + + rows_for_cluster_platforms_kubernetes = kubernetes_services.each_with_index.map do |kubernetes_service, i| + + # Create PlatformsKubernetes instance for generating an encrypted token + platforms_kubernetes = + MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes + .new(token: kubernetes_service.token) + + { + cluster_id: inserted_cluster_ids[i], api_url: kubernetes_service.api_url, ca_cert: kubernetes_service.ca_pem, namespace: kubernetes_service.namespace, username: nil, # KubernetesService doesn't have encrypted_password: nil, # KubernetesService doesn't have encrypted_password_iv: nil, # KubernetesService doesn't have - token: kubernetes_service.token # encrypted_token and encrypted_token_iv - } ) + encrypted_token: platforms_kubernetes.encrypted_token, # encrypted_token and encrypted_token_iv + encrypted_token_iv: platforms_kubernetes.encrypted_token_iv, # encrypted_token and encrypted_token_iv + created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), + updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) + } + end + + Gitlab::Database.bulk_insert('cluster_platforms_kubernetes', rows_for_cluster_platforms_kubernetes) + + rows_for_cluster_projects = kubernetes_services.each_with_index.map do |kubernetes_service, i| + { + cluster_id: inserted_cluster_ids[i], + project_id: kubernetes_service.project_id, + created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), + updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) + } + end + + Gitlab::Database.bulk_insert('cluster_projects', rows_for_cluster_projects) end MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service_without_template.update_all(active: false) -- cgit v1.2.1 From acfb8464bef7b43b730ddf433a8223fc240d968a Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 5 Jan 2018 15:22:45 +0900 Subject: Fix static anylysy --- ...ate_kubernetes_service_to_new_clusters_architectures.rb | 5 +++-- ...ubernetes_service_to_new_clusters_architectures_spec.rb | 14 +++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index f2455a3d8b0..5ea115cea90 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -66,7 +66,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") .select('1') ) .order(project_id: :asc) - end + end scope :kubernetes_service_without_template, -> do where(category: 'deployment') @@ -85,9 +85,10 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati unique_iid = 0 # If it's still conflicted, finding an unique environment scope incrementaly - while true + loop do candidate = "migrated#{unique_iid}/*" return candidate if environment_scopes.exclude?(candidate) + unique_iid += 1 end end diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index d6643a63f0d..cade4d7df28 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -19,7 +19,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end it 'migrates the KubernetesService to Platform::Kubernetes' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + expect { migrate! }.to change { Clusters::Cluster.count }.by(sample_num) projects.each do |project| project.clusters.last.tap do |cluster| @@ -76,7 +76,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end it 'migrates the KubernetesService to Platform::Kubernetes without template' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + expect { migrate! }.to change { Clusters::Cluster.count }.by(sample_num) projects.each do |project| project.clusters.last.tap do |cluster| @@ -104,7 +104,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end it 'does not migrate the KubernetesService and disables the kubernetes_service' do # Because the corresponding Platform::Kubernetes already exists - expect{ migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { Clusters::Cluster.count } kubernetes_service.reload expect(kubernetes_service).not_to be_active @@ -117,7 +117,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } it 'migrates the KubernetesService to Platform::Kubernetes' do - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { Clusters::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| @@ -136,7 +136,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { Clusters::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| @@ -156,7 +156,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect{ migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { Clusters::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| @@ -173,7 +173,7 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let!(:project) { create(:project) } it 'does not migrate the KubernetesService' do - expect{ migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { Clusters::Cluster.count } end end end -- cgit v1.2.1 From 8bc3221f2fe096e6c1f2070a2fcdb18903c2c599 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 5 Jan 2018 19:43:03 +0900 Subject: Fix query to look for proper unmanaged kubernetes service --- ...rnetes_service_to_new_clusters_architectures.rb | 29 +++++++++------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 5ea115cea90..aff934c2ab4 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -55,24 +55,16 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' scope :unmanaged_kubernetes_service, -> do - where(category: 'deployment') + joins('LEFT JOIN projects ON projects.id = services.project_id') + .joins('LEFT JOIN cluster_projects ON cluster_projects.project_id = projects.id') + .joins('LEFT JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = cluster_projects.cluster_id') + .where(category: 'deployment') .where(type: 'KubernetesService') .where(template: false) - .where("NOT EXISTS (?)", - MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes - .joins('INNER JOIN projects ON projects.id = services.project_id') - .joins('INNER JOIN cluster_projects ON cluster_projects.project_id = projects.id') - .where('cluster_projects.cluster_id = cluster_platforms_kubernetes.cluster_id') - .where("services.properties LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')") - .select('1') ) - .order(project_id: :asc) - end - - scope :kubernetes_service_without_template, -> do - where(category: 'deployment') - .where(type: 'KubernetesService') - .where(template: false) - .order(project_id: :asc) + .where("services.properties LIKE '%api_url%'") + .where("(services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')) OR cluster_platforms_kubernetes.api_url IS NULL") + .group(:id) + .order(id: :asc) end end @@ -148,7 +140,10 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati Gitlab::Database.bulk_insert('cluster_projects', rows_for_cluster_projects) end - MigrateKubernetesServiceToNewClustersArchitectures::Service.kubernetes_service_without_template.update_all(active: false) + connection.execute <<~SQL + UPDATE services SET active = false + WHERE category = 'deployment' AND type = 'KubernetesService' AND template = false + SQL end def down -- cgit v1.2.1 From 2d3c7d29b2950110f51cde9b6c8c39f9d7404884 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 5 Jan 2018 23:24:51 +0900 Subject: Use batch update for Service deactivation --- ...7_migrate_kubernetes_service_to_new_clusters_architectures.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index aff934c2ab4..2808c8e0222 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -140,10 +140,11 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati Gitlab::Database.bulk_insert('cluster_projects', rows_for_cluster_projects) end - connection.execute <<~SQL - UPDATE services SET active = false - WHERE category = 'deployment' AND type = 'KubernetesService' AND template = false - SQL + MigrateKubernetesServiceToNewClustersArchitectures::Service + .where(category: 'deployment', type: 'KubernetesService', template: false) + .each_batch(of: 100) do |batch| + batch.update_all(active: false) + end end def down -- cgit v1.2.1 From 1c404c91b6e0bb0fac335083eff2f286d0da6df1 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 00:37:40 +0900 Subject: Add a new test for emptified params --- ...es_service_to_new_clusters_architectures_spec.rb | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index cade4d7df28..5f135ad274e 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -42,8 +42,8 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do context 'when KubernetesService is not active' do let(:active) { false } - # Platforms::Kubernetes validates `token` reagdless of the activeness - # KubernetesService validates `token` if only it's activated + # Platforms::Kubernetes validates `token` reagdless of the activeness, + # whereas KubernetesService validates `token` if only it's activated # However, in this migration file, there are no validations because of the migration specific model class # therefore, Validation Error will not happen in this case and just migrate data let(:token) { '' } @@ -169,6 +169,23 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end + context 'when KubernetesService has nullified parameters' do + let(:project) { create(:project) } + + before do + ActiveRecord::Base.connection.execute <<~SQL + INSERT INTO services (project_id, active, category, type, properties) + VALUES (#{project.id}, false, 'deployment', 'KubernetesService', '{}'); + SQL + end + + it 'does not migrate the KubernetesService and disables the kubernetes_service' do + expect { migrate! }.not_to change { Clusters::Cluster.count } + + expect(project.kubernetes_service).not_to be_active + end + end + context 'when KubernetesService does not exist' do let!(:project) { create(:project) } -- cgit v1.2.1 From 183dbdc8b8a68bdcbfb465abcbdae2d2292d9386 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 01:11:06 +0900 Subject: Revert bulk_insert and bring back AR insert(one by one) --- ...rnetes_service_to_new_clusters_architectures.rb | 78 ++++++++-------------- ...s_service_to_new_clusters_architectures_spec.rb | 40 +++++++---- 2 files changed, 54 insertions(+), 64 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 2808c8e0222..27d56fe5b42 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -1,7 +1,11 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + DOWNTIME = false DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService'.freeze + disable_ddl_transaction! + class Project < ActiveRecord::Base self.table_name = 'projects' @@ -58,14 +62,16 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati joins('LEFT JOIN projects ON projects.id = services.project_id') .joins('LEFT JOIN cluster_projects ON cluster_projects.project_id = projects.id') .joins('LEFT JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = cluster_projects.cluster_id') - .where(category: 'deployment') - .where(type: 'KubernetesService') - .where(template: false) + .where(category: 'deployment', type: 'KubernetesService', template: false) .where("services.properties LIKE '%api_url%'") .where("(services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')) OR cluster_platforms_kubernetes.api_url IS NULL") .group(:id) .order(id: :asc) end + + scope :kubernetes_service_without_template, -> do + where(category: 'deployment', type: 'KubernetesService', template: false) + end end def find_dedicated_environement_scope(project) @@ -86,65 +92,33 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end def up - MigrateKubernetesServiceToNewClustersArchitectures::Service - .unmanaged_kubernetes_service.each_batch(of: 100) do |kubernetes_services| - - rows_for_clusters = kubernetes_services.map do |kubernetes_service| - { + ActiveRecord::Base.transaction do + MigrateKubernetesServiceToNewClustersArchitectures::Service + .unmanaged_kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( enabled: kubernetes_service.active, user_id: nil, # KubernetesService doesn't have name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], + projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], environment_scope: find_dedicated_environement_scope(kubernetes_service.project), - created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), - updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) - } - end - - inserted_cluster_ids = Gitlab::Database.bulk_insert('clusters', rows_for_clusters, return_ids: true) - - rows_for_cluster_platforms_kubernetes = kubernetes_services.each_with_index.map do |kubernetes_service, i| - - # Create PlatformsKubernetes instance for generating an encrypted token - platforms_kubernetes = - MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes - .new(token: kubernetes_service.token) - - { - cluster_id: inserted_cluster_ids[i], - api_url: kubernetes_service.api_url, - ca_cert: kubernetes_service.ca_pem, - namespace: kubernetes_service.namespace, - username: nil, # KubernetesService doesn't have - encrypted_password: nil, # KubernetesService doesn't have - encrypted_password_iv: nil, # KubernetesService doesn't have - encrypted_token: platforms_kubernetes.encrypted_token, # encrypted_token and encrypted_token_iv - encrypted_token_iv: platforms_kubernetes.encrypted_token_iv, # encrypted_token and encrypted_token_iv - created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), - updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) - } - end - - Gitlab::Database.bulk_insert('cluster_platforms_kubernetes', rows_for_cluster_platforms_kubernetes) - - rows_for_cluster_projects = kubernetes_services.each_with_index.map do |kubernetes_service, i| - { - cluster_id: inserted_cluster_ids[i], - project_id: kubernetes_service.project_id, - created_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.created_at), - updated_at: Gitlab::Database.sanitize_timestamp(kubernetes_service.updated_at) - } + platform_kubernetes_attributes: { + api_url: kubernetes_service.api_url, + ca_cert: kubernetes_service.ca_pem, + namespace: kubernetes_service.namespace, + username: nil, # KubernetesService doesn't have + encrypted_password: nil, # KubernetesService doesn't have + encrypted_password_iv: nil, # KubernetesService doesn't have + token: kubernetes_service.token # encrypted_token and encrypted_token_iv + } ) end - - Gitlab::Database.bulk_insert('cluster_projects', rows_for_cluster_projects) end MigrateKubernetesServiceToNewClustersArchitectures::Service - .where(category: 'deployment', type: 'KubernetesService', template: false) - .each_batch(of: 100) do |batch| - batch.update_all(active: false) - end + .kubernetes_service_without_template.each_batch(of: 100) do |kubernetes_service| + kubernetes_service.update_all(active: false) + end end def down diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 5f135ad274e..85ed1328211 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -38,18 +38,6 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do it_behaves_like 'KubernetesService migration' end - - context 'when KubernetesService is not active' do - let(:active) { false } - - # Platforms::Kubernetes validates `token` reagdless of the activeness, - # whereas KubernetesService validates `token` if only it's activated - # However, in this migration file, there are no validations because of the migration specific model class - # therefore, Validation Error will not happen in this case and just migrate data - let(:token) { '' } - - it_behaves_like 'KubernetesService migration' - end end context 'when unique KubernetesService spawned from Service Template' do @@ -186,6 +174,34 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end end + # Platforms::Kubernetes validates `token` reagdless of the activeness, + # whereas KubernetesService validates `token` if only it's activated + # However, in this migration file, there are no validations because of the re-defined model class + # therefore, we should safely add this raw to Platform::Kubernetes + context 'when KubernetesService has empty token' do + let(:project) { create(:project) } + + before do + ActiveRecord::Base.connection.execute <<~SQL + INSERT INTO services (project_id, active, category, type, properties) + VALUES (#{project.id}, false, 'deployment', 'KubernetesService', '{"namespace":"prod","api_url":"http://111.111.111.111","ca_pem":"a","token":""}'); + SQL + end + + it 'does not migrate the KubernetesService and disables the kubernetes_service' do + expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('*') + expect(cluster.platform_kubernetes.namespace).to eq('prod') + expect(cluster.platform_kubernetes.api_url).to eq('http://111.111.111.111') + expect(cluster.platform_kubernetes.ca_pem).to eq('a') + expect(cluster.platform_kubernetes.token).to be_empty + expect(project.kubernetes_service).not_to be_active + end + end + end + context 'when KubernetesService does not exist' do let!(:project) { create(:project) } -- cgit v1.2.1 From 54d20d1bad9cfa23bd2935a3f355ac07727bfdab Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 01:20:13 +0900 Subject: Add changelog --- ...-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml diff --git a/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml new file mode 100644 index 00000000000..eb7678b6e28 --- /dev/null +++ b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml @@ -0,0 +1,5 @@ +--- +title: Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes +merge_request: +author: +type: changed -- cgit v1.2.1 From 290c224847d5623541d4300f7a1bfe2a83bb3b26 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 01:25:45 +0900 Subject: Fix change log --- ...ing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml index eb7678b6e28..5e158d831a6 100644 --- a/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml +++ b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml @@ -1,5 +1,5 @@ --- title: Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes -merge_request: +merge_request: 15589 author: type: changed -- cgit v1.2.1 From 58d074e0013fb6123a084275edd14bb52955c21e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 6 Jan 2018 02:16:04 +0900 Subject: Fix StaticSnalysys --- ...24104327_migrate_kubernetes_service_to_new_clusters_architectures.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 27d56fe5b42..0445044cc3c 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -117,7 +117,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati MigrateKubernetesServiceToNewClustersArchitectures::Service .kubernetes_service_without_template.each_batch(of: 100) do |kubernetes_service| - kubernetes_service.update_all(active: false) + kubernetes_service.update_all(active: false) end end -- cgit v1.2.1 From df658c7b1337afc71d9f3d66d3e8ad55db26b523 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 8 Jan 2018 16:17:35 +0900 Subject: Disable STI of ActiveRecord. Refactoring specs. --- ...rnetes_service_to_new_clusters_architectures.rb | 22 +- ...s_service_to_new_clusters_architectures_spec.rb | 228 +++++++++++++++------ 2 files changed, 184 insertions(+), 66 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 0445044cc3c..3fe0a4941d5 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -12,6 +12,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' has_many :clusters, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' has_many :services, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service' + has_one :kubernetes_service, -> { where(category: 'deployment', type: 'KubernetesService') }, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service', inverse_of: :project, foreign_key: :project_id end class Cluster < ActiveRecord::Base @@ -55,8 +56,9 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati include EachBatch self.table_name = 'services' + self.inheritance_column = :_type_disabled # Disable STI, otherwise KubernetesModel will be looked up - belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' + belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project', foreign_key: :project_id scope :unmanaged_kubernetes_service, -> do joins('LEFT JOIN projects ON projects.id = services.project_id') @@ -72,6 +74,22 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati scope :kubernetes_service_without_template, -> do where(category: 'deployment', type: 'KubernetesService', template: false) end + + def api_url + JSON.parse(self.properties)['api_url'] + end + + def ca_pem + JSON.parse(self.properties)['ca_pem'] + end + + def namespace + JSON.parse(self.properties)['namespace'] + end + + def token + JSON.parse(self.properties)['token'] + end end def find_dedicated_environement_scope(project) @@ -101,7 +119,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], - projects: [kubernetes_service.project.becomes(MigrateKubernetesServiceToNewClustersArchitectures::Project)], + projects: [kubernetes_service.project], environment_scope: find_dedicated_environement_scope(kubernetes_service.project), platform_kubernetes_attributes: { api_url: kubernetes_service.api_url, diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 85ed1328211..54ae4c97d4e 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -5,27 +5,32 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do context 'when unique KubernetesService exists' do shared_examples 'KubernetesService migration' do let(:sample_num) { 2 } - let(:projects) { create_list(:project, sample_num) } + + let(:projects) do + (1..sample_num).each_with_object([]) do |n, array| + array << MigrateKubernetesServiceToNewClustersArchitectures::Project.create! + end + end let!(:kubernetes_services) do projects.map do |project| - create(:kubernetes_service, - project: project, - active: active, - api_url: "https://kubernetes#{project.id}.com", - token: defined?(token) ? token : "token#{project.id}", - ca_pem: "ca_pem#{project.id}") + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: active, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"https://kubernetes#{project.id}.com\",\"ca_pem\":\"ca_pem#{project.id}\",\"token\":\"token#{project.id}\"}") end end it 'migrates the KubernetesService to Platform::Kubernetes' do - expect { migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(sample_num) projects.each do |project| project.clusters.last.tap do |cluster| expect(cluster.enabled).to eq(active) expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(project.kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) expect(project.kubernetes_service).not_to be_active end @@ -42,34 +47,38 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do context 'when unique KubernetesService spawned from Service Template' do let(:sample_num) { 2 } - let(:projects) { create_list(:project, sample_num) } + + let(:projects) do + (1..sample_num).each_with_object([]) do |n, array| + array << MigrateKubernetesServiceToNewClustersArchitectures::Project.create! + end + end let!(:kubernetes_service_template) do - create(:kubernetes_service, - project: nil, - template: true, - api_url: "https://sample.kubernetes.com", - token: "token-sample", - ca_pem: "ca_pem-sample") + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + template: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"https://sample.kubernetes.com\",\"ca_pem\":\"ca_pem-sample\",\"token\":\"token-sample\"}") end let!(:kubernetes_services) do projects.map do |project| - create(:kubernetes_service, - project: project, - api_url: kubernetes_service_template.api_url, - token: kubernetes_service_template.token, - ca_pem: kubernetes_service_template.ca_pem) + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"#{kubernetes_service_template.api_url}\",\"ca_pem\":\"#{kubernetes_service_template.ca_pem}\",\"token\":\"#{kubernetes_service_template.token}\"}") end end it 'migrates the KubernetesService to Platform::Kubernetes without template' do - expect { migrate! }.to change { Clusters::Cluster.count }.by(sample_num) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(sample_num) projects.each do |project| project.clusters.last.tap do |cluster| expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(project.kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) expect(project.kubernetes_service).not_to be_active end @@ -78,21 +87,32 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when managed KubernetesService exists' do - let(:project) { create(:project) } - let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - let!(:platform_kubernetes) { cluster.platform_kubernetes } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end let!(:kubernetes_service) do - create(:kubernetes_service, - project: project, - active: cluster.enabled, - api_url: platform_kubernetes.api_url, - token: platform_kubernetes.token, - ca_pem: platform_kubernetes.ca_cert) + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: cluster.enabled, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"#{cluster.platform_kubernetes.api_url}\"}") end it 'does not migrate the KubernetesService and disables the kubernetes_service' do # Because the corresponding Platform::Kubernetes already exists - expect { migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } kubernetes_service.reload expect(kubernetes_service).not_to be_active @@ -100,18 +120,39 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when production cluster has already been existed' do # i.e. There are no environment_scope conflicts - let(:project) { create(:project) } - let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'production/*', projects: [project]) } - let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: 'production/*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"https://debug.kube.com\"}") + end it 'migrates the KubernetesService to Platform::Kubernetes' do - expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| expect(cluster.environment_scope).to eq('*') expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active end @@ -119,18 +160,39 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when default cluster has already been existed' do - let(:project) { create(:project) } - let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } - let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let!(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: '*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"https://debug.kube.com\"}") + end it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| expect(cluster.environment_scope).to eq('migrated/*') expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active end @@ -138,19 +200,53 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when default cluster and migrated cluster has already been existed' do - let(:project) { create(:project) } - let!(:cluster) { create(:cluster, :provided_by_gcp, environment_scope: '*', projects: [project]) } - let!(:migrated_cluster) { create(:cluster, :provided_by_gcp, environment_scope: 'migrated/*', projects: [project]) } - let!(:kubernetes_service) { create(:kubernetes_service, api_url: 'https://debug.kube.com', active: true, project: project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let!(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: '*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:migrated_cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: 'migrated/*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"https://debug.kube.com\"}") + end it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated - expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) kubernetes_service.reload project.clusters.last.tap do |cluster| expect(cluster.environment_scope).to eq('migrated0/*') expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) - expect(cluster.platform_kubernetes.ca_pem).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) expect(kubernetes_service).not_to be_active end @@ -158,17 +254,19 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when KubernetesService has nullified parameters' do - let(:project) { create(:project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } before do - ActiveRecord::Base.connection.execute <<~SQL - INSERT INTO services (project_id, active, category, type, properties) - VALUES (#{project.id}, false, 'deployment', 'KubernetesService', '{}'); - SQL + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: false, + category: 'deployment', + type: 'KubernetesService', + properties: "{}") end it 'does not migrate the KubernetesService and disables the kubernetes_service' do - expect { migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } expect(project.kubernetes_service).not_to be_active end @@ -179,23 +277,25 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do # However, in this migration file, there are no validations because of the re-defined model class # therefore, we should safely add this raw to Platform::Kubernetes context 'when KubernetesService has empty token' do - let(:project) { create(:project) } + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } before do - ActiveRecord::Base.connection.execute <<~SQL - INSERT INTO services (project_id, active, category, type, properties) - VALUES (#{project.id}, false, 'deployment', 'KubernetesService', '{"namespace":"prod","api_url":"http://111.111.111.111","ca_pem":"a","token":""}'); - SQL + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: false, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"http://111.111.111.111\",\"ca_pem\":\"a\",\"token\":\"\"}") end it 'does not migrate the KubernetesService and disables the kubernetes_service' do - expect { migrate! }.to change { Clusters::Cluster.count }.by(1) + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) project.clusters.last.tap do |cluster| expect(cluster.environment_scope).to eq('*') expect(cluster.platform_kubernetes.namespace).to eq('prod') expect(cluster.platform_kubernetes.api_url).to eq('http://111.111.111.111') - expect(cluster.platform_kubernetes.ca_pem).to eq('a') + expect(cluster.platform_kubernetes.ca_cert).to eq('a') expect(cluster.platform_kubernetes.token).to be_empty expect(project.kubernetes_service).not_to be_active end @@ -203,10 +303,10 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do end context 'when KubernetesService does not exist' do - let!(:project) { create(:project) } + let!(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } it 'does not migrate the KubernetesService' do - expect { migrate! }.not_to change { Clusters::Cluster.count } + expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } end end end -- cgit v1.2.1 From c425ff750179542a94da69af3a507c70cd77ca48 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 8 Jan 2018 17:01:29 +0900 Subject: Fix static analysys --- ...netes_service_to_new_clusters_architectures_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb index 54ae4c97d4e..df0015b6dd3 100644 --- a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -91,15 +91,15 @@ describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do let(:cluster) do MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( - projects: [project], - name: 'sample-cluster', - platform_type: :kubernetes, - provider_type: :user, - platform_kubernetes_attributes: { - api_url: 'https://sample.kubernetes.com', - ca_cert: 'ca_pem-sample', - token: 'token-sample' - } ) + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) end let!(:kubernetes_service) do -- cgit v1.2.1 From 6732795231dd71f2d5cd8a851372db1894ba0a3f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 8 Jan 2018 22:24:23 +0900 Subject: Add memoization for properties --- ...ate_kubernetes_service_to_new_clusters_architectures.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb index 3fe0a4941d5..11b581e4b57 100644 --- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -76,19 +76,25 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati end def api_url - JSON.parse(self.properties)['api_url'] + parsed_properties['api_url'] end def ca_pem - JSON.parse(self.properties)['ca_pem'] + parsed_properties['ca_pem'] end def namespace - JSON.parse(self.properties)['namespace'] + parsed_properties['namespace'] end def token - JSON.parse(self.properties)['token'] + parsed_properties['token'] + end + + private + + def parsed_properties + @parsed_properties ||= JSON.parse(self.properties) end end -- cgit v1.2.1