From 525edec78bc39ae41284927f1866def56d21a12a Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Mon, 24 Jun 2019 09:48:16 +1200 Subject: Add cluster_id to deployments table as an FK We nullify when cluster is deleted as we want to keep the deployment record around. Add cluster as an optional association We will have only cluster for deployments where the build deploys to a kubernetes cluster --- app/models/deployment.rb | 1 + .../20190623212503_add_cluster_id_to_deployments.rb | 9 +++++++++ ...051902_add_cluster_id_index_fk_to_deployments.rb | 21 +++++++++++++++++++++ db/schema.rb | 5 ++++- spec/models/deployment_spec.rb | 1 + 5 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20190623212503_add_cluster_id_to_deployments.rb create mode 100644 db/migrate/20190627051902_add_cluster_id_index_fk_to_deployments.rb diff --git a/app/models/deployment.rb b/app/models/deployment.rb index f0fa5974787..8332656a7a6 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -7,6 +7,7 @@ class Deployment < ApplicationRecord belongs_to :project, required: true belongs_to :environment, required: true + belongs_to :cluster, class_name: 'Clusters::Cluster', optional: true belongs_to :user belongs_to :deployable, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations diff --git a/db/migrate/20190623212503_add_cluster_id_to_deployments.rb b/db/migrate/20190623212503_add_cluster_id_to_deployments.rb new file mode 100644 index 00000000000..cd0c4191568 --- /dev/null +++ b/db/migrate/20190623212503_add_cluster_id_to_deployments.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddClusterIdToDeployments < ActiveRecord::Migration[5.1] + DOWNTIME = false + + def change + add_column :deployments, :cluster_id, :integer + end +end diff --git a/db/migrate/20190627051902_add_cluster_id_index_fk_to_deployments.rb b/db/migrate/20190627051902_add_cluster_id_index_fk_to_deployments.rb new file mode 100644 index 00000000000..f41e5c80269 --- /dev/null +++ b/db/migrate/20190627051902_add_cluster_id_index_fk_to_deployments.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddClusterIdIndexFkToDeployments < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :deployments, :cluster_id + + add_concurrent_foreign_key :deployments, :clusters, column: :cluster_id, on_delete: :nullify + end + + def down + remove_foreign_key :deployments, :clusters + + remove_concurrent_index :deployments, :cluster_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 054dbc7201f..80e2356629f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190625184066) do +ActiveRecord::Schema.define(version: 20190627051902) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1066,6 +1066,8 @@ ActiveRecord::Schema.define(version: 20190625184066) do t.string "on_stop" t.integer "status", limit: 2, null: false t.datetime_with_timezone "finished_at" + t.integer "cluster_id" + t.index ["cluster_id"], name: "index_deployments_on_cluster_id", using: :btree t.index ["created_at"], name: "index_deployments_on_created_at", using: :btree t.index ["deployable_type", "deployable_id"], name: "index_deployments_on_deployable_type_and_deployable_id", using: :btree t.index ["environment_id", "id"], name: "index_deployments_on_environment_id_and_id", using: :btree @@ -3650,6 +3652,7 @@ ActiveRecord::Schema.define(version: 20190625184066) do add_foreign_key "dependency_proxy_blobs", "namespaces", column: "group_id", name: "fk_db58bbc5d7", on_delete: :cascade add_foreign_key "dependency_proxy_group_settings", "namespaces", column: "group_id", name: "fk_616ddd680a", on_delete: :cascade add_foreign_key "deploy_keys_projects", "projects", name: "fk_58a901ca7e", on_delete: :cascade + add_foreign_key "deployments", "clusters", name: "fk_289bba3222", on_delete: :nullify add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade add_foreign_key "design_management_designs", "issues", on_delete: :cascade add_foreign_key "design_management_designs", "projects", on_delete: :cascade diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index a433878f3bc..341ed31d19b 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -7,6 +7,7 @@ describe Deployment do it { is_expected.to belong_to(:project).required } it { is_expected.to belong_to(:environment).required } + it { is_expected.to belong_to(:cluster).class_name('Clusters::Cluster') } it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:deployable) } -- cgit v1.2.1 From 2f96e5d3b9e750c5faef1fc8d95b18cb02002516 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Mon, 24 Jun 2019 11:33:32 +1200 Subject: Populate cluster_id when creating a deployment --- app/models/concerns/deployable.rb | 1 + changelogs/unreleased/add-clusters-to-deployment.yml | 5 +++++ spec/models/concerns/deployable_spec.rb | 14 ++++++++++---- 3 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/add-clusters-to-deployment.yml diff --git a/app/models/concerns/deployable.rb b/app/models/concerns/deployable.rb index bc12b06b5af..957b72f3721 100644 --- a/app/models/concerns/deployable.rb +++ b/app/models/concerns/deployable.rb @@ -18,6 +18,7 @@ module Deployable return unless environment.persisted? create_deployment!( + cluster_id: environment.deployment_platform&.cluster_id, project_id: environment.project_id, environment: environment, ref: ref, diff --git a/changelogs/unreleased/add-clusters-to-deployment.yml b/changelogs/unreleased/add-clusters-to-deployment.yml new file mode 100644 index 00000000000..c85bd3635cc --- /dev/null +++ b/changelogs/unreleased/add-clusters-to-deployment.yml @@ -0,0 +1,5 @@ +--- +title: Persist the cluster a deployment was deployed to +merge_request: 29960 +author: +type: fixed diff --git a/spec/models/concerns/deployable_spec.rb b/spec/models/concerns/deployable_spec.rb index 42bed9434f5..bb73dd8ade0 100644 --- a/spec/models/concerns/deployable_spec.rb +++ b/spec/models/concerns/deployable_spec.rb @@ -7,10 +7,6 @@ describe Deployable do let(:deployment) { job.deployment } let(:environment) { deployment&.environment } - before do - job.reload - end - context 'when the deployable object will deploy to production' do let!(:job) { create(:ci_build, :start_review_app) } @@ -26,6 +22,16 @@ describe Deployable do end end + context 'when the deployable object will deploy to a cluster' do + let(:project) { create(:project) } + let!(:cluster) { create(:cluster, :provided_by_user, projects: [project]) } + let!(:job) { create(:ci_build, :start_review_app, project: project) } + + it 'creates a deployment with cluster association' do + expect(deployment.cluster).to eq(cluster) + end + end + context 'when the deployable object will stop an environment' do let!(:job) { create(:ci_build, :stop_review_app) } -- cgit v1.2.1 From 2cdb72ea0382ef80f6cd24606826094321a4e643 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Tue, 25 Jun 2019 16:24:54 +1200 Subject: Use deployment's cluster for kubernetes prereq A deployment will have a cluster associated on creation if there is one. Otherwise fallback to deployment_platform for legacy deployments. --- lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb b/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb index 49c680605ea..48598fcae7e 100644 --- a/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb +++ b/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb @@ -20,7 +20,9 @@ module Gitlab private def deployment_cluster - build.deployment&.deployment_platform_cluster + strong_memoize(:deployment_cluster) do + build.deployment&.cluster || build.deployment&.deployment_platform_cluster + end end def kubernetes_namespace -- cgit v1.2.1 From 4615dca1d90975d16b1534292c1b2886da1b2cd5 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Tue, 25 Jun 2019 17:06:37 +1200 Subject: Drop fallback to deployment platform All deployments should have already their cluster_id filled in on creation. Legacy deployments will not be retried as:- * Ci::Build#retry calls `Ci::RetryBuildService` * Ci::Pipeline#retry calls `Ci::RetryPipelineService` which also calls `Ci::RetryBuildService` * `Ci::RetryBuildService` will clone a build to retry It is also impossibly to backfill Deployment#cluster_id from Project#deployment_platform correctly as clusters could have been deleted, added or altered in the intervening time. --- .../ci/build/prerequisite/kubernetes_namespace.rb | 4 +--- .../prerequisite/kubernetes_namespace_spec.rb | 22 ++++------------------ 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb b/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb index 48598fcae7e..e6e0aaab60b 100644 --- a/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb +++ b/lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb @@ -20,9 +20,7 @@ module Gitlab private def deployment_cluster - strong_memoize(:deployment_cluster) do - build.deployment&.cluster || build.deployment&.deployment_platform_cluster - end + build.deployment&.cluster end def kubernetes_namespace diff --git a/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb index 51e16c99688..d88a2097ba2 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb @@ -17,15 +17,12 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do end context 'build has a deployment' do - let!(:deployment) { create(:deployment, deployable: build) } + let!(:deployment) { create(:deployment, deployable: build, cluster: cluster) } + let(:cluster) { nil } context 'and a cluster to deploy to' do let(:cluster) { create(:cluster, :group) } - before do - allow(build.deployment).to receive(:deployment_platform_cluster).and_return(cluster) - end - it { is_expected.to be_truthy } context 'and the cluster is not managed' do @@ -48,28 +45,21 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do end context 'and no cluster to deploy to' do - before do - expect(deployment.deployment_platform_cluster).to be_nil - end - it { is_expected.to be_falsey } end end end describe '#complete!' do - let!(:deployment) { create(:deployment, deployable: build) } + let!(:deployment) { create(:deployment, deployable: build, cluster: cluster) } let(:service) { double(execute: true) } + let(:cluster) { nil } subject { described_class.new(build).complete! } context 'completion is required' do let(:cluster) { create(:cluster, :group) } - before do - allow(build.deployment).to receive(:deployment_platform_cluster).and_return(cluster) - end - it 'creates a kubernetes namespace' do expect(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService) .to receive(:new) @@ -83,10 +73,6 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do end context 'completion is not required' do - before do - expect(deployment.deployment_platform_cluster).to be_nil - end - it 'does not create a namespace' do expect(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService).not_to receive(:new) -- cgit v1.2.1 From 04af6132b14e594aeddef2a6d0c171af667c9539 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Mon, 24 Jun 2019 14:08:42 +1200 Subject: Use #cluster for prometheus_adapter We still fallback to environment.deployment_platform until we can backfill --- app/models/deployment.rb | 17 +++++++++++- spec/factories/deployments.rb | 4 +++ spec/models/deployment_spec.rb | 61 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 8332656a7a6..a8f5642f726 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -197,7 +197,22 @@ class Deployment < ApplicationRecord private def prometheus_adapter - environment.prometheus_adapter + service = project.find_or_initialize_service('prometheus') + + if service.can_query? + service + else + cluster_prometheus + end + end + + # TODO remove fallback case to deployment_platform_cluster. + # Otherwise we will continue to pay the performance penalty described in + # https://gitlab.com/gitlab-org/gitlab-ce/issues/63475 + def cluster_prometheus + cluster_with_fallback = cluster || deployment_platform_cluster + + cluster_with_fallback.application_prometheus if cluster_with_fallback&.application_prometheus_available? end def ref_path diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index db438ad32d3..1c7787bc1a6 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -22,6 +22,10 @@ FactoryBot.define do ref 'pages-deploy' end + trait :on_cluster do + cluster factory: %i(cluster provided_by_gcp) + end + trait :running do status :running end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 341ed31d19b..8d0eb0f4a06 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -295,6 +295,67 @@ describe Deployment do end end + describe '#has_metrics?' do + subject { deployment.has_metrics? } + + context 'when deployment is failed' do + let(:deployment) { create(:deployment, :failed) } + + it { is_expected.to be_falsy } + end + + context 'when deployment is success' do + let(:deployment) { create(:deployment, :success) } + + context 'without a monitoring service' do + it { is_expected.to be_falsy } + end + + context 'with a Prometheus Service' do + let(:prometheus_service) { double(:prometheus_service, can_query?: true) } + + before do + allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it { is_expected.to be_truthy } + end + + context 'with a Prometheus Service that cannot query' do + let(:prometheus_service) { double(:prometheus_service, can_query?: false) } + + before do + allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it { is_expected.to be_falsy } + end + + context 'with a cluster Prometheus' do + let(:deployment) { create(:deployment, :success, :on_cluster) } + let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: deployment.cluster) } + + before do + expect(deployment.cluster.application_prometheus).to receive(:can_query?).and_return(true) + end + + it { is_expected.to be_truthy } + end + + context 'fallback deployment platform' do + let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [deployment.project]) } + let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } + + before do + expect(deployment.project).to receive(:deployment_platform).and_return(cluster.platform) + expect(cluster.application_prometheus).to receive(:can_query?).and_return(true) + end + + it { is_expected.to be_truthy } + end + end + end + describe '#metrics' do let(:deployment) { create(:deployment, :success) } let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } -- cgit v1.2.1