diff options
| -rw-r--r-- | .gitlab/ci/review.gitlab-ci.yml | 13 | ||||
| -rw-r--r-- | changelogs/unreleased/include-worker-attributes-in-sidekiq-metrics.yml | 5 | ||||
| -rw-r--r-- | doc/administration/gitaly/index.md | 4 | ||||
| -rw-r--r-- | doc/development/contributing/index.md | 4 | ||||
| -rw-r--r-- | doc/user/application_security/container_scanning/index.md | 7 | ||||
| -rw-r--r-- | doc/user/application_security/security_dashboard/index.md | 2 | ||||
| -rw-r--r-- | lib/gitlab/repository_cache_adapter.rb | 13 | ||||
| -rw-r--r-- | lib/gitlab/repository_set_cache.rb | 11 | ||||
| -rw-r--r-- | lib/gitlab/sidekiq_middleware/metrics.rb | 22 | ||||
| -rwxr-xr-x | scripts/review_apps/review-apps.sh | 50 | ||||
| -rw-r--r-- | spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb | 92 | ||||
| -rwxr-xr-x[-rw-r--r--] | vendor/gitignore/C++.gitignore | 0 | ||||
| -rwxr-xr-x[-rw-r--r--] | vendor/gitignore/Java.gitignore | 0 |
13 files changed, 123 insertions, 100 deletions
diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index a790dd3e02f..8e91d053ca0 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.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 diff --git a/changelogs/unreleased/include-worker-attributes-in-sidekiq-metrics.yml b/changelogs/unreleased/include-worker-attributes-in-sidekiq-metrics.yml new file mode 100644 index 00000000000..ea7e229d0ea --- /dev/null +++ b/changelogs/unreleased/include-worker-attributes-in-sidekiq-metrics.yml @@ -0,0 +1,5 @@ +--- +title: Add worker attributes to Sidekiq metrics +merge_request: 19491 +author: +type: other diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index 1b41d862020..a2da37dac97 100644 --- a/doc/administration/gitaly/index.md +++ b/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/ ``` diff --git a/doc/development/contributing/index.md b/doc/development/contributing/index.md index 92dd040a2bd..481a18aac3d 100644 --- a/doc/development/contributing/index.md +++ b/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) diff --git a/doc/user/application_security/container_scanning/index.md b/doc/user/application_security/container_scanning/index.md index 14dae56f087..0da9157a636 100644 --- a/doc/user/application_security/container_scanning/index.md +++ b/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. diff --git a/doc/user/application_security/security_dashboard/index.md b/doc/user/application_security/security_dashboard/index.md index 0e26206f070..17f63577f0c 100644 --- a/doc/user/application_security/security_dashboard/index.md +++ b/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. diff --git a/lib/gitlab/repository_cache_adapter.rb b/lib/gitlab/repository_cache_adapter.rb index 6d216217bdf..b2dc92ce010 100644 --- a/lib/gitlab/repository_cache_adapter.rb +++ b/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) + if strong_memoized?(name) || !redis_set_cache.exist?(name) + return __send__(name).include?(value) # rubocop:disable GitlabSecurity/PublicSend + end - __send__(name).include?(value) # rubocop:disable GitlabSecurity/PublicSend + redis_set_cache.include?(name, value) end end diff --git a/lib/gitlab/repository_set_cache.rb b/lib/gitlab/repository_set_cache.rb index 8035946090a..6d3ac53a787 100644 --- a/lib/gitlab/repository_set_cache.rb +++ b/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) diff --git a/lib/gitlab/sidekiq_middleware/metrics.rb b/lib/gitlab/sidekiq_middleware/metrics.rb index bd819843bd4..64e77a2d828 100644 --- a/lib/gitlab/sidekiq_middleware/metrics.rb +++ b/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 diff --git a/scripts/review_apps/review-apps.sh b/scripts/review_apps/review-apps.sh index fbef3ebd231..b405e992b63 100755 --- a/scripts/review_apps/review-apps.sh +++ b/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"; - ' -} diff --git a/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb index 0d8cff3a295..c9399f591da 100644 --- a/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb @@ -1,8 +1,11 @@ # 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) + 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 - expect(running_jobs_metric).to receive(:increment).with(labels, 1) - expect(running_jobs_metric).to receive(:increment).with(labels, -1) + 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 - 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 '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 - 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 } - 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) - it 'does not set the queue_duration_seconds histogram' do - middleware.call(worker, job, :test) { nil } + middleware.call(worker, job, :test) { nil } + end end - end - context 'when job is retried' do - let(:job) { { 'retry_count' => 1 } } + 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) + it 'sets sidekiq_jobs_retried_total metric' do + expect(retried_total_metric).to receive(:increment) - middleware.call(worker, job, :test) { nil } + middleware.call(worker, job, :test) { nil } + end end - end - context 'when error is raised' do - let(:job_status) { :fail } + 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) + 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") + expect { middleware.call(worker, job, :test) { raise StandardError, "Failed" } }.to raise_error(StandardError, "Failed") + end end end end diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore index 259148fa18f..259148fa18f 100644..100755 --- a/vendor/gitignore/C++.gitignore +++ b/vendor/gitignore/C++.gitignore diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore index a1c2a238a96..a1c2a238a96 100644..100755 --- a/vendor/gitignore/Java.gitignore +++ b/vendor/gitignore/Java.gitignore |