diff options
60 files changed, 759 insertions, 148 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a54b38d284d..fd3a9ce0986 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -180,6 +180,7 @@ update-knapsack: <<: *only-canonical-masters stage: post-test script: + - retry gem install fog-aws mime-types - scripts/merge-reports ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/rspec-pg_node_*.json - scripts/merge-reports ${KNAPSACK_SPINACH_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/spinach-pg_node_*.json - '[[ -z ${KNAPSACK_S3_BUCKET} ]] || scripts/sync-reports put $KNAPSACK_S3_BUCKET $KNAPSACK_RSPEC_SUITE_REPORT_PATH $KNAPSACK_SPINACH_SUITE_REPORT_PATH' diff --git a/CHANGELOG.md b/CHANGELOG.md index c181aba0205..b6f955553e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 9.3.7 (2017-07-18) + +- Prevent bad data being added to application settings when Redis is unavailable. !12750 +- Return `is_admin` attribute in the GET /user endpoint for admins. !12811 + ## 9.3.6 (2017-07-12) - Fix API Scoping. !12300 diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c5523bd09b1..59dad104b0b 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.17.0 +0.21.2 @@ -390,3 +390,6 @@ gem 'toml-rb', '~> 0.3.15', require: false # Feature toggles gem 'flipper', '~> 0.10.2' gem 'flipper-active_record', '~> 0.10.2' + +# Structured logging +gem 'lograge', '~> 0.5' diff --git a/Gemfile.lock b/Gemfile.lock index df14fbec2c6..63eb0712ef3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -443,6 +443,10 @@ GEM logging (2.2.2) little-plugger (~> 1.1) multi_json (~> 1.10) + lograge (0.5.1) + actionpack (>= 4, < 5.2) + activesupport (>= 4, < 5.2) + railties (>= 4, < 5.2) loofah (2.0.3) nokogiri (>= 1.5.9) mail (2.6.5) @@ -998,6 +1002,7 @@ DEPENDENCIES letter_opener_web (~> 1.3.0) license_finder (~> 2.1.0) licensee (~> 8.7.0) + lograge (~> 0.5) loofah (~> 2.0.3) mail_room (~> 0.9.1) method_source (~> 0.8) diff --git a/app/assets/stylesheets/framework/avatar.scss b/app/assets/stylesheets/framework/avatar.scss index 4ae2b164d2e..06f7af33f94 100644 --- a/app/assets/stylesheets/framework/avatar.scss +++ b/app/assets/stylesheets/framework/avatar.scss @@ -60,7 +60,7 @@ } &:not([href]):hover { - border-color: rgba($avatar-border, .2); + border-color: darken($avatar-border, 10%); } } @@ -99,7 +99,7 @@ .avatar-counter { background-color: $gray-darkest; color: $white-light; - border: 1px solid $border-color; + border: 1px solid $avatar-border; border-radius: 1em; font-family: $regular_font; font-size: 9px; diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index fbf49f3a813..5e410cbf563 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -205,6 +205,7 @@ @media (max-width: $screen-sm-min) { width: 100%; + min-width: 180px; } &.dropdown-open-left { @@ -288,12 +289,6 @@ padding: 5px 8px; color: $gl-text-color-secondary; } - - .badge { - position: absolute; - right: 8px; - top: 5px; - } } .droplab-dropdown { @@ -466,10 +461,6 @@ left: auto; right: 0; margin-top: -5px; - - @media (max-width: $screen-xs-max) { - left: 0; - } } .dropdown-menu-selectable { diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index e59cd0eea82..868e65a8f46 100644 --- a/app/assets/stylesheets/framework/lists.scss +++ b/app/assets/stylesheets/framework/lists.scss @@ -236,6 +236,8 @@ ul.content-list { ul.controls { float: right; list-style: none; + display: flex; + align-items: center; .btn { padding: 10px 14px; @@ -259,6 +261,12 @@ ul.controls { } } } + + .issuable-pipeline-broken a, + .issuable-pipeline-status a, + .author_link { + display: flex; + } } ul.indent-list { diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index 28b2a7cfacd..e71bf04aec7 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -325,7 +325,7 @@ position: absolute; top: 7px; right: 15px; - z-index: 2; + z-index: 300; li.active { font-weight: bold; diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 785b09e622f..77b7d901f9a 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -2,6 +2,10 @@ color: $gl-text-color; word-wrap: break-word; + [dir="auto"] { + text-align: initial; + } + a { color: $md-link-color; } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 8bd69faf84c..7016208f624 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -379,7 +379,7 @@ $issue-boards-card-shadow: rgba(186, 186, 186, 0.5); * Avatar */ $avatar_radius: 50%; -$avatar-border: rgba(0, 0, 0, .1); +$avatar-border: $border-color; $gl-avatar-size: 40px; /* diff --git a/app/assets/stylesheets/new_sidebar.scss b/app/assets/stylesheets/new_sidebar.scss index 82cabefa129..bd9a5d7392d 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -65,7 +65,6 @@ $new-sidebar-width: 220px; .settings-avatar { background-color: $white-light; - transition: background-color 100ms linear; i { font-size: 20px; @@ -73,7 +72,6 @@ $new-sidebar-width: 220px; color: $gl-text-color-secondary; text-align: center; align-self: center; - transition: color 100ms linear; } } @@ -90,6 +88,7 @@ $new-sidebar-width: 220px; box-shadow: inset -2px 0 0 $border-color; a { + transition: none; text-decoration: none; } @@ -177,7 +176,6 @@ $new-sidebar-width: 220px; color: $hover-color; .badge { - transition: background-color 100ms linear, color 100ms linear; background-color: $indigo-500; color: $hover-color; } diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index e858ef427b0..aa04e490649 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -346,13 +346,9 @@ display: none; } - .avatar:hover, - .avatar-counter:hover { - border-color: $issuable-sidebar-color; - } - .avatar-counter:hover { color: $issuable-sidebar-color; + border-color: $issuable-sidebar-color; } .btn-clipboard { diff --git a/app/models/commit.rb b/app/models/commit.rb index 21b906e1110..1e19f00106a 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -1,5 +1,6 @@ class Commit extend ActiveModel::Naming + extend Gitlab::Cache::RequestCache include ActiveModel::Conversion include Noteable @@ -169,19 +170,9 @@ class Commit end def author - if RequestStore.active? - key = "commit_author:#{author_email.downcase}" - # nil is a valid value since no author may exist in the system - if RequestStore.store.key?(key) - @author = RequestStore.store[key] - else - @author = find_author_by_any_email - RequestStore.store[key] = @author - end - else - @author ||= find_author_by_any_email - end + User.find_by_any_email(author_email.downcase) end + request_cache(:author) { author_email.downcase } def committer @committer ||= User.find_by_any_email(committer_email.downcase) @@ -368,10 +359,6 @@ class Commit end end - def find_author_by_any_email - User.find_by_any_email(author_email.downcase) - end - def repo_changes changes = { added: [], modified: [], removed: [] } diff --git a/app/models/note.rb b/app/models/note.rb index 3d39047d32f..d0e3bc0bfed 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -190,7 +190,7 @@ class Note < ActiveRecord::Base # override to return commits, which are not active record def noteable if for_commit? - project.commit(commit_id) + @commit ||= project.commit(commit_id) else super end diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index ac222ad8c82..be7d27df2a0 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/app/views/layouts/nav/_dashboard.html.haml @@ -42,18 +42,18 @@ .key = icon('arrow-up', 'aria-label' => 'hidden') I + %span.badge.pull-right= number_with_delimiter(assigned_issuables_count(:issues)) %span Issues - .badge= number_with_delimiter(assigned_issuables_count(:issues)) = nav_link(path: 'dashboard#merge_requests') do = link_to assigned_mrs_dashboard_path, title: 'Merge Requests', class: 'dashboard-shortcuts-merge_requests' do .shortcut-mappings .key = icon('arrow-up', 'aria-label' => 'hidden') M + %span.badge.pull-right= number_with_delimiter(assigned_issuables_count(:merge_requests)) %span Merge Requests - .badge= number_with_delimiter(assigned_issuables_count(:merge_requests)) = nav_link(controller: 'dashboard/snippets') do = link_to dashboard_snippets_path, class: 'dashboard-shortcuts-snippets', title: 'Snippets' do .shortcut-mappings diff --git a/app/views/layouts/nav/_new_group_sidebar.html.haml b/app/views/layouts/nav/_new_group_sidebar.html.haml index c80308ed0de..6e0c45739f1 100644 --- a/app/views/layouts/nav/_new_group_sidebar.html.haml +++ b/app/views/layouts/nav/_new_group_sidebar.html.haml @@ -6,15 +6,15 @@ = @group.name %ul.sidebar-top-level-items = nav_link(path: ['groups#show', 'groups#activity', 'groups#subgroups'], html_options: { class: 'home' }) do - = link_to group_path(@group), title: 'Home' do + = link_to group_path(@group), title: 'About group' do %span - Group + About %ul.sidebar-sub-level-items = nav_link(path: ['groups#show', 'groups#subgroups'], html_options: { class: 'home' }) do - = link_to group_path(@group), title: 'Group Home' do + = link_to group_path(@group), title: 'Group details' do %span - Home + Details = nav_link(path: 'groups#activity') do = link_to activity_group_path(@group), title: 'Activity' do diff --git a/app/views/layouts/nav/_new_project_sidebar.html.haml b/app/views/layouts/nav/_new_project_sidebar.html.haml index 7c9822c5a6a..882123c0b0a 100644 --- a/app/views/layouts/nav/_new_project_sidebar.html.haml +++ b/app/views/layouts/nav/_new_project_sidebar.html.haml @@ -7,14 +7,14 @@ = @project.name %ul.sidebar-top-level-items = nav_link(path: ['projects#show', 'projects#activity', 'cycle_analytics#show'], html_options: { class: 'home' }) do - = link_to project_path(@project), title: 'Project', class: 'shortcuts-project' do + = link_to project_path(@project), title: 'About project', class: 'shortcuts-project' do %span - Project + About %ul.sidebar-sub-level-items = nav_link(path: 'projects#show') do - = link_to project_path(@project), title: _('Project home'), class: 'shortcuts-project' do - %span= _('Home') + = link_to project_path(@project), title: _('Project details'), class: 'shortcuts-project' do + %span= _('Details') = nav_link(path: 'projects#activity') do = link_to activity_project_path(@project), title: _('Activity'), class: 'shortcuts-project-activity' do diff --git a/app/views/projects/artifacts/browse.html.haml b/app/views/projects/artifacts/browse.html.haml index 576e5b385af..a33743c2f57 100644 --- a/app/views/projects/artifacts/browse.html.haml +++ b/app/views/projects/artifacts/browse.html.haml @@ -5,12 +5,6 @@ .tree-holder .nav-block - .tree-controls - = link_to download_project_job_artifacts_path(@project, @build), - rel: 'nofollow', download: '', class: 'btn btn-default download' do - = icon('download') - Download artifacts archive - %ul.breadcrumb.repo-breadcrumb %li = link_to 'Artifacts', browse_project_job_artifacts_path(@project, @build) @@ -18,6 +12,12 @@ %li = link_to truncate(title, length: 40), browse_project_job_artifacts_path(@project, @build, path) + .tree-controls + = link_to download_project_job_artifacts_path(@project, @build), + rel: 'nofollow', download: '', class: 'btn btn-default download' do + = icon('download') + Download artifacts archive + .tree-content-holder %table.table.tree-table %thead diff --git a/changelogs/unreleased/12892-reset-css-text-align-to-initial-for-rtl.md b/changelogs/unreleased/12892-reset-css-text-align-to-initial-for-rtl.md new file mode 100644 index 00000000000..87e95240bba --- /dev/null +++ b/changelogs/unreleased/12892-reset-css-text-align-to-initial-for-rtl.md @@ -0,0 +1,4 @@ +--- +title: "reset text-align to initial to let elements with dir="auto" align texts to right in RTL languages ( default is left )" +merge_request: 12892 +author: goshhob diff --git a/changelogs/unreleased/34325-reinstate-is_admin-for-user-api.yml b/changelogs/unreleased/34325-reinstate-is_admin-for-user-api.yml deleted file mode 100644 index 3bed1fbe16e..00000000000 --- a/changelogs/unreleased/34325-reinstate-is_admin-for-user-api.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Return `is_admin` attribute in the GET /user endpoint for admins -merge_request: 12811 -author: diff --git a/changelogs/unreleased/34468-remove-extra-blank-on-admin-on-mobile.yml b/changelogs/unreleased/34468-remove-extra-blank-on-admin-on-mobile.yml new file mode 100644 index 00000000000..99291b4c75a --- /dev/null +++ b/changelogs/unreleased/34468-remove-extra-blank-on-admin-on-mobile.yml @@ -0,0 +1,4 @@ +--- +title: Use smaller min-width for dropdown-menu-nav only on mobile +merge_request: 12528 +author: Takuya Noguchi diff --git a/changelogs/unreleased/34728-fix-application-setting-created-when-redis-down.yml b/changelogs/unreleased/34728-fix-application-setting-created-when-redis-down.yml deleted file mode 100644 index 4fddabebf36..00000000000 --- a/changelogs/unreleased/34728-fix-application-setting-created-when-redis-down.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Prevent bad data being added to application settings when Redis is unavailable -merge_request: 12750 -author: diff --git a/changelogs/unreleased/35087-mr-status-misaligned.yml b/changelogs/unreleased/35087-mr-status-misaligned.yml new file mode 100644 index 00000000000..3be43125a61 --- /dev/null +++ b/changelogs/unreleased/35087-mr-status-misaligned.yml @@ -0,0 +1,4 @@ +--- +title: Fix alignment of controls in mr issuable list +merge_request: +author: diff --git a/changelogs/unreleased/request-store-wrap.yml b/changelogs/unreleased/request-store-wrap.yml new file mode 100644 index 00000000000..8017054b77b --- /dev/null +++ b/changelogs/unreleased/request-store-wrap.yml @@ -0,0 +1,4 @@ +--- +title: Add RequestCache which makes caching with RequestStore easier +merge_request: 12920 +author: diff --git a/changelogs/unreleased/sh-structured-logging.yml b/changelogs/unreleased/sh-structured-logging.yml new file mode 100644 index 00000000000..d89eb93f689 --- /dev/null +++ b/changelogs/unreleased/sh-structured-logging.yml @@ -0,0 +1,4 @@ +--- +title: Add structured logging for Rails processes +merge_request: +author: diff --git a/config/environments/test.rb b/config/environments/test.rb index c3b788c038e..986107150cf 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -43,4 +43,9 @@ Rails.application.configure do config.cache_store = :null_store config.active_job.queue_adapter = :test + + if ENV['CI'] && !ENV['RAILS_ENABLE_TEST_LOG'] + config.logger = Logger.new(nil) + config.log_level = :fatal + end end diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb new file mode 100644 index 00000000000..14902316240 --- /dev/null +++ b/config/initializers/lograge.rb @@ -0,0 +1,21 @@ +# Only use Lograge for Rails +unless Sidekiq.server? + filename = File.join(Rails.root, 'log', "#{Rails.env}_json.log") + + Rails.application.configure do + config.lograge.enabled = true + # Store the lograge JSON files in a separate file + config.lograge.keep_original_rails_log = true + # Don't use the Logstash formatter since this requires logstash-event, an + # unmaintained gem that monkey patches `Time` + config.lograge.formatter = Lograge::Formatters::Json.new + config.lograge.logger = ActiveSupport::Logger.new(filename) + # Add request parameters to log output + config.lograge.custom_options = lambda do |event| + { + time: event.time, + params: event.payload[:params].except(%w(controller action format)) + } + end + end +end diff --git a/db/migrate/20170713104829_add_foreign_key_to_merge_requests.rb b/db/migrate/20170713104829_add_foreign_key_to_merge_requests.rb new file mode 100644 index 00000000000..c25d4fd5986 --- /dev/null +++ b/db/migrate/20170713104829_add_foreign_key_to_merge_requests.rb @@ -0,0 +1,45 @@ +class AddForeignKeyToMergeRequests < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + class MergeRequest < ActiveRecord::Base + self.table_name = 'merge_requests' + include ::EachBatch + end + + def up + scope = <<-SQL.strip_heredoc + head_pipeline_id IS NOT NULL + AND NOT EXISTS ( + SELECT 1 FROM ci_pipelines + WHERE ci_pipelines.id = merge_requests.head_pipeline_id + ) + SQL + + MergeRequest.where(scope).each_batch(of: 1000) do |merge_requests| + merge_requests.update_all(head_pipeline_id: nil) + end + + unless foreign_key_exists?(:merge_requests, :head_pipeline_id) + add_concurrent_foreign_key(:merge_requests, :ci_pipelines, + column: :head_pipeline_id, on_delete: :nullify) + end + end + + def down + if foreign_key_exists?(:merge_requests, :head_pipeline_id) + remove_foreign_key(:merge_requests, column: :head_pipeline_id) + end + end + + private + + def foreign_key_exists?(table, column) + foreign_keys(table).any? do |key| + key.options[:column] == column.to_s + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 5264fc99557..9c8c64fe2d0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170707184244) do +ActiveRecord::Schema.define(version: 20170713104829) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1615,6 +1615,7 @@ ActiveRecord::Schema.define(version: 20170707184244) do add_foreign_key "merge_request_diffs", "merge_requests", name: "fk_8483f3258f", on_delete: :cascade add_foreign_key "merge_request_metrics", "ci_pipelines", column: "pipeline_id", on_delete: :cascade add_foreign_key "merge_request_metrics", "merge_requests", on_delete: :cascade + add_foreign_key "merge_requests", "ci_pipelines", column: "head_pipeline_id", name: "fk_fd82eae0b9", on_delete: :nullify add_foreign_key "merge_requests", "projects", column: "target_project_id", name: "fk_a6963e8447", on_delete: :cascade add_foreign_key "merge_requests_closing_issues", "issues", on_delete: :cascade add_foreign_key "merge_requests_closing_issues", "merge_requests", on_delete: :cascade diff --git a/doc/administration/monitoring/prometheus/index.md b/doc/administration/monitoring/prometheus/index.md index 695fdf09a87..f43c89dad87 100644 --- a/doc/administration/monitoring/prometheus/index.md +++ b/doc/administration/monitoring/prometheus/index.md @@ -95,8 +95,9 @@ Sample Prometheus queries: ## Configuring Prometheus to monitor Kubernetes > Introduced in GitLab 9.0. +> Pod monitoring introduced in GitLab 9.4. -If your GitLab server is running within Kubernetes, Prometheus will collect metrics from the Nodes in the cluster including performance data on each container. This is particularly helpful if your CI/CD environments run in the same cluster, as you can use the [Prometheus project integration][] to monitor them. +If your GitLab server is running within Kubernetes, Prometheus will collect metrics from the Nodes and [annotated Pods](https://prometheus.io/docs/operating/configuration/#<kubernetes_sd_config>) in the cluster, including performance data on each container. This is particularly helpful if your CI/CD environments run in the same cluster, as you can use the [Prometheus project integration][] to monitor them. To disable the monitoring of Kubernetes: diff --git a/doc/ci/environments.md b/doc/ci/environments.md index 3393030210e..df5c66a4c85 100644 --- a/doc/ci/environments.md +++ b/doc/ci/environments.md @@ -602,9 +602,8 @@ exist, you should see something like: >**Notes:** > - For the monitor dashboard to appear, you need to: - - Have enabled the [Kubernetes integration][kube] - - Have your app deployed on Kubernetes - Have enabled the [Prometheus integration][prom] + - Configured Prometheus to collect at least one [supported metric](../user/project/integrations/prometheus_library/metrics.md) - With GitLab 9.2, all deployments to an environment are shown directly on the monitoring dashboard diff --git a/doc/ci/img/environments_monitoring.png b/doc/ci/img/environments_monitoring.png Binary files differindex 387b6c54b61..d9c46ea4c95 100644 --- a/doc/ci/img/environments_monitoring.png +++ b/doc/ci/img/environments_monitoring.png diff --git a/doc/development/testing.md b/doc/development/testing.md index cf3ea2ccfc2..fc84932354b 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -479,6 +479,11 @@ slowest test files and try to improve them. run the suite against MySQL for tags, `master`, and any branch that includes `mysql` in the name. - On EE, the test suite always runs both PostgreSQL and MySQL. +- Rails logging to `log/test.log` is disabled by default in CI [for + performance reasons][logging]. To override this setting, provide the + `RAILS_ENABLE_TEST_LOG` environment variable. + +[logging]: https://jtway.co/speed-up-your-rails-test-suite-by-6-in-1-line-13fedb869ec4 ## Spinach (feature) tests diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index 86ceb14b965..6f15765751c 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -17,35 +17,30 @@ the settings page with a default template. To configure the template, see the Integration with Prometheus requires the following: 1. GitLab 9.0 or higher -1. The [Kubernetes integration must be enabled][kube] on your project -1. Your app must be deployed on [Kubernetes][] -1. Prometheus must be configured to collect Kubernetes metrics +1. Prometheus must be configured to collect one of the [supported metrics](prometheus_library/metrics.md) 1. Each metric must be have a label to indicate the environment -1. GitLab must have network connectivity to the Prometheus sever +1. GitLab must have network connectivity to the Prometheus server -There are a few steps necessary to set up integration between Prometheus and -GitLab. +## Getting started with Prometheus monitoring -## Configuring Prometheus to collect Kubernetes metrics +Depending on your deployment and where you have located your GitLab server, there are a few options to get started with Prometheus monitoring. -In order for Prometheus to collect Kubernetes metrics, you first must have a -Prometheus server up and running. You have two options here: +* If both GitLab and your applications are installed in the same Kubernetes cluster, you can leverage the [bundled Prometheus server within GitLab](#configuring-omnibus-gitlab-prometheus-to-monitor-kubernetes). +* If your applications are deployed on Kubernetes, but GitLab is not in the same cluster, then you can [configure a Prometheus server in your Kubernetes cluster](#configuring-your-own-prometheus-server-within-kubernetes). +* If your applications are not running in Kubernetes, [get started with Prometheus](#getting-started-with-prometheus-outside-of-kubernetes). -- If you installed Omnibus GitLab inside of Kubernetes, you can simply use the - [bundled version of Prometheus][promgldocs]. In that case, follow the info in the - [Omnibus GitLab section](#configuring-omnibus-gitlab-prometheus-to-monitor-kubernetes) - below. -- If you are using GitLab.com or installed GitLab outside of Kubernetes, you - will likely need to run a Prometheus server within the Kubernetes cluster. - Once installed, the easiest way to monitor Kubernetes is to simply use - Prometheus' support for [Kubernetes Service Discovery][prometheus-k8s-sd]. - In that case, follow the instructions on - [configuring your own Prometheus server within Kubernetes](#configuring-your-own-prometheus-server-within-kubernetes). +### Getting started with Prometheus outside of Kubernetes -### Configuring Omnibus GitLab Prometheus to monitor Kubernetes +Installing and configuring Prometheus to monitor applications is fairly straight forward. + +1. [Install Prometheus](https://prometheus.io/docs/introduction/install/) +1. Set up one of the [supported monitoring targets](prometheus_library/metrics.md) +1. Configure the Prometheus server to [collect their metrics](https://prometheus.io/docs/operating/configuration/#scrape_config) + +### Configuring Omnibus GitLab Prometheus to monitor Kubernetes deployments With Omnibus GitLab running inside of Kubernetes, you can leverage the bundled -version of Prometheus to collect the required metrics. +version of Prometheus to collect the supported metrics. Once enabled, Prometheus will automatically begin monitoring Kubernetes Nodes and any [annotated Pods](https://prometheus.io/docs/operating/configuration/#<kubernetes_sd_config>). 1. Read how to configure the bundled Prometheus server in the [Administration guide][gitlab-prometheus-k8s-monitor]. @@ -74,7 +69,7 @@ kubectl apply -f path/to/prometheus.yml Once deployed, you should see the Prometheus service, deployment, and pod start within the `prometheus` namespace. The server will begin to collect metrics from each Kubernetes Node in the cluster, based on the configuration -provided in the template. +provided in the template. It will also attempt to collect metrics from any Kubernetes Pods that have been [annotated for Prometheus](https://prometheus.io/docs/operating/configuration/#pod). Since GitLab is not running within Kubernetes, the template provides external network access via a `NodePort` running on `30090`. This method allows access @@ -133,30 +128,6 @@ to integrate with.  -## Metrics and Labels - -GitLab retrieves performance data from two metrics, `container_cpu_usage_seconds_total` -and `container_memory_usage_bytes`. These metrics are collected from the -Kubernetes pods via Prometheus, and report CPU and Memory utilization of each -container or Pod running in the cluster. - -In order to isolate and only display relevant metrics for a given environment -however, GitLab needs a method to detect which pods are associated. To do that, -GitLab will specifically request metrics that have an `environment` tag that -matches the [$CI_ENVIRONMENT_SLUG][ci-environment-slug]. - -If you are using [GitLab Auto-Deploy][autodeploy] and one of the methods of -configuring Prometheus above, the `environment` will be automatically added. - -### GitLab Prometheus queries - -The queries utilized by GitLab are shown in the following table. - -| Metric | Query | -| ------ | ----- | -| Average Memory (MB) | `(sum(container_memory_usage_bytes{container_name!="POD",environment="$CI_ENVIRONMENT_SLUG"}) / count(container_memory_usage_bytes{container_name!="POD",environment="$CI_ENVIRONMENT_SLUG"})) /1024/1024` | -| Average CPU Utilization (%) | `sum(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="$CI_ENVIRONMENT_SLUG"}[2m])) / count(container_cpu_usage_seconds_total{container_name!="POD",environment="$CI_ENVIRONMENT_SLUG"}) * 100` | - ## Monitoring CI/CD Environments Once configured, GitLab will attempt to retrieve performance metrics for any @@ -168,8 +139,9 @@ environment which has had a successful deployment. > [Introduced][ce-10408] in GitLab 9.2. > GitLab 9.3 added the [numeric comparison](https://gitlab.com/gitlab-org/gitlab-ce/issues/27439) of the 30 minute averages. +> Requires [Kubernetes](prometheus_library/kubernetes.md) metrics -Developers can view the performance impact of their changes within the merge +Developers can view theperformance impact of their changes within the merge request workflow. When a source branch has been deployed to an environment, a sparkline and numeric comparison of the average memory consumption will appear. On the sparkline, a dot indicates when the current changes were deployed, with up to 30 minutes of performance data displayed before and after. The comparison shows the difference between the 30 minute average before and after the deployment. This information is updated after diff --git a/doc/user/project/integrations/prometheus_library/cloudwatch.md b/doc/user/project/integrations/prometheus_library/cloudwatch.md new file mode 100644 index 00000000000..cc5cee36d28 --- /dev/null +++ b/doc/user/project/integrations/prometheus_library/cloudwatch.md @@ -0,0 +1,25 @@ +# Monitoring AWS Resources +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12621) in GitLab 9.4 + +GitLab has support for automatically detecting and monitoring AWS resources, starting with the [Elastic Load Balancer](https://aws.amazon.com/elasticloadbalancing/). This is provided by leveraging the official [Cloudwatch exporter](https://github.com/prometheus/cloudwatch_exporter), which translates [Cloudwatch metrics](https://aws.amazon.com/cloudwatch/) into a Prometheus readable form. + +## Metrics supported + +| Name | Query | +| ---- | ----- | +| Throughput (req/sec) | sum(aws_elb_request_count_sum{%{environment_filter}}) / 60 | +| Latency (ms) | avg(aws_elb_latency_average{%{environment_filter}}) * 1000 | +| HTTP Error Rate (%) | sum(aws_elb_httpcode_backend_5_xx_sum{%{environment_filter}}) / sum(aws_elb_request_count_sum{%{environment_filter}}) | + +## Configuring Prometheus to monitor for Cloudwatch metrics + +To get started with Cloudwatch monitoring, you should install and configure the [Cloudwatch exporter](https://github.com/hnlq715/nginx-vts-exporter) which retrieves and parses the specified Cloudwatch metrics and translates them into a Prometheus monitoring endpoint. + +Right now, the only AWS resource supported is the Elastic Load Balancer, whose Cloudwatch metrics can be found [here](http://docs.aws.amazon.com/elasticloadbalancing/latest/classic/elb-cloudwatch-metrics.html). + +A sample Cloudwatch Exporter configuration file, configured for basic AWS ELB monitoring, is [available for download](../samples/cloudwatch.yml). + +## Specifying the Environment label + +In order to isolate and only display relevant metrics for a given environment +however, GitLab needs a method to detect which labels are associated. To do this, GitLab will [look for an `environment` label](metrics.md#identifying-environments). diff --git a/doc/user/project/integrations/prometheus_library/kubernetes.md b/doc/user/project/integrations/prometheus_library/kubernetes.md new file mode 100644 index 00000000000..eb8cd821ddc --- /dev/null +++ b/doc/user/project/integrations/prometheus_library/kubernetes.md @@ -0,0 +1,26 @@ +# Monitoring Kubernetes +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8935) in GitLab 9.0 + +GitLab has support for automatically detecting and monitoring Kubernetes metrics. Kubernetes exposes Node level metrics out of the box via the built-in [Prometheus metrics support in cAdvisor](https://github.com/google/cadvisor). No additional services or exporters are needed. + +## Metrics supported + +| Name | Query | +| ---- | ----- | +| Average Memory Usage (MB) | (sum(container_memory_usage_bytes{container_name!="POD",%{environment_filter}}) / count(container_memory_usage_bytes{container_name!="POD",%{environment_filter}})) /1024/1024 | +| Average CPU Utilization (%) | sum(rate(container_cpu_usage_seconds_total{container_name!="POD",%{environment_filter}}[2m])) / count(container_cpu_usage_seconds_total{container_name!="POD",%{environment_filter}}) * 100 | + +## Configuring Prometheus to monitor for Kubernetes node metrics + +In order for Prometheus to collect Kubernetes metrics, you first must have a +Prometheus server up and running. You have two options here: + +- If you have an Omnibus based GitLab installation within your Kubernetes cluster, you can leverage the bundled Prometheus server to [monitor Kubernetes](../../../../administration/monitoring/prometheus/index.md#configuring-prometheus-to-monitor-kubernetes). +- To configure your own Prometheus server, you can follow the [Prometheus documentation](https://prometheus.io/docs/introduction/overview/) or [our guide](../../../../administration/monitoring/prometheus/index.md#configuring-your-own-prometheus-server-within-kubernetes). + +## Specifying the Environment label + +In order to isolate and only display relevant metrics for a given environment +however, GitLab needs a method to detect which labels are associated. To do this, GitLab will [look for an `environment` label](metrics.md#identifying-environments). + +If you are using [GitLab Auto-Deploy][autodeploy] and one of the two [provided Kubernetes monitoring solutions](../prometheus.md#getting-started-with-prometheus-monitoring), the `environment` label will be automatically added. diff --git a/doc/user/project/integrations/prometheus_library/metrics.md b/doc/user/project/integrations/prometheus_library/metrics.md new file mode 100644 index 00000000000..55146e57370 --- /dev/null +++ b/doc/user/project/integrations/prometheus_library/metrics.md @@ -0,0 +1,25 @@ +# Prometheus Metrics library +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8935) in GitLab 9.0 + +GitLab offers automatic detection of select [Prometheus exporters](https://prometheus.io/docs/instrumenting/exporters/). Currently supported exporters are: +* [Kubernetes](kubernetes.md) +* [NGINX](nginx.md) +* [Amazon Cloud Watch](cloudwatch.md) + +We have tried to surface the most important metrics for each exporter, and will be continuing to add support for additional exporters in future releases. If you would like to add support for other official exporters, [contributions](#adding-to-the-library) are welcome. + +## Identifying Environments + +GitLab retrieves performance data from the configured Prometheus server, and attempts to identifying the presence of known metrics. Once identified, GitLab then needs to be able to map the data to a particular environment. + +In order to isolate and only display relevant metrics for a given environment, GitLab needs a method to detect which labels are associated. To do that, +GitLab will look for the required metrics which have a label that +matches the [$CI_ENVIRONMENT_SLUG][ci-environment-slug]. + +For example if you are deploying to an environment named `production`, there must be a label for the metric with the value of `production`. + +## Adding to the library + +We strive to support the 2-4 most important metrics for each common system service that supports Prometheus. If you are looking for support for a particular exporter which has not yet been added to the library, additions can be made [to the `additional_metrics.yml`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/prometheus/additional_metrics.yml) file. + +> Note: The library is only for monitoring public, common, system services which all customers can benefit from. Support for monitoring [customer proprietary metrics](https://gitlab.com/gitlab-org/gitlab-ee/issues/2273) will be added in a subsequent release. diff --git a/doc/user/project/integrations/prometheus_library/nginx.md b/doc/user/project/integrations/prometheus_library/nginx.md new file mode 100644 index 00000000000..fe238e74e36 --- /dev/null +++ b/doc/user/project/integrations/prometheus_library/nginx.md @@ -0,0 +1,23 @@ +# Monitoring NGINX +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12621) in GitLab 9.4 + +GitLab has support for automatically detecting and monitoring NGINX. This is provided by leveraging the [NGINX VTS exporter](https://github.com/hnlq715/nginx-vts-exporter), which translates [VTS statistics](https://github.com/vozlt/nginx-module-vts) into a Prometheus readable form. + +## Metrics supported + +| Name | Query | +| ---- | ----- | +| Throughput (req/sec) | sum(rate(nginx_requests_total{server_zone!="*", server_zone!="_", %{environment_filter}}[2m])) | +| Latency (ms) | avg(nginx_upstream_response_msecs_avg{%{environment_filter}}) * 1000 | +| HTTP Error Rate (%) | sum(nginx_responses_total{status_code="5xx", %{environment_filter}}) / sum(nginx_responses_total{server_zone!="*", server_zone!="_", %{environment_filter}}) | + +## Configuring Prometheus to monitor for NGINX metrics + +To get started with NGINX monitoring, you should first enable the [VTS statistics](https://github.com/vozlt/nginx-module-vts)) module for your NGINX server. This will capture and display statistics in an HTML readable form. Next, you should install and configure the [NGINX VTS exporter](https://github.com/hnlq715/nginx-vts-exporter) which parses these statistics and translates them into a Prometheus monitoring endpoint. + +If you are using NGINX as your Kubernetes ingress, there is [upcoming direct support](https://github.com/kubernetes/ingress/pull/423) for enabling Prometheus monitoring in the 0.9.0 release. + +## Specifying the Environment label + +In order to isolate and only display relevant metrics for a given environment +however, GitLab needs a method to detect which labels are associated. To do this, GitLab will [look for an `environment` label](metrics.md#identifying-environments). diff --git a/doc/user/project/integrations/samples/cloudwatch.yml b/doc/user/project/integrations/samples/cloudwatch.yml new file mode 100644 index 00000000000..d9b58f52c32 --- /dev/null +++ b/doc/user/project/integrations/samples/cloudwatch.yml @@ -0,0 +1,26 @@ +region: us-east-1 + metrics: + - aws_namespace: AWS/ELB + aws_metric_name: RequestCount + aws_dimensions: [AvailabilityZone, LoadBalancerName] + aws_dimension_select: + LoadBalancerName: [gitlab-ha-lb] + aws_statistics: [Sum] + - aws_namespace: AWS/ELB + aws_metric_name: Latency + aws_dimensions: [AvailabilityZone, LoadBalancerName] + aws_dimension_select: + LoadBalancerName: [gitlab-ha-lb] + aws_statistics: [Average] + - aws_namespace: AWS/ELB + aws_metric_name: HTTPCode_Backend_2XX + aws_dimensions: [AvailabilityZone, LoadBalancerName] + aws_dimension_select: + LoadBalancerName: [gitlab-ha-lb] + aws_statistics: [Sum] + - aws_namespace: AWS/ELB + aws_metric_name: HTTPCode_Backend_5XX + aws_dimensions: [AvailabilityZone, LoadBalancerName] + aws_dimension_select: + LoadBalancerName: [gitlab-ha-lb] + aws_statistics: [Sum] diff --git a/doc/user/project/integrations/samples/prometheus.yml b/doc/user/project/integrations/samples/prometheus.yml index 01bbcaffe1e..30b59e172a1 100644 --- a/doc/user/project/integrations/samples/prometheus.yml +++ b/doc/user/project/integrations/samples/prometheus.yml @@ -24,6 +24,44 @@ data: target_label: environment regex: (.+)-.+-.+ replacement: $1 + - job_name: kubernetes-pods + tls_config: + ca_file: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" + insecure_skip_verify: true + bearer_token_file: "/var/run/secrets/kubernetes.io/serviceaccount/token" + kubernetes_sd_configs: + - role: pod + api_server: https://kubernetes.default.svc:443 + tls_config: + ca_file: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" + bearer_token_file: "/var/run/secrets/kubernetes.io/serviceaccount/token" + relabel_configs: + - source_labels: + - __meta_kubernetes_pod_annotation_prometheus_io_scrape + action: keep + regex: 'true' + - source_labels: + - __meta_kubernetes_pod_annotation_prometheus_io_path + action: replace + target_label: __metrics_path__ + regex: "(.+)" + - source_labels: + - __address__ + - __meta_kubernetes_pod_annotation_prometheus_io_port + action: replace + regex: "([^:]+)(?::[0-9]+)?;([0-9]+)" + replacement: "$1:$2" + target_label: __address__ + - action: labelmap + regex: __meta_kubernetes_pod_label_(.+) + - source_labels: + - __meta_kubernetes_namespace + action: replace + target_label: kubernetes_namespace + - source_labels: + - __meta_kubernetes_pod_name + action: replace + target_label: kubernetes_pod_name --- apiVersion: v1 kind: Service diff --git a/lib/gitlab/cache/request_cache.rb b/lib/gitlab/cache/request_cache.rb new file mode 100644 index 00000000000..f1a04affd38 --- /dev/null +++ b/lib/gitlab/cache/request_cache.rb @@ -0,0 +1,94 @@ +module Gitlab + module Cache + # This module provides a simple way to cache values in RequestStore, + # and the cache key would be based on the class name, method name, + # optionally customized instance level values, optionally customized + # method level values, and optional method arguments. + # + # A simple example: + # + # class UserAccess + # extend Gitlab::Cache::RequestCache + # + # request_cache_key do + # [user&.id, project&.id] + # end + # + # request_cache def can_push_to_branch?(ref) + # # ... + # end + # end + # + # This way, the result of `can_push_to_branch?` would be cached in + # `RequestStore.store` based on the cache key. If RequestStore is not + # currently active, then it would be stored in a hash saved in an + # instance variable, so the cache logic would be the same. + # Here's another example using customized method level values: + # + # class Commit + # extend Gitlab::Cache::RequestCache + # + # def author + # User.find_by_any_email(author_email.downcase) + # end + # request_cache(:author) { author_email.downcase } + # end + # + # So that we could have different strategies for different methods + # + module RequestCache + def self.extended(klass) + return if klass < self + + extension = Module.new + klass.const_set(:RequestCacheExtension, extension) + klass.prepend(extension) + end + + def request_cache_key(&block) + if block_given? + @request_cache_key = block + else + @request_cache_key + end + end + + def request_cache(method_name, &method_key_block) + const_get(:RequestCacheExtension).module_eval do + cache_key_method_name = "#{method_name}_cache_key" + + define_method(method_name) do |*args| + store = + if RequestStore.active? + RequestStore.store + else + ivar_name = # ! and ? cannot be used as ivar name + "@cache_#{method_name.to_s.tr('!?', "\u2605\u2606")}" + + instance_variable_get(ivar_name) || + instance_variable_set(ivar_name, {}) + end + + key = __send__(cache_key_method_name, args) + + store.fetch(key) { store[key] = super(*args) } + end + + define_method(cache_key_method_name) do |args| + klass = self.class + + instance_key = instance_exec(&klass.request_cache_key) if + klass.request_cache_key + + method_key = instance_exec(&method_key_block) if method_key_block + + [klass.name, method_name, *instance_key, *method_key, *args] + .join(':') + end + + private cache_key_method_name + end + end + end + end +end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 0643c56db9b..69ca9aa596b 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -140,6 +140,8 @@ module Gitlab return add_foreign_key(source, target, column: column, on_delete: on_delete) + else + on_delete = 'SET NULL' if on_delete == :nullify end disable_statement_timeout @@ -155,7 +157,7 @@ module Gitlab ADD CONSTRAINT #{key_name} FOREIGN KEY (#{column}) REFERENCES #{target} (id) - #{on_delete ? "ON DELETE #{on_delete}" : ''} + #{on_delete ? "ON DELETE #{on_delete.upcase}" : ''} NOT VALID; EOF diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index d0f04d25db2..76a562f356e 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -98,7 +98,15 @@ module Gitlab # Commit.between(repo, '29eda46b', 'master') # def between(repo, base, head) - repo.commits_between(base, head).map do |commit| + commits = Gitlab::GitalyClient.migrate(:commits_between) do |is_enabled| + if is_enabled + repo.gitaly_commit_client.between(base, head) + else + repo.commits_between(base, head) + end + end + + commits.map do |commit| decorate(commit) end rescue Rugged::ReferenceError @@ -210,6 +218,8 @@ module Gitlab init_from_hash(raw_commit) elsif raw_commit.is_a?(Rugged::Commit) init_from_rugged(raw_commit) + elsif raw_commit.is_a?(Gitaly::GitCommit) + init_from_gitaly(raw_commit) else raise "Invalid raw commit type: #{raw_commit.class}" end @@ -371,6 +381,22 @@ module Gitlab @parent_ids = commit.parents.map(&:oid) end + def init_from_gitaly(commit) + @raw_commit = commit + @id = commit.id + # TODO: Once gitaly "takes over" Rugged consider separating the + # subject from the message to make it clearer when there's one + # available but not the other. + @message = (commit.body.presence || commit.subject).dup + @authored_date = Time.at(commit.author.date.seconds) + @author_name = commit.author.name.dup + @author_email = commit.author.email.dup + @committed_date = Time.at(commit.committer.date.seconds) + @committer_name = commit.committer.name.dup + @committer_email = commit.committer.email.dup + @parent_ids = commit.parent_ids + end + def serialize_keys SERIALIZE_KEYS end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index c091bb9dcfe..63eebadff2e 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -807,6 +807,14 @@ module Gitlab Gitlab::GitalyClient::Util.repository(@storage, @relative_path) end + def gitaly_ref_client + @gitaly_ref_client ||= Gitlab::GitalyClient::RefService.new(self) + end + + def gitaly_commit_client + @gitaly_commit_client ||= Gitlab::GitalyClient::CommitService.new(self) + end + private # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'. @@ -1105,14 +1113,6 @@ module Gitlab end end - def gitaly_ref_client - @gitaly_ref_client ||= Gitlab::GitalyClient::RefService.new(self) - end - - def gitaly_commit_client - @gitaly_commit_client ||= Gitlab::GitalyClient::CommitService.new(self) - end - def gitaly_migrate(method, &block) Gitlab::GitalyClient.migrate(method, &block) rescue GRPC::NotFound => e diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 470e3ac8779..8f5738fed06 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -65,6 +65,17 @@ module Gitlab GitalyClient.call(@repository.storage, :commit_service, :count_commits, request).count end + def between(from, to) + request = Gitaly::CommitsBetweenRequest.new( + repository: @gitaly_repo, + from: from, + to: to + ) + + response = GitalyClient.call(@repository.storage, :commit_service, :commits_between, request) + consume_commits_response(response) + end + private def commit_diff_request_params(commit, options = {}) @@ -77,6 +88,10 @@ module Gitlab paths: options.fetch(:paths, []) } end + + def consume_commits_response(response) + response.flat_map { |r| r.commits } + end end end end diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index f541887843d..2c3d53410ac 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -96,11 +96,11 @@ module Gitlab id: response.commit_id, message: message, authored_date: Time.at(response.commit_author.date.seconds), - author_name: response.commit_author.name, - author_email: response.commit_author.email, + author_name: response.commit_author.name.dup, + author_email: response.commit_author.email.dup, committed_date: Time.at(response.commit_committer.date.seconds), - committer_name: response.commit_committer.name, - committer_email: response.commit_committer.email + committer_name: response.commit_committer.name.dup, + committer_email: response.commit_committer.email.dup } Gitlab::Git::Commit.decorate(hash) diff --git a/lib/gitlab/performance_bar/peek_query_tracker.rb b/lib/gitlab/performance_bar/peek_query_tracker.rb index f97e895dbd0..67fee8c227d 100644 --- a/lib/gitlab/performance_bar/peek_query_tracker.rb +++ b/lib/gitlab/performance_bar/peek_query_tracker.rb @@ -37,7 +37,7 @@ module Gitlab def track_query(raw_query, bindings, start, finish) query = Gitlab::Sherlock::Query.new(raw_query, start, finish) - query_info = { duration: '%.3f' % query.duration, sql: query.formatted_query } + query_info = { duration: query.duration.round(3), sql: query.formatted_query } PEEK_DB_CLIENT.query_details << query_info end diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 3b922da7ced..8e91ee7287c 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -1,5 +1,11 @@ module Gitlab class UserAccess + extend Gitlab::Cache::RequestCache + + request_cache_key do + [user&.id, project&.id] + end + attr_reader :user, :project def initialize(user, project: nil) @@ -28,7 +34,7 @@ module Gitlab true end - def can_create_tag?(ref) + request_cache def can_create_tag?(ref) return false unless can_access_git? if ProtectedTag.protected?(project, ref) @@ -38,7 +44,7 @@ module Gitlab end end - def can_delete_branch?(ref) + request_cache def can_delete_branch?(ref) return false unless can_access_git? if ProtectedBranch.protected?(project, ref) @@ -48,7 +54,7 @@ module Gitlab end end - def can_push_to_branch?(ref) + request_cache def can_push_to_branch?(ref) return false unless can_access_git? if ProtectedBranch.protected?(project, ref) @@ -60,7 +66,7 @@ module Gitlab end end - def can_merge_to_branch?(ref) + request_cache def can_merge_to_branch?(ref) return false unless can_access_git? if ProtectedBranch.protected?(project, ref) diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index 68114d149c4..39806901274 100644 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -10,7 +10,7 @@ fi # Only install knapsack after bundle install! Otherwise oddly some native # gems could not be found under some circumstance. No idea why, hours wasted. -retry gem install knapsack fog-aws mime-types +retry gem install knapsack cp config/gitlab.yml.example config/gitlab.yml diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb index 36b9645438a..89e260cf65b 100644 --- a/spec/factories/commits.rb +++ b/spec/factories/commits.rb @@ -4,14 +4,19 @@ FactoryGirl.define do factory :commit do git_commit RepoHelpers.sample_commit project factory: :empty_project - author { build(:author) } initialize_with do new(git_commit, project) end + after(:build) do |commit| + allow(commit).to receive(:author).and_return build(:author) + end + trait :without_author do - author nil + after(:build) do |commit| + allow(commit).to receive(:author).and_return nil + end end end end diff --git a/spec/features/participants_autocomplete_spec.rb b/spec/features/participants_autocomplete_spec.rb index 382d83ca051..81b0a2f541b 100644 --- a/spec/features/participants_autocomplete_spec.rb +++ b/spec/features/participants_autocomplete_spec.rb @@ -54,7 +54,8 @@ feature 'Member autocomplete', :js do let(:note) { create(:note_on_commit, project: project, commit_id: project.commit.id) } before do - allow_any_instance_of(Commit).to receive(:author).and_return(author) + allow(User).to receive(:find_by_any_email) + .with(noteable.author_email.downcase).and_return(author) visit project_commit_path(project, noteable) end diff --git a/spec/lib/gitlab/cache/request_cache_spec.rb b/spec/lib/gitlab/cache/request_cache_spec.rb new file mode 100644 index 00000000000..5b82c216a13 --- /dev/null +++ b/spec/lib/gitlab/cache/request_cache_spec.rb @@ -0,0 +1,133 @@ +require 'spec_helper' + +describe Gitlab::Cache::RequestCache do + let(:klass) do + Class.new do + extend Gitlab::Cache::RequestCache + + attr_accessor :id, :name, :result, :extra + + def self.name + 'ExpensiveAlgorithm' + end + + def initialize(id, name, result, extra = nil) + self.id = id + self.name = name + self.result = result + self.extra = nil + end + + request_cache def compute(arg) + result << arg + end + + request_cache def repute(arg) + result << arg + end + + def dispute(arg) + result << arg + end + request_cache(:dispute) { extra } + end + end + + let(:algorithm) { klass.new('id', 'name', []) } + + shared_examples 'cache for the same instance' do + it 'does not compute twice for the same argument' do + algorithm.compute(true) + result = algorithm.compute(true) + + expect(result).to eq([true]) + end + + it 'computes twice for the different argument' do + algorithm.compute(true) + result = algorithm.compute(false) + + expect(result).to eq([true, false]) + end + + it 'computes twice for the different class name' do + algorithm.compute(true) + allow(klass).to receive(:name).and_return('CheapAlgo') + result = algorithm.compute(true) + + expect(result).to eq([true, true]) + end + + it 'computes twice for the different method' do + algorithm.compute(true) + result = algorithm.repute(true) + + expect(result).to eq([true, true]) + end + + context 'when request_cache_key is provided' do + before do + klass.request_cache_key do + [id, name] + end + end + + it 'computes twice for the different keys, id' do + algorithm.compute(true) + algorithm.id = 'ad' + result = algorithm.compute(true) + + expect(result).to eq([true, true]) + end + + it 'computes twice for the different keys, name' do + algorithm.compute(true) + algorithm.name = 'same' + result = algorithm.compute(true) + + expect(result).to eq([true, true]) + end + + it 'uses extra method cache key if provided' do + algorithm.dispute(true) # miss + algorithm.extra = true + algorithm.dispute(true) # miss + result = algorithm.dispute(true) # hit + + expect(result).to eq([true, true]) + end + end + end + + context 'when RequestStore is active', :request_store do + it_behaves_like 'cache for the same instance' + + it 'computes once for different instances when keys are the same' do + algorithm.compute(true) + result = klass.new('id', 'name', algorithm.result).compute(true) + + expect(result).to eq([true]) + end + + it 'computes twice if RequestStore starts over' do + algorithm.compute(true) + RequestStore.end! + RequestStore.clear! + RequestStore.begin! + result = algorithm.compute(true) + + expect(result).to eq([true, true]) + end + end + + context 'when RequestStore is inactive' do + it_behaves_like 'cache for the same instance' + + it 'computes twice for different instances even if keys are the same' do + algorithm.compute(true) + result = klass.new('id', 'name', algorithm.result).compute(true) + + expect(result).to eq([true, true]) + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 4259be3f522..a2acd15c8fb 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -174,13 +174,23 @@ describe Gitlab::Database::MigrationHelpers, lib: true do allow(Gitlab::Database).to receive(:mysql?).and_return(false) end - it 'creates a concurrent foreign key' do + it 'creates a concurrent foreign key and validates it' do expect(model).to receive(:disable_statement_timeout) expect(model).to receive(:execute).ordered.with(/NOT VALID/) expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/) model.add_concurrent_foreign_key(:projects, :users, column: :user_id) end + + it 'appends a valid ON DELETE statement' do + expect(model).to receive(:disable_statement_timeout) + expect(model).to receive(:execute).with(/ON DELETE SET NULL/) + expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/) + + model.add_concurrent_foreign_key(:projects, :users, + column: :user_id, + on_delete: :nullify) + end end end end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index f20a14155dc..60de91324f0 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -64,6 +64,52 @@ describe Gitlab::Git::Commit, seed_helper: true do end end + describe "Commit info from gitaly commit" do + let(:id) { 'f00' } + let(:subject) { "My commit".force_encoding('ASCII-8BIT') } + let(:body) { subject + "My body".force_encoding('ASCII-8BIT') } + let(:committer) do + Gitaly::CommitAuthor.new( + name: generate(:name), + email: generate(:email), + date: Google::Protobuf::Timestamp.new(seconds: 123) + ) + end + let(:author) do + Gitaly::CommitAuthor.new( + name: generate(:name), + email: generate(:email), + date: Google::Protobuf::Timestamp.new(seconds: 456) + ) + end + let(:gitaly_commit) do + Gitaly::GitCommit.new( + id: id, + subject: subject, + body: body, + author: author, + committer: committer + ) + end + let(:commit) { described_class.new(gitaly_commit) } + + it { expect(commit.short_id).to eq(id[0..10]) } + it { expect(commit.id).to eq(id) } + it { expect(commit.sha).to eq(id) } + it { expect(commit.safe_message).to eq(body) } + it { expect(commit.created_at).to eq(Time.at(committer.date.seconds)) } + it { expect(commit.author_email).to eq(author.email) } + it { expect(commit.author_name).to eq(author.name) } + it { expect(commit.committer_name).to eq(committer.name) } + it { expect(commit.committer_email).to eq(committer.email) } + + context 'no body' do + let(:body) { "".force_encoding('ASCII-8BIT') } + + it { expect(commit.safe_message).to eq(subject) } + end + end + context 'Class methods' do describe '.find' do it "should return first head commit if without params" do diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index fee5bb45fe5..93affb12f2b 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -1,7 +1,6 @@ require 'spec_helper' describe Gitlab::GitalyClient::CommitService do - let(:diff_stub) { double('Gitaly::DiffService::Stub') } let(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:repository_message) { repository.gitaly_repository } @@ -82,4 +81,19 @@ describe Gitlab::GitalyClient::CommitService do end end end + + describe '#between' do + let(:from) { 'master' } + let(:to) { '4b825dc642cb6eb9a060e54bf8d69288fbee4904' } + it 'sends an RPC request' do + request = Gitaly::CommitsBetweenRequest.new( + repository: repository_message, from: from, to: to + ) + + expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:commits_between) + .with(request, kind_of(Hash)).and_return([]) + + described_class.new(repository).between(from, to) + end + end end diff --git a/spec/migrations/add_foreign_key_to_merge_requests_spec.rb b/spec/migrations/add_foreign_key_to_merge_requests_spec.rb new file mode 100644 index 00000000000..d9ad9a585f0 --- /dev/null +++ b/spec/migrations/add_foreign_key_to_merge_requests_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20170713104829_add_foreign_key_to_merge_requests.rb') + +describe AddForeignKeyToMergeRequests, :migration do + let(:projects) { table(:projects) } + let(:merge_requests) { table(:merge_requests) } + let(:pipelines) { table(:ci_pipelines) } + + before do + projects.create!(name: 'gitlab', path: 'gitlab-org/gitlab-ce') + pipelines.create!(project_id: projects.first.id, + ref: 'some-branch', + sha: 'abc12345') + + # merge request without a pipeline + create_merge_request(head_pipeline_id: nil) + + # merge request with non-existent pipeline + create_merge_request(head_pipeline_id: 1234) + + # merge reqeust with existing pipeline assigned + create_merge_request(head_pipeline_id: pipelines.first.id) + end + + it 'correctly adds a foreign key to head_pipeline_id' do + migrate! + + expect(merge_requests.first.head_pipeline_id).to be_nil + expect(merge_requests.second.head_pipeline_id).to be_nil + expect(merge_requests.third.head_pipeline_id).to eq pipelines.first.id + end + + def create_merge_request(**opts) + merge_requests.create!(source_project_id: projects.first.id, + target_project_id: projects.first.id, + source_branch: 'some-branch', + target_branch: 'master', **opts) + end +end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 6056d78da4e..528b211c9d6 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -19,17 +19,15 @@ describe Commit, models: true do expect(commit.author).to eq(user) end - it 'caches the author' do - allow(RequestStore).to receive(:active?).and_return(true) + it 'caches the author', :request_store do user = create(:user, email: commit.author_email) - expect_any_instance_of(Commit).to receive(:find_author_by_any_email).and_call_original + expect(User).to receive(:find_by_any_email).and_call_original expect(commit.author).to eq(user) - key = "commit_author:#{commit.author_email}" + key = "Commit:author:#{commit.author_email.downcase}" expect(RequestStore.store[key]).to eq(user) expect(commit.author).to eq(user) - RequestStore.store.clear end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 3f77ed10069..c493c08a7ae 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -108,7 +108,7 @@ describe GitPushService, services: true do it { is_expected.to include(id: @commit.id) } it { is_expected.to include(message: @commit.safe_message) } - it { is_expected.to include(timestamp: @commit.date.xmlschema) } + it { expect(subject[:timestamp].in_time_zone).to eq(@commit.date.in_time_zone) } it do is_expected.to include( url: [ @@ -163,7 +163,7 @@ describe GitPushService, services: true do execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) end end - + context "Sends System Push data" do it "when pushing on a branch" do expect(SystemHookPushWorker).to receive(:perform_async).with(@push_data, :push_hooks) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index f1e00c1163b..4fc5eb0a527 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -383,7 +383,7 @@ describe NotificationService, services: true do before do build_team(note.project) reset_delivered_emails! - allow_any_instance_of(Commit).to receive(:author).and_return(@u_committer) + allow(note.noteable).to receive(:author).and_return(@u_committer) update_custom_notification(:new_note, @u_guest_custom, resource: project) update_custom_notification(:new_note, @u_custom_global) end |
