This instance will be upgraded to Heptapod 0.26.0rc1 on 2021-10-26 at 14:30 UTC+2 (a few minutes of down time)

Commit 458db961 authored by GitLab Bot's avatar GitLab Bot
Browse files

Add latest changes from gitlab-org/gitlab@master

parent 560c79491c5d
......@@ -5,12 +5,9 @@ class ApplicationSetting < ApplicationRecord
include CacheMarkdownField
include TokenAuthenticatable
include ChronicDurationAttribute
include IgnorableColumns
# Only remove this >= %12.6 and >= 2019-12-01
self.ignored_columns += %i[
pendo_enabled
pendo_url
]
ignore_columns :pendo_enabled, :pendo_url, remove_after: '2019-12-01', remove_with: '12.6'
add_authentication_token_field :runners_registration_token, encrypted: -> { Feature.enabled?(:application_settings_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
add_authentication_token_field :health_check_access_token
......
......@@ -13,17 +13,11 @@ class Build < CommitStatus
include Importable
include Gitlab::Utils::StrongMemoize
include HasRef
include IgnorableColumns
BuildArchivedError = Class.new(StandardError)
self.ignored_columns += %i[
artifacts_file
artifacts_file_store
artifacts_metadata
artifacts_metadata_store
artifacts_size
commands
]
ignore_columns :artifacts_file, :artifacts_file_store, :artifacts_metadata, :artifacts_metadata_store, :artifacts_size, :commands, remove_after: '2019-12-15', remove_with: '12.7'
belongs_to :project, inverse_of: :builds
belongs_to :runner
......
......@@ -8,6 +8,7 @@ class Runner < ApplicationRecord
include ChronicDurationAttribute
include FromUnion
include TokenAuthenticatable
include IgnorableColumns
add_authentication_token_field :token, encrypted: -> { Feature.enabled?(:ci_runners_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
......@@ -35,7 +36,7 @@ class Runner < ApplicationRecord
FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level maximum_timeout_human_readable].freeze
self.ignored_columns += %i[is_shared]
ignore_column :is_shared, remove_after: '2019-12-15', remove_with: '12.6'
has_many :builds
has_many :runner_projects, inverse_of: :runner, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
......
# frozen_string_literal: true
module IgnorableColumns
extend ActiveSupport::Concern
ColumnIgnore = Struct.new(:remove_after, :remove_with) do
def safe_to_remove?
Date.today > remove_after
end
def to_s
"(#{remove_after}, #{remove_with})"
end
end
class_methods do
# Ignore database columns in a model
#
# Indicate the earliest date and release we can stop ignoring the column with +remove_after+ (a date string) and +remove_with+ (a release)
def ignore_columns(*columns, remove_after:, remove_with:)
raise ArgumentError, 'Please indicate when we can stop ignoring columns with remove_after (date string YYYY-MM-DD), example: ignore_columns(:name, remove_after: \'2019-12-01\', remove_with: \'12.6\')' unless remove_after =~ Gitlab::Regex.utc_date_regex
raise ArgumentError, 'Please indicate in which release we can stop ignoring columns with remove_with, example: ignore_columns(:name, remove_after: \'2019-12-01\', remove_with: \'12.6\')' unless remove_with
self.ignored_columns += columns.flatten # rubocop:disable Cop/IgnoredColumns
columns.flatten.each do |column|
self.ignored_columns_details[column.to_sym] = ColumnIgnore.new(Date.parse(remove_after), remove_with)
end
end
alias_method :ignore_column, :ignore_columns
def ignored_columns_details
unless defined?(@ignored_columns_details)
IGNORE_COLUMN_MUTEX.synchronize do
@ignored_columns_details ||= superclass.try(:ignored_columns_details)&.dup || {}
end
end
@ignored_columns_details
end
IGNORE_COLUMN_MUTEX = Mutex.new
end
end
......@@ -2,6 +2,7 @@
class DeployKey < Key
include FromUnion
include IgnorableColumns
has_many :deploy_keys_projects, inverse_of: :deploy_key, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :projects, through: :deploy_keys_projects
......@@ -10,7 +11,7 @@ class DeployKey < Key
scope :are_public, -> { where(public: true) }
scope :with_projects, -> { includes(deploy_keys_projects: { project: [:route, :namespace] }) }
self.ignored_columns += %i[can_push]
ignore_column :can_push, remove_after: '2019-12-15', remove_with: '12.6'
accepts_nested_attributes_for :deploy_keys_projects
......
......@@ -3,7 +3,9 @@
# Placeholder class for model that is implemented in EE
# It reserves '&' as a reference prefix, but the table does not exists in CE
class Epic < ApplicationRecord
self.ignored_columns += %i[milestone_id]
include IgnorableColumns
ignore_column :milestone_id, remove_after: '2019-12-15', remove_with: '12.7'
def self.link_reference_pattern
nil
......
......@@ -14,6 +14,7 @@ class Issue < ApplicationRecord
include TimeTrackable
include ThrottledTouch
include LabelEventable
include IgnorableColumns
DueDateStruct = Struct.new(:title, :name).freeze
NoDueDate = DueDateStruct.new('No Due Date', '0').freeze
......@@ -68,8 +69,7 @@ class Issue < ApplicationRecord
scope :counts_by_state, -> { reorder(nil).group(:state_id).count }
# Only remove after 2019-12-22 and with %12.7
self.ignored_columns += %i[state]
ignore_column :state, remove_with: '12.7', remove_after: '2019-12-22'
after_commit :expire_etag_cache
after_save :ensure_metrics, unless: :imported?
......
......@@ -17,6 +17,7 @@ class MergeRequest < ApplicationRecord
include FromUnion
include DeprecatedAssignee
include ShaAttribute
include IgnorableColumns
sha_attribute :squash_commit_sha
......@@ -239,8 +240,7 @@ def check_state?(merge_status)
with_state(:opened).where(auto_merge_enabled: true)
end
# Only remove after 2019-12-22 and with %12.7
self.ignored_columns += %i[state]
ignore_column :state, remove_with: '12.7', remove_after: '2019-12-22'
after_save :keep_around_commit
......
......@@ -31,6 +31,7 @@ class Project < ApplicationRecord
include FeatureGate
include OptionallySearch
include FromUnion
include IgnorableColumns
extend Gitlab::Cache::RequestCache
extend Gitlab::ConfigHelper
......@@ -64,7 +65,7 @@ class Project < ApplicationRecord
# TODO: remove once GitLab 12.5 is released
# https://gitlab.com/gitlab-org/gitlab/issues/34638
self.ignored_columns += %i[merge_requests_require_code_owner_approval]
ignore_column :merge_requests_require_code_owner_approval, remove_after: '2019-12-01', remove_with: '12.6'
default_value_for :archived, false
default_value_for :resolve_outdated_diff_discussions, false
......
# frozen_string_literal: true
class ProjectCiCdSetting < ApplicationRecord
# TODO: remove once GitLab 12.7 is released
include IgnorableColumns
# https://gitlab.com/gitlab-org/gitlab/issues/36651
self.ignored_columns += %i[merge_trains_enabled]
ignore_column :merge_trains_enabled, remove_with: '12.7', remove_after: '2019-12-22'
belongs_to :project, inverse_of: :ci_cd_settings
# The version of the schema that first introduced this model/table.
......
......@@ -101,6 +101,7 @@ and details for a database reviewer:
- Check consistency with `db/schema.rb` and that migrations are [reversible](migration_style_guide.md#reversibility)
- Check queries timing (If any): Queries executed in a migration
need to fit comfortably within `15s` - preferably much less than that - on GitLab.com.
- For column removals, make sure the column has been [ignored in a previous release](what_requires_downtime.md#dropping-columns)
- Check [background migrations](background_migrations.md):
- Establish a time estimate for execution on GitLab.com.
- They should only be used when migrating data in larger tables.
......
......@@ -37,11 +37,19 @@ information on how to use this method.
## Dropping Columns
Removing columns is tricky because running GitLab processes may still be using
the columns. To work around this you will need two separate merge requests and
releases: one to ignore and then remove the column, and one to remove the ignore
rule.
the columns. To work around this safely, you will need three steps in three releases:
### Step 1: Ignoring The Column
1. Ignoring the column (release M)
1. Dropping the column (release M+1)
1. Removing the ignore rule (release M+2)
The reason we spread this out across three releases is that dropping a column is
a destructive operation that can't be rolled back easily.
Following this procedure helps us to make sure there are no deployments to GitLab.com
and upgrade processes for self-hosted installations that lump together any of these steps.
### Step 1: Ignoring the column (release M)
The first step is to ignore the column in the application code. This is
necessary because Rails caches the columns and re-uses this cache in various
......@@ -50,18 +58,46 @@ places. This can be done by defining the columns to ignore. For example, to igno
```ruby
class User < ApplicationRecord
self.ignored_columns += %i[updated_at]
include IgnorableColumns
ignore_column :updated_at, remove_with: '12.7', remove_after: '2019-12-22'
end
```
Once added you should create a _post-deployment_ migration that removes the
column. Both these changes should be submitted in the same merge request.
Multiple columns can be ignored, too:
```ruby
ignore_columns %i[updated_at created_at], remove_with: '12.7', remove_after: '2019-12-22'
```
We require indication of when it is safe to remove the column ignore with:
- `remove_with`: set to a GitLab release typically two releases (M+2) after adding the
column ignore.
- `remove_after`: set to a date after which we consider it safe to remove the column
ignore, typically within the development cycle of release M+2.
This information allows us to reason better about column ignores and makes sure we
don't remove column ignores too early for both regular releases and deployments to GitLab.com. For
example, this avoids a situation where we deploy a bulk of changes that include both changes
to ignore the column and subsequently remove the column ignore (which would result in a downtime).
In this example, the change to ignore the column went into release 12.5.
### Step 2: Dropping the column (release M+1)
Continuing our example, dropping the column goes into a _post-deployment_ migration in release 12.6:
```ruby
remove_column :user, :updated_at
```
### Step 3: Removing the ignore rule (release M+2)
### Step 2: Removing The Ignore Rule
With the next release, in this example 12.7, we set up another merge request to remove the ignore rule.
This removes the `ignore_column` line and - if not needed anymore - also the inclusion of `IgnoreableColumns`.
Once the changes from step 1 have been released & deployed you can set up a
separate merge request that removes the ignore rule. This merge request can
simply remove the `self.ignored_columns` line.
This should only get merged with the release indicated with `remove_with` and once
the `remove_after` date has passed.
## Renaming Columns
......
......@@ -23,8 +23,15 @@ def execute
private
def ignored_columns_safe_to_remove_for(klass)
ignored = klass.ignored_columns.map(&:to_s)
ignores = ignored_and_not_present(klass).each_with_object({}) do |col, h|
h[col] = klass.ignored_columns_details[col.to_sym]
end
ignores.select { |_, i| i&.safe_to_remove? }
end
def ignored_and_not_present(klass)
ignored = klass.ignored_columns.map(&:to_s)
return [] if ignored.empty?
schema = klass.connection.schema_cache.columns_hash(klass.table_name)
......
......@@ -8,7 +8,10 @@ task 'db:obsolete_ignored_columns' => :environment do
puts 'The following `ignored_columns` are obsolete and can be removed:'
list.each do |name, ignored_columns|
puts "- #{name}: #{ignored_columns.join(', ')}"
puts "#{name}:"
ignored_columns.each do |column, removal|
puts " - #{column.ljust(30)} Remove after #{removal.remove_after} with #{removal.remove_with}"
end
end
puts <<~TEXT
......
# frozen_string_literal: true
module RuboCop
module Cop
# Cop that blacklists the usage of Group.public_or_visible_to_user
class IgnoredColumns < RuboCop::Cop::Cop
MSG = 'Use `IgnoredColumns` concern instead of adding to `self.ignored_columns`.'
def_node_matcher :ignored_columns?, <<~PATTERN
(send (self) :ignored_columns)
PATTERN
def on_send(node)
return unless ignored_columns?(node)
add_offense(node, location: :expression)
end
end
end
end
......@@ -54,3 +54,4 @@
require_relative 'cop/inject_enterprise_edition_module'
require_relative 'cop/graphql/authorize_types'
require_relative 'cop/graphql/descriptions'
require_relative 'cop/ignored_columns'
......@@ -8,21 +8,27 @@ class MyBase < ApplicationRecord
end
class SomeAbstract < MyBase
include IgnorableColumns
self.abstract_class = true
self.table_name = 'projects'
self.ignored_columns += %i[unused]
ignore_column :unused, remove_after: '2019-01-01', remove_with: '12.0'
end
class B < MyBase
include IgnorableColumns
self.table_name = 'issues'
self.ignored_columns += %i[id other]
ignore_column :id, :other, remove_after: '2019-01-01', remove_with: '12.0'
ignore_column :not_used_but_still_ignored, remove_after: Date.today.to_s, remove_with: '12.1'
end
class A < SomeAbstract
self.ignored_columns += %i[id also_unused]
ignore_column :also_unused, remove_after: '2019-02-01', remove_with: '12.1'
ignore_column :not_used_but_still_ignored, remove_after: Date.today.to_s, remove_with: '12.1'
end
class C < MyBase
......@@ -35,8 +41,13 @@ class C < MyBase
describe '#execute' do
it 'returns a list of class names and columns pairs' do
expect(subject.execute).to eq([
['Testing::A', %w(unused also_unused)],
['Testing::B', %w(other)]
['Testing::A', {
'unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0'),
'also_unused' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-02-01'), '12.1')
}],
['Testing::B', {
'other' => IgnorableColumns::ColumnIgnore.new(Date.parse('2019-01-01'), '12.0')
}]
])
end
end
......
# frozen_string_literal: true
require 'spec_helper'
describe IgnorableColumns do
let(:record_class) do
Class.new(ApplicationRecord) do
include IgnorableColumns
end
end
subject { record_class }
it 'adds columns to ignored_columns' do
expect do
subject.ignore_columns(:name, :created_at, remove_after: '2019-12-01', remove_with: '12.6')
end.to change { subject.ignored_columns }.from([]).to(%w(name created_at))
end
it 'adds columns to ignored_columns (array version)' do
expect do
subject.ignore_columns(%i[name created_at], remove_after: '2019-12-01', remove_with: '12.6')
end.to change { subject.ignored_columns }.from([]).to(%w(name created_at))
end
it 'requires remove_after attribute to be set' do
expect { subject.ignore_columns(:name, remove_after: nil, remove_with: 12.6) }.to raise_error(ArgumentError, /Please indicate/)
end
it 'requires remove_after attribute to be set' do
expect { subject.ignore_columns(:name, remove_after: "not a date", remove_with: 12.6) }.to raise_error(ArgumentError, /Please indicate/)
end
it 'requires remove_with attribute to be set' do
expect { subject.ignore_columns(:name, remove_after: '2019-12-01', remove_with: nil) }.to raise_error(ArgumentError, /Please indicate/)
end
describe '.ignored_columns_details' do
shared_examples_for 'storing removal information' do
it 'storing removal information' do
subject.ignore_column(columns, remove_after: '2019-12-01', remove_with: '12.6')
[columns].flatten.each do |column|
expect(subject.ignored_columns_details[column].remove_after).to eq(Date.parse('2019-12-01'))
expect(subject.ignored_columns_details[column].remove_with).to eq('12.6')
end
end
end
context 'with single column' do
let(:columns) { :name }
it_behaves_like 'storing removal information'
end
context 'with array column' do
let(:columns) { %i[name created_at] }
it_behaves_like 'storing removal information'
end
it 'defaults to empty Hash' do
expect(subject.ignored_columns_details).to eq({})
end
end
describe IgnorableColumns::ColumnIgnore do
subject { described_class.new(remove_after, remove_with) }
let(:remove_with) { double }
describe '#safe_to_remove?' do
context 'after remove_after date has passed' do
let(:remove_after) { Date.parse('2019-01-10') }
it 'returns true (safe to remove)' do
expect(subject.safe_to_remove?).to be_truthy
end
end
context 'before remove_after date has passed' do
let(:remove_after) { Date.today }
it 'returns false (not safe to remove)' do
expect(subject.safe_to_remove?).to be_falsey
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/ignored_columns'
describe RuboCop::Cop::IgnoredColumns do
include CopHelper
subject(:cop) { described_class.new }
it 'flags the use of destroy_all with a local variable receiver' do
inspect_source(<<~RUBY)
class Foo < ApplicationRecord
self.ignored_columns += %i[id]
end
RUBY
expect(cop.offenses.size).to eq(1)
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