From 2ab69f0d0c825f8546f189a61189246d6c90b7ff Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 5 Dec 2017 12:02:07 +0100 Subject: Schedule full build stage migration in the background For builds that are still missing `stage_id`. --- ...0171205101928_schedule_build_stage_migration.rb | 20 ++++++++++ .../schedule_build_stage_migration_spec.rb | 43 ++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 db/post_migrate/20171205101928_schedule_build_stage_migration.rb create mode 100644 spec/migrations/schedule_build_stage_migration_spec.rb diff --git a/db/post_migrate/20171205101928_schedule_build_stage_migration.rb b/db/post_migrate/20171205101928_schedule_build_stage_migration.rb new file mode 100644 index 00000000000..60293c60e8e --- /dev/null +++ b/db/post_migrate/20171205101928_schedule_build_stage_migration.rb @@ -0,0 +1,20 @@ +class ScheduleBuildStageMigration < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + MIGRATION = 'MigrateBuildStage'.freeze + BATCH = 10_000 + + class Build < ActiveRecord::Base + include EachBatch + self.table_name = 'ci_builds' + end + + def change + Build.where('stage_id IS NULL').each_batch(of: BATCH) do |builds, index| + builds.pluck(:id).map { |id| [MIGRATION, [id]] }.tap do |migrations| + BackgroundMigrationWorker.perform_bulk_in(index.minutes, migrations) + end + end + end +end diff --git a/spec/migrations/schedule_build_stage_migration_spec.rb b/spec/migrations/schedule_build_stage_migration_spec.rb new file mode 100644 index 00000000000..f80c0a94b32 --- /dev/null +++ b/spec/migrations/schedule_build_stage_migration_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171205101928_schedule_build_stage_migration') + +describe ScheduleBuildStageMigration, :migration do + let(:projects) { table(:projects) } + let(:pipelines) { table(:ci_pipelines) } + let(:stages) { table(:ci_stages) } + let(:jobs) { table(:ci_builds) } + + before do + ## + # Dependencies + # + projects.create!(id: 123, name: 'gitlab', path: 'gitlab-ce') + pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') + stages.create!(id: 1, project_id: 123, pipeline_id: 1, name: 'test') + + ## + # CI/CD jobs + # + jobs.create!(id: 10, commit_id: 1, project_id: 123, stage_id: nil) + jobs.create!(id: 20, commit_id: 1, project_id: 123, stage_id: nil) + jobs.create!(id: 30, commit_id: 1, project_id: 123, stage_id: nil) + jobs.create!(id: 40, commit_id: 1, project_id: 123, stage_id: 1) + end + + before do + stub_const("#{described_class}::BATCH", 1) + end + + it 'schedules background migrations in batches in bulk' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_migration(1.minutes, 10) + expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 20) + expect(described_class::MIGRATION).to be_scheduled_migration(3.minutes, 30) + expect(BackgroundMigrationWorker.jobs.size).to eq 3 + end + end + end +end -- cgit v1.2.1 From cd6d0dbd5fe9548ab7c0cd7b09e640a7b425daf1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 5 Dec 2017 12:42:24 +0100 Subject: Migrate a build stage completely in a background migration --- .../background_migration/migrate_build_stage.rb | 67 ++++++++++++++++++++++ .../migrate_build_stage_spec.rb | 48 ++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 lib/gitlab/background_migration/migrate_build_stage.rb create mode 100644 spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb diff --git a/lib/gitlab/background_migration/migrate_build_stage.rb b/lib/gitlab/background_migration/migrate_build_stage.rb new file mode 100644 index 00000000000..05edaab29d2 --- /dev/null +++ b/lib/gitlab/background_migration/migrate_build_stage.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true +# rubocop:disable Metrics/AbcSize +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + class MigrateBuildStage + def perform(id) + Ci::Build.find_by(id: id).try do |build| + Stage.new(build).tap do |stage| + return if stage.exists? + + stage.ensure! + stage.migrate_reference! + stage.migrate_status! + end + end + end + + private + + class Ci::Stage < ActiveRecord::Base + self.table_name = 'ci_stages' + end + + class Ci::Build < ActiveRecord::Base + self.table_name = 'ci_builds' + end + + class Stage + def initialize(build) + @build = build + end + + def exists? + @build.reload.stage_id.present? + end + + def ensure! + find || create! + end + + def find + Ci::Stage.find_by(name: @build.stage, + pipeline_id: @build.commit_id, + project_id: @build.project_id) + end + + def create! + Ci::Stage.create!(name: @build.stage, + pipeline_id: @build.commit_id, + project_id: @build.project_id) + end + + def migrate_reference! + MigrateBuildStageIdReference.new.perform(@build.id, @build.id) + end + + def migrate_status! + raise ArgumentError unless exists? + + MigrateStageStatus.new.perform(@build.stage_id, @build.stage_id) + end + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb new file mode 100644 index 00000000000..baa9c532c6d --- /dev/null +++ b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 20171205101928 do + let(:projects) { table(:projects) } + let(:pipelines) { table(:ci_pipelines) } + let(:stages) { table(:ci_stages) } + let(:jobs) { table(:ci_builds) } + + STATUSES = { created: 0, pending: 1, running: 2, success: 3, + failed: 4, canceled: 5, skipped: 6, manual: 7 }.freeze + + before do + ## + # Dependencies + # + projects.create!(id: 123, name: 'gitlab', path: 'gitlab-ce') + pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') + + ## + # CI/CD jobs + # + jobs.create!(id: 1, commit_id: 1, project_id: 123, + stage_idx: 2, stage: 'build', status: :success) + jobs.create!(id: 2, commit_id: 1, project_id: 123, + stage_idx: 2, stage: 'build', status: :success) + jobs.create!(id: 3, commit_id: 1, project_id: 123, + stage_idx: 1, stage: 'test', status: :failed) + jobs.create!(id: 4, commit_id: 1, project_id: 123, + stage_idx: 1, stage: 'test', status: :success) + jobs.create!(id: 5, commit_id: 1, project_id: 123, + stage_idx: 3, stage: 'deploy', status: :pending) + end + + it 'correctly migrates builds stages' do + expect(stages.count).to be_zero + + jobs.all.find_each do |job| + described_class.new.perform(job.id) + end + + expect(stages.count).to eq 3 + expect(stages.all.pluck(:name)).to match_array %w[test build deploy] + expect(jobs.where(stage_id: nil)).to be_empty + expect(stages.all.pluck(:status)).to match_array [STATUSES[:success], + STATUSES[:failed], + STATUSES[:pending]] + end +end -- cgit v1.2.1 From b50270a54efaa448e06018ad9a824ef4a512da31 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 5 Dec 2017 14:20:17 +0100 Subject: Fix Rubocop offenses in build status migrations :cop: --- lib/gitlab/background_migration/migrate_build_stage.rb | 4 +--- spec/migrations/schedule_build_stage_migration_spec.rb | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/background_migration/migrate_build_stage.rb b/lib/gitlab/background_migration/migrate_build_stage.rb index 05edaab29d2..a870609e68d 100644 --- a/lib/gitlab/background_migration/migrate_build_stage.rb +++ b/lib/gitlab/background_migration/migrate_build_stage.rb @@ -8,7 +8,7 @@ module Gitlab def perform(id) Ci::Build.find_by(id: id).try do |build| Stage.new(build).tap do |stage| - return if stage.exists? + break if stage.exists? stage.ensure! stage.migrate_reference! @@ -17,8 +17,6 @@ module Gitlab end end - private - class Ci::Stage < ActiveRecord::Base self.table_name = 'ci_stages' end diff --git a/spec/migrations/schedule_build_stage_migration_spec.rb b/spec/migrations/schedule_build_stage_migration_spec.rb index f80c0a94b32..c840d0b77fc 100644 --- a/spec/migrations/schedule_build_stage_migration_spec.rb +++ b/spec/migrations/schedule_build_stage_migration_spec.rb @@ -33,7 +33,7 @@ describe ScheduleBuildStageMigration, :migration do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_migration(1.minutes, 10) + expect(described_class::MIGRATION).to be_scheduled_migration(1.minute, 10) expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 20) expect(described_class::MIGRATION).to be_scheduled_migration(3.minutes, 30) expect(BackgroundMigrationWorker.jobs.size).to eq 3 -- cgit v1.2.1 From 1ab0ffe3830340c0c9c2def74f59c95a44bcf9f7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 6 Jan 2018 14:21:45 +0100 Subject: Update background stages migration timestamp --- .../20171205101928_schedule_build_stage_migration.rb | 20 -------------------- .../20180105101928_schedule_build_stage_migration.rb | 20 ++++++++++++++++++++ db/schema.rb | 2 +- .../background_migration/migrate_build_stage_spec.rb | 2 +- .../schedule_build_stage_migration_spec.rb | 2 +- 5 files changed, 23 insertions(+), 23 deletions(-) delete mode 100644 db/post_migrate/20171205101928_schedule_build_stage_migration.rb create mode 100644 db/post_migrate/20180105101928_schedule_build_stage_migration.rb diff --git a/db/post_migrate/20171205101928_schedule_build_stage_migration.rb b/db/post_migrate/20171205101928_schedule_build_stage_migration.rb deleted file mode 100644 index 60293c60e8e..00000000000 --- a/db/post_migrate/20171205101928_schedule_build_stage_migration.rb +++ /dev/null @@ -1,20 +0,0 @@ -class ScheduleBuildStageMigration < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - MIGRATION = 'MigrateBuildStage'.freeze - BATCH = 10_000 - - class Build < ActiveRecord::Base - include EachBatch - self.table_name = 'ci_builds' - end - - def change - Build.where('stage_id IS NULL').each_batch(of: BATCH) do |builds, index| - builds.pluck(:id).map { |id| [MIGRATION, [id]] }.tap do |migrations| - BackgroundMigrationWorker.perform_bulk_in(index.minutes, migrations) - end - end - end -end diff --git a/db/post_migrate/20180105101928_schedule_build_stage_migration.rb b/db/post_migrate/20180105101928_schedule_build_stage_migration.rb new file mode 100644 index 00000000000..3d7f6278244 --- /dev/null +++ b/db/post_migrate/20180105101928_schedule_build_stage_migration.rb @@ -0,0 +1,20 @@ +class ScheduleBuildStageMigration < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + MIGRATION = 'MigrateBuildStage'.freeze + BATCH = 10_000 + + class Build < ActiveRecord::Base + include EachBatch + self.table_name = 'ci_builds' + end + + def change + Build.where('stage_id IS NULL').each_batch(of: BATCH) do |builds, index| + builds.pluck(:id).map { |id| [MIGRATION, [id]] }.tap do |migrations| + BackgroundMigrationWorker.bulk_perform_in(index.minutes, migrations) + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 740e80ccfd4..1eba30b28c8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171230123729) do +ActiveRecord::Schema.define(version: 20180105101928) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb index baa9c532c6d..b675db463af 100644 --- a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 20171205101928 do +describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 20180105101928 do let(:projects) { table(:projects) } let(:pipelines) { table(:ci_pipelines) } let(:stages) { table(:ci_stages) } diff --git a/spec/migrations/schedule_build_stage_migration_spec.rb b/spec/migrations/schedule_build_stage_migration_spec.rb index c840d0b77fc..485ff84819d 100644 --- a/spec/migrations/schedule_build_stage_migration_spec.rb +++ b/spec/migrations/schedule_build_stage_migration_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20171205101928_schedule_build_stage_migration') +require Rails.root.join('db', 'post_migrate', '20180105101928_schedule_build_stage_migration') describe ScheduleBuildStageMigration, :migration do let(:projects) { table(:projects) } -- cgit v1.2.1 From aaa678cbc69ad5768d076a8e1cc3f249b2adeb29 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 6 Jan 2018 14:27:07 +0100 Subject: Update specs for delayed background migration of stages --- spec/migrations/schedule_build_stage_migration_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/migrations/schedule_build_stage_migration_spec.rb b/spec/migrations/schedule_build_stage_migration_spec.rb index 485ff84819d..c96acceeeda 100644 --- a/spec/migrations/schedule_build_stage_migration_spec.rb +++ b/spec/migrations/schedule_build_stage_migration_spec.rb @@ -28,14 +28,14 @@ describe ScheduleBuildStageMigration, :migration do stub_const("#{described_class}::BATCH", 1) end - it 'schedules background migrations in batches in bulk' do + it 'schedules delayed background migrations in batches in bulk' do Sidekiq::Testing.fake! do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_migration(1.minute, 10) - expect(described_class::MIGRATION).to be_scheduled_migration(2.minutes, 20) - expect(described_class::MIGRATION).to be_scheduled_migration(3.minutes, 30) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(1.minute, 10) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 20) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(3.minutes, 30) expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end -- cgit v1.2.1 From 3c89252e53147fed0d134988a6ec29aa2c02abe5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 6 Jan 2018 14:29:35 +0100 Subject: Reduce batch size of stages we schedule for a migration --- db/post_migrate/20180105101928_schedule_build_stage_migration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20180105101928_schedule_build_stage_migration.rb b/db/post_migrate/20180105101928_schedule_build_stage_migration.rb index 3d7f6278244..b33127140bb 100644 --- a/db/post_migrate/20180105101928_schedule_build_stage_migration.rb +++ b/db/post_migrate/20180105101928_schedule_build_stage_migration.rb @@ -3,7 +3,7 @@ class ScheduleBuildStageMigration < ActiveRecord::Migration DOWNTIME = false MIGRATION = 'MigrateBuildStage'.freeze - BATCH = 10_000 + BATCH = 1000 class Build < ActiveRecord::Base include EachBatch -- cgit v1.2.1 From 2d2f9e51594e33f04bfda4012e3d4c7e48539fe3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 6 Jan 2018 14:44:43 +0100 Subject: Do not attempt to migrate legacy pipeline stages --- .../background_migration/migrate_build_stage.rb | 33 ++++++++++++++-------- .../migrate_build_stage_spec.rb | 5 +++- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/lib/gitlab/background_migration/migrate_build_stage.rb b/lib/gitlab/background_migration/migrate_build_stage.rb index a870609e68d..7139ac03737 100644 --- a/lib/gitlab/background_migration/migrate_build_stage.rb +++ b/lib/gitlab/background_migration/migrate_build_stage.rb @@ -6,9 +6,9 @@ module Gitlab module BackgroundMigration class MigrateBuildStage def perform(id) - Ci::Build.find_by(id: id).try do |build| - Stage.new(build).tap do |stage| - break if stage.exists? + DatabaseBuild.find_by(id: id).try do |build| + MigratableStage.new(build).tap do |stage| + break if stage.exists? || stage.legacy? stage.ensure! stage.migrate_reference! @@ -17,15 +17,15 @@ module Gitlab end end - class Ci::Stage < ActiveRecord::Base + class DatabaseStage < ActiveRecord::Base self.table_name = 'ci_stages' end - class Ci::Build < ActiveRecord::Base + class DatabaseBuild < ActiveRecord::Base self.table_name = 'ci_builds' end - class Stage + class MigratableStage def initialize(build) @build = build end @@ -34,20 +34,29 @@ module Gitlab @build.reload.stage_id.present? end + ## + # We can have some very old stages that do not have `ci_builds.stage` set. + # + # In that case we just don't migrate such stage. + # + def legacy? + @build.stage.nil? + end + def ensure! find || create! end def find - Ci::Stage.find_by(name: @build.stage, - pipeline_id: @build.commit_id, - project_id: @build.project_id) + DatabaseStage.find_by(name: @build.stage, + pipeline_id: @build.commit_id, + project_id: @build.project_id) end def create! - Ci::Stage.create!(name: @build.stage, - pipeline_id: @build.commit_id, - project_id: @build.project_id) + DatabaseStage.create!(name: @build.stage, + pipeline_id: @build.commit_id, + project_id: @build.project_id) end def migrate_reference! diff --git a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb index b675db463af..d20b5c29a71 100644 --- a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb @@ -29,6 +29,8 @@ describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 201 stage_idx: 1, stage: 'test', status: :success) jobs.create!(id: 5, commit_id: 1, project_id: 123, stage_idx: 3, stage: 'deploy', status: :pending) + jobs.create!(id: 6, commit_id: 1, project_id: 123, + stage_idx: 3, stage: nil, status: :pending) end it 'correctly migrates builds stages' do @@ -40,7 +42,8 @@ describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 201 expect(stages.count).to eq 3 expect(stages.all.pluck(:name)).to match_array %w[test build deploy] - expect(jobs.where(stage_id: nil)).to be_empty + expect(jobs.where(stage_id: nil)).to be_one + expect(jobs.find_by(stage_id: nil).id).to eq 6 expect(stages.all.pluck(:status)).to match_array [STATUSES[:success], STATUSES[:failed], STATUSES[:pending]] -- cgit v1.2.1 From fefb8ef74f2c269f1aecffc59fba32eb90c8ba73 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 6 Jan 2018 14:56:09 +0100 Subject: Randomize build ids in stages migration test data --- spec/migrations/schedule_build_stage_migration_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/migrations/schedule_build_stage_migration_spec.rb b/spec/migrations/schedule_build_stage_migration_spec.rb index c96acceeeda..79f00fdbcf0 100644 --- a/spec/migrations/schedule_build_stage_migration_spec.rb +++ b/spec/migrations/schedule_build_stage_migration_spec.rb @@ -18,10 +18,10 @@ describe ScheduleBuildStageMigration, :migration do ## # CI/CD jobs # - jobs.create!(id: 10, commit_id: 1, project_id: 123, stage_id: nil) - jobs.create!(id: 20, commit_id: 1, project_id: 123, stage_id: nil) - jobs.create!(id: 30, commit_id: 1, project_id: 123, stage_id: nil) - jobs.create!(id: 40, commit_id: 1, project_id: 123, stage_id: 1) + jobs.create!(id: 11, commit_id: 1, project_id: 123, stage_id: nil) + jobs.create!(id: 206, commit_id: 1, project_id: 123, stage_id: nil) + jobs.create!(id: 3413, commit_id: 1, project_id: 123, stage_id: nil) + jobs.create!(id: 4109, commit_id: 1, project_id: 123, stage_id: 1) end before do @@ -33,9 +33,9 @@ describe ScheduleBuildStageMigration, :migration do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_delayed_migration(1.minute, 10) - expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 20) - expect(described_class::MIGRATION).to be_scheduled_delayed_migration(3.minutes, 30) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(1.minute, 11) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 206) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(3.minutes, 3413) expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end -- cgit v1.2.1 From c181683d99e4447c44a5963afe3c2680f0c82a9a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 6 Jan 2018 16:07:55 +0100 Subject: Fix rubocop offense in build stage migration classes --- lib/gitlab/background_migration/migrate_build_stage.rb | 2 +- spec/migrations/schedule_build_stage_migration_spec.rb | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/gitlab/background_migration/migrate_build_stage.rb b/lib/gitlab/background_migration/migrate_build_stage.rb index 7139ac03737..6819810b145 100644 --- a/lib/gitlab/background_migration/migrate_build_stage.rb +++ b/lib/gitlab/background_migration/migrate_build_stage.rb @@ -40,7 +40,7 @@ module Gitlab # In that case we just don't migrate such stage. # def legacy? - @build.stage.nil? + @build.stage.nil? end def ensure! diff --git a/spec/migrations/schedule_build_stage_migration_spec.rb b/spec/migrations/schedule_build_stage_migration_spec.rb index 79f00fdbcf0..b475e1fe7de 100644 --- a/spec/migrations/schedule_build_stage_migration_spec.rb +++ b/spec/migrations/schedule_build_stage_migration_spec.rb @@ -8,6 +8,8 @@ describe ScheduleBuildStageMigration, :migration do let(:jobs) { table(:ci_builds) } before do + stub_const("#{described_class}::BATCH", 1) + ## # Dependencies # @@ -24,10 +26,6 @@ describe ScheduleBuildStageMigration, :migration do jobs.create!(id: 4109, commit_id: 1, project_id: 123, stage_id: 1) end - before do - stub_const("#{described_class}::BATCH", 1) - end - it 'schedules delayed background migrations in batches in bulk' do Sidekiq::Testing.fake! do Timecop.freeze do -- cgit v1.2.1 From d4d564c8e7d2cbc3e6742475a793ba0f630167e3 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 1 Feb 2018 18:48:32 +0800 Subject: Try not to hold env and release the controller after the request. This way, we could release the project referred from the controller, which potentially referred a repository which potentially allocated a lot of memories. Before this change, we could hold the last request data and cannot release the memory. After this change, the largest request data should be able to be collected from GC. This might not impact the instances having heavy load, as the last request should be changing all the time, and GC won't kick in for each request anyway. However it could still potentially allow us to free more memories for each GC runs, because now we could free one more request anyway. --- config.ru | 1 + lib/gitlab/middleware/read_only.rb | 2 +- lib/gitlab/middleware/release_controller.rb | 9 +++++++++ .../lib/gitlab/middleware/release_controller_spec.rb | 20 ++++++++++++++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 lib/gitlab/middleware/release_controller.rb create mode 100644 spec/lib/gitlab/middleware/release_controller_spec.rb diff --git a/config.ru b/config.ru index de0400f4f67..c4bef72308e 100644 --- a/config.ru +++ b/config.ru @@ -23,5 +23,6 @@ warmup do |app| end map ENV['RAILS_RELATIVE_URL_ROOT'] || "/" do + use Gitlab::ReleaseController run Gitlab::Application end diff --git a/lib/gitlab/middleware/read_only.rb b/lib/gitlab/middleware/read_only.rb index c26656704d7..a68c6c3d15c 100644 --- a/lib/gitlab/middleware/read_only.rb +++ b/lib/gitlab/middleware/read_only.rb @@ -28,7 +28,7 @@ module Gitlab end end - @app.call(env) + @app.call(env).tap { @env = nil } end private diff --git a/lib/gitlab/middleware/release_controller.rb b/lib/gitlab/middleware/release_controller.rb new file mode 100644 index 00000000000..a21d718d51c --- /dev/null +++ b/lib/gitlab/middleware/release_controller.rb @@ -0,0 +1,9 @@ +module Gitlab + module Middleware + ReleaseController = Struct.new(:app) do + def call(env) + app.call(env).tap { env.delete('action_controller.instance') } + end + end + end +end diff --git a/spec/lib/gitlab/middleware/release_controller_spec.rb b/spec/lib/gitlab/middleware/release_controller_spec.rb new file mode 100644 index 00000000000..854bac6e751 --- /dev/null +++ b/spec/lib/gitlab/middleware/release_controller_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Gitlab::Middleware::ReleaseController do + let(:inner_app) { double(:app) } + let(:app) { described_class.new(inner_app) } + let(:env) { { 'action_controller.instance' => 'something' } } + + before do + expect(inner_app).to receive(:call).with(env).and_return('yay') + end + + describe '#call' do + it 'calls the app and delete the controller' do + result = app.call(env) + + expect(result).to eq('yay') + expect(env).to be_empty + end + end +end -- cgit v1.2.1 From bbfce29ba8d75df5344dae34dc472dfb3b3acf4b Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 2 Feb 2018 22:12:28 +0800 Subject: Use a controller to hold request values So that we don't need to hold env after the request. This makes it much harder to test, especially Rails session is acting weirdly, so we need `dig('flash', 'flashes', 'alert')` to dig the actual flash value. --- lib/gitlab/middleware/read_only.rb | 125 ++++++++++++++------------- spec/lib/gitlab/middleware/read_only_spec.rb | 23 ++++- 2 files changed, 87 insertions(+), 61 deletions(-) diff --git a/lib/gitlab/middleware/read_only.rb b/lib/gitlab/middleware/read_only.rb index a68c6c3d15c..b7649ea01db 100644 --- a/lib/gitlab/middleware/read_only.rb +++ b/lib/gitlab/middleware/read_only.rb @@ -5,86 +5,95 @@ module Gitlab APPLICATION_JSON = 'application/json'.freeze API_VERSIONS = (3..4) - def initialize(app) - @app = app - @whitelisted = internal_routes - end - - def call(env) - @env = env - @route_hash = nil + class Controller + def initialize(app, env) + @app = app + @env = env + end - if disallowed_request? && Gitlab::Database.read_only? - Rails.logger.debug('GitLab ReadOnly: preventing possible non read-only operation') - error_message = 'You cannot do writing operations on a read-only GitLab instance' + def call + if disallowed_request? && Gitlab::Database.read_only? + Rails.logger.debug('GitLab ReadOnly: preventing possible non read-only operation') + error_message = 'You cannot do writing operations on a read-only GitLab instance' - if json_request? - return [403, { 'Content-Type' => 'application/json' }, [{ 'message' => error_message }.to_json]] - else - rack_flash.alert = error_message - rack_session['flash'] = rack_flash.to_session_value + if json_request? + return [403, { 'Content-Type' => 'application/json' }, [{ 'message' => error_message }.to_json]] + else + rack_flash.alert = error_message + rack_session['flash'] = rack_flash.to_session_value - return [301, { 'Location' => last_visited_url }, []] + return [301, { 'Location' => last_visited_url }, []] + end end + + @app.call(@env) end - @app.call(env).tap { @env = nil } - end + private - private + def disallowed_request? + DISALLOWED_METHODS.include?(@env['REQUEST_METHOD']) && + !whitelisted_routes + end - def internal_routes - API_VERSIONS.flat_map { |version| "api/v#{version}/internal" } - end + def json_request? + request.media_type == APPLICATION_JSON + end - def disallowed_request? - DISALLOWED_METHODS.include?(@env['REQUEST_METHOD']) && !whitelisted_routes - end + def rack_flash + @rack_flash ||= ActionDispatch::Flash::FlashHash.from_session_value(rack_session) + end - def json_request? - request.media_type == APPLICATION_JSON - end + def rack_session + @env['rack.session'] + end - def rack_flash - @rack_flash ||= ActionDispatch::Flash::FlashHash.from_session_value(rack_session) - end + def request + @env['rack.request'] ||= Rack::Request.new(@env) + end - def rack_session - @env['rack.session'] - end + def last_visited_url + @env['HTTP_REFERER'] || rack_session['user_return_to'] || Gitlab::Routing.url_helpers.root_url + end - def request - @env['rack.request'] ||= Rack::Request.new(@env) - end + def route_hash + @route_hash ||= Rails.application.routes.recognize_path(request.url, { method: request.request_method }) rescue {} + end - def last_visited_url - @env['HTTP_REFERER'] || rack_session['user_return_to'] || Gitlab::Routing.url_helpers.root_url - end + def whitelisted_routes + grack_route || ReadOnly.internal_routes.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route + end - def route_hash - @route_hash ||= Rails.application.routes.recognize_path(request.url, { method: request.request_method }) rescue {} - end + def sidekiq_route + request.path.start_with?('/admin/sidekiq') + end - def whitelisted_routes - grack_route || @whitelisted.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route - end + def grack_route + # Calling route_hash may be expensive. Only do it if we think there's a possible match + return false unless request.path.end_with?('.git/git-upload-pack') - def sidekiq_route - request.path.start_with?('/admin/sidekiq') - end + route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack' + end + + def lfs_route + # Calling route_hash may be expensive. Only do it if we think there's a possible match + return false unless request.path.end_with?('/info/lfs/objects/batch') - def grack_route - # Calling route_hash may be expensive. Only do it if we think there's a possible match - return false unless request.path.end_with?('.git/git-upload-pack') + route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch' + end + end - route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack' + def self.internal_routes + @internal_routes ||= + API_VERSIONS.map { |version| "api/v#{version}/internal" } end - def lfs_route - # Calling route_hash may be expensive. Only do it if we think there's a possible match - return false unless request.path.end_with?('/info/lfs/objects/batch') + def initialize(app) + @app = app + end - route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch' + def call(env) + Controller.new(@app, env).call end end end diff --git a/spec/lib/gitlab/middleware/read_only_spec.rb b/spec/lib/gitlab/middleware/read_only_spec.rb index 07ba11b93a3..b3c85142b82 100644 --- a/spec/lib/gitlab/middleware/read_only_spec.rb +++ b/spec/lib/gitlab/middleware/read_only_spec.rb @@ -11,8 +11,10 @@ describe Gitlab::Middleware::ReadOnly do RSpec::Matchers.define :disallow_request do match do |middleware| - flash = middleware.send(:rack_flash) - flash['alert'] && flash['alert'].include?('You cannot do writing operations') + alert = middleware.env['rack.session'].to_hash + .dig('flash', 'flashes', 'alert') + + alert&.include?('You cannot do writing operations') end end @@ -34,7 +36,22 @@ describe Gitlab::Middleware::ReadOnly do rack.to_app end - subject { described_class.new(fake_app) } + let(:observe_env) do + Module.new do + attr_reader :env + + def call(env) + @env = env + super + end + end + end + + subject do + app = described_class.new(fake_app) + app.extend(observe_env) + app + end let(:request) { Rack::MockRequest.new(rack_stack) } -- cgit v1.2.1 From 31f1ec59a7cf7517cd5935ef3af540aceba07bb3 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 7 Feb 2018 22:44:05 +0800 Subject: Release the entire env --- config.ru | 2 +- lib/gitlab/middleware/release_controller.rb | 9 --------- lib/gitlab/middleware/release_env.rb | 14 ++++++++++++++ .../lib/gitlab/middleware/release_controller_spec.rb | 20 -------------------- spec/lib/gitlab/middleware/release_env_spec.rb | 20 ++++++++++++++++++++ 5 files changed, 35 insertions(+), 30 deletions(-) delete mode 100644 lib/gitlab/middleware/release_controller.rb create mode 100644 lib/gitlab/middleware/release_env.rb delete mode 100644 spec/lib/gitlab/middleware/release_controller_spec.rb create mode 100644 spec/lib/gitlab/middleware/release_env_spec.rb diff --git a/config.ru b/config.ru index c4bef72308e..7b15939c6ff 100644 --- a/config.ru +++ b/config.ru @@ -23,6 +23,6 @@ warmup do |app| end map ENV['RAILS_RELATIVE_URL_ROOT'] || "/" do - use Gitlab::ReleaseController + use Gitlab::Middleware::ReleaseEnv run Gitlab::Application end diff --git a/lib/gitlab/middleware/release_controller.rb b/lib/gitlab/middleware/release_controller.rb deleted file mode 100644 index a21d718d51c..00000000000 --- a/lib/gitlab/middleware/release_controller.rb +++ /dev/null @@ -1,9 +0,0 @@ -module Gitlab - module Middleware - ReleaseController = Struct.new(:app) do - def call(env) - app.call(env).tap { env.delete('action_controller.instance') } - end - end - end -end diff --git a/lib/gitlab/middleware/release_env.rb b/lib/gitlab/middleware/release_env.rb new file mode 100644 index 00000000000..f8d0a135965 --- /dev/null +++ b/lib/gitlab/middleware/release_env.rb @@ -0,0 +1,14 @@ +module Gitlab + module Middleware + # Some of middleware would hold env for no good reason even after the + # request had already been processed, and we could not garbage collect + # them due to this. Put this middleware as the first middleware so that + # it would clear the env after the request is done, allowing GC gets a + # chance to release memory for the last request. + ReleaseEnv = Struct.new(:app) do + def call(env) + app.call(env).tap { env.clear } + end + end + end +end diff --git a/spec/lib/gitlab/middleware/release_controller_spec.rb b/spec/lib/gitlab/middleware/release_controller_spec.rb deleted file mode 100644 index 854bac6e751..00000000000 --- a/spec/lib/gitlab/middleware/release_controller_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Middleware::ReleaseController do - let(:inner_app) { double(:app) } - let(:app) { described_class.new(inner_app) } - let(:env) { { 'action_controller.instance' => 'something' } } - - before do - expect(inner_app).to receive(:call).with(env).and_return('yay') - end - - describe '#call' do - it 'calls the app and delete the controller' do - result = app.call(env) - - expect(result).to eq('yay') - expect(env).to be_empty - end - end -end diff --git a/spec/lib/gitlab/middleware/release_env_spec.rb b/spec/lib/gitlab/middleware/release_env_spec.rb new file mode 100644 index 00000000000..657b705502a --- /dev/null +++ b/spec/lib/gitlab/middleware/release_env_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Gitlab::Middleware::ReleaseEnv do + let(:inner_app) { double(:app) } + let(:app) { described_class.new(inner_app) } + let(:env) { { 'action_controller.instance' => 'something' } } + + before do + expect(inner_app).to receive(:call).with(env).and_return('yay') + end + + describe '#call' do + it 'calls the app and delete the controller' do + result = app.call(env) + + expect(result).to eq('yay') + expect(env).to be_empty + end + end +end -- cgit v1.2.1 From 5309d4457aea74729a8c6be9ec76d535f922bf8a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 7 Feb 2018 22:44:18 +0800 Subject: Put controller in its separate file --- lib/gitlab/middleware/read_only.rb | 80 +------------------------- lib/gitlab/middleware/read_only/controller.rb | 83 +++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 79 deletions(-) create mode 100644 lib/gitlab/middleware/read_only/controller.rb diff --git a/lib/gitlab/middleware/read_only.rb b/lib/gitlab/middleware/read_only.rb index b7649ea01db..19b74c0c122 100644 --- a/lib/gitlab/middleware/read_only.rb +++ b/lib/gitlab/middleware/read_only.rb @@ -5,84 +5,6 @@ module Gitlab APPLICATION_JSON = 'application/json'.freeze API_VERSIONS = (3..4) - class Controller - def initialize(app, env) - @app = app - @env = env - end - - def call - if disallowed_request? && Gitlab::Database.read_only? - Rails.logger.debug('GitLab ReadOnly: preventing possible non read-only operation') - error_message = 'You cannot do writing operations on a read-only GitLab instance' - - if json_request? - return [403, { 'Content-Type' => 'application/json' }, [{ 'message' => error_message }.to_json]] - else - rack_flash.alert = error_message - rack_session['flash'] = rack_flash.to_session_value - - return [301, { 'Location' => last_visited_url }, []] - end - end - - @app.call(@env) - end - - private - - def disallowed_request? - DISALLOWED_METHODS.include?(@env['REQUEST_METHOD']) && - !whitelisted_routes - end - - def json_request? - request.media_type == APPLICATION_JSON - end - - def rack_flash - @rack_flash ||= ActionDispatch::Flash::FlashHash.from_session_value(rack_session) - end - - def rack_session - @env['rack.session'] - end - - def request - @env['rack.request'] ||= Rack::Request.new(@env) - end - - def last_visited_url - @env['HTTP_REFERER'] || rack_session['user_return_to'] || Gitlab::Routing.url_helpers.root_url - end - - def route_hash - @route_hash ||= Rails.application.routes.recognize_path(request.url, { method: request.request_method }) rescue {} - end - - def whitelisted_routes - grack_route || ReadOnly.internal_routes.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route - end - - def sidekiq_route - request.path.start_with?('/admin/sidekiq') - end - - def grack_route - # Calling route_hash may be expensive. Only do it if we think there's a possible match - return false unless request.path.end_with?('.git/git-upload-pack') - - route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack' - end - - def lfs_route - # Calling route_hash may be expensive. Only do it if we think there's a possible match - return false unless request.path.end_with?('/info/lfs/objects/batch') - - route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch' - end - end - def self.internal_routes @internal_routes ||= API_VERSIONS.map { |version| "api/v#{version}/internal" } @@ -93,7 +15,7 @@ module Gitlab end def call(env) - Controller.new(@app, env).call + ReadOnly::Controller.new(@app, env).call end end end diff --git a/lib/gitlab/middleware/read_only/controller.rb b/lib/gitlab/middleware/read_only/controller.rb new file mode 100644 index 00000000000..053cb6f0a9f --- /dev/null +++ b/lib/gitlab/middleware/read_only/controller.rb @@ -0,0 +1,83 @@ +module Gitlab + module Middleware + class ReadOnly + class Controller + def initialize(app, env) + @app = app + @env = env + end + + def call + if disallowed_request? && Gitlab::Database.read_only? + Rails.logger.debug('GitLab ReadOnly: preventing possible non read-only operation') + error_message = 'You cannot do writing operations on a read-only GitLab instance' + + if json_request? + return [403, { 'Content-Type' => 'application/json' }, [{ 'message' => error_message }.to_json]] + else + rack_flash.alert = error_message + rack_session['flash'] = rack_flash.to_session_value + + return [301, { 'Location' => last_visited_url }, []] + end + end + + @app.call(@env) + end + + private + + def disallowed_request? + DISALLOWED_METHODS.include?(@env['REQUEST_METHOD']) && + !whitelisted_routes + end + + def json_request? + request.media_type == APPLICATION_JSON + end + + def rack_flash + @rack_flash ||= ActionDispatch::Flash::FlashHash.from_session_value(rack_session) + end + + def rack_session + @env['rack.session'] + end + + def request + @env['rack.request'] ||= Rack::Request.new(@env) + end + + def last_visited_url + @env['HTTP_REFERER'] || rack_session['user_return_to'] || Gitlab::Routing.url_helpers.root_url + end + + def route_hash + @route_hash ||= Rails.application.routes.recognize_path(request.url, { method: request.request_method }) rescue {} + end + + def whitelisted_routes + grack_route || ReadOnly.internal_routes.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route + end + + def sidekiq_route + request.path.start_with?('/admin/sidekiq') + end + + def grack_route + # Calling route_hash may be expensive. Only do it if we think there's a possible match + return false unless request.path.end_with?('.git/git-upload-pack') + + route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack' + end + + def lfs_route + # Calling route_hash may be expensive. Only do it if we think there's a possible match + return false unless request.path.end_with?('/info/lfs/objects/batch') + + route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch' + end + end + end + end +end -- cgit v1.2.1 From 461ecbcf07f0785b5ea50c62b114bf8217ac5199 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 7 Feb 2018 22:55:50 +0800 Subject: Update peek-performance_bar which doesn't hold env --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index e78c3c5f794..546ba2f2a18 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -601,7 +601,7 @@ GEM atomic (>= 1.0.0) mysql2 peek - peek-performance_bar (1.3.0) + peek-performance_bar (1.3.1) peek (>= 0.1.0) peek-pg (1.3.0) concurrent-ruby -- cgit v1.2.1 From 89c652b8fe24dd1784acb7b9d475eec6775b6a04 Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Tue, 6 Feb 2018 16:44:38 +0200 Subject: Move BoardNewIssue vue component --- .../javascripts/boards/components/board_list.vue | 2 +- .../boards/components/board_new_issue.js | 100 ------------------ .../boards/components/board_new_issue.vue | 113 +++++++++++++++++++++ app/assets/javascripts/boards/models/issue.js | 2 + ...refactor-move-board-new-issue-vue-component.yml | 5 + spec/javascripts/boards/board_new_issue_spec.js | 2 +- 6 files changed, 122 insertions(+), 102 deletions(-) delete mode 100644 app/assets/javascripts/boards/components/board_new_issue.js create mode 100644 app/assets/javascripts/boards/components/board_new_issue.vue create mode 100644 changelogs/unreleased/refactor-move-board-new-issue-vue-component.yml diff --git a/app/assets/javascripts/boards/components/board_list.vue b/app/assets/javascripts/boards/components/board_list.vue index 9a0442e2afe..6637904d87d 100644 --- a/app/assets/javascripts/boards/components/board_list.vue +++ b/app/assets/javascripts/boards/components/board_list.vue @@ -1,6 +1,6 @@ + + diff --git a/app/assets/javascripts/boards/models/issue.js b/app/assets/javascripts/boards/models/issue.js index 81edd95bf2b..3bfb6d39ad5 100644 --- a/app/assets/javascripts/boards/models/issue.js +++ b/app/assets/javascripts/boards/models/issue.js @@ -110,3 +110,5 @@ class ListIssue { } window.ListIssue = ListIssue; + +export default ListIssue; diff --git a/changelogs/unreleased/refactor-move-board-new-issue-vue-component.yml b/changelogs/unreleased/refactor-move-board-new-issue-vue-component.yml new file mode 100644 index 00000000000..20d05530513 --- /dev/null +++ b/changelogs/unreleased/refactor-move-board-new-issue-vue-component.yml @@ -0,0 +1,5 @@ +--- +title: Move BoardNewIssue vue component +merge_request: 16947 +author: George Tsiolis +type: performance diff --git a/spec/javascripts/boards/board_new_issue_spec.js b/spec/javascripts/boards/board_new_issue_spec.js index e204985f039..d5fbfdeaa91 100644 --- a/spec/javascripts/boards/board_new_issue_spec.js +++ b/spec/javascripts/boards/board_new_issue_spec.js @@ -4,7 +4,7 @@ import Vue from 'vue'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; -import boardNewIssue from '~/boards/components/board_new_issue'; +import boardNewIssue from '~/boards/components/board_new_issue.vue'; import '~/boards/models/list'; import { listObj, boardsMockInterceptor, mockBoardService } from './mock_data'; -- cgit v1.2.1 From acc70f7da1c21071a11bb07b86486c8e61799be8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Feb 2018 13:23:03 +0100 Subject: Move pipeline stages background migration in time --- .../20180105101928_schedule_build_stage_migration.rb | 20 -------------------- .../20180212101928_schedule_build_stage_migration.rb | 20 ++++++++++++++++++++ db/schema.rb | 2 +- .../schedule_build_stage_migration_spec.rb | 2 +- 4 files changed, 22 insertions(+), 22 deletions(-) delete mode 100644 db/post_migrate/20180105101928_schedule_build_stage_migration.rb create mode 100644 db/post_migrate/20180212101928_schedule_build_stage_migration.rb diff --git a/db/post_migrate/20180105101928_schedule_build_stage_migration.rb b/db/post_migrate/20180105101928_schedule_build_stage_migration.rb deleted file mode 100644 index b33127140bb..00000000000 --- a/db/post_migrate/20180105101928_schedule_build_stage_migration.rb +++ /dev/null @@ -1,20 +0,0 @@ -class ScheduleBuildStageMigration < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - MIGRATION = 'MigrateBuildStage'.freeze - BATCH = 1000 - - class Build < ActiveRecord::Base - include EachBatch - self.table_name = 'ci_builds' - end - - def change - Build.where('stage_id IS NULL').each_batch(of: BATCH) do |builds, index| - builds.pluck(:id).map { |id| [MIGRATION, [id]] }.tap do |migrations| - BackgroundMigrationWorker.bulk_perform_in(index.minutes, migrations) - end - end - end -end diff --git a/db/post_migrate/20180212101928_schedule_build_stage_migration.rb b/db/post_migrate/20180212101928_schedule_build_stage_migration.rb new file mode 100644 index 00000000000..b33127140bb --- /dev/null +++ b/db/post_migrate/20180212101928_schedule_build_stage_migration.rb @@ -0,0 +1,20 @@ +class ScheduleBuildStageMigration < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + MIGRATION = 'MigrateBuildStage'.freeze + BATCH = 1000 + + class Build < ActiveRecord::Base + include EachBatch + self.table_name = 'ci_builds' + end + + def change + Build.where('stage_id IS NULL').each_batch(of: BATCH) do |builds, index| + builds.pluck(:id).map { |id| [MIGRATION, [id]] }.tap do |migrations| + BackgroundMigrationWorker.bulk_perform_in(index.minutes, migrations) + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index d07a4c31618..e3474a4995f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180206200543) do +ActiveRecord::Schema.define(version: 20180212101928) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/migrations/schedule_build_stage_migration_spec.rb b/spec/migrations/schedule_build_stage_migration_spec.rb index b475e1fe7de..5e18d2e72f9 100644 --- a/spec/migrations/schedule_build_stage_migration_spec.rb +++ b/spec/migrations/schedule_build_stage_migration_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20180105101928_schedule_build_stage_migration') +require Rails.root.join('db', 'post_migrate', '20180212101928_schedule_build_stage_migration') describe ScheduleBuildStageMigration, :migration do let(:projects) { table(:projects) } -- cgit v1.2.1 From 8488c98e70561c6d7dde025131c15df4692b20b1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Feb 2018 16:18:09 +0100 Subject: Schedule pipeline stages migration every 5 minutes [ci skip] --- .../20180212101928_schedule_build_stage_migration.rb | 18 +++++++++++++----- spec/migrations/schedule_build_stage_migration_spec.rb | 14 ++++---------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/db/post_migrate/20180212101928_schedule_build_stage_migration.rb b/db/post_migrate/20180212101928_schedule_build_stage_migration.rb index b33127140bb..f03a3f8d355 100644 --- a/db/post_migrate/20180212101928_schedule_build_stage_migration.rb +++ b/db/post_migrate/20180212101928_schedule_build_stage_migration.rb @@ -3,18 +3,26 @@ class ScheduleBuildStageMigration < ActiveRecord::Migration DOWNTIME = false MIGRATION = 'MigrateBuildStage'.freeze - BATCH = 1000 + BATCH_SIZE = 500 + + disable_ddl_transaction! class Build < ActiveRecord::Base include EachBatch self.table_name = 'ci_builds' end - def change - Build.where('stage_id IS NULL').each_batch(of: BATCH) do |builds, index| - builds.pluck(:id).map { |id| [MIGRATION, [id]] }.tap do |migrations| - BackgroundMigrationWorker.bulk_perform_in(index.minutes, migrations) + def up + disable_statement_timeout + + Build.where('stage_id IS NULL').each_batch(of: BATCH_SIZE) do |builds, index| + builds.pluck('MIN(id)', 'MAX(id)').first.tap do |range| + BackgroundMigrationWorker.perform_in(index * 5.minutes, MIGRATION, range) end end end + + def down + # noop + end end diff --git a/spec/migrations/schedule_build_stage_migration_spec.rb b/spec/migrations/schedule_build_stage_migration_spec.rb index 5e18d2e72f9..06657410173 100644 --- a/spec/migrations/schedule_build_stage_migration_spec.rb +++ b/spec/migrations/schedule_build_stage_migration_spec.rb @@ -8,18 +8,12 @@ describe ScheduleBuildStageMigration, :migration do let(:jobs) { table(:ci_builds) } before do - stub_const("#{described_class}::BATCH", 1) + stub_const("#{described_class}::BATCH_SIZE", 1) - ## - # Dependencies - # projects.create!(id: 123, name: 'gitlab', path: 'gitlab-ce') pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') stages.create!(id: 1, project_id: 123, pipeline_id: 1, name: 'test') - ## - # CI/CD jobs - # jobs.create!(id: 11, commit_id: 1, project_id: 123, stage_id: nil) jobs.create!(id: 206, commit_id: 1, project_id: 123, stage_id: nil) jobs.create!(id: 3413, commit_id: 1, project_id: 123, stage_id: nil) @@ -31,9 +25,9 @@ describe ScheduleBuildStageMigration, :migration do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_delayed_migration(1.minute, 11) - expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 206) - expect(described_class::MIGRATION).to be_scheduled_delayed_migration(3.minutes, 3413) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 11, 11) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 206, 206) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(15.minutes, 3413, 3413) expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end -- cgit v1.2.1 From 378b2bad11b893fabebc083d000295c1ec499b23 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 14 Feb 2018 13:59:16 +0100 Subject: Migrate pipeline stages in batches instead of single row --- .../background_migration/migrate_build_stage.rb | 84 ++++++++-------------- .../migrate_build_stage_spec.rb | 12 +--- 2 files changed, 32 insertions(+), 64 deletions(-) diff --git a/lib/gitlab/background_migration/migrate_build_stage.rb b/lib/gitlab/background_migration/migrate_build_stage.rb index 6819810b145..3e297a90b19 100644 --- a/lib/gitlab/background_migration/migrate_build_stage.rb +++ b/lib/gitlab/background_migration/migrate_build_stage.rb @@ -5,69 +5,45 @@ module Gitlab module BackgroundMigration class MigrateBuildStage - def perform(id) - DatabaseBuild.find_by(id: id).try do |build| - MigratableStage.new(build).tap do |stage| - break if stage.exists? || stage.legacy? - - stage.ensure! - stage.migrate_reference! - stage.migrate_status! - end - end - end - - class DatabaseStage < ActiveRecord::Base - self.table_name = 'ci_stages' - end - - class DatabaseBuild < ActiveRecord::Base - self.table_name = 'ci_builds' - end - - class MigratableStage - def initialize(build) - @build = build - end - - def exists? - @build.reload.stage_id.present? + module Migratable + class Stage < ActiveRecord::Base + self.table_name = 'ci_stages' end - ## - # We can have some very old stages that do not have `ci_builds.stage` set. - # - # In that case we just don't migrate such stage. - # - def legacy? - @build.stage.nil? - end + class Build < ActiveRecord::Base + self.table_name = 'ci_builds' - def ensure! - find || create! - end + def ensure_stage! + find || create! + rescue ActiveRecord::RecordNotUnique + # TODO + end - def find - DatabaseStage.find_by(name: @build.stage, - pipeline_id: @build.commit_id, - project_id: @build.project_id) - end + def find + Stage.find_by(name: self.stage, + pipeline_id: self.commit_id, + project_id: self.project_id) + end - def create! - DatabaseStage.create!(name: @build.stage, - pipeline_id: @build.commit_id, - project_id: @build.project_id) + def create! + Stage.create!(name: self.stage || 'test', + pipeline_id: self.commit_id, + project_id: self.project_id) + end end + end - def migrate_reference! - MigrateBuildStageIdReference.new.perform(@build.id, @build.id) - end + def perform(start_id, stop_id) + # TODO, should we disable_statement_timeout? + # TODO, use plain SQL query? - def migrate_status! - raise ArgumentError unless exists? + stages = Migratable::Build.where('stage_id IS NULL') + .where("id BETWEEN #{start_id.to_i} AND #{stop_id.to_i}") + .map { |build| build.ensure_stage! } + .compact.map(&:id) - MigrateStageStatus.new.perform(@build.stage_id, @build.stage_id) - end + MigrateBuildStageIdReference.new.perform(start_id, stop_id) + MigrateStageStatus.new.perform(stages.min, stages.max) end end end diff --git a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb index d20b5c29a71..27cb63848de 100644 --- a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 20180105101928 do +describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 20180212101928 do let(:projects) { table(:projects) } let(:pipelines) { table(:ci_pipelines) } let(:stages) { table(:ci_stages) } @@ -10,15 +10,9 @@ describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 201 failed: 4, canceled: 5, skipped: 6, manual: 7 }.freeze before do - ## - # Dependencies - # projects.create!(id: 123, name: 'gitlab', path: 'gitlab-ce') pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') - ## - # CI/CD jobs - # jobs.create!(id: 1, commit_id: 1, project_id: 123, stage_idx: 2, stage: 'build', status: :success) jobs.create!(id: 2, commit_id: 1, project_id: 123, @@ -36,9 +30,7 @@ describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 201 it 'correctly migrates builds stages' do expect(stages.count).to be_zero - jobs.all.find_each do |job| - described_class.new.perform(job.id) - end + described_class.new.perform(1, 6) expect(stages.count).to eq 3 expect(stages.all.pluck(:name)).to match_array %w[test build deploy] -- cgit v1.2.1 From b3d8a1e29a44fdb7770929b466b24c286e271208 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 14 Feb 2018 16:09:56 +0200 Subject: update changelog --- changelogs/unreleased/4826-create-empty-wiki-when-it-s-enabled.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/4826-create-empty-wiki-when-it-s-enabled.yml diff --git a/changelogs/unreleased/4826-create-empty-wiki-when-it-s-enabled.yml b/changelogs/unreleased/4826-create-empty-wiki-when-it-s-enabled.yml new file mode 100644 index 00000000000..c0fa8e2e377 --- /dev/null +++ b/changelogs/unreleased/4826-create-empty-wiki-when-it-s-enabled.yml @@ -0,0 +1,5 @@ +--- +title: Make sure wiki exists when it's enabled +merge_request: +author: +type: fixed -- cgit v1.2.1 From c1828eaed56159998d1eaafdaa135f1b3480549b Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Mon, 12 Feb 2018 14:22:15 +1100 Subject: Persist external IP of ingress controller created for GKE (#42643) --- app/models/clusters/applications/ingress.rb | 7 ++ app/models/clusters/concerns/application_core.rb | 4 ++ app/serializers/cluster_application_entity.rb | 1 + .../check_ingress_ip_address_service.rb | 37 ++++++++++ .../check_installation_progress_service.rb | 1 + app/workers/all_queues.yml | 1 + .../cluster_wait_for_ingress_ip_address_worker.rb | 14 ++++ ...external_ip_to_clusters_applications_ingress.rb | 31 +++++++++ db/schema.rb | 3 +- spec/fixtures/api/schemas/cluster_status.json | 3 +- spec/models/clusters/applications/ingress_spec.rb | 14 ++++ .../serializers/cluster_application_entity_spec.rb | 14 ++++ .../check_ingress_ip_address_service_spec.rb | 81 ++++++++++++++++++++++ ...ster_wait_for_ingress_ip_address_worker_spec.rb | 20 ++++++ 14 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 app/services/clusters/applications/check_ingress_ip_address_service.rb create mode 100644 app/workers/cluster_wait_for_ingress_ip_address_worker.rb create mode 100644 db/migrate/20180212030105_add_external_ip_to_clusters_applications_ingress.rb create mode 100644 spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb create mode 100644 spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index aa5cf97756f..5e9086aecca 100644 --- a/app/models/clusters/applications/ingress.rb +++ b/app/models/clusters/applications/ingress.rb @@ -13,6 +13,8 @@ module Clusters nginx: 1 } + IP_ADDRESS_FETCH_RETRIES = 3 + def chart 'stable/nginx-ingress' end @@ -24,6 +26,11 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new(name, chart: chart, chart_values_file: chart_values_file) end + + def post_install + ClusterWaitForIngressIpAddressWorker.perform_in( + ClusterWaitForIngressIpAddressWorker::INTERVAL, name, id, IP_ADDRESS_FETCH_RETRIES) + end end end end diff --git a/app/models/clusters/concerns/application_core.rb b/app/models/clusters/concerns/application_core.rb index a98fa85a5ff..b047fbce214 100644 --- a/app/models/clusters/concerns/application_core.rb +++ b/app/models/clusters/concerns/application_core.rb @@ -23,6 +23,10 @@ module Clusters def name self.class.application_name end + + def post_install + # Override for any extra work that needs to be done after install + end end end end diff --git a/app/serializers/cluster_application_entity.rb b/app/serializers/cluster_application_entity.rb index 3f9a275ad08..b22a0b666ef 100644 --- a/app/serializers/cluster_application_entity.rb +++ b/app/serializers/cluster_application_entity.rb @@ -2,4 +2,5 @@ class ClusterApplicationEntity < Grape::Entity expose :name expose :status_name, as: :status expose :status_reason + expose :external_ip, if: -> (e, _) { e.respond_to?(:external_ip) } end diff --git a/app/services/clusters/applications/check_ingress_ip_address_service.rb b/app/services/clusters/applications/check_ingress_ip_address_service.rb new file mode 100644 index 00000000000..cf132676aa6 --- /dev/null +++ b/app/services/clusters/applications/check_ingress_ip_address_service.rb @@ -0,0 +1,37 @@ +module Clusters + module Applications + class CheckIngressIpAddressService < BaseHelmService + def execute(retries_remaining) + return if app.external_ip + + service = get_service + + if service.status.loadBalancer.ingress + resolve_external_ip(service) + else + retry_if_necessary(retries_remaining) + end + + rescue KubeException + retry_if_necessary(retries_remaining) + end + + private + + def resolve_external_ip(service) + app.update!( external_ip: service.status.loadBalancer.ingress[0].ip) + end + + def get_service + kubeclient.get_service('ingress-nginx-ingress-controller', Gitlab::Kubernetes::Helm::NAMESPACE) + end + + def retry_if_necessary(retries_remaining) + if retries_remaining > 0 + ClusterWaitForIngressIpAddressWorker.perform_in( + ClusterWaitForIngressIpAddressWorker::INTERVAL, app.name, app.id, retries_remaining - 1) + end + end + end + end +end diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb index bde090eaeec..7dcddc1c3f7 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -20,6 +20,7 @@ module Clusters def on_success app.make_installed! + app.post_install ensure remove_installation_pod end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index f2c20114534..35ffa5d5fda 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -23,6 +23,7 @@ - gcp_cluster:cluster_wait_for_app_installation - gcp_cluster:wait_for_cluster_creation - gcp_cluster:check_gcp_project_billing +- gcp_cluster:cluster_wait_for_ingress_ip_address - github_import_advance_stage - github_importer:github_import_import_diff_note diff --git a/app/workers/cluster_wait_for_ingress_ip_address_worker.rb b/app/workers/cluster_wait_for_ingress_ip_address_worker.rb new file mode 100644 index 00000000000..829417484cf --- /dev/null +++ b/app/workers/cluster_wait_for_ingress_ip_address_worker.rb @@ -0,0 +1,14 @@ +class ClusterWaitForIngressIpAddressWorker + include ApplicationWorker + include ClusterQueue + include ClusterApplications + + INTERVAL = 10.seconds + TIMEOUT = 20.minutes + + def perform(app_name, app_id, retries_remaining) + find_application(app_name, app_id) do |app| + Clusters::Applications::CheckIngressIpAddressService.new(app).execute(retries_remaining) + end + end +end diff --git a/db/migrate/20180212030105_add_external_ip_to_clusters_applications_ingress.rb b/db/migrate/20180212030105_add_external_ip_to_clusters_applications_ingress.rb new file mode 100644 index 00000000000..c18d1d7a67b --- /dev/null +++ b/db/migrate/20180212030105_add_external_ip_to_clusters_applications_ingress.rb @@ -0,0 +1,31 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddExternalIpToClustersApplicationsIngress < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :clusters_applications_ingress, :external_ip, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 6b43fc8403c..e13415926ec 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180208183958) do +ActiveRecord::Schema.define(version: 20180212030105) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -568,6 +568,7 @@ ActiveRecord::Schema.define(version: 20180208183958) do t.string "version", null: false t.string "cluster_ip" t.text "status_reason" + t.string "external_ip" end create_table "clusters_applications_prometheus", force: :cascade do |t| diff --git a/spec/fixtures/api/schemas/cluster_status.json b/spec/fixtures/api/schemas/cluster_status.json index 489d563be2b..9617157ee37 100644 --- a/spec/fixtures/api/schemas/cluster_status.json +++ b/spec/fixtures/api/schemas/cluster_status.json @@ -30,7 +30,8 @@ ] } }, - "status_reason": { "type": ["string", "null"] } + "status_reason": { "type": ["string", "null"] }, + "external_ip": { "type": ["string", null] } }, "required" : [ "name", "status" ] } diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index 619c088b0bf..da9535f9d16 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -5,4 +5,18 @@ describe Clusters::Applications::Ingress do it { is_expected.to validate_presence_of(:cluster) } include_examples 'cluster application specs', described_class + + describe '#post_install' do + let(:application) { create(:clusters_applications_ingress, :installed) } + + before do + allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) + application.post_install + end + + it 'schedules a ClusterWaitForIngressIpAddressWorker' do + expect(ClusterWaitForIngressIpAddressWorker).to have_received(:perform_in) + .with(ClusterWaitForIngressIpAddressWorker::INTERVAL, 'ingress', application.id, 3) + end + end end diff --git a/spec/serializers/cluster_application_entity_spec.rb b/spec/serializers/cluster_application_entity_spec.rb index b5a55b4ef6e..70b274083b2 100644 --- a/spec/serializers/cluster_application_entity_spec.rb +++ b/spec/serializers/cluster_application_entity_spec.rb @@ -26,5 +26,19 @@ describe ClusterApplicationEntity do expect(subject[:status_reason]).to eq(application.status_reason) end end + + context 'for ingress application' do + let(:application) do + build( + :clusters_applications_ingress, + :installed, + external_ip: '111.222.111.222', + ) + end + + it 'includes external_ip' do + expect(subject[:external_ip]).to eq('111.222.111.222') + end + end end end diff --git a/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb b/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb new file mode 100644 index 00000000000..5d99196fbe6 --- /dev/null +++ b/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb @@ -0,0 +1,81 @@ +require 'spec_helper' + +describe Clusters::Applications::CheckIngressIpAddressService do + let(:application) { create(:clusters_applications_ingress, :installed) } + let(:service) { described_class.new(application) } + let(:kube_service) do + ::Kubeclient::Resource.new( + { + status: { + loadBalancer: { + ingress: ingress + } + } + } + ) + end + let(:kubeclient) { double(::Kubeclient::Client, get_service: kube_service) } + let(:ingress) { [{ ip: '111.222.111.222' }] } + + before do + allow(application.cluster).to receive(:kubeclient).and_return(kubeclient) + end + + describe '#execute' do + context 'when the ingress ip address is available' do + it 'updates the external_ip for the app and does not schedule another worker' do + expect(ClusterWaitForIngressIpAddressWorker).not_to receive(:perform_in) + + service.execute(1) + + expect(application.external_ip).to eq('111.222.111.222') + end + end + + context 'when the ingress ip address is not available' do + let(:ingress) { nil } + + it 'it schedules another worker with 1 less retry' do + expect(ClusterWaitForIngressIpAddressWorker) + .to receive(:perform_in) + .with(ClusterWaitForIngressIpAddressWorker::INTERVAL, 'ingress', application.id, 0) + + service.execute(1) + end + + context 'when no more retries remaining' do + it 'does not schedule another worker' do + expect(ClusterWaitForIngressIpAddressWorker).not_to receive(:perform_in) + + service.execute(0) + end + end + end + + context 'when there is already an external_ip' do + let(:application) { create(:clusters_applications_ingress, :installed, external_ip: '001.111.002.111') } + + it 'does nothing' do + expect(kubeclient).not_to receive(:get_service) + + service.execute(1) + + expect(application.external_ip).to eq('001.111.002.111') + end + end + + context 'when a kubernetes error occurs' do + before do + allow(kubeclient).to receive(:get_service).and_raise(KubeException.new(500, 'something blew up', nil)) + end + + it 'it schedules another worker with 1 less retry' do + expect(ClusterWaitForIngressIpAddressWorker) + .to receive(:perform_in) + .with(ClusterWaitForIngressIpAddressWorker::INTERVAL, 'ingress', application.id, 0) + + service.execute(1) + end + end + end +end diff --git a/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb b/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb new file mode 100644 index 00000000000..9f8bf35f604 --- /dev/null +++ b/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe ClusterWaitForIngressIpAddressWorker do + describe '#perform' do + let(:service) { instance_double(Clusters::Applications::CheckIngressIpAddressService) } + let(:application) { instance_double(Clusters::Applications::Ingress) } + let(:worker) { described_class.new } + + it 'finds the application and calls CheckIngressIpAddressService#execute' do + expect(worker).to receive(:find_application).with('ingress', 117).and_yield(application) + expect(Clusters::Applications::CheckIngressIpAddressService) + .to receive(:new) + .with(application) + .and_return(service) + expect(service).to receive(:execute).with(2) + + worker.perform('ingress', 117, 2) + end + end +end -- cgit v1.2.1 From 5190ef57a3c5a4333020a5281904e56c00519e91 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 15 Feb 2018 17:24:59 +1100 Subject: Ensure CheckIngressIpAddressService obtains exclusive lease per ingress controller (#42643) --- .../applications/check_ingress_ip_address_service.rb | 9 +++++++++ .../check_ingress_ip_address_service_spec.rb | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/app/services/clusters/applications/check_ingress_ip_address_service.rb b/app/services/clusters/applications/check_ingress_ip_address_service.rb index cf132676aa6..3262aa59a90 100644 --- a/app/services/clusters/applications/check_ingress_ip_address_service.rb +++ b/app/services/clusters/applications/check_ingress_ip_address_service.rb @@ -1,8 +1,11 @@ module Clusters module Applications class CheckIngressIpAddressService < BaseHelmService + LEASE_TIMEOUT = 3.seconds.to_i + def execute(retries_remaining) return if app.external_ip + return unless try_obtain_lease service = get_service @@ -18,6 +21,12 @@ module Clusters private + def try_obtain_lease + Gitlab::ExclusiveLease + .new("check_ingress_ip_address_service:#{app.id}", timeout: LEASE_TIMEOUT) + .try_obtain + end + def resolve_external_ip(service) app.update!( external_ip: service.status.loadBalancer.ingress[0].ip) end diff --git a/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb b/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb index 5d99196fbe6..6c81acfcf84 100644 --- a/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb +++ b/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb @@ -16,9 +16,14 @@ describe Clusters::Applications::CheckIngressIpAddressService do end let(:kubeclient) { double(::Kubeclient::Client, get_service: kube_service) } let(:ingress) { [{ ip: '111.222.111.222' }] } + let(:exclusive_lease) { instance_double(Gitlab::ExclusiveLease, try_obtain: true) } before do allow(application.cluster).to receive(:kubeclient).and_return(kubeclient) + allow(Gitlab::ExclusiveLease) + .to receive(:new) + .with("check_ingress_ip_address_service:#{application.id}", timeout: 3.seconds.to_i) + .and_return(exclusive_lease) end describe '#execute' do @@ -52,6 +57,20 @@ describe Clusters::Applications::CheckIngressIpAddressService do end end + context 'when the exclusive lease cannot be obtained' do + before do + allow(exclusive_lease) + .to receive(:try_obtain) + .and_return(false) + end + + it 'does not call kubeclient' do + expect(kubeclient).not_to receive(:get_service) + + service.execute(1) + end + end + context 'when there is already an external_ip' do let(:application) { create(:clusters_applications_ingress, :installed, external_ip: '001.111.002.111') } -- cgit v1.2.1 From 6cb5b7c8729151c95d1610f0b2f7255fdac2bdec Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 15 Feb 2018 10:13:20 +0100 Subject: Recover from unique constraint violation in stages migration --- .../20180212101928_schedule_build_stage_migration.rb | 2 +- lib/gitlab/background_migration/migrate_build_stage.rb | 18 +++++++++--------- .../background_migration/migrate_build_stage_spec.rb | 11 +++++++++++ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/db/post_migrate/20180212101928_schedule_build_stage_migration.rb b/db/post_migrate/20180212101928_schedule_build_stage_migration.rb index f03a3f8d355..ad65a6fb16e 100644 --- a/db/post_migrate/20180212101928_schedule_build_stage_migration.rb +++ b/db/post_migrate/20180212101928_schedule_build_stage_migration.rb @@ -3,7 +3,7 @@ class ScheduleBuildStageMigration < ActiveRecord::Migration DOWNTIME = false MIGRATION = 'MigrateBuildStage'.freeze - BATCH_SIZE = 500 + BATCH_SIZE = 800 disable_ddl_transaction! diff --git a/lib/gitlab/background_migration/migrate_build_stage.rb b/lib/gitlab/background_migration/migrate_build_stage.rb index 3e297a90b19..adca16751af 100644 --- a/lib/gitlab/background_migration/migrate_build_stage.rb +++ b/lib/gitlab/background_migration/migrate_build_stage.rb @@ -13,19 +13,20 @@ module Gitlab class Build < ActiveRecord::Base self.table_name = 'ci_builds' - def ensure_stage! - find || create! + def ensure_stage!(attempts: 2) + find_stage || create_stage! rescue ActiveRecord::RecordNotUnique - # TODO + retry if (attempts -= 1) > 0 + raise end - def find - Stage.find_by(name: self.stage, + def find_stage + Stage.find_by(name: self.stage || 'test', pipeline_id: self.commit_id, project_id: self.project_id) end - def create! + def create_stage! Stage.create!(name: self.stage || 'test', pipeline_id: self.commit_id, project_id: self.project_id) @@ -34,11 +35,10 @@ module Gitlab end def perform(start_id, stop_id) - # TODO, should we disable_statement_timeout? - # TODO, use plain SQL query? + # TODO, statement timeout? stages = Migratable::Build.where('stage_id IS NULL') - .where("id BETWEEN #{start_id.to_i} AND #{stop_id.to_i}") + .where('id BETWEEN ? AND ?', start_id, stop_id) .map { |build| build.ensure_stage! } .compact.map(&:id) diff --git a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb index 27cb63848de..e112e9e9e3d 100644 --- a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb @@ -40,4 +40,15 @@ describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 201 STATUSES[:failed], STATUSES[:pending]] end + + it 'recovers from unique constraint violation only twice' do + allow(described_class::Migratable::Stage) + .to receive(:find_by).and_return(nil) + + expect(described_class::Migratable::Stage) + .to receive(:find_by).exactly(3).times + + expect { described_class.new.perform(1, 6) } + .to raise_error ActiveRecord::RecordNotUnique + end end -- cgit v1.2.1 From 0af391d2f45f23fb806584a5c80747145a07b7c0 Mon Sep 17 00:00:00 2001 From: Constance Okoghenun Date: Thu, 15 Feb 2018 12:26:38 +0100 Subject: Refactored use of boards/boards_bundle.js, created a dipatcher import --- app/assets/javascripts/boards/boards_bundle.js | 239 --------------------- app/assets/javascripts/boards/index.js | 239 +++++++++++++++++++++ app/assets/javascripts/dispatcher.js | 5 + .../javascripts/pages/projects/boards/index.js | 6 +- app/views/shared/boards/_show.html.haml | 1 - config/webpack.config.js | 1 - spec/javascripts/test_bundle.js | 2 +- 7 files changed, 249 insertions(+), 244 deletions(-) delete mode 100644 app/assets/javascripts/boards/boards_bundle.js create mode 100644 app/assets/javascripts/boards/index.js diff --git a/app/assets/javascripts/boards/boards_bundle.js b/app/assets/javascripts/boards/boards_bundle.js deleted file mode 100644 index 90166b3d3d1..00000000000 --- a/app/assets/javascripts/boards/boards_bundle.js +++ /dev/null @@ -1,239 +0,0 @@ -/* eslint-disable one-var, quote-props, comma-dangle, space-before-function-paren */ - -import _ from 'underscore'; -import Vue from 'vue'; -import Flash from '../flash'; -import { __ } from '../locale'; -import FilteredSearchBoards from './filtered_search_boards'; -import eventHub from './eventhub'; -import sidebarEventHub from '../sidebar/event_hub'; -import './models/issue'; -import './models/label'; -import './models/list'; -import './models/milestone'; -import './models/assignee'; -import './stores/boards_store'; -import './stores/modal_store'; -import BoardService from './services/board_service'; -import './mixins/modal_mixins'; -import './mixins/sortable_default_options'; -import './filters/due_date_filters'; -import './components/board'; -import './components/board_sidebar'; -import './components/new_list_dropdown'; -import './components/modal/index'; -import '../vue_shared/vue_resource_interceptor'; - -$(() => { - const $boardApp = document.getElementById('board-app'); - const Store = gl.issueBoards.BoardsStore; - const ModalStore = gl.issueBoards.ModalStore; - - window.gl = window.gl || {}; - - if (gl.IssueBoardsApp) { - gl.IssueBoardsApp.$destroy(true); - } - - Store.create(); - - // hack to allow sidebar scripts like milestone_select manipulate the BoardsStore - gl.issueBoards.boardStoreIssueSet = (...args) => Vue.set(Store.detail.issue, ...args); - gl.issueBoards.boardStoreIssueDelete = (...args) => Vue.delete(Store.detail.issue, ...args); - - gl.IssueBoardsApp = new Vue({ - el: $boardApp, - components: { - 'board': gl.issueBoards.Board, - 'board-sidebar': gl.issueBoards.BoardSidebar, - 'board-add-issues-modal': gl.issueBoards.IssuesModal, - }, - data: { - state: Store.state, - loading: true, - boardsEndpoint: $boardApp.dataset.boardsEndpoint, - listsEndpoint: $boardApp.dataset.listsEndpoint, - boardId: $boardApp.dataset.boardId, - disabled: $boardApp.dataset.disabled === 'true', - issueLinkBase: $boardApp.dataset.issueLinkBase, - rootPath: $boardApp.dataset.rootPath, - bulkUpdatePath: $boardApp.dataset.bulkUpdatePath, - detailIssue: Store.detail, - defaultAvatar: $boardApp.dataset.defaultAvatar, - }, - computed: { - detailIssueVisible () { - return Object.keys(this.detailIssue.issue).length; - }, - }, - created () { - gl.boardService = new BoardService({ - boardsEndpoint: this.boardsEndpoint, - listsEndpoint: this.listsEndpoint, - bulkUpdatePath: this.bulkUpdatePath, - boardId: this.boardId, - }); - Store.rootPath = this.boardsEndpoint; - - eventHub.$on('updateTokens', this.updateTokens); - eventHub.$on('newDetailIssue', this.updateDetailIssue); - eventHub.$on('clearDetailIssue', this.clearDetailIssue); - sidebarEventHub.$on('toggleSubscription', this.toggleSubscription); - }, - beforeDestroy() { - eventHub.$off('updateTokens', this.updateTokens); - eventHub.$off('newDetailIssue', this.updateDetailIssue); - eventHub.$off('clearDetailIssue', this.clearDetailIssue); - sidebarEventHub.$off('toggleSubscription', this.toggleSubscription); - }, - mounted () { - this.filterManager = new FilteredSearchBoards(Store.filter, true); - this.filterManager.setup(); - - Store.disabled = this.disabled; - gl.boardService.all() - .then(res => res.data) - .then((data) => { - data.forEach((board) => { - const list = Store.addList(board, this.defaultAvatar); - - if (list.type === 'closed') { - list.position = Infinity; - } else if (list.type === 'backlog') { - list.position = -1; - } - }); - - this.state.lists = _.sortBy(this.state.lists, 'position'); - - Store.addBlankState(); - this.loading = false; - }) - .catch(() => { - Flash('An error occurred while fetching the board lists. Please try again.'); - }); - }, - methods: { - updateTokens() { - this.filterManager.updateTokens(); - }, - updateDetailIssue(newIssue) { - const sidebarInfoEndpoint = newIssue.sidebarInfoEndpoint; - if (sidebarInfoEndpoint && newIssue.subscribed === undefined) { - newIssue.setFetchingState('subscriptions', true); - BoardService.getIssueInfo(sidebarInfoEndpoint) - .then(res => res.data) - .then((data) => { - newIssue.setFetchingState('subscriptions', false); - newIssue.updateData({ - subscribed: data.subscribed, - }); - }) - .catch(() => { - newIssue.setFetchingState('subscriptions', false); - Flash(__('An error occurred while fetching sidebar data')); - }); - } - - Store.detail.issue = newIssue; - }, - clearDetailIssue() { - Store.detail.issue = {}; - }, - toggleSubscription(id) { - const issue = Store.detail.issue; - if (issue.id === id && issue.toggleSubscriptionEndpoint) { - issue.setFetchingState('subscriptions', true); - BoardService.toggleIssueSubscription(issue.toggleSubscriptionEndpoint) - .then(() => { - issue.setFetchingState('subscriptions', false); - issue.updateData({ - subscribed: !issue.subscribed, - }); - }) - .catch(() => { - issue.setFetchingState('subscriptions', false); - Flash(__('An error occurred when toggling the notification subscription')); - }); - } - } - }, - }); - - gl.IssueBoardsSearch = new Vue({ - el: document.getElementById('js-add-list'), - data: { - filters: Store.state.filters, - }, - mounted () { - gl.issueBoards.newListDropdownInit(); - }, - }); - - gl.IssueBoardsModalAddBtn = new Vue({ - el: document.getElementById('js-add-issues-btn'), - mixins: [gl.issueBoards.ModalMixins], - data() { - return { - modal: ModalStore.store, - store: Store.state, - }; - }, - computed: { - disabled() { - if (!this.store) { - return true; - } - return !this.store.lists.filter(list => !list.preset).length; - }, - tooltipTitle() { - if (this.disabled) { - return 'Please add a list to your board first'; - } - - return ''; - }, - }, - watch: { - disabled() { - this.updateTooltip(); - }, - }, - mounted() { - this.updateTooltip(); - }, - methods: { - updateTooltip() { - const $tooltip = $(this.$refs.addIssuesButton); - - this.$nextTick(() => { - if (this.disabled) { - $tooltip.tooltip(); - } else { - $tooltip.tooltip('destroy'); - } - }); - }, - openModal() { - if (!this.disabled) { - this.toggleModal(true); - } - }, - }, - template: ` -
- -
- `, - }); -}); diff --git a/app/assets/javascripts/boards/index.js b/app/assets/javascripts/boards/index.js new file mode 100644 index 00000000000..7adbf442f00 --- /dev/null +++ b/app/assets/javascripts/boards/index.js @@ -0,0 +1,239 @@ +/* eslint-disable one-var, quote-props, comma-dangle, space-before-function-paren */ + +import _ from 'underscore'; +import Vue from 'vue'; +import Flash from '../flash'; +import { __ } from '../locale'; +import FilteredSearchBoards from './filtered_search_boards'; +import eventHub from './eventhub'; +import sidebarEventHub from '../sidebar/event_hub'; +import './models/issue'; +import './models/label'; +import './models/list'; +import './models/milestone'; +import './models/assignee'; +import './stores/boards_store'; +import './stores/modal_store'; +import BoardService from './services/board_service'; +import './mixins/modal_mixins'; +import './mixins/sortable_default_options'; +import './filters/due_date_filters'; +import './components/board'; +import './components/board_sidebar'; +import './components/new_list_dropdown'; +import './components/modal/index'; +import '../vue_shared/vue_resource_interceptor'; + +export default function initBoards() { + const $boardApp = document.getElementById('board-app'); + const Store = gl.issueBoards.BoardsStore; + const ModalStore = gl.issueBoards.ModalStore; + + window.gl = window.gl || {}; + + if (gl.IssueBoardsApp) { + gl.IssueBoardsApp.$destroy(true); + } + + Store.create(); + + // hack to allow sidebar scripts like milestone_select manipulate the BoardsStore + gl.issueBoards.boardStoreIssueSet = (...args) => Vue.set(Store.detail.issue, ...args); + gl.issueBoards.boardStoreIssueDelete = (...args) => Vue.delete(Store.detail.issue, ...args); + + gl.IssueBoardsApp = new Vue({ + el: $boardApp, + components: { + 'board': gl.issueBoards.Board, + 'board-sidebar': gl.issueBoards.BoardSidebar, + 'board-add-issues-modal': gl.issueBoards.IssuesModal, + }, + data: { + state: Store.state, + loading: true, + boardsEndpoint: $boardApp.dataset.boardsEndpoint, + listsEndpoint: $boardApp.dataset.listsEndpoint, + boardId: $boardApp.dataset.boardId, + disabled: $boardApp.dataset.disabled === 'true', + issueLinkBase: $boardApp.dataset.issueLinkBase, + rootPath: $boardApp.dataset.rootPath, + bulkUpdatePath: $boardApp.dataset.bulkUpdatePath, + detailIssue: Store.detail, + defaultAvatar: $boardApp.dataset.defaultAvatar, + }, + computed: { + detailIssueVisible () { + return Object.keys(this.detailIssue.issue).length; + }, + }, + created () { + gl.boardService = new BoardService({ + boardsEndpoint: this.boardsEndpoint, + listsEndpoint: this.listsEndpoint, + bulkUpdatePath: this.bulkUpdatePath, + boardId: this.boardId, + }); + Store.rootPath = this.boardsEndpoint; + + eventHub.$on('updateTokens', this.updateTokens); + eventHub.$on('newDetailIssue', this.updateDetailIssue); + eventHub.$on('clearDetailIssue', this.clearDetailIssue); + sidebarEventHub.$on('toggleSubscription', this.toggleSubscription); + }, + beforeDestroy() { + eventHub.$off('updateTokens', this.updateTokens); + eventHub.$off('newDetailIssue', this.updateDetailIssue); + eventHub.$off('clearDetailIssue', this.clearDetailIssue); + sidebarEventHub.$off('toggleSubscription', this.toggleSubscription); + }, + mounted () { + this.filterManager = new FilteredSearchBoards(Store.filter, true); + this.filterManager.setup(); + + Store.disabled = this.disabled; + gl.boardService.all() + .then(res => res.data) + .then((data) => { + data.forEach((board) => { + const list = Store.addList(board, this.defaultAvatar); + + if (list.type === 'closed') { + list.position = Infinity; + } else if (list.type === 'backlog') { + list.position = -1; + } + }); + + this.state.lists = _.sortBy(this.state.lists, 'position'); + + Store.addBlankState(); + this.loading = false; + }) + .catch(() => { + Flash('An error occurred while fetching the board lists. Please try again.'); + }); + }, + methods: { + updateTokens() { + this.filterManager.updateTokens(); + }, + updateDetailIssue(newIssue) { + const sidebarInfoEndpoint = newIssue.sidebarInfoEndpoint; + if (sidebarInfoEndpoint && newIssue.subscribed === undefined) { + newIssue.setFetchingState('subscriptions', true); + BoardService.getIssueInfo(sidebarInfoEndpoint) + .then(res => res.data) + .then((data) => { + newIssue.setFetchingState('subscriptions', false); + newIssue.updateData({ + subscribed: data.subscribed, + }); + }) + .catch(() => { + newIssue.setFetchingState('subscriptions', false); + Flash(__('An error occurred while fetching sidebar data')); + }); + } + + Store.detail.issue = newIssue; + }, + clearDetailIssue() { + Store.detail.issue = {}; + }, + toggleSubscription(id) { + const issue = Store.detail.issue; + if (issue.id === id && issue.toggleSubscriptionEndpoint) { + issue.setFetchingState('subscriptions', true); + BoardService.toggleIssueSubscription(issue.toggleSubscriptionEndpoint) + .then(() => { + issue.setFetchingState('subscriptions', false); + issue.updateData({ + subscribed: !issue.subscribed, + }); + }) + .catch(() => { + issue.setFetchingState('subscriptions', false); + Flash(__('An error occurred when toggling the notification subscription')); + }); + } + } + }, + }); + + gl.IssueBoardsSearch = new Vue({ + el: document.getElementById('js-add-list'), + data: { + filters: Store.state.filters, + }, + mounted () { + gl.issueBoards.newListDropdownInit(); + }, + }); + + gl.IssueBoardsModalAddBtn = new Vue({ + el: document.getElementById('js-add-issues-btn'), + mixins: [gl.issueBoards.ModalMixins], + data() { + return { + modal: ModalStore.store, + store: Store.state, + }; + }, + computed: { + disabled() { + if (!this.store) { + return true; + } + return !this.store.lists.filter(list => !list.preset).length; + }, + tooltipTitle() { + if (this.disabled) { + return 'Please add a list to your board first'; + } + + return ''; + }, + }, + watch: { + disabled() { + this.updateTooltip(); + }, + }, + mounted() { + this.updateTooltip(); + }, + methods: { + updateTooltip() { + const $tooltip = $(this.$refs.addIssuesButton); + + this.$nextTick(() => { + if (this.disabled) { + $tooltip.tooltip(); + } else { + $tooltip.tooltip('destroy'); + } + }); + }, + openModal() { + if (!this.disabled) { + this.toggleModal(true); + } + }, + }, + template: ` +
+ +
+ `, + }); +} diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index f8082c74943..872ddcbbbe6 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -180,6 +180,11 @@ var Dispatcher; .then(callDefault) .catch(fail); break; + case 'projects:boards:index': + import('./pages/projects/boards') + .then(callDefault) + .catch(fail); + break; case 'projects:issues:new': import('./pages/projects/issues/new') .then(callDefault) diff --git a/app/assets/javascripts/pages/projects/boards/index.js b/app/assets/javascripts/pages/projects/boards/index.js index 3aeeedbb45d..0bda9fcc421 100644 --- a/app/assets/javascripts/pages/projects/boards/index.js +++ b/app/assets/javascripts/pages/projects/boards/index.js @@ -1,7 +1,9 @@ import UsersSelect from '~/users_select'; import ShortcutsNavigation from '~/shortcuts_navigation'; +import initBoards from '~/boards'; -document.addEventListener('DOMContentLoaded', () => { +export default () => { new UsersSelect(); // eslint-disable-line no-new new ShortcutsNavigation(); // eslint-disable-line no-new -}); + initBoards(); +}; diff --git a/app/views/shared/boards/_show.html.haml b/app/views/shared/boards/_show.html.haml index ee8ad8e3999..de470730f5e 100644 --- a/app/views/shared/boards/_show.html.haml +++ b/app/views/shared/boards/_show.html.haml @@ -7,7 +7,6 @@ - content_for :page_specific_javascripts do = webpack_bundle_tag 'common_vue' = webpack_bundle_tag 'filtered_search' - = webpack_bundle_tag 'boards' %script#js-board-template{ type: "text/x-template" }= render "shared/boards/components/board" %script#js-board-modal-filter{ type: "text/x-template" }= render "shared/issuable/search_bar", type: :boards_modal diff --git a/config/webpack.config.js b/config/webpack.config.js index a4e6c64fce5..ba7003f3673 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -51,7 +51,6 @@ var config = { account: './profile/account/index.js', balsamiq_viewer: './blob/balsamiq_viewer.js', blob: './blob_edit/blob_bundle.js', - boards: './boards/boards_bundle.js', common: './commons/index.js', common_vue: './vue_shared/vue_resource_interceptor.js', cycle_analytics: './cycle_analytics/cycle_analytics_bundle.js', diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 9b2a5379855..96671041cd2 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -113,7 +113,7 @@ if (process.env.BABEL_ENV === 'coverage') { // exempt these files from the coverage report const troubleMakers = [ './blob_edit/blob_bundle.js', - './boards/boards_bundle.js', + './boards/index.js', './cycle_analytics/cycle_analytics_bundle.js', './cycle_analytics/components/stage_plan_component.js', './cycle_analytics/components/stage_staging_component.js', -- cgit v1.2.1 From 7282dbea094e5dffa3af29f00e183d063a3717b1 Mon Sep 17 00:00:00 2001 From: Constance Okoghenun Date: Fri, 16 Feb 2018 01:17:23 +0100 Subject: Fixed failing test --- app/assets/javascripts/dispatcher.js | 1 + spec/javascripts/test_bundle.js | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 872ddcbbbe6..99ac424c433 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -180,6 +180,7 @@ var Dispatcher; .then(callDefault) .catch(fail); break; + case 'projects:boards:show': case 'projects:boards:index': import('./pages/projects/boards') .then(callDefault) diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 96671041cd2..9d2dee990fd 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -113,7 +113,6 @@ if (process.env.BABEL_ENV === 'coverage') { // exempt these files from the coverage report const troubleMakers = [ './blob_edit/blob_bundle.js', - './boards/index.js', './cycle_analytics/cycle_analytics_bundle.js', './cycle_analytics/components/stage_plan_component.js', './cycle_analytics/components/stage_staging_component.js', -- cgit v1.2.1 From b284ce0428d855600899764a1c9bf0a7f3e88277 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Fri, 16 Feb 2018 11:47:42 +1100 Subject: Fix rubocop warning (#42643) --- spec/serializers/cluster_application_entity_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/serializers/cluster_application_entity_spec.rb b/spec/serializers/cluster_application_entity_spec.rb index 70b274083b2..852b6af9f7f 100644 --- a/spec/serializers/cluster_application_entity_spec.rb +++ b/spec/serializers/cluster_application_entity_spec.rb @@ -32,7 +32,7 @@ describe ClusterApplicationEntity do build( :clusters_applications_ingress, :installed, - external_ip: '111.222.111.222', + external_ip: '111.222.111.222' ) end -- cgit v1.2.1 From dfef5437a26320fc4a55fc857b6e59d5e92c13c3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 16 Feb 2018 12:28:34 +0100 Subject: Use a helper to schedule pipeline stages migration --- .../20180212101928_schedule_build_stage_migration.rb | 11 ++++++----- lib/gitlab/background_migration/migrate_build_stage.rb | 2 -- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/db/post_migrate/20180212101928_schedule_build_stage_migration.rb b/db/post_migrate/20180212101928_schedule_build_stage_migration.rb index ad65a6fb16e..df15b2cd9d4 100644 --- a/db/post_migrate/20180212101928_schedule_build_stage_migration.rb +++ b/db/post_migrate/20180212101928_schedule_build_stage_migration.rb @@ -3,7 +3,7 @@ class ScheduleBuildStageMigration < ActiveRecord::Migration DOWNTIME = false MIGRATION = 'MigrateBuildStage'.freeze - BATCH_SIZE = 800 + BATCH_SIZE = 500 disable_ddl_transaction! @@ -15,10 +15,11 @@ class ScheduleBuildStageMigration < ActiveRecord::Migration def up disable_statement_timeout - Build.where('stage_id IS NULL').each_batch(of: BATCH_SIZE) do |builds, index| - builds.pluck('MIN(id)', 'MAX(id)').first.tap do |range| - BackgroundMigrationWorker.perform_in(index * 5.minutes, MIGRATION, range) - end + Build.where('stage_id IS NULL').tap do |relation| + queue_background_migration_jobs_by_range_at_intervals(relation, + MIGRATION, + 5.minutes, + batch_size: BATCH_SIZE) end end diff --git a/lib/gitlab/background_migration/migrate_build_stage.rb b/lib/gitlab/background_migration/migrate_build_stage.rb index adca16751af..8fe4f1a2289 100644 --- a/lib/gitlab/background_migration/migrate_build_stage.rb +++ b/lib/gitlab/background_migration/migrate_build_stage.rb @@ -35,8 +35,6 @@ module Gitlab end def perform(start_id, stop_id) - # TODO, statement timeout? - stages = Migratable::Build.where('stage_id IS NULL') .where('id BETWEEN ? AND ?', start_id, stop_id) .map { |build| build.ensure_stage! } -- cgit v1.2.1 From 4efcc0618ccf147db301f4afe50fb800fcf490c7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 16 Feb 2018 13:13:23 +0100 Subject: Add pipeline expression statement class --- lib/gitlab/ci/pipeline/expression/statement.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 lib/gitlab/ci/pipeline/expression/statement.rb diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb new file mode 100644 index 00000000000..adc36896fd3 --- /dev/null +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -0,0 +1,20 @@ +module Gitlab + module Ci + module Pipeline + module Expression + class Statement + def initialize(pipeline, statement) + @pipeline = pipeline + @statement = statement + end + + def errors + end + + def matches? + end + end + end + end + end +end -- cgit v1.2.1 From f0b27f9b406579a03e55fa16cbc7095009dc8c2b Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 19 Feb 2018 14:03:41 +0000 Subject: Adds support to render the IP address in the application ingress row Updates components to use a slot to allow to reuse the clipboard button Adds tests --- .../clusters/components/application_row.vue | 11 +- .../clusters/components/applications.vue | 123 +++++++++++++++++---- .../vue_shared/components/clipboard_button.vue | 7 +- ...rsist-external-ip-of-ingress-controller-gke.yml | 5 + .../clusters/components/applications_spec.js | 70 ++++++++++++ 5 files changed, 189 insertions(+), 27 deletions(-) create mode 100644 changelogs/unreleased/42643-persist-external-ip-of-ingress-controller-gke.yml diff --git a/app/assets/javascripts/clusters/components/application_row.vue b/app/assets/javascripts/clusters/components/application_row.vue index 50e35bbbba5..428a762a9c8 100644 --- a/app/assets/javascripts/clusters/components/application_row.vue +++ b/app/assets/javascripts/clusters/components/application_row.vue @@ -36,10 +36,6 @@ type: String, required: false, }, - description: { - type: String, - required: true, - }, status: { type: String, required: false, @@ -148,11 +144,14 @@ class="table-section section-wrap" role="gridcell" > -
+
- ${descriptionParagraph} -

${extraCostParagraph}

@@ -136,33 +129,123 @@ id="helm" :title="applications.helm.title" title-link="https://docs.helm.sh/" - :description="helmTillerDescription" :status="applications.helm.status" :status-reason="applications.helm.statusReason" :request-status="applications.helm.requestStatus" :request-reason="applications.helm.requestReason" - /> + > +
+ {{ s__( + `ClusterIntegration|Helm streamlines installing and managing Kubernetes applications. + Tiller runs inside of your Kubernetes Cluster, and manages releases of your charts.`, + ) }} +
+ + > +
+

+ {{ s__( + `ClusterIntegration|Ingress gives you a way to route + requests to services based on the request host or path, + centralizing a number of services into a single entrypoint.`, + ) }} +

+ + + +
+ + > +
+
+ ### Summary -- cgit v1.2.1 From 4be20ba923c010c105e30ef8bba3b9bf82b2d0ca Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 22 Feb 2018 10:51:00 -0800 Subject: Respond 404 when repo does not exist --- .../mk-fix-error-code-for-repo-does-not-exist.yml | 5 +++++ lib/gitlab/git_access.rb | 2 +- spec/lib/gitlab/git_access_spec.rb | 13 +++++++++++++ spec/lib/gitlab/git_access_wiki_spec.rb | 2 +- spec/requests/git_http_spec.rb | 6 +++--- 5 files changed, 23 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml diff --git a/changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml b/changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml new file mode 100644 index 00000000000..a761d610da1 --- /dev/null +++ b/changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml @@ -0,0 +1,5 @@ +--- +title: Return a 404 instead of 403 if the repository does not exist on disk +merge_request: 17341 +author: +type: fixed diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index bbdb593d4e2..6400089a22f 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -199,7 +199,7 @@ module Gitlab def check_repository_existence! unless repository.exists? - raise UnauthorizedError, ERROR_MESSAGES[:no_repo] + raise NotFoundError, ERROR_MESSAGES[:no_repo] end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 19d3f55501e..6f07e423c1b 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -534,6 +534,19 @@ describe Gitlab::GitAccess do expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.') end + context 'when the project repository does not exist' do + it 'returns not found' do + project.add_guest(user) + repo = project.repository + FileUtils.rm_rf(repo.path) + + # Sanity check for rm_rf + expect(repo.exists?).to eq(false) + + expect { pull_access_check }.to raise_error(Gitlab::GitAccess::NotFoundError, 'A repository for this project does not exist yet.') + end + end + describe 'without access to project' do context 'pull code' do it { expect { pull_access_check }.to raise_not_found } diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 215f1ecc9c5..730ede99fc9 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -57,7 +57,7 @@ describe Gitlab::GitAccessWiki do # Sanity check for rm_rf expect(wiki_repo.exists?).to eq(false) - expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'A repository for this project does not exist yet.') + expect { subject }.to raise_error(Gitlab::GitAccess::NotFoundError, 'A repository for this project does not exist yet.') end end end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index c6fdda203ad..1b0a5eac9b0 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -597,7 +597,7 @@ describe 'Git HTTP requests' do context "when a gitlab ci token is provided" do let(:project) { create(:project, :repository) } let(:build) { create(:ci_build, :running) } - let(:other_project) { create(:project) } + let(:other_project) { create(:project, :repository) } before do build.update!(project: project) # can't associate it on factory create @@ -648,10 +648,10 @@ describe 'Git HTTP requests' do context 'when the repo does not exist' do let(:project) { create(:project) } - it 'rejects pulls with 403 Forbidden' do + it 'rejects pulls with 404 Not Found' do clone_get path, env - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) expect(response.body).to eq(git_access_error(:no_repo)) end end -- cgit v1.2.1 From 20897ebc7afe498c7a521dbe9f9c197ee63d346b Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Sun, 25 Feb 2018 12:45:26 +1100 Subject: Increase lease timeout for CheckIngressIpAddressService to reduce polling frequency (#42643) --- app/services/clusters/applications/check_ingress_ip_address_service.rb | 2 +- .../clusters/applications/check_ingress_ip_address_service_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/clusters/applications/check_ingress_ip_address_service.rb b/app/services/clusters/applications/check_ingress_ip_address_service.rb index a98e05b25cb..e572b1e5d99 100644 --- a/app/services/clusters/applications/check_ingress_ip_address_service.rb +++ b/app/services/clusters/applications/check_ingress_ip_address_service.rb @@ -5,7 +5,7 @@ module Clusters Error = Class.new(StandardError) - LEASE_TIMEOUT = 3.seconds.to_i + LEASE_TIMEOUT = 15.seconds.to_i def execute return if app.external_ip diff --git a/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb b/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb index e14c82d32d4..bf038595a4d 100644 --- a/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb +++ b/spec/services/clusters/applications/check_ingress_ip_address_service_spec.rb @@ -25,7 +25,7 @@ describe Clusters::Applications::CheckIngressIpAddressService do allow(application.cluster).to receive(:kubeclient).and_return(kubeclient) allow(Gitlab::ExclusiveLease) .to receive(:new) - .with("check_ingress_ip_address_service:#{application.id}", timeout: 3.seconds.to_i) + .with("check_ingress_ip_address_service:#{application.id}", timeout: 15.seconds.to_i) .and_return(exclusive_lease) end -- cgit v1.2.1 From 233cf07083faabcc11b2fa8de04123156d712efb Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Sun, 25 Feb 2018 12:46:16 +1100 Subject: Remove redundant code and increase delay for ClusterWaitForIngressIpAddressWorker (#42643) --- app/models/clusters/applications/ingress.rb | 7 +++---- app/workers/cluster_wait_for_ingress_ip_address_worker.rb | 4 +--- spec/models/clusters/applications/ingress_spec.rb | 4 ++-- spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index 57ced0f5c44..9f583342c19 100644 --- a/app/models/clusters/applications/ingress.rb +++ b/app/models/clusters/applications/ingress.rb @@ -14,13 +14,13 @@ module Clusters nginx: 1 } - IP_ADDRESS_FETCH_RETRIES = 3 + FETCH_IP_ADDRESS_DELAY = 30.seconds state_machine :status do before_transition any => [:installed] do |application| application.run_after_commit do ClusterWaitForIngressIpAddressWorker.perform_in( - ClusterWaitForIngressIpAddressWorker::INTERVAL, application.name, application.id, IP_ADDRESS_FETCH_RETRIES) + FETCH_IP_ADDRESS_DELAY, application.name, application.id) end end end @@ -41,8 +41,7 @@ module Clusters return unless installed? return if external_ip - ClusterWaitForIngressIpAddressWorker.perform_async( - name, id, IP_ADDRESS_FETCH_RETRIES) + ClusterWaitForIngressIpAddressWorker.perform_async(name, id) end end end diff --git a/app/workers/cluster_wait_for_ingress_ip_address_worker.rb b/app/workers/cluster_wait_for_ingress_ip_address_worker.rb index 72b3c8b49e0..8ba5951750c 100644 --- a/app/workers/cluster_wait_for_ingress_ip_address_worker.rb +++ b/app/workers/cluster_wait_for_ingress_ip_address_worker.rb @@ -3,9 +3,7 @@ class ClusterWaitForIngressIpAddressWorker include ClusterQueue include ClusterApplications - INTERVAL = 30.seconds - - def perform(app_name, app_id, retries_remaining) + def perform(app_name, app_id) find_application(app_name, app_id) do |app| Clusters::Applications::CheckIngressIpAddressService.new(app).execute end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index 80c450c30cb..a34f4ff2b48 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -20,7 +20,7 @@ describe Clusters::Applications::Ingress do it 'schedules a ClusterWaitForIngressIpAddressWorker' do expect(ClusterWaitForIngressIpAddressWorker).to have_received(:perform_in) - .with(ClusterWaitForIngressIpAddressWorker::INTERVAL, 'ingress', application.id, 3) + .with(Clusters::Applications::Ingress::FETCH_IP_ADDRESS_DELAY, 'ingress', application.id) end end @@ -33,7 +33,7 @@ describe Clusters::Applications::Ingress do it 'schedules a ClusterWaitForIngressIpAddressWorker' do expect(ClusterWaitForIngressIpAddressWorker).to have_received(:perform_async) - .with('ingress', application.id, 3) + .with('ingress', application.id) end context 'when the application is not installed' do diff --git a/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb b/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb index baa295984bf..2e2e9afd25a 100644 --- a/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb +++ b/spec/workers/cluster_wait_for_ingress_ip_address_worker_spec.rb @@ -22,7 +22,7 @@ describe ClusterWaitForIngressIpAddressWorker do end it 'finds the application and calls CheckIngressIpAddressService#execute' do - worker.perform('ingress', 117, 2) + worker.perform('ingress', 117) expect(service).to have_received(:execute) end -- cgit v1.2.1 From e1de8a77d5d9749eaa596bfa15e6d11f1828db95 Mon Sep 17 00:00:00 2001 From: Ian Baum Date: Sun, 25 Feb 2018 11:42:19 -0600 Subject: Update CHANGELOG.md for 10.5.2 [ci skip] --- CHANGELOG.md | 21 +++++++++++++++++++++ changelogs/unreleased/34130-null-pipes.yml | 5 ----- .../41461-project-members-slow-due-to-sql.yml | 5 ----- ...legacy-authorization-for-new-clusters-on-gke.yml | 5 ----- .../unreleased/42877-snippets-dashboard-slow.yml | 5 ----- .../unreleased/43373-fix-cache-index-appending.yml | 5 ----- .../unreleased/fix-500-for-invalid-upload-path.yml | 5 ----- changelogs/unreleased/flipper-caching.yml | 5 ----- .../unreleased/jej-fix-slow-lfs-object-check.yml | 5 ----- .../kp-fix-stacked-bar-progress-value-clipping.yml | 5 ----- .../unreleased/sh-guard-read-only-user-updates.yml | 5 ----- changelogs/unreleased/users-autocomplete.yml | 5 ----- .../unreleased/zj-branch-contains-git-message.yml | 5 ----- 13 files changed, 21 insertions(+), 60 deletions(-) delete mode 100644 changelogs/unreleased/34130-null-pipes.yml delete mode 100644 changelogs/unreleased/41461-project-members-slow-due-to-sql.yml delete mode 100644 changelogs/unreleased/41619-turn-on-legacy-authorization-for-new-clusters-on-gke.yml delete mode 100644 changelogs/unreleased/42877-snippets-dashboard-slow.yml delete mode 100644 changelogs/unreleased/43373-fix-cache-index-appending.yml delete mode 100644 changelogs/unreleased/fix-500-for-invalid-upload-path.yml delete mode 100644 changelogs/unreleased/flipper-caching.yml delete mode 100644 changelogs/unreleased/jej-fix-slow-lfs-object-check.yml delete mode 100644 changelogs/unreleased/kp-fix-stacked-bar-progress-value-clipping.yml delete mode 100644 changelogs/unreleased/sh-guard-read-only-user-updates.yml delete mode 100644 changelogs/unreleased/users-autocomplete.yml delete mode 100644 changelogs/unreleased/zj-branch-contains-git-message.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 869884f8ca6..c8d399b2b98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,27 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 10.5.2 (2018-02-25) + +### Fixed (7 changes) + +- Fix single digit value clipping for stacked progress bar. !17217 +- Fix issue with cache key being empty when variable used as the key. !17260 +- Enable Legacy Authorization by default on Cluster creations. !17302 +- Allow branch names to be named the same as the sha it points to. +- Fix 500 error when loading an invalid upload URL. +- Don't attempt to update user tracked fields if database is in read-only. +- Prevent MR Widget error when no CI configured. + +### Performance (5 changes) + +- Improve query performance for snippets dashboard. !17088 +- Only check LFS integrity for first ref in a push to avoid timeout. !17098 +- Improve query performance of MembersFinder. !17190 +- Increase feature flag cache TTL to one hour. +- Improve performance of searching for and autocompleting of users. + + ## 10.5.1 (2018-02-22) - No changes. diff --git a/changelogs/unreleased/34130-null-pipes.yml b/changelogs/unreleased/34130-null-pipes.yml deleted file mode 100644 index a56e5cf8db2..00000000000 --- a/changelogs/unreleased/34130-null-pipes.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Prevent MR Widget error when no CI configured -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/41461-project-members-slow-due-to-sql.yml b/changelogs/unreleased/41461-project-members-slow-due-to-sql.yml deleted file mode 100644 index 27eee7d943b..00000000000 --- a/changelogs/unreleased/41461-project-members-slow-due-to-sql.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improve query performance of MembersFinder. -merge_request: 17190 -author: -type: performance diff --git a/changelogs/unreleased/41619-turn-on-legacy-authorization-for-new-clusters-on-gke.yml b/changelogs/unreleased/41619-turn-on-legacy-authorization-for-new-clusters-on-gke.yml deleted file mode 100644 index 507367c98c4..00000000000 --- a/changelogs/unreleased/41619-turn-on-legacy-authorization-for-new-clusters-on-gke.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Enable Legacy Authorization by default on Cluster creations -merge_request: 17302 -author: -type: fixed diff --git a/changelogs/unreleased/42877-snippets-dashboard-slow.yml b/changelogs/unreleased/42877-snippets-dashboard-slow.yml deleted file mode 100644 index 839b44ad272..00000000000 --- a/changelogs/unreleased/42877-snippets-dashboard-slow.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improve query performance for snippets dashboard. -merge_request: 17088 -author: -type: performance diff --git a/changelogs/unreleased/43373-fix-cache-index-appending.yml b/changelogs/unreleased/43373-fix-cache-index-appending.yml deleted file mode 100644 index fdb293ea04d..00000000000 --- a/changelogs/unreleased/43373-fix-cache-index-appending.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix issue with cache key being empty when variable used as the key -merge_request: 17260 -author: -type: fixed diff --git a/changelogs/unreleased/fix-500-for-invalid-upload-path.yml b/changelogs/unreleased/fix-500-for-invalid-upload-path.yml deleted file mode 100644 index a4ce00c64c4..00000000000 --- a/changelogs/unreleased/fix-500-for-invalid-upload-path.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix 500 error when loading an invalid upload URL -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/flipper-caching.yml b/changelogs/unreleased/flipper-caching.yml deleted file mode 100644 index 6db27fd579e..00000000000 --- a/changelogs/unreleased/flipper-caching.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Increase feature flag cache TTL to one hour -merge_request: -author: -type: performance diff --git a/changelogs/unreleased/jej-fix-slow-lfs-object-check.yml b/changelogs/unreleased/jej-fix-slow-lfs-object-check.yml deleted file mode 100644 index 09112fba85e..00000000000 --- a/changelogs/unreleased/jej-fix-slow-lfs-object-check.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Only check LFS integrity for first ref in a push to avoid timeout -merge_request: 17098 -author: -type: performance diff --git a/changelogs/unreleased/kp-fix-stacked-bar-progress-value-clipping.yml b/changelogs/unreleased/kp-fix-stacked-bar-progress-value-clipping.yml deleted file mode 100644 index 690536a533b..00000000000 --- a/changelogs/unreleased/kp-fix-stacked-bar-progress-value-clipping.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix single digit value clipping for stacked progress bar -merge_request: 17217 -author: -type: fixed diff --git a/changelogs/unreleased/sh-guard-read-only-user-updates.yml b/changelogs/unreleased/sh-guard-read-only-user-updates.yml deleted file mode 100644 index b8dbd840ed9..00000000000 --- a/changelogs/unreleased/sh-guard-read-only-user-updates.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Don't attempt to update user tracked fields if database is in read-only -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/users-autocomplete.yml b/changelogs/unreleased/users-autocomplete.yml deleted file mode 100644 index 2cb078a3a7c..00000000000 --- a/changelogs/unreleased/users-autocomplete.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improve performance of searching for and autocompleting of users -merge_request: -author: -type: performance diff --git a/changelogs/unreleased/zj-branch-contains-git-message.yml b/changelogs/unreleased/zj-branch-contains-git-message.yml deleted file mode 100644 index ce034e7ec87..00000000000 --- a/changelogs/unreleased/zj-branch-contains-git-message.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Allow branch names to be named the same as the sha it points to -merge_request: -author: -type: fixed -- cgit v1.2.1 From 936280934ba820846c646f3b10ef2dc533c7a76e Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Mon, 26 Feb 2018 12:42:55 +0530 Subject: Use imported `DropdownUtils` --- app/assets/javascripts/labels_select.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 5de48aa49a9..7151ac05a09 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -213,7 +213,7 @@ export default class LabelsSelect { } } if (label.duplicate) { - color = gl.DropdownUtils.duplicateLabelColor(label.color); + color = DropdownUtils.duplicateLabelColor(label.color); } else { if (label.color != null) { -- cgit v1.2.1 From c0145298b3f65151da3bb169c00b631fb8fd7040 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Mon, 26 Feb 2018 12:49:24 +0530 Subject: Add changelog entry --- changelogs/unreleased/43598-fix-duplicate-label-load-failure.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/43598-fix-duplicate-label-load-failure.yml diff --git a/changelogs/unreleased/43598-fix-duplicate-label-load-failure.yml b/changelogs/unreleased/43598-fix-duplicate-label-load-failure.yml new file mode 100644 index 00000000000..bda4ec84e5c --- /dev/null +++ b/changelogs/unreleased/43598-fix-duplicate-label-load-failure.yml @@ -0,0 +1,5 @@ +--- +title: Fix Group labels load failure when there are duplicate labels present +merge_request: 17353 +author: +type: fixed -- cgit v1.2.1 From e351f67874fb98d1d40bd32dca8d9a44a4fc8582 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Fri, 23 Feb 2018 14:25:14 +0000 Subject: Suppress whitespace warnings in squash error messages These are obscuring the real error, which is confusing for everyone. --- lib/gitlab/git/repository.rb | 2 +- spec/lib/gitlab/git/repository_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index e3cbf017e55..ddb9cf433eb 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -2206,7 +2206,7 @@ module Gitlab with_worktree(squash_path, branch, sparse_checkout_files: diff_files, env: env) do # Apply diff of the `diff_range` to the worktree diff = run_git!(%W(diff --binary #{diff_range})) - run_git!(%w(apply --index), chdir: squash_path, env: env) do |stdin| + run_git!(%w(apply --index --whitespace=nowarn), chdir: squash_path, env: env) do |stdin| stdin.binmode stdin.write(diff) end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index d601a383a98..13358995383 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2283,6 +2283,20 @@ describe Gitlab::Git::Repository, seed_helper: true do expect(subject).to match(/\h{40}/) end end + + context 'with trailing whitespace in an invalid patch', :skip_gitaly_mock do + let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+ \n ====== \n \n Sample repo for testing gitlab features\n" } + + it 'does not include whitespace warnings in the error' do + allow(repository).to receive(:run_git!).and_call_original + allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT')) + + expect { subject }.to raise_error do |error| + expect(error).to be_a(described_class::GitError) + expect(error.message).not_to include('trailing whitespace') + end + end + end end end -- cgit v1.2.1 From 22addf26732e15bd63112c21f03a7f20c5feabc9 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Fri, 23 Feb 2018 15:45:32 +0100 Subject: Don't convert issuable_initial_data into JSON Instead of converting hash into JSON inside issuable_initial_data method, return hash and convert to JSON later. This allows us to easily extend basic issuable data with resource specific values. For example for Epic these data should include also labels, so we can then do something like: issuable_initial_data(@epic).merge(labels: @epic.labels).to_json --- app/helpers/issuables_helper.rb | 2 +- app/views/projects/issues/show.html.haml | 2 +- spec/helpers/issuables_helper_spec.rb | 32 ++++++++++++++++---------------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 7cd84fe69c9..44ecc2212f2 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -234,7 +234,7 @@ module IssuablesHelper data.merge!(updated_at_by(issuable)) - data.to_json + data end def updated_at_by(issuable) diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 7bc5c46d64a..d63443c9da5 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -55,7 +55,7 @@ .issue-details.issuable-details .detail-page-description.content-block - %script#js-issuable-app-initial-data{ type: "application/json" }= issuable_initial_data(@issue) + %script#js-issuable-app-initial-data{ type: "application/json" }= issuable_initial_data(@issue).to_json #js-issuable-app %h2.title= markdown_field(@issue, :title) - if @issue.description.present? diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 7fa665aecdc..2fecd1a3d27 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -173,23 +173,23 @@ describe IssuablesHelper do @project = issue.project expected_data = { - 'endpoint' => "/#{@project.full_path}/issues/#{issue.iid}", - 'updateEndpoint' => "/#{@project.full_path}/issues/#{issue.iid}.json", - 'canUpdate' => true, - 'canDestroy' => true, - 'issuableRef' => "##{issue.iid}", - 'markdownPreviewPath' => "/#{@project.full_path}/preview_markdown", - 'markdownDocsPath' => '/help/user/markdown', - 'issuableTemplates' => [], - 'projectPath' => @project.path, - 'projectNamespace' => @project.namespace.path, - 'initialTitleHtml' => issue.title, - 'initialTitleText' => issue.title, - 'initialDescriptionHtml' => '

issue text

', - 'initialDescriptionText' => 'issue text', - 'initialTaskStatus' => '0 of 0 tasks completed' + endpoint: "/#{@project.full_path}/issues/#{issue.iid}", + updateEndpoint: "/#{@project.full_path}/issues/#{issue.iid}.json", + canUpdate: true, + canDestroy: true, + issuableRef: "##{issue.iid}", + markdownPreviewPath: "/#{@project.full_path}/preview_markdown", + markdownDocsPath: '/help/user/markdown', + issuableTemplates: [], + projectPath: @project.path, + projectNamespace: @project.namespace.path, + initialTitleHtml: issue.title, + initialTitleText: issue.title, + initialDescriptionHtml: '

issue text

', + initialDescriptionText: 'issue text', + initialTaskStatus: '0 of 0 tasks completed' } - expect(JSON.parse(helper.issuable_initial_data(issue))).to eq(expected_data) + expect(helper.issuable_initial_data(issue)).to eq(expected_data) end end -- cgit v1.2.1 From 006753110a462e62f549cdf3c410e73eed068dbf Mon Sep 17 00:00:00 2001 From: "Jacob Vosmaer (GitLab)" Date: Mon, 26 Feb 2018 09:54:20 +0000 Subject: Restart Unicorn and Sidekiq when GRPC throws 14:Endpoint read failed --- changelogs/unreleased/grpc-unavailable-restart.yml | 5 + config/initializers/sidekiq.rb | 2 +- lib/gitlab/gitaly_client.rb | 23 ++++ lib/gitlab/sidekiq_middleware/memory_killer.rb | 67 ----------- lib/gitlab/sidekiq_middleware/shutdown.rb | 133 +++++++++++++++++++++ .../sidekiq_middleware/memory_killer_spec.rb | 63 ---------- .../lib/gitlab/sidekiq_middleware/shutdown_spec.rb | 88 ++++++++++++++ 7 files changed, 250 insertions(+), 131 deletions(-) create mode 100644 changelogs/unreleased/grpc-unavailable-restart.yml delete mode 100644 lib/gitlab/sidekiq_middleware/memory_killer.rb create mode 100644 lib/gitlab/sidekiq_middleware/shutdown.rb delete mode 100644 spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb create mode 100644 spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb diff --git a/changelogs/unreleased/grpc-unavailable-restart.yml b/changelogs/unreleased/grpc-unavailable-restart.yml new file mode 100644 index 00000000000..5ce08d66004 --- /dev/null +++ b/changelogs/unreleased/grpc-unavailable-restart.yml @@ -0,0 +1,5 @@ +--- +title: Restart Unicorn and Sidekiq when GRPC throws 14:Endpoint read failed +merge_request: 17293 +author: +type: fixed diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 0f164e628f9..161fb185c9b 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -10,7 +10,7 @@ Sidekiq.configure_server do |config| config.server_middleware do |chain| chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS'] - chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] + chain.add Gitlab::SidekiqMiddleware::Shutdown chain.add Gitlab::SidekiqMiddleware::RequestStoreMiddleware unless ENV['SIDEKIQ_REQUEST_STORE'] == '0' chain.add Gitlab::SidekiqStatus::ServerMiddleware end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index c5d3e944f7d..9cd76630484 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -125,6 +125,8 @@ module Gitlab kwargs = yield(kwargs) if block_given? stub(service, storage).__send__(rpc, request, kwargs) # rubocop:disable GitlabSecurity/PublicSend + rescue GRPC::Unavailable => ex + handle_grpc_unavailable!(ex) ensure duration = Gitlab::Metrics::System.monotonic_time - start @@ -135,6 +137,27 @@ module Gitlab duration) end + def self.handle_grpc_unavailable!(ex) + status = ex.to_status + raise ex unless status.details == 'Endpoint read failed' + + # There is a bug in grpc 1.8.x that causes a client process to get stuck + # always raising '14:Endpoint read failed'. The only thing that we can + # do to recover is to restart the process. + # + # See https://gitlab.com/gitlab-org/gitaly/issues/1029 + + if Sidekiq.server? + raise Gitlab::SidekiqMiddleware::Shutdown::WantShutdown.new(ex.to_s) + else + # SIGQUIT requests a Unicorn worker to shut down gracefully after the current request. + Process.kill('QUIT', Process.pid) + end + + raise ex + end + private_class_method :handle_grpc_unavailable! + def self.current_transaction_labels Gitlab::Metrics::Transaction.current&.labels || {} end diff --git a/lib/gitlab/sidekiq_middleware/memory_killer.rb b/lib/gitlab/sidekiq_middleware/memory_killer.rb deleted file mode 100644 index b89ae2505c9..00000000000 --- a/lib/gitlab/sidekiq_middleware/memory_killer.rb +++ /dev/null @@ -1,67 +0,0 @@ -module Gitlab - module SidekiqMiddleware - class MemoryKiller - # Default the RSS limit to 0, meaning the MemoryKiller is disabled - MAX_RSS = (ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] || 0).to_s.to_i - # Give Sidekiq 15 minutes of grace time after exceeding the RSS limit - GRACE_TIME = (ENV['SIDEKIQ_MEMORY_KILLER_GRACE_TIME'] || 15 * 60).to_s.to_i - # Wait 30 seconds for running jobs to finish during graceful shutdown - SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i - - # Create a mutex used to ensure there will be only one thread waiting to - # shut Sidekiq down - MUTEX = Mutex.new - - def call(worker, job, queue) - yield - - current_rss = get_rss - - return unless MAX_RSS > 0 && current_rss > MAX_RSS - - Thread.new do - # Return if another thread is already waiting to shut Sidekiq down - return unless MUTEX.try_lock - - Sidekiq.logger.warn "Sidekiq worker PID-#{pid} current RSS #{current_rss}"\ - " exceeds maximum RSS #{MAX_RSS} after finishing job #{worker.class} JID-#{job['jid']}" - Sidekiq.logger.warn "Sidekiq worker PID-#{pid} will stop fetching new jobs in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later" - - # Wait `GRACE_TIME` to give the memory intensive job time to finish. - # Then, tell Sidekiq to stop fetching new jobs. - wait_and_signal(GRACE_TIME, 'SIGSTP', 'stop fetching new jobs') - - # Wait `SHUTDOWN_WAIT` to give already fetched jobs time to finish. - # Then, tell Sidekiq to gracefully shut down by giving jobs a few more - # moments to finish, killing and requeuing them if they didn't, and - # then terminating itself. - wait_and_signal(SHUTDOWN_WAIT, 'SIGTERM', 'gracefully shut down') - - # Wait for Sidekiq to shutdown gracefully, and kill it if it didn't. - wait_and_signal(Sidekiq.options[:timeout] + 2, 'SIGKILL', 'die') - end - end - - private - - def get_rss - output, status = Gitlab::Popen.popen(%W(ps -o rss= -p #{pid}), Rails.root.to_s) - return 0 unless status.zero? - - output.to_i - end - - def wait_and_signal(time, signal, explanation) - Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" - sleep(time) - - Sidekiq.logger.warn "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" - Process.kill(signal, pid) - end - - def pid - Process.pid - end - end - end -end diff --git a/lib/gitlab/sidekiq_middleware/shutdown.rb b/lib/gitlab/sidekiq_middleware/shutdown.rb new file mode 100644 index 00000000000..c2b8d6de66e --- /dev/null +++ b/lib/gitlab/sidekiq_middleware/shutdown.rb @@ -0,0 +1,133 @@ +require 'mutex_m' + +module Gitlab + module SidekiqMiddleware + class Shutdown + extend Mutex_m + + # Default the RSS limit to 0, meaning the MemoryKiller is disabled + MAX_RSS = (ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] || 0).to_s.to_i + # Give Sidekiq 15 minutes of grace time after exceeding the RSS limit + GRACE_TIME = (ENV['SIDEKIQ_MEMORY_KILLER_GRACE_TIME'] || 15 * 60).to_s.to_i + # Wait 30 seconds for running jobs to finish during graceful shutdown + SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i + + # This exception can be used to request that the middleware start shutting down Sidekiq + WantShutdown = Class.new(StandardError) + + ShutdownWithoutRaise = Class.new(WantShutdown) + private_constant :ShutdownWithoutRaise + + # For testing only, to avoid race conditions (?) in Rspec mocks. + attr_reader :trace + + # We store the shutdown thread in a class variable to ensure that there + # can be only one shutdown thread in the process. + def self.create_shutdown_thread + mu_synchronize do + return unless @shutdown_thread.nil? + + @shutdown_thread = Thread.new { yield } + end + end + + # For testing only: so we can wait for the shutdown thread to finish. + def self.shutdown_thread + mu_synchronize { @shutdown_thread } + end + + # For testing only: so that we can reset the global state before each test. + def self.clear_shutdown_thread + mu_synchronize { @shutdown_thread = nil } + end + + def initialize + @trace = Queue.new if Rails.env.test? + end + + def call(worker, job, queue) + shutdown_exception = nil + + begin + yield + check_rss! + rescue WantShutdown => ex + shutdown_exception = ex + end + + return unless shutdown_exception + + self.class.create_shutdown_thread do + do_shutdown(worker, job, shutdown_exception) + end + + raise shutdown_exception unless shutdown_exception.is_a?(ShutdownWithoutRaise) + end + + private + + def do_shutdown(worker, job, shutdown_exception) + Sidekiq.logger.warn "Sidekiq worker PID-#{pid} shutting down because of #{shutdown_exception} after job "\ + "#{worker.class} JID-#{job['jid']}" + Sidekiq.logger.warn "Sidekiq worker PID-#{pid} will stop fetching new jobs in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later" + + # Wait `GRACE_TIME` to give the memory intensive job time to finish. + # Then, tell Sidekiq to stop fetching new jobs. + wait_and_signal(GRACE_TIME, 'SIGTSTP', 'stop fetching new jobs') + + # Wait `SHUTDOWN_WAIT` to give already fetched jobs time to finish. + # Then, tell Sidekiq to gracefully shut down by giving jobs a few more + # moments to finish, killing and requeuing them if they didn't, and + # then terminating itself. + wait_and_signal(SHUTDOWN_WAIT, 'SIGTERM', 'gracefully shut down') + + # Wait for Sidekiq to shutdown gracefully, and kill it if it didn't. + wait_and_signal(Sidekiq.options[:timeout] + 2, 'SIGKILL', 'die') + end + + def check_rss! + return unless MAX_RSS > 0 + + current_rss = get_rss + return unless current_rss > MAX_RSS + + raise ShutdownWithoutRaise.new("current RSS #{current_rss} exceeds maximum RSS #{MAX_RSS}") + end + + def get_rss + output, status = Gitlab::Popen.popen(%W(ps -o rss= -p #{pid}), Rails.root.to_s) + return 0 unless status.zero? + + output.to_i + end + + def wait_and_signal(time, signal, explanation) + Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" + sleep(time) + + Sidekiq.logger.warn "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" + kill(signal, pid) + end + + def pid + Process.pid + end + + def sleep(time) + if Rails.env.test? + @trace << [:sleep, time] + else + Kernel.sleep(time) + end + end + + def kill(signal, pid) + if Rails.env.test? + @trace << [:kill, signal, pid] + else + Process.kill(signal, pid) + end + end + end + end +end diff --git a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb deleted file mode 100644 index 8fdbbacd04d..00000000000 --- a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb +++ /dev/null @@ -1,63 +0,0 @@ -require 'spec_helper' - -describe Gitlab::SidekiqMiddleware::MemoryKiller do - subject { described_class.new } - let(:pid) { 999 } - - let(:worker) { double(:worker, class: 'TestWorker') } - let(:job) { { 'jid' => 123 } } - let(:queue) { 'test_queue' } - - def run - thread = subject.call(worker, job, queue) { nil } - thread&.join - end - - before do - allow(subject).to receive(:get_rss).and_return(10.kilobytes) - allow(subject).to receive(:pid).and_return(pid) - end - - context 'when MAX_RSS is set to 0' do - before do - stub_const("#{described_class}::MAX_RSS", 0) - end - - it 'does nothing' do - expect(subject).not_to receive(:sleep) - - run - end - end - - context 'when MAX_RSS is exceeded' do - before do - stub_const("#{described_class}::MAX_RSS", 5.kilobytes) - end - - it 'sends the STP, TERM and KILL signals at expected times' do - expect(subject).to receive(:sleep).with(15 * 60).ordered - expect(Process).to receive(:kill).with('SIGSTP', pid).ordered - - expect(subject).to receive(:sleep).with(30).ordered - expect(Process).to receive(:kill).with('SIGTERM', pid).ordered - - expect(subject).to receive(:sleep).with(10).ordered - expect(Process).to receive(:kill).with('SIGKILL', pid).ordered - - run - end - end - - context 'when MAX_RSS is not exceeded' do - before do - stub_const("#{described_class}::MAX_RSS", 15.kilobytes) - end - - it 'does nothing' do - expect(subject).not_to receive(:sleep) - - run - end - end -end diff --git a/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb b/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb new file mode 100644 index 00000000000..0001795c3f0 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +describe Gitlab::SidekiqMiddleware::Shutdown do + subject { described_class.new } + + let(:pid) { Process.pid } + let(:worker) { double(:worker, class: 'TestWorker') } + let(:job) { { 'jid' => 123 } } + let(:queue) { 'test_queue' } + let(:block) { proc { nil } } + + def run + subject.call(worker, job, queue) { block.call } + described_class.shutdown_thread&.join + end + + def pop_trace + subject.trace.pop(true) + end + + before do + allow(subject).to receive(:get_rss).and_return(10.kilobytes) + described_class.clear_shutdown_thread + end + + context 'when MAX_RSS is set to 0' do + before do + stub_const("#{described_class}::MAX_RSS", 0) + end + + it 'does nothing' do + expect(subject).not_to receive(:sleep) + + run + end + end + + def expect_shutdown_sequence + expect(pop_trace).to eq([:sleep, 15 * 60]) + expect(pop_trace).to eq([:kill, 'SIGTSTP', pid]) + + expect(pop_trace).to eq([:sleep, 30]) + expect(pop_trace).to eq([:kill, 'SIGTERM', pid]) + + expect(pop_trace).to eq([:sleep, 10]) + expect(pop_trace).to eq([:kill, 'SIGKILL', pid]) + end + + context 'when MAX_RSS is exceeded' do + before do + stub_const("#{described_class}::MAX_RSS", 5.kilobytes) + end + + it 'sends the TSTP, TERM and KILL signals at expected times' do + run + + expect_shutdown_sequence + end + end + + context 'when MAX_RSS is not exceeded' do + before do + stub_const("#{described_class}::MAX_RSS", 15.kilobytes) + end + + it 'does nothing' do + expect(subject).not_to receive(:sleep) + + run + end + end + + context 'when WantShutdown is raised' do + let(:block) { proc { raise described_class::WantShutdown } } + + it 'starts the shutdown sequence and re-raises the exception' do + expect { run }.to raise_exception(described_class::WantShutdown) + + # We can't expect 'run' to have joined on the shutdown thread, because + # it hit an exception. + shutdown_thread = described_class.shutdown_thread + expect(shutdown_thread).not_to be_nil + shutdown_thread.join + + expect_shutdown_sequence + end + end +end -- cgit v1.2.1 From 4497f829d82dbe4cd6775555a60f986929e0b510 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 14 Feb 2018 16:10:17 +0200 Subject: Make sure wiki exists when it's enabled --- app/services/projects/update_service.rb | 15 +++++++++++++++ spec/services/projects/update_service_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 0e235a6d2a0..379a8068023 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -15,6 +15,8 @@ module Projects return error("Could not set the default branch") unless project.change_head(params[:default_branch]) end + ensure_wiki_exists if enabling_wiki? + if project.update_attributes(params.except(:default_branch)) if project.previous_changes.include?('path') project.rename_repo @@ -52,5 +54,18 @@ module Projects project.repository.exists? && new_branch && new_branch != project.default_branch end + + def enabling_wiki? + return false if @project.wiki_enabled? + + params[:project_feature_attributes][:wiki_access_level].to_i > ProjectFeature::DISABLED + end + + def ensure_wiki_exists + ProjectWiki.new(project, project.owner).wiki + rescue ProjectWiki::CouldNotCreateWikiError + log_error("Could not create wiki for #{project.full_name}") + Gitlab::Metrics.counter(:wiki_can_not_be_created_total, 'Counts the times we failed to create a wiki') + end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index a0b97ceead9..da42bba6f10 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -123,6 +123,29 @@ describe Projects::UpdateService do end end + context 'when enabling a wiki' do + it 'creates a wiki' do + project.project_feature.update(wiki_access_level: ProjectFeature::DISABLED) + FileUtils.rm_rf(project.wiki.repository.path) + + result = update_project(project, user, project_feature_attributes: { wiki_access_level: ProjectFeature::ENABLED }) + + expect(result).to eq({ status: :success }) + expect(project.wiki_repository_exists?).to be true + expect(project.wiki_enabled?).to be true + end + + it 'logs an error and creates a metric when wiki can not be created' do + project.project_feature.update(wiki_access_level: ProjectFeature::DISABLED) + + expect_any_instance_of(ProjectWiki).to receive(:wiki).and_raise(ProjectWiki::CouldNotCreateWikiError) + expect_any_instance_of(described_class).to receive(:log_error).with("Could not create wiki for #{project.full_name}") + expect(Gitlab::Metrics).to receive(:counter) + + update_project(project, user, project_feature_attributes: { wiki_access_level: ProjectFeature::ENABLED }) + end + end + context 'when updating a project that contains container images' do before do stub_container_registry_config(enabled: true) -- cgit v1.2.1 From d74aa827c4cb9f327ab028497182341921e56fbf Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 26 Feb 2018 11:18:45 +0000 Subject: Add missing class to parent element and remove extra CSS --- .../projects/tree/components/commit_pipeline_status_component.vue | 2 +- app/assets/stylesheets/pages/commits.scss | 8 -------- app/views/projects/commits/_commit.html.haml | 2 +- changelogs/unreleased/43315-gpg-popover.yml | 2 +- 4 files changed, 3 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue b/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue index c53ce0549f5..63f20a0041d 100644 --- a/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue +++ b/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue @@ -98,7 +98,7 @@ };