Add latest changes from gitlab-org/gitlab@master

.gitlab/ci/review.gitlab-ci.yml

@@ -94,7 +94,10 @@ schedule:review-build-cng:
   variables:
     HOST_SUFFIX: "${CI_ENVIRONMENT_SLUG}"
     DOMAIN: "-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN}"
-    GITLAB_HELM_CHART_REF: "v2.4.4"
+    # v2.4.4 + two improvements:
+    # - Allow to pass an EE license when installing the chart: https://gitlab.com/gitlab-org/charts/gitlab/merge_requests/1008
+    # - Allow to customize the livenessProbe for `gitlab-shell`: https://gitlab.com/gitlab-org/charts/gitlab/merge_requests/1021
+    GITLAB_HELM_CHART_REF: "6c655ed77e60f1f7f533afb97bef8c9cb7dc61eb"
     GITLAB_EDITION: "ce"
   environment:
     name: review/${CI_COMMIT_REF_NAME}
@@ -116,21 +119,13 @@ schedule:review-build-cng:
     - source scripts/review_apps/review-apps.sh
     - export REVIEW_APP_CONFIG_CHANGED=$(base_config_changed)
   script:
-    - date
     - check_kube_domain
-    - date
     - ensure_namespace
-    - date
     - install_tiller
-    - date
     - install_external_dns
-    - date
     - download_chart
     - date
     - deploy || (display_deployment_debug && exit 1)
-    - date
-    - add_license
-    - date
   artifacts:
     paths: [review_app_url.txt]
     expire_in: 2 days

changelogs/unreleased/include-worker-attributes-in-sidekiq-metrics.yml

+---
+title: Add worker attributes to Sidekiq metrics
+merge_request: 19491
+author:
+type: other

doc/administration/gitaly/index.md

@@ -407,11 +407,11 @@ To configure Gitaly with TLS:
    ```
 1. Save the file and [reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure) on client node(s).
-1. Create the `/etc/gitlab/ssl` directory and copy your key and certificate there:
+1. On the Gitaly server, create the `/etc/gitlab/ssl` directory and copy your key and certificate there:
    ```sh
    sudo mkdir -p /etc/gitlab/ssl
-   sudo chmod 700 /etc/gitlab/ssl
+   sudo chmod 755 /etc/gitlab/ssl
    sudo cp key.pem cert.pem /etc/gitlab/ssl/
    ```

doc/development/contributing/index.md

@@ -118,6 +118,10 @@ This [documentation](merge_request_workflow.md) outlines the current merge reque
 This [documentation](style_guides.md) outlines the current style guidelines.
+## Getting an Enterprise Edition License
+
+If you need a license for contributing to an EE-feature, please [follow these instructions](https://about.gitlab.com/handbook/marketing/community-relations/code-contributor-program/#for-contributors-to-the-gitlab-enterprise-edition-ee).
+
 ---
 [Return to Development documentation](../README.md)

doc/user/application_security/container_scanning/index.md

@@ -40,10 +40,9 @@ to perform audits for your Docker-based apps.
 To enable Container Scanning in your pipeline, you need:
 - A GitLab Runner with the
-  [`docker`](https://docs.gitlab.com/runner/executors/docker.html#use-docker-in-docker-with-privileged-mode) or
-  [`kubernetes`](https://docs.gitlab.com/runner/install/kubernetes.html#running-privileged-containers-for-the-runners)
-  executor running in privileged mode. If you're using the shared Runners on GitLab.com,
-  this is enabled by default.
+  [`docker`](https://docs.gitlab.com/runner/executors/docker.html) or
+  [`kubernetes`](https://docs.gitlab.com/runner/install/kubernetes.html)
+  executor.
 - Docker `18.09.03` or higher installed on the machine where the Runners are
   running. If you're using the shared Runners on GitLab.com, this is already
   the case.

doc/user/application_security/security_dashboard/index.md

@@ -71,7 +71,7 @@ Once you're on the dashboard, at the top you should see a series of filters for:
 - Report type
 - Project
-To the right of the filters, you should see a **Hide dismissed** toggle button.
+To the right of the filters, you should see a **Hide dismissed** toggle button ([available in GitLab Ultimate 12.5](https://gitlab.com/gitlab-org/gitlab/issues/9102)).
 NOTE: **Note:**
 The dashboard only shows projects with [security reports](#supported-reports) enabled in a group.

lib/gitlab/repository_cache_adapter.rb

@@ -58,16 +58,11 @@ module Gitlab
         # wrong answer. We handle that by querying the full list - which fills
         # the cache - and using it directly to answer the question.
         define_method("#{name}_include?") do |value|
-          return __send__(name).include?(value) if strong_memoized?(name) # rubocop:disable GitlabSecurity/PublicSend
-
-          # If the member exists in the set, return as such early.
-          return true if redis_set_cache.include?(name, value)
-
-          # If it did not, make sure the collection exists.
-          # If the collection exists, then item does not.
-          return false if redis_set_cache.exist?(name)
-          __send__(name).include?(value) # rubocop:disable GitlabSecurity/PublicSend
+          if strong_memoized?(name) || !redis_set_cache.exist?(name)
+            return __send__(name).include?(value) # rubocop:disable GitlabSecurity/PublicSend
+          end
+          redis_set_cache.include?(name, value)
         end
       end

lib/gitlab/repository_set_cache.rb

@@ -25,7 +25,7 @@ module Gitlab
     end
     def read(key)
-      with { |redis| redis.sscan_each(cache_key(key)).to_a }
+      with { |redis| redis.smembers(cache_key(key)) }
     end
     def write(key, value)
@@ -47,10 +47,11 @@ module Gitlab
     end
     def fetch(key, &block)
-      result = read(key)
-      return result unless result.empty?
-
-      write(key, yield)
+      if exist?(key)
+        read(key)
+      else
+        write(key, yield)
+      end
     end
     def include?(key, value)

lib/gitlab/sidekiq_middleware/metrics.rb

@@ -13,8 +13,8 @@ module Gitlab
         @metrics[:sidekiq_concurrency].set({}, Sidekiq.options[:concurrency].to_i)
       end
-      def call(_worker, job, queue)
-        labels = create_labels(queue)
+      def call(worker, job, queue)
+        labels = create_labels(worker, queue)
         queue_duration = ::Gitlab::InstrumentationHelper.queue_duration_for_job(job)
         @metrics[:sidekiq_jobs_queue_duration_seconds].observe(labels, queue_duration) if queue_duration
@@ -62,10 +62,20 @@ module Gitlab
         }
       end
-      def create_labels(queue)
-        {
-          queue: queue
-        }
+      def create_labels(worker, queue)
+        labels = { queue: queue }
+        return labels unless worker.include? WorkerAttributes
+
+        labels[:latency_sensitive] = true if worker.latency_sensitive_worker?
+        labels[:external_deps] = true if worker.worker_has_external_dependencies?
+
+        feature_category = worker.get_feature_category
+        labels[:feat_cat] = feature_category if feature_category
+
+        resource_boundary = worker.get_worker_resource_boundary
+        labels[:boundary] = resource_boundary if resource_boundary && resource_boundary != :unknown
+
+        labels
       end
       def get_thread_cputime

scripts/review_apps/review-apps.sh

@@ -179,6 +179,17 @@ function create_application_secret() {
     "${CI_ENVIRONMENT_SLUG}-gitlab-initial-root-password" \
     --from-literal="password=${REVIEW_APPS_ROOT_PASSWORD}" \
     --dry-run -o json | kubectl apply -f -
+
+  if [ -z "${REVIEW_APPS_EE_LICENSE}" ]; then echo "License not found" && return; fi
+
+  echoinfo "Creating the ${CI_ENVIRONMENT_SLUG}-gitlab-license secret in the ${KUBE_NAMESPACE} namespace..." true
+
+  echo "${REVIEW_APPS_EE_LICENSE}" > /tmp/license.gitlab
+
+  kubectl create secret generic -n "$KUBE_NAMESPACE" \
+    "${CI_ENVIRONMENT_SLUG}-gitlab-license" \
+    --from-file=license=/tmp/license.gitlab \
+    --dry-run -o json | kubectl apply -f -
 }
 function download_chart() {
@@ -252,6 +263,14 @@ HELM_CMD=$(cat << EOF
 EOF
 )
+if [ -n "${REVIEW_APPS_EE_LICENSE}" ]; then
+HELM_CMD=$(cat << EOF
+  ${HELM_CMD} \
+  --set global.gitlab.license.secret="${CI_ENVIRONMENT_SLUG}-gitlab-license"
+EOF
+)
+fi
+
 HELM_CMD=$(cat << EOF
   ${HELM_CMD} \
   --namespace="$KUBE_NAMESPACE" \
@@ -276,34 +295,3 @@ function display_deployment_debug() {
   echoinfo "Unsuccessful Jobs for release ${CI_ENVIRONMENT_SLUG}"
   kubectl get jobs -n "$KUBE_NAMESPACE" -lrelease=${CI_ENVIRONMENT_SLUG} --field-selector=status.successful!=1
 }
-
-function add_license() {
-  if [ -z "${REVIEW_APPS_EE_LICENSE}" ]; then echo "License not found" && return; fi
-
-  task_runner_pod=$(get_pod "task-runner");
-  if [ -z "${task_runner_pod}" ]; then echo "Task runner pod not found" && return; fi
-
-  echoinfo "Installing license..." true
-
-  echo "${REVIEW_APPS_EE_LICENSE}" > /tmp/license.gitlab
-  kubectl -n "$KUBE_NAMESPACE" cp /tmp/license.gitlab "${task_runner_pod}":/tmp/license.gitlab
-  rm /tmp/license.gitlab
-
-  kubectl -n "$KUBE_NAMESPACE" exec -it "${task_runner_pod}" -- /srv/gitlab/bin/rails runner -e production \
-    '
-    content = File.read("/tmp/license.gitlab").strip;
-    FileUtils.rm_f("/tmp/license.gitlab");
-
-    unless License.where(data:content).empty?
-      puts "License already exists";
-      Kernel.exit 0;
-    end
-
-    unless License.new(data: content).save
-      puts "Could not add license";
-      Kernel.exit 0;
-    end
-
-    puts "License added";
-    '
-}

spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb

 # frozen_string_literal: true
 require 'fast_spec_helper'
+require 'rspec-parameterized'
 describe Gitlab::SidekiqMiddleware::Metrics do
+  using RSpec::Parameterized::TableSyntax
+
   let(:middleware) { described_class.new }
   let(:concurrency_metric) { double('concurrency metric') }
@@ -45,7 +48,7 @@ describe Gitlab::SidekiqMiddleware::Metrics do
     let(:job) { {} }
     let(:job_status) { :done }
     let(:labels) { { queue: :test } }
-    let(:labels_with_job_status) { { queue: :test, job_status: job_status } }
+    let(:labels_with_job_status) { labels.merge(job_status: job_status) }
     let(:thread_cputime_before) { 1 }
     let(:thread_cputime_after) { 2 }
@@ -57,52 +60,75 @@ describe Gitlab::SidekiqMiddleware::Metrics do
     let(:queue_duration_for_job) { 0.01 }
-    before do
-      allow(middleware).to receive(:get_thread_cputime).and_return(thread_cputime_before, thread_cputime_after)
-      allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after)
-      allow(Gitlab::InstrumentationHelper).to receive(:queue_duration_for_job).with(job).and_return(queue_duration_for_job)
-      expect(running_jobs_metric).to receive(:increment).with(labels, 1)
-      expect(running_jobs_metric).to receive(:increment).with(labels, -1)
-      expect(queue_duration_seconds).to receive(:observe).with(labels, queue_duration_for_job) if queue_duration_for_job
-      expect(user_execution_seconds_metric).to receive(:observe).with(labels_with_job_status, thread_cputime_duration)
-      expect(completion_seconds_metric).to receive(:observe).with(labels_with_job_status, monotonic_time_duration)
-    end
-    it 'yields block' do
-      expect { |b| middleware.call(worker, job, :test, &b) }.to yield_control.once
-    end
-    it 'sets queue specific metrics' do
-      middleware.call(worker, job, :test) { nil }
-    end
-    context 'when job_duration is not available' do
-      let(:queue_duration_for_job) { nil }
-      it 'does not set the queue_duration_seconds histogram' do
-        middleware.call(worker, job, :test) { nil }
+    where(:worker_has_attributes, :worker_is_latency_sensitive, :worker_has_external_dependencies, :worker_feature_category, :worker_resource_boundary, :labels) do
+      false | false | false | nil             | nil      | { queue: :test }
+      true  | false | false | nil             | nil      | { queue: :test }
+      true  | true  | false | nil             | nil      | { queue: :test, latency_sensitive: true }
+      true  | false | true  | nil             | nil      | { queue: :test, external_deps: true }
+      true  | false | false | :authentication | nil      | { queue: :test, feat_cat: :authentication }
+      true  | false | false | nil             | :cpu     | { queue: :test, boundary: :cpu }
+      true  | false | false | nil             | :memory  | { queue: :test, boundary: :memory }
+      true  | false | false | nil             | :unknown | { queue: :test }
+      true  | true  | true  | :authentication | :cpu     | { queue: :test, latency_sensitive: true, external_deps: true, feat_cat: :authentication, boundary: :cpu }
+    end
+    with_them do
+      before do
+        allow(middleware).to receive(:get_thread_cputime).and_return(thread_cputime_before, thread_cputime_after)
+        allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after)
+        allow(Gitlab::InstrumentationHelper).to receive(:queue_duration_for_job).with(job).and_return(queue_duration_for_job)
+
+        # Attributes
+        allow(worker).to receive(:include?).with(WorkerAttributes).and_return(worker_has_attributes)
+        allow(worker).to receive(:latency_sensitive_worker?).and_return(worker_is_latency_sensitive)
+        allow(worker).to receive(:worker_has_external_dependencies?).and_return(worker_has_external_dependencies)
+        allow(worker).to receive(:get_worker_resource_boundary).and_return(worker_resource_boundary)
+        allow(worker).to receive(:get_feature_category).and_return(worker_feature_category)
+
+        expect(running_jobs_metric).to receive(:increment).with(labels, 1)
+        expect(running_jobs_metric).to receive(:increment).with(labels, -1)
+
+        expect(queue_duration_seconds).to receive(:observe).with(labels, queue_duration_for_job) if queue_duration_for_job
+        expect(user_execution_seconds_metric).to receive(:observe).with(labels_with_job_status, thread_cputime_duration)
+        expect(completion_seconds_metric).to receive(:observe).with(labels_with_job_status, monotonic_time_duration)
+      end
+      it 'yields block' do
+        expect { |b| middleware.call(worker, job, :test, &b) }.to yield_control.once
+      end
+      it 'sets queue specific metrics' do
+        middleware.call(worker, job, :test) { nil }
+      end
+      context 'when job_duration is not available' do
+        let(:queue_duration_for_job) { nil }
+        it 'does not set the queue_duration_seconds histogram' do
+          expect(queue_duration_seconds).not_to receive(:observe)
+          middleware.call(worker, job, :test) { nil }
+        end
       end
-    end
-    context 'when job is retried' do
-      let(:job) { { 'retry_count' => 1 } }
-      it 'sets sidekiq_jobs_retried_total metric' do
-        expect(retried_total_metric).to receive(:increment)
-        middleware.call(worker, job, :test) { nil }
+      context 'when job is retried' do
+        let(:job) { { 'retry_count' => 1 } }
+        it 'sets sidekiq_jobs_retried_total metric' do
+          expect(retried_total_metric).to receive(:increment)
+          middleware.call(worker, job, :test) { nil }
+        end
       end
-    end
-    context 'when error is raised' do
-      let(:job_status) { :fail }
-      it 'sets sidekiq_jobs_failed_total and reraises' do
-        expect(failed_total_metric).to receive(:increment).with(labels, 1)
-        expect { middleware.call(worker, job, :test) { raise StandardError, "Failed" } }.to raise_error(StandardError, "Failed")
+      context 'when error is raised' do
+        let(:job_status) { :fail }
+        it 'sets sidekiq_jobs_failed_total and reraises' do
+          expect(failed_total_metric).to receive(:increment).with(labels, 1)
+          expect { middleware.call(worker, job, :test) { raise StandardError, "Failed" } }.to raise_error(StandardError, "Failed")
+        end
       end
     end
   end