Automatic merge of gitlab-org/gitlab master

.gitlab/ci/test-metadata.gitlab-ci.yml

@@ -26,6 +26,7 @@ update-tests-metadata:
     - .test-metadata:rules:update-tests-metadata
   stage: post-test
   dependencies:
+    - retrieve-tests-metadata
     - setup-test-env
     - rspec migration pg12
     - rspec frontend_fixture

app/models/protected_branch.rb

@@ -26,7 +26,9 @@ class ProtectedBranch < ApplicationRecord
   def self.protected?(project, ref_name)
     return true if project.empty_repo? && project.default_branch_protected?
 
-    self.matching(ref_name, protected_refs: protected_refs(project)).present?
+    Rails.cache.fetch("protected_ref-#{ref_name}-#{project.cache_key}") do
+      self.matching(ref_name, protected_refs: protected_refs(project)).present?
+    end
   end
 
   def self.allow_force_push?(project, ref_name)

app/services/issues/base_service.rb

@@ -84,7 +84,8 @@ module Issues
 
     # @param object [Issue, Project]
     def issue_type_allowed?(object)
-      can?(current_user, :"create_#{params[:issue_type]}", object)
+      WorkItem::Type.base_types.key?(params[:issue_type]) &&
+        can?(current_user, :"create_#{params[:issue_type]}", object)
     end
 
     # @param issue [Issue]

doc/administration/operations/sidekiq_memory_killer.md

@@ -4,7 +4,7 @@ group: Memory
 info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
 ---
 
-# Sidekiq MemoryKiller
+# Sidekiq MemoryKiller **(FREE SELF)**
 
 The GitLab Rails application code suffers from memory leaks. For web requests
 this problem is made manageable using

ee/app/views/projects/iteration_cadences/index.html.haml

-- page_title _('Iterations|Iteration cadences')
+- page_title s_('Iterations|Iteration cadences')
 
 .js-iteration-cadence-app{ data: { full_path: @project.full_path,
   cadences_list_path: project_iteration_cadences_path(@project),

scripts/rspec_helpers.sh

 #!/usr/bin/env bash
 
 function retrieve_tests_metadata() {
-  mkdir -p knapsack/ rspec_flaky/ rspec_profiling/
+  mkdir -p $(dirname "$KNAPSACK_RSPEC_SUITE_REPORT_PATH") $(dirname "$FLAKY_RSPEC_SUITE_REPORT_PATH") rspec_profiling/
 
-  # ${CI_DEFAULT_BRANCH} might not be master in other forks but we want to
-  # always target the canonical project here, so the branch must be hardcoded
-  local project_path="gitlab-org/gitlab"
-  local artifact_branch="master"
-  local test_metadata_job_id
+  if [[ -n "${RETRIEVE_TESTS_METADATA_FROM_PAGES}" ]]; then
+    if [[ ! -f "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" ]]; then
+      curl --location -o "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" "https://gitlab-org.gitlab.io/gitlab/${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}"
+    fi
 
-  # Ruby
-  test_metadata_job_id=$(scripts/api/get_job_id.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" -q "status=success" -q "ref=${artifact_branch}" -q "username=gitlab-bot" -Q "scope=success" --job-name "update-tests-metadata")
+    if [[ ! -f "${FLAKY_RSPEC_SUITE_REPORT_PATH}" ]]; then
+      curl --location -o "${FLAKY_RSPEC_SUITE_REPORT_PATH}" "https://gitlab-org.gitlab.io/gitlab/${FLAKY_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${FLAKY_RSPEC_SUITE_REPORT_PATH}"
+    fi
+  else
+    # ${CI_DEFAULT_BRANCH} might not be master in other forks but we want to
+    # always target the canonical project here, so the branch must be hardcoded
+    local project_path="gitlab-org/gitlab"
+    local artifact_branch="master"
+    local test_metadata_job_id
 
-  if [[ ! -f "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" ]]; then
-    scripts/api/download_job_artifact.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}"
-  fi
+    # Ruby
+    test_metadata_job_id=$(scripts/api/get_job_id.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" -q "status=success" -q "ref=${artifact_branch}" -q "username=gitlab-bot" -Q "scope=success" --job-name "update-tests-metadata")
 
-  if [[ ! -f "${FLAKY_RSPEC_SUITE_REPORT_PATH}" ]]; then
-    scripts/api/download_job_artifact.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${FLAKY_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${FLAKY_RSPEC_SUITE_REPORT_PATH}"
+    if [[ ! -f "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" ]]; then
+      scripts/api/download_job_artifact.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}"
+    fi
+
+    if [[ ! -f "${FLAKY_RSPEC_SUITE_REPORT_PATH}" ]]; then
+      scripts/api/download_job_artifact.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${FLAKY_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${FLAKY_RSPEC_SUITE_REPORT_PATH}"
+    fi
   fi
 }
 
@@ -40,18 +50,24 @@ function update_tests_metadata() {
 }
 
 function retrieve_tests_mapping() {
-  mkdir -p crystalball/
+  mkdir -p $(dirname "$RSPEC_PACKED_TESTS_MAPPING_PATH")
 
-  # ${CI_DEFAULT_BRANCH} might not be master in other forks but we want to
-  # always target the canonical project here, so the branch must be hardcoded
-  local project_path="gitlab-org/gitlab"
-  local artifact_branch="master"
-  local test_metadata_with_mapping_job_id
+  if [[ -n "${RETRIEVE_TESTS_METADATA_FROM_PAGES}" ]]; then
+    if [[ ! -f "${RSPEC_PACKED_TESTS_MAPPING_PATH}" ]]; then
+      (curl --location  -o "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" "https://gitlab-org.gitlab.io/gitlab/${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" && gzip -d "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz") || echo "{}" > "${RSPEC_PACKED_TESTS_MAPPING_PATH}"
+    fi
+  else
+    # ${CI_DEFAULT_BRANCH} might not be master in other forks but we want to
+    # always target the canonical project here, so the branch must be hardcoded
+    local project_path="gitlab-org/gitlab"
+    local artifact_branch="master"
+    local test_metadata_with_mapping_job_id
 
-  test_metadata_with_mapping_job_id=$(scripts/api/get_job_id.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" -q "status=success" -q "ref=${artifact_branch}" -q "username=gitlab-bot" -Q "scope=success" --job-name "update-tests-metadata" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz")
+    test_metadata_with_mapping_job_id=$(scripts/api/get_job_id.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" -q "status=success" -q "ref=${artifact_branch}" -q "username=gitlab-bot" -Q "scope=success" --job-name "update-tests-metadata" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz")
 
-  if [[ ! -f "${RSPEC_PACKED_TESTS_MAPPING_PATH}" ]]; then
-   (scripts/api/download_job_artifact.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" --job-id "${test_metadata_with_mapping_job_id}" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" && gzip -d "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz") || echo "{}" > "${RSPEC_PACKED_TESTS_MAPPING_PATH}"
+    if [[ ! -f "${RSPEC_PACKED_TESTS_MAPPING_PATH}" ]]; then
+     (scripts/api/download_job_artifact.rb --endpoint "https://gitlab.com/api/v4" --project "${project_path}" --job-id "${test_metadata_with_mapping_job_id}" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" && gzip -d "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz") || echo "{}" > "${RSPEC_PACKED_TESTS_MAPPING_PATH}"
+    fi
   fi
 
   scripts/unpack-test-mapping "${RSPEC_PACKED_TESTS_MAPPING_PATH}" "${RSPEC_TESTS_MAPPING_PATH}"

scripts/static-analysis

@@ -14,51 +14,71 @@ class StaticAnalysis
     "Browserslist: caniuse-lite is outdated. Please run next command `yarn upgrade`"
   ].freeze
 
+  Task = Struct.new(:command, :duration) do
+    def cmd
+      command.join(' ')
+    end
+  end
+  NodeAssignment = Struct.new(:index, :tasks) do
+    def total_duration
+      return 0 if tasks.empty?
+
+      tasks.sum(&:duration)
+    end
+  end
+
   # `gettext:updated_check` and `gitlab:sidekiq:sidekiq_queues_yml:check` will fail on FOSS installations
   # (e.g. gitlab-org/gitlab-foss) since they test against a single
   # file that is generated by an EE installation, which can
   # contain values that a FOSS installation won't find. To work
   # around this we will only enable this task on EE installations.
-  TASKS_BY_DURATIONS_SECONDS_DESC = {
-    %w[bin/rake lint:haml] => 800,
+  TASKS_WITH_DURATIONS_SECONDS = [
+    Task.new(%w[bin/rake lint:haml], 562),
     # We need to disable the cache for this cop since it creates files under tmp/feature_flags/*.used,
     # the cache would prevent these files from being created.
-    %w[bundle exec rubocop --only Gitlab/MarkUsedFeatureFlags --cache false] => 600,
-    (Gitlab.ee? ? %w[bin/rake gettext:updated_check] : nil) => 360,
-    %w[yarn run lint:eslint:all] => 312,
-    %w[yarn run lint:prettier] => 162,
-    %w[bin/rake gettext:lint] => 65,
-    %w[bundle exec license_finder] => 61,
-    %w[bin/rake lint:static_verification] => 45,
-    %w[bundle exec rubocop --parallel] => 40,
-    %w[bin/rake config_lint] => 26,
-    %w[bin/rake gitlab:sidekiq:all_queues_yml:check] => 15,
-    (Gitlab.ee? ? %w[bin/rake gitlab:sidekiq:sidekiq_queues_yml:check] : nil) => 11,
-    %w[yarn run internal:stylelint] => 8,
-    %w[scripts/lint-conflicts.sh] => 1,
-    %w[yarn run block-dependencies] => 1,
-    %w[scripts/lint-rugged] => 1,
-    %w[scripts/gemfile_lock_changed.sh] => 1,
-    %w[scripts/frontend/check_no_partial_karma_jest.sh] => 1
-  }.reject { |k| k.nil? }.sort_by { |a| -a[1] }.to_h.keys.freeze
-
-  def run_tasks!
-    tasks = tasks_to_run((ENV['CI_NODE_INDEX'] || 1).to_i, (ENV['CI_NODE_TOTAL'] || 1).to_i)
+    Task.new(%w[bundle exec rubocop --only Gitlab/MarkUsedFeatureFlags --cache false], 800),
+    (Gitlab.ee? ? Task.new(%w[bin/rake gettext:updated_check], 360) : nil),
+    Task.new(%w[yarn run lint:eslint:all], 312),
+    Task.new(%w[bundle exec rubocop --parallel], 60),
+    Task.new(%w[yarn run lint:prettier], 160),
+    Task.new(%w[bin/rake gettext:lint], 85),
+    Task.new(%w[bundle exec license_finder], 20),
+    Task.new(%w[bin/rake lint:static_verification], 35),
+    Task.new(%w[bin/rake config_lint], 10),
+    Task.new(%w[bin/rake gitlab:sidekiq:all_queues_yml:check], 15),
+    (Gitlab.ee? ? Task.new(%w[bin/rake gitlab:sidekiq:sidekiq_queues_yml:check], 11) : nil),
+    Task.new(%w[yarn run internal:stylelint], 8),
+    Task.new(%w[scripts/lint-conflicts.sh], 1),
+    Task.new(%w[yarn run block-dependencies], 1),
+    Task.new(%w[scripts/lint-rugged], 1),
+    Task.new(%w[scripts/gemfile_lock_changed.sh], 1),
+    Task.new(%w[scripts/frontend/check_no_partial_karma_jest.sh], 1)
+  ].compact.freeze
+
+  def run_tasks!(options = {})
+    node_assignment = tasks_to_run((ENV['CI_NODE_TOTAL'] || 1).to_i)[(ENV['CI_NODE_INDEX'] || 1).to_i - 1]
+
+    if options[:dry_run]
+      puts "Dry-run mode!"
+      return
+    end
 
     static_analysis = Gitlab::Popen::Runner.new
-
-    static_analysis.run(tasks) do |cmd, &run|
+    start_time = Time.now
+    static_analysis.run(node_assignment.tasks.map(&:command)) do |command, &run|
+      task = node_assignment.tasks.find { |task| task.command == command }
       puts
-      puts "$ #{cmd.join(' ')}"
+      puts "$ #{task.cmd}"
 
       result = run.call
 
-      puts "==> Finished in #{result.duration} seconds"
+      puts "==> Finished in #{result.duration} seconds (expected #{task.duration} seconds)"
       puts
     end
 
     puts
     puts '==================================================='
+    puts "Node finished running all tasks in #{Time.now - start_time} seconds (expected #{node_assignment.total_duration})"
     puts
     puts
 
@@ -107,16 +127,66 @@ class StaticAnalysis
       .count { |result| !ALLOWED_WARNINGS.include?(result.stderr.strip) }
   end
 
-  def tasks_to_run(node_index, node_total)
-    tasks = []
-    TASKS_BY_DURATIONS_SECONDS_DESC.each_with_index do |task, i|
-      tasks << task if i % node_total == (node_index - 1)
+  def tasks_to_run(node_total)
+    total_time = TASKS_WITH_DURATIONS_SECONDS.sum(&:duration).to_f
+    ideal_time_per_node = total_time / node_total
+    tasks_by_duration_desc = TASKS_WITH_DURATIONS_SECONDS.sort_by { |a| -a.duration }
+    nodes = Array.new(node_total) { |i| NodeAssignment.new(i + 1, []) }
+
+    puts "Total expected time: #{total_time}; ideal time per job: #{ideal_time_per_node}.\n\n"
+    puts "Tasks to distribute:"
+    tasks_by_duration_desc.each { |task| puts "* #{task.cmd} (#{task.duration}s)" }
+
+    # Distribute tasks optimally first
+    puts "\nAssigning tasks optimally."
+    distribute_tasks(tasks_by_duration_desc, nodes, ideal_time_per_node: ideal_time_per_node)
+
+    # Distribute remaining tasks, ordered by ascending duration
+    leftover_tasks = tasks_by_duration_desc - nodes.flat_map(&:tasks)
+
+    if leftover_tasks.any?
+      puts "\n\nAssigning remaining tasks: #{leftover_tasks.flat_map(&:cmd)}"
+      distribute_tasks(leftover_tasks, nodes.sort_by { |node| node.total_duration })
+    end
+
+    nodes.each do |node|
+      puts "\nExpected duration for node #{node.index}: #{node.total_duration} seconds"
+      node.tasks.each { |task| puts "* #{task.cmd} (#{task.duration}s)" }
+    end
+
+    nodes
+  end
+
+  def distribute_tasks(tasks, nodes, ideal_time_per_node: nil)
+    condition =
+      if ideal_time_per_node
+        ->(task, node, ideal_time_per_node) { (task.duration + node.total_duration) <= ideal_time_per_node }
+      else
+        ->(*) { true }
+      end
+
+    tasks.each do |task|
+      nodes.each do |node|
+        if condition.call(task, node, ideal_time_per_node)
+          assign_task_to_node(tasks, node, task)
+          break
+        end
+      end
     end
+  end
 
-    tasks
+  def assign_task_to_node(remaining_tasks, node, task)
+    node.tasks << task
+    puts "Assigning #{task.command} (#{task.duration}s) to node ##{node.index}. Node total duration: #{node.total_duration}s."
   end
 end
 
 if $0 == __FILE__
-  StaticAnalysis.new.run_tasks!
+  options = {}
+
+  if ARGV.include?('--dry-run')
+    options[:dry_run] = true
+  end
+
+  StaticAnalysis.new.run_tasks!(options)
 end

spec/models/protected_branch_spec.rb

@@ -162,6 +162,30 @@ RSpec.describe ProtectedBranch do
 
         expect(described_class.protected?(project, 'staging/some-branch')).to eq(false)
       end
+
+      context 'with caching', :use_clean_rails_memory_store_caching do
+        let_it_be(:project) { create(:project, :repository) }
+        let_it_be(:protected_branch) { create(:protected_branch, project: project, name: "jawn") }
+
+        before do
+          allow(described_class).to receive(:matching).once.and_call_original
+          # the original call works and warms the cache
+          described_class.protected?(project, 'jawn')
+        end
+
+        it 'correctly invalidates a cache' do
+          expect(described_class).to receive(:matching).once.and_call_original
+
+          create(:protected_branch, project: project, name: "bar")
+          # the cache is invalidated because the project has been "updated"
+          expect(described_class.protected?(project, 'jawn')).to eq(true)
+        end
+
+        it 'correctly uses the cached version' do
+          expect(described_class).not_to receive(:matching)
+          expect(described_class.protected?(project, 'jawn')).to eq(true)
+        end
+      end
     end
 
     context 'new project' do

spec/services/issues/build_service_spec.rb

@@ -183,8 +183,8 @@ RSpec.describe Issues::BuildService do
           expect(issue).to be_incident
         end
 
-        it 'cannot set invalid type' do
-          issue = build_issue(issue_type: 'invalid type')
+        it 'cannot set invalid issue type' do
+          issue = build_issue(issue_type: 'project')
 
           expect(issue).to be_issue
         end