Skip to content

Don't remove mock ci connection with multiple dbs

Patrick Bair requested to merge pb-fix-mocked-ci-connection into master

What does this MR do and why?

Changes mocked_ci_connection to not call remove_connection when the ci database is configured. Calling remove_connection in this instance clears the connection_specification_name for Ci::ApplicationRecord, so when it reconnects, it picks uses the default specification name of ActiveRecord::Base, causing it to use the default connection as well.

This was causing flaky test failures on the decomposed pipeline, when running these specs in this order:

rspec spec/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics_spec.rb spec/models/acts_as_taggable_on/tagging_spec.rb

How to set up and validate locally

  1. Verify that this command fails on master:
    GITLAB_LOAD_BALANCING_REUSE_PRIMARY_ci=main GITLAB_USE_MODEL_LOAD_BALANCING=true bundle exec rspec spec/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics_spec.rb spec/models/acts_as_taggable_on/tagging_spec.rb
    with an error like:
      1) ActsAsTaggableOn::Tagging has the same connection as Ci::ApplicationRecord
      Failure/Error: expect(described_class.retrieve_connection.execute(query).first).to eq(Ci::ApplicationRecord.retrieve_connection.execute(query).first)
        expected: {"current_database"=>"gitlabhq_test"}
             got: {"current_database"=>"gitlabhq_test_ci"}
  2. Run the same test above with the change from this branch, and it should pass.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Patrick Bair

Merge request reports