Merge branch 'sy-backfill-escalation-policies' into 'master'

Backfill escalation policies for on-call schedules [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!62233

db/post_migrate/20210519220019_backfill_escalation_policies_for_oncall_schedules.rb

+# frozen_string_literal: true
+
+class BackfillEscalationPoliciesForOncallSchedules < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  # Creates a single new escalation policy for projects which have
+  # existing on-call schedules. Only one schedule is expected
+  # per project, but it is possible to have multiple.
+  #
+  # An escalation rule is created for each existing schedule,
+  # configured to immediately notify the schedule of an incoming
+  # alert payload unless the alert has already been acknowledged.
+  # For projects with multiple schedules, the name of the first saved
+  # schedule will be used for the policy's description.
+  #
+  # Skips projects which already have escalation policies & schedules.
+  #
+  # EX)
+  # For these existing records:
+  #   Project #3
+  #   IncidentManagement::OncallSchedules #13
+  #     project_id: 3
+  #     name: 'Awesome Schedule'
+  #     description: null
+  #   IncidentManagement::OncallSchedules #14
+  #     project_id: 3
+  #     name: '2ndary sched'
+  #     description: 'Backup on-call'
+  #
+  # These will be inserted:
+  #   EscalationPolicy #1
+  #     project_id: 3
+  #     name: 'On-call Escalation Policy'
+  #     description: 'Immediately notify Awesome Schedule'
+  #   EscalationRule #1
+  #     policy_id: 1,
+  #     oncall_schedule_id: 13
+  #     status: 1 # Acknowledged status
+  #     elapsed_time_seconds: 0
+  #   EscalationRule #2
+  #     policy_id: 1,
+  #     oncall_schedule_id: 14
+  #     status: 1 # Acknowledged status
+  #     elapsed_time_seconds: 0
+  def up
+    ApplicationRecord.connection.exec_query(<<~SQL.squish)
+      WITH new_escalation_policies AS (
+        INSERT INTO incident_management_escalation_policies (
+          project_id,
+          name,
+          description
+        )
+        SELECT
+          DISTINCT ON (project_id) project_id,
+          'On-call Escalation Policy',
+          CONCAT('Immediately notify ', name)
+        FROM incident_management_oncall_schedules
+        WHERE project_id NOT IN (
+          SELECT DISTINCT project_id
+          FROM incident_management_escalation_policies
+        )
+        ORDER BY project_id, id
+        RETURNING id, project_id
+      )
+
+      INSERT INTO incident_management_escalation_rules (
+        policy_id,
+        oncall_schedule_id,
+        status,
+        elapsed_time_seconds
+      )
+      SELECT
+        new_escalation_policies.id,
+        incident_management_oncall_schedules.id,
+        1,
+        0
+      FROM new_escalation_policies
+      INNER JOIN incident_management_oncall_schedules
+        ON new_escalation_policies.project_id = incident_management_oncall_schedules.project_id
+    SQL
+  end
+
+  # There is no way to distinguish between policies created
+  # via the backfill or as a result of a user creating a new
+  # on-call schedule.
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20210519220019

+6c687ffd41f242dcd0ecf1ff82652aba79130d2d54016729a817dafa0bac6184
\ No newline at end of file

ee/app/models/incident_management/escalation_rule.rb

@@ -13,7 +13,7 @@ module IncidentManagement
     validates :oncall_schedule, presence: true
     validates :elapsed_time_seconds,
               presence: true,
-              numericality: { only_integer: true, greater_than_or_equal_to: 1, less_than_or_equal_to: 24.hours }
+              numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 24.hours }
 
     validates :policy_id, uniqueness: { scope: [:oncall_schedule_id, :status, :elapsed_time_seconds], message: _('Must have a unique policy, status, and elapsed time') }
   end

ee/app/models/incident_management/oncall_schedule.rb

@@ -25,10 +25,41 @@ module IncidentManagement
 
     delegate :name, to: :project, prefix: true
 
+    after_create :backfill_escalation_policy
+
+    def default_escalation_rule
+      EscalationRule.new(
+        elapsed_time_seconds: 0,
+        oncall_schedule: self,
+        status: :acknowledged
+      )
+    end
+
     private
 
     def timezones
       @timezones ||= ActiveSupport::TimeZone.all.map { |tz| tz.tzinfo.identifier }
     end
+
+    # While escalation policies are in development, we want to
+    # backfill a policy for any project with an OncallSchedule.
+    # Once escalation policies are enabled, users will need to
+    # configure a policy directly in order to direct alerts
+    # to a schedule.
+    def backfill_escalation_policy
+      return if ::Feature.enabled?(:escalation_policies_mvc, project, default_enabled: :yaml)
+      return if ::Feature.disabled?(:escalation_policies_backfill, project, default_enabled: :yaml)
+
+      if policy = project.incident_management_escalation_policies.first
+        policy.rules << default_escalation_rule
+      else
+        EscalationPolicy.create!(
+          project: project,
+          name: 'On-call Escalation Policy',
+          description: "Immediately notify #{name}",
+          rules: project.incident_management_oncall_schedules.map(&:default_escalation_rule)
+        )
+      end
+    end
   end
 end

ee/config/feature_flags/development/escalation_policies_backfill.yml

+---
+name: escalation_policies_backfill
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62233
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/268066#implementation-plan
+milestone: '14.0'
+type: development
+group: group::monitor
+default_enabled: true

ee/spec/models/incident_management/escalation_rule_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe IncidentManagement::EscalationRule do
   describe 'validations' do
     it { is_expected.to validate_presence_of(:status) }
     it { is_expected.to validate_presence_of(:elapsed_time_seconds) }
-    it { is_expected.to validate_numericality_of(:elapsed_time_seconds).is_greater_than_or_equal_to(1).is_less_than_or_equal_to(24.hours) }
+    it { is_expected.to validate_numericality_of(:elapsed_time_seconds).is_greater_than_or_equal_to(0).is_less_than_or_equal_to(24.hours) }
     it { is_expected.to validate_uniqueness_of(:policy_id).scoped_to([:oncall_schedule_id, :status, :elapsed_time_seconds] ).with_message('Must have a unique policy, status, and elapsed time') }
   end
 end

ee/spec/models/incident_management/oncall_schedule_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe IncidentManagement::OncallSchedule do
-  let_it_be(:project) { create(:project) }
+  let_it_be_with_reload(:project) { create(:project) }
 
   describe '.associations' do
     it { is_expected.to belong_to(:project) }
@@ -62,4 +62,96 @@ RSpec.describe IncidentManagement::OncallSchedule do
       expect(described_class.for_iid(oncall_schedule1.iid)).to contain_exactly(oncall_schedule1)
     end
   end
+
+  describe '#backfill_escalation_policy' do
+    subject(:schedule) { create(:incident_management_oncall_schedule, project: project) }
+
+    context 'when the escalation policies feature is disabled' do
+      before do
+        stub_feature_flags(escalation_policies_mvc: false)
+      end
+
+      context 'with an existing escalation policy' do
+        let_it_be(:policy) { create(:incident_management_escalation_policy, project: project) }
+        let_it_be(:rule) { policy.rules.first }
+
+        it 'creates an new escalation rule on the existing policy' do
+          expect { schedule }
+            .to change(::IncidentManagement::EscalationPolicy, :count).by(0)
+            .and change(::IncidentManagement::EscalationRule, :count).by(1)
+
+          expect(policy.reload.rules.length).to eq(2)
+          expect(policy.rules.first).to eq(rule)
+          expect(policy.rules.second).to have_attributes(
+            elapsed_time_seconds: 0,
+            oncall_schedule: schedule,
+            status: 'acknowledged'
+          )
+        end
+      end
+
+      context 'without an existing escalation policy' do
+        let(:policy) { ::IncidentManagement::EscalationPolicy.last! }
+
+        it 'creates a new escalation policy and rule' do
+          expect { schedule }
+            .to change(::IncidentManagement::EscalationPolicy, :count).by(1)
+            .and change(::IncidentManagement::EscalationRule, :count).by(1)
+
+          expect(policy).to have_attributes(
+            name: 'On-call Escalation Policy',
+            description: "Immediately notify #{schedule.name}"
+          )
+          expect(policy.rules.length).to eq(1)
+          expect(policy.rules.first).to have_attributes(
+            elapsed_time_seconds: 0,
+            oncall_schedule: schedule,
+            status: 'acknowledged'
+          )
+        end
+
+        context 'with a previously created schedule which has not yet been backfilled' do
+          let_it_be(:existing_schedule) { create(:incident_management_oncall_schedule, project: project) }
+
+          it 'creates an new escalation rule on the existing policy' do
+            expect { schedule }
+              .to change(::IncidentManagement::EscalationPolicy, :count).by(1)
+              .and change(::IncidentManagement::EscalationRule, :count).by(2)
+
+            expect(policy.rules.length).to eq(2)
+            expect(policy.rules.first).to have_attributes(
+              elapsed_time_seconds: 0,
+              oncall_schedule: existing_schedule,
+              status: 'acknowledged'
+            )
+            expect(policy.rules.second).to have_attributes(
+              elapsed_time_seconds: 0,
+              oncall_schedule: schedule,
+              status: 'acknowledged'
+            )
+          end
+        end
+      end
+
+      context 'when the backfill is disabled directly' do
+        before do
+          stub_feature_flags(escalation_policies_mvc: false, escalation_policies_backfill: false)
+        end
+
+        it 'does not alter the escalation policies' do
+          expect { schedule }
+            .to not_change(::IncidentManagement::EscalationPolicy, :count)
+            .and not_change(::IncidentManagement::EscalationRule, :count)
+        end
+      end
+    end
+
+    context 'when the escalation policies feature is enabled' do
+      it 'does not alter the escalation policies' do
+        expect { schedule }
+          .to not_change(::IncidentManagement::EscalationPolicy, :count)
+          .and not_change(::IncidentManagement::EscalationRule, :count)
+      end
+    end
+  end
 end

spec/migrations/backfill_escalation_policies_for_oncall_schedules_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20210519220019_backfill_escalation_policies_for_oncall_schedules.rb')
+
+RSpec.describe BackfillEscalationPoliciesForOncallSchedules do
+  let_it_be(:projects) { table(:projects) }
+  let_it_be(:schedules) { table(:incident_management_oncall_schedules) }
+  let_it_be(:policies) { table(:incident_management_escalation_policies) }
+  let_it_be(:rules) { table(:incident_management_escalation_rules) }
+
+  # Project with no schedules
+  let_it_be(:namespace) { table(:namespaces).create!(name: 'gitlab', path: 'gitlab') }
+  let_it_be(:project_a) { projects.create!(namespace_id: namespace.id) }
+
+  context 'with backfill-able schedules' do
+    # Project with one schedule
+    let_it_be(:project_b) { projects.create!(namespace_id: namespace.id) }
+    let_it_be(:schedule_b1) { schedules.create!(project_id: project_b.id, iid: 1, name: 'Schedule B1') }
+
+    # Project with multiple schedules
+    let_it_be(:project_c) { projects.create!(namespace_id: namespace.id) }
+    let_it_be(:schedule_c1) { schedules.create!(project_id: project_c.id, iid: 1, name: 'Schedule C1') }
+    let_it_be(:schedule_c2) { schedules.create!(project_id: project_c.id, iid: 2, name: 'Schedule C2') }
+
+    # Project with a single schedule which already has a policy
+    let_it_be(:project_d) { projects.create!(namespace_id: namespace.id) }
+    let_it_be(:schedule_d1) { schedules.create!(project_id: project_d.id, iid: 1, name: 'Schedule D1') }
+    let_it_be(:policy_d1) { policies.create!(project_id: project_d.id, name: 'Policy D1') }
+    let_it_be(:rule_d1) { rules.create!(policy_id: policy_d1.id, oncall_schedule_id: schedule_d1.id, status: 2, elapsed_time_seconds: 60) }
+
+    # Project with a multiple schedule, one of which already has a policy
+    let_it_be(:project_e) { projects.create!(namespace_id: namespace.id) }
+    let_it_be(:schedule_e1) { schedules.create!(project_id: project_e.id, iid: 1, name: 'Schedule E1') }
+    let_it_be(:schedule_e2) { schedules.create!(project_id: project_e.id, iid: 2, name: 'Schedule E2') }
+    let_it_be(:policy_e1) { policies.create!(project_id: project_e.id, name: 'Policy E1') }
+    let_it_be(:rule_e1) { rules.create!(policy_id: policy_e1.id, oncall_schedule_id: schedule_e2.id, status: 2, elapsed_time_seconds: 60) }
+
+    # Project with a multiple schedule, with multiple policies
+    let_it_be(:project_f) { projects.create!(namespace_id: namespace.id) }
+    let_it_be(:schedule_f1) { schedules.create!(project_id: project_f.id, iid: 1, name: 'Schedule F1') }
+    let_it_be(:schedule_f2) { schedules.create!(project_id: project_f.id, iid: 2, name: 'Schedule F2') }
+    let_it_be(:policy_f1) { policies.create!(project_id: project_f.id, name: 'Policy F1') }
+    let_it_be(:rule_f1) { rules.create!(policy_id: policy_f1.id, oncall_schedule_id: schedule_f1.id, status: 2, elapsed_time_seconds: 60) }
+    let_it_be(:rule_f2) { rules.create!(policy_id: policy_f1.id, oncall_schedule_id: schedule_f2.id, status: 2, elapsed_time_seconds: 60) }
+    let_it_be(:policy_f2) { policies.create!(project_id: project_f.id, name: 'Policy F2') }
+    let_it_be(:rule_f3) { rules.create!(policy_id: policy_f2.id, oncall_schedule_id: schedule_f2.id, status: 1, elapsed_time_seconds: 10) }
+
+    it 'backfills escalation policies correctly' do
+      expect { migrate! }
+        .to change(policies, :count).by(2)
+        .and change(rules, :count).by(3)
+
+      new_policy_b1, new_policy_c1 = new_polices = policies.last(2)
+      new_rules = rules.last(3)
+
+      expect(new_polices).to all have_attributes(name: 'On-call Escalation Policy')
+      expect(new_policy_b1.description).to eq('Immediately notify Schedule B1')
+      expect(new_policy_c1.description).to eq('Immediately notify Schedule C1')
+      expect(policies.pluck(:project_id)).to eq([
+        project_d.id,
+        project_e.id,
+        project_f.id,
+        project_f.id,
+        project_b.id,
+        project_c.id
+      ])
+
+      expect(new_rules).to all have_attributes(status: 1, elapsed_time_seconds: 0)
+      expect(rules.pluck(:policy_id)).to eq([
+        rule_d1.policy_id,
+        rule_e1.policy_id,
+        rule_f1.policy_id,
+        rule_f2.policy_id,
+        rule_f3.policy_id,
+        new_policy_b1.id,
+        new_policy_c1.id,
+        new_policy_c1.id
+      ])
+      expect(rules.pluck(:oncall_schedule_id)).to eq([
+        rule_d1.oncall_schedule_id,
+        rule_e1.oncall_schedule_id,
+        rule_f1.oncall_schedule_id,
+        rule_f2.oncall_schedule_id,
+        rule_f3.oncall_schedule_id,
+        schedule_b1.id,
+        schedule_c1.id,
+        schedule_c2.id
+      ])
+    end
+  end
+
+  context 'with no schedules' do
+    it 'does nothing' do
+      expect { migrate! }
+        .to not_change(policies, :count)
+        .and not_change(rules, :count)
+    end
+  end
+end