diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keep.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keep.rb
index ecd55008a80a7f702cbfbcdaf71a3b5287642981..d982e3ba9bbe4e622d82618cb83da06d81df0e70 100644
--- a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keep.rb
+++ b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keep.rb
@@ -15,6 +15,10 @@ module Housekeeper
# )
# ```
class Keep
+ def initialize(logger: nil)
+ @logger = logger || Logger.new(nil)
+ end
+
# The each_change method must update local working copy files and yield a Change object which describes the
# specific changed files and other data that will be used to generate a merge request. This is the core
# implementation details for a specific housekeeper keep. This does not need to commit the changes or create the
diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keeps/rubocop_fixer.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keeps/rubocop_fixer.rb
index 5a78e7c584c7c468e129ec31f5f264905a3a1b82..f0085e17ed7834e01ca3e7dc0b8da3e89dea9c62 100644
--- a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keeps/rubocop_fixer.rb
+++ b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/keeps/rubocop_fixer.rb
@@ -9,13 +9,15 @@ class RubocopFixer < Keep
LIMIT_FIXES = 20
RUBOCOP_TODO_DIR_PATTERN = ".rubocop_todo/**/*.yml"
- def initialize(todo_dir_pattern: RUBOCOP_TODO_DIR_PATTERN, limit_fixes: LIMIT_FIXES)
+ def initialize(logger: nil, todo_dir_pattern: RUBOCOP_TODO_DIR_PATTERN, limit_fixes: LIMIT_FIXES)
+ super(logger: logger)
@todo_dir_pattern = todo_dir_pattern
@limit_fixes = limit_fixes
end
def each_change
each_allowed_rubocop_rule do |rule, rule_file_path, violating_files|
+ @logger.puts "RubopCop rule #{rule}"
remove_allow_rule = true
if violating_files.count > @limit_fixes
@@ -41,6 +43,7 @@ def each_change
begin
::Gitlab::Housekeeper::Shell.execute('rubocop', '--autocorrect', *violating_files)
rescue ::Gitlab::Housekeeper::Shell::Error
+ @logger.warn "Failed to autocorrect files. Reverting"
# Ignore when it cannot be automatically fixed. But we need to checkout any files we might have updated.
::Gitlab::Housekeeper::Shell.execute('git', 'checkout', rule_file_path, *violating_files)
next
diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/logger.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/logger.rb
new file mode 100644
index 0000000000000000000000000000000000000000..8a4829e59543903ca71da2763c0e5abfb864d98d
--- /dev/null
+++ b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/logger.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Housekeeper
+ class Logger < ::Logger
+ def initialize(...)
+ super
+
+ self.formatter = Formatter.new
+ end
+
+ def puts(*args)
+ args = [nil] if args.empty?
+ args.each { |arg| self << "#{arg}\n" }
+ end
+
+ class Formatter < ::Logger::Formatter
+ def call(severity, _time, _progname, msg)
+ format("%s: %s\n", severity, msg2str(msg))
+ end
+ end
+ end
+ end
+end
diff --git a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb
index 640e3bbeaaa82eaa6da4a6c3f0b5d5ac4903cae3..006f47c3f4de7d75f5bd98ab646f577dedcb1be6 100644
--- a/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb
+++ b/gems/gitlab-housekeeper/lib/gitlab/housekeeper/runner.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
require 'active_support/core_ext/string'
+require 'gitlab/housekeeper/logger'
require 'gitlab/housekeeper/keep'
require 'gitlab/housekeeper/keeps/rubocop_fixer'
require 'gitlab/housekeeper/gitlab_client'
@@ -34,10 +35,11 @@ def run
git.with_clean_state do
@keeps.each do |keep_class|
- keep = keep_class.new
+ @logger.puts "Running keep #{keep_class}"
+ keep = keep_class.new(logger: @logger)
keep.each_change do |change|
unless change.valid?
- puts "Ignoring invalid change from: #{keep_class}"
+ @logger.warn "Ignoring invalid change from: #{keep_class}"
next
end
@@ -56,9 +58,10 @@ def run
git.create_commit(change)
end
- puts "Skipping change: #{change.identifiers} due to not matching filter."
- puts "Modified files have been committed to branch #{branch_name.yellowish}, but will not be pushed."
- puts
+ @logger.puts "Skipping change: #{change.identifiers} due to not matching filter."
+ @logger.puts "Modified files have been committed to branch #{branch_name.yellowish}," \
+ "but will not be pushed."
+ @logger.puts
next
end
@@ -99,8 +102,8 @@ def print_completion_message(mrs_created_count)
"Housekeeper created #{mr_count_string}."
end
- puts completion_message.yellowish
- puts
+ @logger.puts completion_message.yellowish
+ @logger.puts
end
def add_standard_change_data(change)
@@ -122,28 +125,28 @@ def print_change_details(change, branch_name)
base_message = "Merge request URL: #{change.mr_web_url || '(known after create)'}, on branch #{branch_name}."
base_message << " CI skipped." if change.push_options.ci_skip
- puts base_message.yellowish
- puts "=> #{change.identifiers.join(': ')}".purple
+ @logger.puts base_message.yellowish
+ @logger.puts "=> #{change.identifiers.join(': ')}".purple
- puts '=> Title:'.purple
- puts change.title.purple
- puts
+ @logger.puts '=> Title:'.purple
+ @logger.puts change.title.purple
+ @logger.puts
- puts '=> Description:'
- puts change.description
- puts
+ @logger.puts '=> Description:'
+ @logger.puts change.description
+ @logger.puts
if change.labels.present? || change.reviewers.present?
- puts '=> Attributes:'
- puts "Labels: #{change.labels.join(', ')}"
- puts "Reviewers: #{change.reviewers.join(', ')}"
- puts
+ @logger.puts '=> Attributes:'
+ @logger.puts "Labels: #{change.labels.join(', ')}"
+ @logger.puts "Reviewers: #{change.reviewers.join(', ')}"
+ @logger.puts
end
- puts '=> Diff:'
- puts Shell.execute('git', '--no-pager', 'diff', '--color=always', @target_branch, branch_name, '--',
+ @logger.puts '=> Diff:'
+ @logger.puts Shell.execute('git', '--no-pager', 'diff', '--color=always', @target_branch, branch_name, '--',
*change.changed_files)
- puts
+ @logger.puts
end
def create(change, branch_name)
diff --git a/gems/gitlab-housekeeper/spec/gitlab/housekeeper/logger_spec.rb b/gems/gitlab-housekeeper/spec/gitlab/housekeeper/logger_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..dc222c66c8ac95cba8f8af6a858d47fef504f210
--- /dev/null
+++ b/gems/gitlab-housekeeper/spec/gitlab/housekeeper/logger_spec.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'stringio'
+require 'gitlab/housekeeper/logger'
+
+RSpec.describe ::Gitlab::Housekeeper::Logger do
+ let(:output) { StringIO.new }
+ let(:logged) { output.string }
+
+ subject(:logger) { described_class.new(output) }
+
+ describe 'formatting' do
+ it 'only prints severity and message' do
+ logger.info
+ logger.debug 42
+ logger.error RuntimeError.new('test')
+ logger.warn :hello
+
+ expect(logged).to eq(<<~OUTPUT)
+ INFO: nil
+ DEBUG: 42
+ ERROR: test (RuntimeError)
+
+ WARN: :hello
+ OUTPUT
+ end
+ end
+
+ describe '#puts' do
+ it 'mimics Kernel#puts and logs without any tags' do
+ logger.puts
+ logger.puts :hello
+ logger.puts "world"
+
+ expect(logged).to eq(<<~OUTPUT)
+
+ hello
+ world
+ OUTPUT
+ end
+ end
+
+ %i[debug info warn error fatal].each do |severity|
+ describe "##{severity}" do
+ it 'tags message with severity only' do
+ tag = severity.to_s.upcase
+
+ logger.public_send(severity, :hello)
+
+ expect(logged).to eq("#{tag}: :hello\n")
+ end
+ end
+ end
+end
diff --git a/keeps/delete_old_feature_flags.rb b/keeps/delete_old_feature_flags.rb
index 39342bf0f0d2cd11bfc28ee330eabce5ce87df96..697cc07b8fcf5a602fd7f75034a9b1dc13084052 100644
--- a/keeps/delete_old_feature_flags.rb
+++ b/keeps/delete_old_feature_flags.rb
@@ -40,7 +40,7 @@ def each_change
def prepare_change(feature_flag)
if feature_flag.milestone.nil?
- puts "#{feature_flag.name} has no milestone set!"
+ @logger.puts "#{feature_flag.name} has no milestone set!"
return
end
diff --git a/keeps/overdue_finalize_background_migration.rb b/keeps/overdue_finalize_background_migration.rb
index 0cd669ecb1bb173b01c3fe1b1bc012d386265a40..a7ebbc041a999372fdaff668a614dd83fad5f814 100644
--- a/keeps/overdue_finalize_background_migration.rb
+++ b/keeps/overdue_finalize_background_migration.rb
@@ -25,8 +25,6 @@ module Keeps
class OverdueFinalizeBackgroundMigration < ::Gitlab::Housekeeper::Keep
CUTOFF_MILESTONE = '16.8' # Only finalize migrations added before this
- def initialize; end
-
def each_change
each_batched_background_migration do |migration_yaml_file, migration|
next unless before_cuttoff_milestone?(migration['milestone'])