Commit 2edc16b9 authored by Stan Hu's avatar Stan Hu

Merge branch 'sh-revert-prometheus-application-migration' into 'master'

Revert problematic background migration and clear it from the Sidekiq queue

See merge request gitlab-org/gitlab!24135
parents f9d97e06 b0ec92be
---
title: Clean backgroud_migration queue from ActivatePrometheusServicesForSharedCluster
jobs.
merge_request: 24135
author:
type: fixed
# frozen_string_literal: true
class DropActivatePrometheusServicesForSharedClusterApplicationsBackgroundMigration < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
DROPPED_JOB_CLASS = 'ActivatePrometheusServicesForSharedClusterApplications'
QUEUE = 'background_migration'
def up
Sidekiq::Queue.new(QUEUE).each do |job|
klass, project_id, *should_be_empty = job.args
next unless klass == DROPPED_JOB_CLASS && project_id.is_a?(Integer) && should_be_empty.empty?
job.delete
end
end
end
# frozen_string_literal: true
class PatchPrometheusServicesForSharedClusterApplications < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
MIGRATION = 'ActivatePrometheusServicesForSharedClusterApplications'.freeze
BATCH_SIZE = 500
DELAY = 2.minutes
disable_ddl_transaction!
module Migratable
module Applications
class Prometheus < ActiveRecord::Base
self.table_name = 'clusters_applications_prometheus'
enum status: {
errored: -1,
installed: 3,
updated: 5
}
end
end
class Project < ActiveRecord::Base
self.table_name = 'projects'
include ::EachBatch
scope :with_application_on_group_clusters, -> {
joins("INNER JOIN namespaces ON namespaces.id = projects.namespace_id")
.joins("INNER JOIN cluster_groups ON cluster_groups.group_id = namespaces.id")
.joins("INNER JOIN clusters ON clusters.id = cluster_groups.cluster_id AND clusters.cluster_type = #{Cluster.cluster_types['group_type']}")
.joins("INNER JOIN clusters_applications_prometheus ON clusters_applications_prometheus.cluster_id = clusters.id
AND clusters_applications_prometheus.status IN (#{Applications::Prometheus.statuses[:installed]}, #{Applications::Prometheus.statuses[:updated]})")
}
scope :without_active_prometheus_services, -> {
joins("LEFT JOIN services ON services.project_id = projects.id AND services.type = 'PrometheusService'")
.where("services.id IS NULL OR (services.active = FALSE AND services.properties = '{}')")
}
end
class Cluster < ActiveRecord::Base
self.table_name = 'clusters'
enum cluster_type: {
instance_type: 1,
group_type: 2
}
def self.has_prometheus_application?
joins("INNER JOIN clusters_applications_prometheus ON clusters_applications_prometheus.cluster_id = clusters.id
AND clusters_applications_prometheus.status IN (#{Applications::Prometheus.statuses[:installed]}, #{Applications::Prometheus.statuses[:updated]})").exists?
end
end
end
def up
projects_without_active_prometheus_service.group('projects.id').each_batch(of: BATCH_SIZE) do |batch, index|
bg_migrations_batch = batch.select('projects.id').map { |project| [MIGRATION, project.id] }
delay = index * DELAY
BackgroundMigrationWorker.bulk_perform_in(delay.seconds, bg_migrations_batch)
end
# no-op
end
def down
# no-op
end
private
def projects_without_active_prometheus_service
scope = Migratable::Project.without_active_prometheus_services
return scope if migrate_instance_cluster?
scope.with_application_on_group_clusters
end
def migrate_instance_cluster?
if instance_variable_defined?('@migrate_instance_cluster')
@migrate_instance_cluster
else
@migrate_instance_cluster = Migratable::Cluster.instance_type.has_prometheus_application?
end
end
end
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# Create missing PrometheusServices records or sets active attribute to true
# for all projects which belongs to cluster with Prometheus Application installed.
class ActivatePrometheusServicesForSharedClusterApplications
module Migratable
# Migration model namespace isolated from application code.
class PrometheusService < ActiveRecord::Base
self.inheritance_column = :_type_disabled
self.table_name = 'services'
default_scope { where("services.type = 'PrometheusService'") }
def self.for_project(project_id)
new(
project_id: project_id,
active: true,
properties: '{}',
type: 'PrometheusService',
template: false,
push_events: true,
issues_events: true,
merge_requests_events: true,
tag_push_events: true,
note_events: true,
category: 'monitoring',
default: false,
wiki_page_events: true,
pipeline_events: true,
confidential_issues_events: true,
commit_events: true,
job_events: true,
confidential_note_events: true,
deployment_events: false
)
end
def managed?
properties == '{}'
end
end
end
def perform(project_id)
service = Migratable::PrometheusService.find_by(project_id: project_id) || Migratable::PrometheusService.for_project(project_id)
service.update!(active: true) if service.managed?
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::BackgroundMigration::ActivatePrometheusServicesForSharedClusterApplications, :migration, schema: 2020_01_14_113341 do
include MigrationHelpers::PrometheusServiceHelpers
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:services) { table(:services) }
let(:namespace) { namespaces.create(name: 'user', path: 'user') }
let(:project) { projects.create(namespace_id: namespace.id) }
let(:columns) do
%w(project_id active properties type template push_events
issues_events merge_requests_events tag_push_events
note_events category default wiki_page_events pipeline_events
confidential_issues_events commit_events job_events
confidential_note_events deployment_events)
end
describe '#perform' do
it 'is idempotent' do
expect { subject.perform(project.id) }.to change { services.order(:id).map { |row| row.attributes } }
expect { subject.perform(project.id) }.not_to change { services.order(:id).map { |row| row.attributes } }
end
context 'non prometheus services' do
it 'does not change them' do
other_type = 'SomeOtherService'
services.create(service_params_for(project.id, active: true, type: other_type))
expect { subject.perform(project.id) }.not_to change { services.where(type: other_type).order(:id).map { |row| row.attributes } }
end
end
context 'prometheus services are configured manually ' do
it 'does not change them' do
properties = '{"api_url":"http://test.dev","manual_configuration":"1"}'
services.create(service_params_for(project.id, properties: properties, active: false))
expect { subject.perform(project.id) }.not_to change { services.order(:id).map { |row| row.attributes } }
end
end
context 'prometheus integration services do not exist' do
it 'creates missing services entries' do
subject.perform(project.id)
rows = services.order(:id).map { |row| row.attributes.slice(*columns).symbolize_keys }
expect([service_params_for(project.id, active: true)]).to eq rows
end
end
context 'prometheus integration services exist' do
context 'in active state' do
it 'does not change them' do
services.create(service_params_for(project.id, active: true))
expect { subject.perform(project.id) }.not_to change { services.order(:id).map { |row| row.attributes } }
end
end
context 'not in active state' do
it 'sets active attribute to true' do
service = services.create(service_params_for(project.id))
expect { subject.perform(project.id) }.to change { service.reload.active? }.from(false).to(true)
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'migrate', '20200116051619_drop_activate_prometheus_services_for_shared_cluster_applications_background_migration.rb')
describe DropActivatePrometheusServicesForSharedClusterApplicationsBackgroundMigration, :sidekiq, :redis, :migration, schema: 2020_01_16_051619 do
subject(:migration) { described_class.new }
describe '#up' do
context 'there are only affected jobs on the queue' do
it 'removes enqueued ActivatePrometheusServicesForSharedClusterApplications background jobs' do
Sidekiq::Testing.disable! do # https://github.com/mperham/sidekiq/wiki/testing#api Sidekiq's API does not have a testing mode
Sidekiq::Client.push('queue' => described_class::QUEUE, 'class' => ::BackgroundMigrationWorker, 'args' => [described_class::DROPPED_JOB_CLASS, 1])
expect { migration.up }.to change { Sidekiq::Queue.new(described_class::QUEUE).size }.from(1).to(0)
end
end
end
context "there aren't any affected jobs on the queue" do
it 'skips other enqueued jobs' do
Sidekiq::Testing.disable! do
Sidekiq::Client.push('queue' => described_class::QUEUE, 'class' => ::BackgroundMigrationWorker, 'args' => ['SomeOtherClass', 1])
expect { migration.up }.not_to change { Sidekiq::Queue.new(described_class::QUEUE).size }
end
end
end
context "there are multiple types of jobs on the queue" do
it 'skips other enqueued jobs' do
Sidekiq::Testing.disable! do
queue = Sidekiq::Queue.new(described_class::QUEUE)
# this job will be deleted
Sidekiq::Client.push('queue' => described_class::QUEUE, 'class' => ::BackgroundMigrationWorker, 'args' => [described_class::DROPPED_JOB_CLASS, 1])
# this jobs will be skipped
skipped_jobs_args = [['SomeOtherClass', 1], [described_class::DROPPED_JOB_CLASS, 'wrong id type'], [described_class::DROPPED_JOB_CLASS, 1, 'some wired argument']]
skipped_jobs_args.each do |args|
Sidekiq::Client.push('queue' => described_class::QUEUE, 'class' => ::BackgroundMigrationWorker, 'args' => args)
end
migration.up
expect(queue.size).to be 3
expect(queue.map(&:args)).to match_array skipped_jobs_args
end
end
end
context "other queues" do
it 'does not modify them' do
Sidekiq::Testing.disable! do
Sidekiq::Client.push('queue' => 'other', 'class' => ::BackgroundMigrationWorker, 'args' => ['SomeOtherClass', 1])
Sidekiq::Client.push('queue' => 'other', 'class' => ::BackgroundMigrationWorker, 'args' => [described_class::DROPPED_JOB_CLASS, 1])
expect { migration.up }.not_to change { Sidekiq::Queue.new('other').size }
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20200114113341_patch_prometheus_services_for_shared_cluster_applications.rb')
describe PatchPrometheusServicesForSharedClusterApplications, :migration do
include MigrationHelpers::PrometheusServiceHelpers
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:services) { table(:services) }
let(:clusters) { table(:clusters) }
let(:cluster_groups) { table(:cluster_groups) }
let(:clusters_applications_prometheus) { table(:clusters_applications_prometheus) }
let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
let(:application_statuses) do
{
errored: -1,
installed: 3,
updated: 5
}
end
let(:cluster_types) do
{
instance_type: 1,
group_type: 2
}
end
describe '#up' do
let!(:project_with_missing_service) { projects.create!(name: 'gitlab', path: 'gitlab-ce', namespace_id: namespace.id) }
let(:project_with_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_manual_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_manual_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_active_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
let(:project_with_inactive_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
before do
services.create(service_params_for(project_with_inactive_service.id, active: false))
services.create(service_params_for(project_with_active_service.id, active: true))
services.create(service_params_for(project_with_active_not_prometheus_service.id, active: true, type: 'other'))
services.create(service_params_for(project_with_inactive_not_prometheus_service.id, active: false, type: 'other'))
services.create(service_params_for(project_with_manual_inactive_service.id, active: false, properties: { some: 'data' }.to_json))
services.create(service_params_for(project_with_manual_active_service.id, active: true, properties: { some: 'data' }.to_json))
end
shared_examples 'patch prometheus services post migration' do
context 'prometheus application is installed on the cluster' do
it 'schedules a background migration' do
clusters_applications_prometheus.create(cluster_id: cluster.id, status: application_statuses[:installed], version: '123')
Sidekiq::Testing.fake! do
Timecop.freeze do
background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_missing_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_active_not_prometheus_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_not_prometheus_service.id]]
migrate!
enqueued_migrations = BackgroundMigrationWorker.jobs.map { |job| job['args'] }
expect(enqueued_migrations).to match_array(background_migrations)
end
end
end
end
context 'prometheus application was recently updated on the cluster' do
it 'schedules a background migration' do
clusters_applications_prometheus.create(cluster_id: cluster.id, status: application_statuses[:updated], version: '123')
Sidekiq::Testing.fake! do
Timecop.freeze do
background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_missing_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_active_not_prometheus_service.id],
["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_not_prometheus_service.id]]
migrate!
enqueued_migrations = BackgroundMigrationWorker.jobs.map { |job| job['args'] }
expect(enqueued_migrations).to match_array(background_migrations)
end
end
end
end
context 'prometheus application failed to install on the cluster' do
it 'does not schedule a background migration' do
clusters_applications_prometheus.create(cluster_id: cluster.id, status: application_statuses[:errored], version: '123')
Sidekiq::Testing.fake! do
Timecop.freeze do
migrate!
expect(BackgroundMigrationWorker.jobs.size).to eq 0
end
end
end
end
context 'prometheus application is NOT installed on the cluster' do
it 'does not schedule a background migration' do
Sidekiq::Testing.fake! do
Timecop.freeze do
migrate!
expect(BackgroundMigrationWorker.jobs.size).to eq 0
end
end
end
end
end
context 'Cluster is group_type' do
let(:cluster) { clusters.create(name: 'cluster', cluster_type: cluster_types[:group_type]) }
before do
cluster_groups.create(group_id: namespace.id, cluster_id: cluster.id)
end
it_behaves_like 'patch prometheus services post migration'
end
context 'Cluster is instance_type' do
let(:cluster) { clusters.create(name: 'cluster', cluster_type: cluster_types[:instance_type]) }
it_behaves_like 'patch prometheus services post migration'
end
end
end
# frozen_string_literal: true
module MigrationHelpers
module PrometheusServiceHelpers
def service_params_for(project_id, params = {})
{
project_id: project_id,
active: false,
properties: '{}',
type: 'PrometheusService',
template: false,
push_events: true,
issues_events: true,
merge_requests_events: true,
tag_push_events: true,
note_events: true,
category: 'monitoring',
default: false,
wiki_page_events: true,
pipeline_events: true,
confidential_issues_events: true,
commit_events: true,
job_events: true,
confidential_note_events: true,
deployment_events: false
}.merge(params)
end
def row_attributes(entity)
entity.attributes.with_indifferent_access.tap do |hash|
hash.merge!(hash.slice(:created_at, :updated_at).transform_values { |v| v.to_s(:db) })
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment