Merge branch '345894-cleanup-saas-incorrect-max_seats_used' into 'master'

Fix incorrect max_seats_used values

See merge request gitlab-org/gitlab!78107

db/post_migrate/20220113111440_schedule_fix_incorrect_max_seats_used.rb

+# frozen_string_literal: true
+
+class ScheduleFixIncorrectMaxSeatsUsed < Gitlab::Database::Migration[1.0]
+  DOWNTIME = false
+  TMP_IDX_NAME = 'tmp_gitlab_subscriptions_max_seats_used_migration'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :gitlab_subscriptions, :id, where: "start_date >= '2021-08-02' AND start_date <= '2021-11-20' AND max_seats_used != 0 AND max_seats_used > seats_in_use AND max_seats_used > seats", name: TMP_IDX_NAME
+
+    return unless Gitlab.com?
+
+    migrate_in(1.hour, 'FixIncorrectMaxSeatsUsed')
+  end
+
+  def down
+    remove_concurrent_index_by_name :gitlab_subscriptions, TMP_IDX_NAME
+  end
+end

db/schema_migrations/20220113111440

+faf899c1aa99e596eb386935ee6ff17a51b7942ee4f6d4cbd1ad2283dd0d40c0
\ No newline at end of file

db/structure.sql

@@ -28157,6 +28157,8 @@ CREATE UNIQUE INDEX taggings_idx ON taggings USING btree (tag_id, taggable_id, t
 
 CREATE UNIQUE INDEX term_agreements_unique_index ON term_agreements USING btree (user_id, term_id);
 
+CREATE INDEX tmp_gitlab_subscriptions_max_seats_used_migration ON gitlab_subscriptions USING btree (id) WHERE ((start_date >= '2021-08-02'::date) AND (start_date <= '2021-11-20'::date) AND (max_seats_used <> 0) AND (max_seats_used > seats_in_use) AND (max_seats_used > seats));
+
 CREATE INDEX tmp_idx_deduplicate_vulnerability_occurrences ON vulnerability_occurrences USING btree (project_id, report_type, location_fingerprint, primary_identifier_id, id);
 
 CREATE INDEX tmp_idx_vulnerability_occurrences_on_id_where_report_type_7_99 ON vulnerability_occurrences USING btree (id) WHERE (report_type = ANY (ARRAY[7, 99]));

ee/lib/ee/gitlab/background_migration/fix_incorrect_max_seats_used.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module BackgroundMigration
+      # Class that fixes the POSSIBLE incorrect `max_seats_used`
+      # in gitlab_subscriptions table
+      module FixIncorrectMaxSeatsUsed
+        class FixIncorrectMaxSeatsUsedJsonLogger < ::Gitlab::JsonLogger
+          def self.file_name_noext
+            'fix_incorrect_max_seats_used_json'
+          end
+        end
+
+        class Namespace < ActiveRecord::Base
+          self.table_name = 'namespaces'
+
+          # disable STI
+          self.inheritance_column = nil
+        end
+
+        class Plan < ActiveRecord::Base
+          self.table_name = 'plans'
+
+          FREE = 'free'
+          BRONZE = 'bronze'
+          SILVER = 'silver'
+          PREMIUM = 'premium'
+          GOLD = 'gold'
+          ULTIMATE = 'ultimate'
+          ULTIMATE_TRIAL = 'ultimate_trial'
+          PREMIUM_TRIAL = 'premium_trial'
+
+          PAID_HOSTED_PLANS = [BRONZE, SILVER, PREMIUM, GOLD, ULTIMATE, ULTIMATE_TRIAL, PREMIUM_TRIAL].freeze
+        end
+
+        class GitlabSubscription < ActiveRecord::Base
+          self.table_name = 'gitlab_subscriptions'
+
+          belongs_to :namespace
+          belongs_to :hosted_plan, class_name: 'Plan'
+          has_many :gitlab_subscription_histories
+
+          before_update :log_previous_state_for_update
+          before_update :reset_seats_for_new_term
+
+          delegate :name, :title, to: :hosted_plan, prefix: :plan, allow_nil: true
+
+          def calculate_seats_owed
+            return 0 unless has_a_paid_hosted_plan?
+
+            [0, max_seats_used - seats].max
+          end
+
+          def has_a_paid_hosted_plan?(include_trials: false)
+            (include_trials || !trial?) &&
+              seats > 0 &&
+              Plan::PAID_HOSTED_PLANS.include?(plan_name)
+          end
+
+          private
+
+          def log_previous_state_for_update
+            attrs = self.attributes.merge(self.attributes_in_database)
+            log_previous_state_to_history(:gitlab_subscription_updated, attrs)
+          end
+
+          def log_previous_state_to_history(change_type, attrs = {})
+            attrs['gitlab_subscription_created_at'] = attrs['created_at']
+            attrs['gitlab_subscription_updated_at'] = attrs['updated_at']
+            attrs['gitlab_subscription_id'] = self.id
+            attrs['change_type'] = change_type
+
+            omitted_attrs = %w(id created_at updated_at seats_in_use seats_owed)
+
+            GitlabSubscriptionHistory.create(attrs.except(*omitted_attrs))
+          end
+
+          def reset_seats_for_new_term
+            return unless new_term?
+
+            self.max_seats_used = attributes['seats_in_use']
+            self.seats_owed = calculate_seats_owed
+          end
+
+          def new_term?
+            persisted? && start_date_changed? && end_date_changed? &&
+              (end_date_was.nil? || start_date >= end_date_was)
+          end
+        end
+
+        class GitlabSubscriptionHistory < ActiveRecord::Base
+          self.table_name = 'gitlab_subscription_histories'
+
+          belongs_to :gitlab_subscription
+        end
+
+        BATCH_SIZE = 200
+
+        def perform
+          eligible_gitlab_subscriptions.find_each(batch_size: BATCH_SIZE) do |gs|
+            gs_histories = gs.gitlab_subscription_histories.to_a.sort_by { |gh| gh.id }
+            gs_histories << gs
+
+            last_new_term_index = find_last_new_term_index(gs_histories)
+            next unless last_new_term_index
+
+            reset(gs) if requires_reset(gs, gs_histories, last_new_term_index)
+          end
+        end
+
+        private
+
+        def file_logger
+          @file_logger ||= FixIncorrectMaxSeatsUsedJsonLogger.build
+        end
+
+        def reset(gs)
+          identified_subscription = gs.attributes.merge(namespace_path: gs.namespace.path)
+
+          gs.max_seats_used = gs.seats_in_use
+          gs.seats_owed = gs.calculate_seats_owed
+
+          changes = gs.changes
+          success = gs.save
+
+          file_logger.info({ identified_subscription: identified_subscription, changes: changes, success: success })
+        end
+
+        def new_term?(gs_histories, index)
+          new_start_date = gs_histories[index].start_date
+          previous_end_date = gs_histories[index - 1].end_date
+
+          return false if new_start_date.nil?
+
+          return true if previous_end_date.nil? || new_start_date >= previous_end_date
+
+          false
+        end
+
+        def find_last_new_term_index(gs_histories)
+          return if gs_histories.size < 2
+
+          # to find the latest new subscription term
+          index = gs_histories.size - 1
+          while index > 0
+            return index if new_term?(gs_histories, index)
+
+            index -= 1
+          end
+        end
+
+        def requires_reset(gs, gs_histories, last_new_term_index)
+          new_term_max_seats_used = gs_histories[last_new_term_index].max_seats_used
+          previous_max_seats_used = gs_histories[last_new_term_index - 1].max_seats_used
+
+          # New term `max_seats_used` started from `0`. No need to reset
+          return false if new_term_max_seats_used == 0
+
+          # New term and previous term value differ, we assume the value was already reset
+          return false if new_term_max_seats_used != previous_max_seats_used
+
+          # `max_seats_used` ever increased after the new term start. No need to reset
+          return false if new_term_max_seats_used < gs.max_seats_used
+
+          # Current gitlab_subscription `seats_in_use` is equal to or larger. No need to reset
+          return false if new_term_max_seats_used <= gs.seats_in_use
+
+          true
+        end
+
+        def eligible_gitlab_subscriptions
+          # Only search subscriptions with `start_date` in range `['2-Aug-2021', '20-Nov-2021']` because:
+          #   - for subscriptions with `start_date < '2-Aug-2021'`, we do not enable QSR(Quarterly Subscription Reconciliation)
+          #   - for subscriptions with `start_date > '20-Nov-2021'`, they should not have such issue
+          #     because the MR https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73078 was merged on `09-Nov-2021`.
+          #     All rails nodes should have deployed new merged code within 10 days.
+          #
+          # Only need to check if max_seats_used is not 0
+          # Only need to check if max_seats_used > seats_in_use
+          # Only need to check if max_seats_used > seats (zuora subscription quantity)
+
+          GitlabSubscription.preload(:namespace, :hosted_plan, :gitlab_subscription_histories)
+            .where('gitlab_subscriptions.start_date >= ?', Date.parse('2-Aug-2021'))
+            .where('gitlab_subscriptions.start_date <= ?', Date.parse('20-Nov-2021'))
+            .where.not(max_seats_used: 0)
+            .where('gitlab_subscriptions.max_seats_used > gitlab_subscriptions.seats_in_use')
+            .where('gitlab_subscriptions.max_seats_used > gitlab_subscriptions.seats')
+        end
+      end
+    end
+  end
+end

ee/spec/lib/ee/gitlab/background_migration/fix_incorrect_max_seats_used_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::FixIncorrectMaxSeatsUsed, :saas do
+  describe '#perform' do
+    subject(:migration) { described_class.new }
+
+    let_it_be(:namespaces) { table(:namespaces) }
+    let_it_be(:plans) { table(:plans) }
+
+    let(:seats) { 2 }
+    let(:seats_in_use) { 5 }
+    let(:max_seats_used) { 10 }
+    let(:seats_owed) { [0, max_seats_used - seats].max }
+
+    let(:start_date) { Date.parse('10-Nov-2021') }
+    let(:end_date) { start_date + 1.year }
+
+    let(:keep_old_seats_attributes_after_renew) { true }
+
+    let(:logger) { instance_spy(Gitlab::BackgroundMigration::FixIncorrectMaxSeatsUsed::FixIncorrectMaxSeatsUsedJsonLogger) }
+
+    let!(:gitlab_subscription) { generate_gitlab_subscription }
+
+    def perform_and_reload
+      migration.perform
+
+      gitlab_subscription.reload
+    end
+
+    def generate_gitlab_subscription
+      initial_start_date = start_date - renewed_count.years
+      initial_end_date = initial_start_date + 1.year
+
+      namespace = namespaces.create!(name: 'gitlab', path: 'gitlab-org', type: 'Group')
+      plan = plans.create!(name: 'gold')
+      gs = Gitlab::BackgroundMigration::FixIncorrectMaxSeatsUsed::GitlabSubscription.create!(namespace_id: namespace.id,
+                  seats: seats, seats_in_use: seats_in_use, max_seats_used: max_seats_used, seats_owed: seats_owed,
+                  start_date: initial_start_date, end_date: initial_end_date, hosted_plan_id: plan.id)
+
+      renewed_count.downto(1) do |i|
+        old_seats_attributes = { seats: gs.seats, max_seats_used: gs.max_seats_used, seats_owed: gs.seats_owed, seats_in_use: gs.seats_in_use }
+
+        renewed_start_date = gs.end_date
+        renewed_end_date = renewed_start_date + 1.year
+
+        gs.update!(start_date: renewed_start_date, end_date: renewed_end_date)
+        gs.update_columns(old_seats_attributes) if keep_old_seats_attributes_after_renew
+      end
+
+      gs.reload
+    end
+
+    shared_examples 'does not reset max_seats_used and seats_owed' do
+      it 'does not reset max_seats_used and seats_owed' do
+        expect do
+          perform_and_reload
+        end.to not_change(gitlab_subscription, :max_seats_used)
+          .and not_change(gitlab_subscription, :seats_owed)
+          .and not_change(GitlabSubscriptionHistory, :count)
+      end
+    end
+
+    shared_examples 'resets max_seats_used and seats_owed' do
+      it 'resets max_seats_used and seats_owed' do
+        gs_before_reset = gitlab_subscription.clone
+
+        expect(Gitlab::BackgroundMigration::FixIncorrectMaxSeatsUsed::FixIncorrectMaxSeatsUsedJsonLogger).to receive(:build).and_return(logger)
+
+        expect do
+          perform_and_reload
+        end.to change(gitlab_subscription, :max_seats_used).from(10).to(5)
+          .and change(gitlab_subscription, :seats_owed).from(8).to(3)
+          .and change(GitlabSubscriptionHistory, :count).by(1)
+
+        expect(logger).to have_received(:info).with(
+          hash_including(
+            identified_subscription: gs_before_reset.attributes.merge(namespace_path: gs_before_reset.namespace.path),
+            changes: hash_including(max_seats_used: [10, 5], seats_owed: [8, 3]),
+            success: true))
+      end
+    end
+
+    shared_examples 'gitlab subscription has one or more renew history' do
+      include_examples 'resets max_seats_used and seats_owed'
+
+      context 'when max_seats_used has already been correctly reset during renew' do
+        let(:keep_old_seats_attributes_after_renew) { false }
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when start_date is before 2-Aug-2021' do
+        let(:start_date) { Date.parse('1-Aug-2021') }
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when start_date is 2-Aug-2021' do
+        let(:start_date) { Date.parse('2-Aug-2021') }
+
+        include_examples 'resets max_seats_used and seats_owed'
+      end
+
+      context 'when start_date is 20-Nov-2021' do
+        let(:start_date) { Date.parse('20-Nov-2021') }
+
+        include_examples 'resets max_seats_used and seats_owed'
+      end
+
+      context 'when start_date is after 20-Nov-2021' do
+        let(:start_date) { Date.parse('21-Nov-2021') }
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when max_seats_used is 0' do
+        let(:max_seats_used) { 0 }
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when max_seats_used equals to seats_in_use' do
+        let(:max_seats_used) { seats_in_use }
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when max_seats_used is less than seats_in_use' do
+        let(:max_seats_used) { seats_in_use - 1 }
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when max_seats_used equals to seats' do
+        let(:max_seats_used) { seats }
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when max_seats_used is less than seats' do
+        let(:max_seats_used) { seats - 1 }
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when max_seats_used increases after renew' do
+        before do
+          gitlab_subscription.update_columns(max_seats_used: max_seats_used + 1)
+        end
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when seats_in_use increased to max_seats_used' do
+        before do
+          gitlab_subscription.update_columns(seats_in_use: max_seats_used)
+        end
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when seats_in_use increased to greater than max_seats_used' do
+        before do
+          gitlab_subscription.update_columns(seats_in_use: max_seats_used + 1)
+        end
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+    end
+
+    context 'when gitlab subscription does not have renew history' do
+      let(:renewed_count) { 0 }
+
+      include_examples 'does not reset max_seats_used and seats_owed'
+    end
+
+    context 'when gitlab subscription has one renew history' do
+      let(:renewed_count) { 1 }
+
+      it_behaves_like 'gitlab subscription has one or more renew history'
+    end
+
+    context 'gitlab subscription has more than one renew histories' do
+      let(:renewed_count) { 2 }
+
+      it_behaves_like 'gitlab subscription has one or more renew history'
+    end
+
+    context 'when gitlab subscription has non-renewal update history' do
+      before do
+        gitlab_subscription.update!(auto_renew: !gitlab_subscription.auto_renew)
+        gitlab_subscription.reload
+      end
+
+      context 'when gitlab subscription does not have renew history' do
+        let(:renewed_count) { 0 }
+
+        include_examples 'does not reset max_seats_used and seats_owed'
+      end
+
+      context 'when gitlab subscription has one renew history' do
+        let(:renewed_count) { 1 }
+
+        it_behaves_like 'gitlab subscription has one or more renew history'
+      end
+
+      context 'gitlab subscription has more than one renew histories' do
+        let(:renewed_count) { 2 }
+
+        it_behaves_like 'gitlab subscription has one or more renew history'
+      end
+    end
+  end
+end
+
+RSpec.describe Gitlab::BackgroundMigration::FixIncorrectMaxSeatsUsed::FixIncorrectMaxSeatsUsedJsonLogger do
+  subject { described_class.new('/dev/null') }
+
+  let(:hash_message) { { 'message' => 'Message', 'project_id' => '123' } }
+  let(:string_message) { 'Information' }
+
+  it 'logs a hash as a JSON' do
+    expect(Gitlab::Json.parse(subject.format_message('INFO', Time.current, nil, hash_message))).to include(hash_message)
+  end
+
+  it 'logs a string as a JSON' do
+    expect(Gitlab::Json.parse(subject.format_message('INFO', Time.current, nil, string_message))).to include('message' => string_message)
+  end
+
+  it 'logs into the expected file' do
+    expect(described_class.file_name).to eq('fix_incorrect_max_seats_used_json.log')
+  end
+end

lib/gitlab/background_migration/fix_incorrect_max_seats_used.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # rubocop: disable Style/Documentation
+    class FixIncorrectMaxSeatsUsed
+      def perform
+      end
+    end
+  end
+end
+
+Gitlab::BackgroundMigration::FixIncorrectMaxSeatsUsed.prepend_mod_with('Gitlab::BackgroundMigration::FixIncorrectMaxSeatsUsed')

spec/migrations/schedule_fix_incorrect_max_seats_used_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe ScheduleFixIncorrectMaxSeatsUsed, :migration do
+  let(:migration) { described_class.new }
+
+  describe '#up' do
+    it 'schedules a job on Gitlab.com' do
+      allow(Gitlab).to receive(:com?).and_return(true)
+
+      expect(migration).to receive(:migrate_in).with(1.hour, 'FixIncorrectMaxSeatsUsed')
+
+      migration.up
+    end
+
+    it 'does not schedule any jobs when not Gitlab.com' do
+      allow(Gitlab::CurrentSettings).to receive(:com?).and_return(false)
+
+      expect(migration).not_to receive(:migrate_in)
+
+      migration.up
+    end
+  end
+end