diff options
60 files changed, 862 insertions, 311 deletions
@@ -1,4 +1,12 @@ -Copyright GitLab B.V. +Copyright (c) 2011-present GitLab B.V. + +Portions of this software are licensed as follows: + +* All content residing under the "doc/" directory of this repository is licensed under "Creative Commons: CC BY-SA 4.0 license". +* All content that resides under the "ee/" directory of this repository, if that directory exists, is licensed under the license defined in "ee/LICENSE". +* All client-side JavaScript (when served directly or after being compiled, arranged, augmented, or combined), is licensed under the "MIT Expat" license. +* All third party components incorporated into the GitLab Software are licensed under the original license provided by the owner of the applicable component. +* Content outside of the above mentioned directories or restrictions above is available under the "MIT Expat" license as defined below. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/README.md b/README.md index 73bfcc988cc..054e2d02461 100644 --- a/README.md +++ b/README.md @@ -20,10 +20,6 @@ To see how GitLab looks please see the [features page on our website](https://ab - Used by more than 100,000 organizations, GitLab is the most popular solution to manage Git repositories on-premises - Completely free and open source (MIT Expat license) -## Hiring - -We're hiring developers, support people, and production engineers all the time, please see our [jobs page](https://about.gitlab.com/jobs/). - ## Editions There are two editions of GitLab: @@ -31,6 +27,15 @@ There are two editions of GitLab: - GitLab Community Edition (CE) is available freely under the MIT Expat license. - GitLab Enterprise Edition (EE) includes [extra features](https://about.gitlab.com/pricing/#compare-options) that are more useful for organizations with more than 100 users. To use EE and get official support please [become a subscriber](https://about.gitlab.com/pricing/). +## Licensing + +See the [LICENSE](LICENSE) file for licensing information as it pertains to +files in this repository. + +## Hiring + +We're hiring developers, support people, and production engineers all the time, please see our [jobs page](https://about.gitlab.com/jobs/). + ## Website On [about.gitlab.com](https://about.gitlab.com/) you can find more information about: @@ -58,14 +63,6 @@ There are various other options to install GitLab, please refer to the [installa GitLab is an open source project and we are very happy to accept community contributions. Please refer to [Contributing to GitLab page](https://about.gitlab.com/contributing/) for more details. -## Licensing - -GitLab Community Edition (CE) is available freely under the MIT Expat license. - -All third party components incorporated into the GitLab Software are licensed under the original license provided by the owner of the applicable component. - -All Documentation content that resides under the `doc/` directory of this repository is licensed under Creative Commons: CC BY-SA 4.0. - ## Install a development environment To work on GitLab itself, we recommend setting up your development environment with [the GitLab Development Kit](https://gitlab.com/gitlab-org/gitlab-development-kit). diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb index 3254229d9cb..c94fdd9483d 100644 --- a/app/controllers/projects/raw_controller.rb +++ b/app/controllers/projects/raw_controller.rb @@ -26,7 +26,7 @@ class Projects::RawController < Projects::ApplicationController limiter.log_request(request, :raw_blob_request_limit, current_user) flash[:alert] = _('You cannot access the raw file. Please wait a minute.') - redirect_to project_blob_path(@project, File.join(@ref, @path)) + redirect_to project_blob_path(@project, File.join(@ref, @path)), status: :too_many_requests end def raw_blob_request_limit diff --git a/app/finders/remote_mirror_finder.rb b/app/finders/remote_mirror_finder.rb deleted file mode 100644 index 420db0077aa..00000000000 --- a/app/finders/remote_mirror_finder.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -class RemoteMirrorFinder - attr_accessor :params - - def initialize(params) - @params = params - end - - # rubocop: disable CodeReuse/ActiveRecord - def execute - RemoteMirror.find_by(id: params[:id]) - end - # rubocop: enable CodeReuse/ActiveRecord -end diff --git a/app/mailers/emails/remote_mirrors.rb b/app/mailers/emails/remote_mirrors.rb index 2d8137843ec..f3938a052b0 100644 --- a/app/mailers/emails/remote_mirrors.rb +++ b/app/mailers/emails/remote_mirrors.rb @@ -3,7 +3,7 @@ module Emails module RemoteMirrors def remote_mirror_update_failed_email(remote_mirror_id, recipient_id) - @remote_mirror = RemoteMirrorFinder.new(id: remote_mirror_id).execute + @remote_mirror = RemoteMirror.find_by_id(remote_mirror_id) @project = @remote_mirror.project mail(to: recipient(recipient_id, @project.group), subject: subject('Remote mirror update failed')) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ac88d9714ac..f705e67121f 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -384,7 +384,7 @@ module Ci return unless has_environment? strong_memoize(:expanded_environment_name) do - ExpandVariables.expand(environment, simple_variables) + ExpandVariables.expand(environment, -> { simple_variables }) end end diff --git a/app/models/project_services/chat_message/pipeline_message.rb b/app/models/project_services/chat_message/pipeline_message.rb index 4edf263433f..a3793d9937b 100644 --- a/app/models/project_services/chat_message/pipeline_message.rb +++ b/app/models/project_services/chat_message/pipeline_message.rb @@ -68,7 +68,7 @@ module ChatMessage title_link: pipeline_url, fields: attachments_fields, footer: project.name, - footer_icon: project.avatar_url, + footer_icon: project.avatar_url(only_path: false), ts: finished_at }] end diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index 6b5605f9999..c9ee0653d86 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -4,6 +4,8 @@ class RemoteMirror < ApplicationRecord include AfterCommitQueue include MirrorAuthentication + MAX_FIRST_RUNTIME = 3.hours + MAX_INCREMENTAL_RUNTIME = 1.hour PROTECTED_BACKOFF_DELAY = 1.minute UNPROTECTED_BACKOFF_DELAY = 5.minutes @@ -31,11 +33,18 @@ class RemoteMirror < ApplicationRecord scope :enabled, -> { where(enabled: true) } scope :started, -> { with_update_status(:started) } - scope :stuck, -> { started.where('last_update_at < ? OR (last_update_at IS NULL AND updated_at < ?)', 1.hour.ago, 3.hours.ago) } + + scope :stuck, -> do + started + .where('(last_update_started_at < ? AND last_update_at IS NOT NULL)', + MAX_INCREMENTAL_RUNTIME.ago) + .or(where('(last_update_started_at < ? AND last_update_at IS NULL)', + MAX_FIRST_RUNTIME.ago)) + end state_machine :update_status, initial: :none do event :update_start do - transition [:none, :finished, :failed] => :started + transition any => :started end event :update_finish do @@ -46,9 +55,14 @@ class RemoteMirror < ApplicationRecord transition started: :failed end + event :update_retry do + transition started: :to_retry + end + state :started state :finished state :failed + state :to_retry after_transition any => :started do |remote_mirror, _| Gitlab::Metrics.add_event(:remote_mirrors_running) @@ -138,16 +152,27 @@ class RemoteMirror < ApplicationRecord end def updated_since?(timestamp) - last_update_started_at && last_update_started_at > timestamp && !update_failed? + return false if failed? + + last_update_started_at && last_update_started_at > timestamp end def mark_for_delete_if_blank_url mark_for_destruction if url.blank? end - def mark_as_failed(error_message) - update_column(:last_error, Gitlab::UrlSanitizer.sanitize(error_message)) - update_fail + def update_error_message(error_message) + self.last_error = Gitlab::UrlSanitizer.sanitize(error_message) + end + + def mark_for_retry!(error_message) + update_error_message(error_message) + update_retry! + end + + def mark_as_failed!(error_message) + update_error_message(error_message) + update_fail! end def url=(value) @@ -190,6 +215,18 @@ class RemoteMirror < ApplicationRecord update_column(:error_notification_sent, true) end + def backoff_delay + if self.only_protected_branches + PROTECTED_BACKOFF_DELAY + else + UNPROTECTED_BACKOFF_DELAY + end + end + + def max_runtime + last_update_at.present? ? MAX_INCREMENTAL_RUNTIME : MAX_FIRST_RUNTIME + end + private def store_credentials @@ -219,14 +256,6 @@ class RemoteMirror < ApplicationRecord self.last_update_started_at >= Time.now - backoff_delay end - def backoff_delay - if self.only_protected_branches - PROTECTED_BACKOFF_DELAY - else - UNPROTECTED_BACKOFF_DELAY - end - end - def reset_fields update_columns( last_error: nil, diff --git a/app/models/repository.rb b/app/models/repository.rb index a89f573e3d6..9d45a12fa6e 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -418,25 +418,29 @@ class Repository end # Runs code before pushing (= creating or removing) a tag. + # + # Note that this doesn't expire the tags. You may need to call + # expire_caches_for_tags or expire_tags_cache. def before_push_tag + repository_event(:push_tag) + end + + def expire_caches_for_tags expire_statistics_caches expire_emptiness_caches expire_tags_cache - - repository_event(:push_tag) end # Runs code before removing a tag. def before_remove_tag - expire_tags_cache - expire_statistics_caches + expire_caches_for_tags repository_event(:remove_tag) end # Runs code after removing a tag. def after_remove_tag - expire_tags_cache + expire_caches_for_tags end # Runs code after the HEAD of a repository is changed. @@ -460,8 +464,8 @@ class Repository end # Runs code after a new branch has been created. - def after_create_branch - expire_branches_cache + def after_create_branch(expire_cache: true) + expire_branches_cache if expire_cache repository_event(:push_branch) end @@ -474,8 +478,8 @@ class Repository end # Runs code after an existing branch has been removed. - def after_remove_branch - expire_branches_cache + def after_remove_branch(expire_cache: true) + expire_branches_cache if expire_cache end def method_missing(msg, *args, &block) diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb index c41f445c3c4..431a5aedf2e 100644 --- a/app/services/git/branch_hooks_service.rb +++ b/app/services/git/branch_hooks_service.rb @@ -63,7 +63,7 @@ module Git end def branch_create_hooks - project.repository.after_create_branch + project.repository.after_create_branch(expire_cache: false) project.after_create_default_branch if default_branch? end @@ -78,7 +78,7 @@ module Git end def branch_remove_hooks - project.repository.after_remove_branch + project.repository.after_remove_branch(expire_cache: false) end # Schedules processing of commit messages diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index 1244a0f72a7..13a467a3ef9 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -2,31 +2,52 @@ module Projects class UpdateRemoteMirrorService < BaseService - attr_reader :errors + MAX_TRIES = 3 - def execute(remote_mirror) + def execute(remote_mirror, tries) return success unless remote_mirror.enabled? - errors = [] + update_mirror(remote_mirror) - begin - remote_mirror.ensure_remote! - repository.fetch_remote(remote_mirror.remote_name, ssh_auth: remote_mirror, no_tags: true) + success + rescue Gitlab::Git::CommandError => e + # This happens if one of the gitaly calls above fail, for example when + # branches have diverged, or the pre-receive hook fails. + retry_or_fail(remote_mirror, e.message, tries) - opts = {} - if remote_mirror.only_protected_branches? - opts[:only_branches_matching] = project.protected_branches.select(:name).map(&:name) - end + error(e.message) + rescue => e + remote_mirror.mark_as_failed!(e.message) + raise e + end + + private + + def update_mirror(remote_mirror) + remote_mirror.update_start! + + remote_mirror.ensure_remote! + repository.fetch_remote(remote_mirror.remote_name, ssh_auth: remote_mirror, no_tags: true) - remote_mirror.update_repository(opts) - rescue => e - errors << e.message.strip + opts = {} + if remote_mirror.only_protected_branches? + opts[:only_branches_matching] = project.protected_branches.select(:name).map(&:name) end - if errors.present? - error(errors.join("\n\n")) + remote_mirror.update_repository(opts) + + remote_mirror.update_finish! + end + + def retry_or_fail(mirror, message, tries) + if tries < MAX_TRIES + mirror.mark_for_retry!(message) else - success + # It's not likely we'll be able to recover from this ourselves, so we'll + # notify the users of the problem, and don't trigger any sidekiq retries + # Instead, we'll wait for the next change to try the push again, or until + # a user manually retries. + mirror.mark_as_failed!(message) end end end diff --git a/app/services/update_deployment_service.rb b/app/services/update_deployment_service.rb index 49a7d0178f4..dcafebae52d 100644 --- a/app/services/update_deployment_service.rb +++ b/app/services/update_deployment_service.rb @@ -42,7 +42,7 @@ class UpdateDeploymentService return unless environment_url @expanded_environment_url = - ExpandVariables.expand(environment_url, variables) + ExpandVariables.expand(environment_url, -> { variables }) end def environment_url diff --git a/app/views/projects/mirrors/_mirror_repos.html.haml b/app/views/projects/mirrors/_mirror_repos.html.haml index 280ec6d715b..eb100e5cf47 100644 --- a/app/views/projects/mirrors/_mirror_repos.html.haml +++ b/app/views/projects/mirrors/_mirror_repos.html.haml @@ -43,7 +43,8 @@ = _('Mirrored repositories') = render_if_exists 'projects/mirrors/mirrored_repositories_count' %th= _('Direction') - %th= _('Last update') + %th= _('Last update attempt') + %th= _('Last successful update') %th %th %tbody.js-mirrors-table-body @@ -53,6 +54,8 @@ %tr.qa-mirrored-repository-row.rspec-mirrored-repository-row{ class: ('bg-secondary' if mirror.disabled?) } %td.qa-mirror-repository-url= mirror.safe_url %td= _('Push') + %td + = mirror.last_update_started_at.present? ? time_ago_with_tooltip(mirror.last_update_started_at) : _('Never') %td.qa-mirror-last-update-at= mirror.last_update_at.present? ? time_ago_with_tooltip(mirror.last_update_at) : _('Never') %td - if mirror.disabled? diff --git a/app/views/projects/update.js.haml b/app/views/projects/update.js.haml index 70f1bf8ef46..c5eecc900b2 100644 --- a/app/views/projects/update.js.haml +++ b/app/views/projects/update.js.haml @@ -4,7 +4,7 @@ location.reload(); - else :plain - $(".project-edit-errors").html("#{escape_javascript(render('errors'))}"); + $(".flash-container").html("#{escape_javascript(render('errors'))}"); $('.save-project-loader').hide(); $('.project-edit-container').show(); $('.edit-project .js-btn-success-general-project-settings').enable(); diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index dba7837bd12..622bd6f1f48 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -42,6 +42,11 @@ class PostReceive user = identify_user(post_received) return false unless user + # Expire the branches cache so we have updated data for this push + post_received.project.repository.expire_branches_cache if post_received.includes_branches? + # We only need to expire tags once per push + post_received.project.repository.expire_caches_for_tags if post_received.includes_tags? + post_received.enum_for(:changes_refs).with_index do |(oldrev, newrev, ref), index| service_klass = if Gitlab::Git.tag_ref?(ref) @@ -72,6 +77,7 @@ class PostReceive def after_project_changes_hooks(post_received, user, refs, changes) hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, user, changes, refs) SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks) + Gitlab::UsageDataCounters::SourceCodeCounter.count(:pushes) end def process_wiki_changes(post_received) diff --git a/app/workers/remote_mirror_notification_worker.rb b/app/workers/remote_mirror_notification_worker.rb index 5bafe8e2046..368abfeda99 100644 --- a/app/workers/remote_mirror_notification_worker.rb +++ b/app/workers/remote_mirror_notification_worker.rb @@ -4,7 +4,7 @@ class RemoteMirrorNotificationWorker include ApplicationWorker def perform(remote_mirror_id) - remote_mirror = RemoteMirrorFinder.new(id: remote_mirror_id).execute + remote_mirror = RemoteMirror.find_by_id(remote_mirror_id) # We check again if there's an error because a newer run since this job was # fired could've completed successfully. diff --git a/app/workers/repository_update_remote_mirror_worker.rb b/app/workers/repository_update_remote_mirror_worker.rb index 03a7ff2cd7a..d13c7641eb3 100644 --- a/app/workers/repository_update_remote_mirror_worker.rb +++ b/app/workers/repository_update_remote_mirror_worker.rb @@ -1,50 +1,53 @@ # frozen_string_literal: true class RepositoryUpdateRemoteMirrorWorker - UpdateAlreadyInProgressError = Class.new(StandardError) UpdateError = Class.new(StandardError) include ApplicationWorker + include Gitlab::ExclusiveLeaseHelpers sidekiq_options retry: 3, dead: false - sidekiq_retry_in { |count| 30 * count } + LOCK_WAIT_TIME = 30.seconds + MAX_TRIES = 3 - sidekiq_retries_exhausted do |msg, _| - Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" - end - - def perform(remote_mirror_id, scheduled_time) - remote_mirror = RemoteMirrorFinder.new(id: remote_mirror_id).execute + def perform(remote_mirror_id, scheduled_time, tries = 0) + remote_mirror = RemoteMirror.find_by_id(remote_mirror_id) + return unless remote_mirror return if remote_mirror.updated_since?(scheduled_time) - raise UpdateAlreadyInProgressError if remote_mirror.update_in_progress? + # If the update is already running, wait for it to finish before running again + # This will wait for a total of 90 seconds in 3 steps + in_lock(remote_mirror_update_lock(remote_mirror.id), + retries: 3, + ttl: remote_mirror.max_runtime, + sleep_sec: LOCK_WAIT_TIME) do + update_mirror(remote_mirror, scheduled_time, tries) + end + rescue Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError + # If an update runs longer than 1.5 minutes, we'll reschedule it + # with a backoff. The next run will check if the previous update would + # include the changes that triggered this update and become a no-op. + self.class.perform_in(remote_mirror.backoff_delay, remote_mirror.id, scheduled_time, tries) + end - remote_mirror.update_start + private - project = remote_mirror.project + def update_mirror(mirror, scheduled_time, tries) + project = mirror.project current_user = project.creator - result = Projects::UpdateRemoteMirrorService.new(project, current_user).execute(remote_mirror) - raise UpdateError, result[:message] if result[:status] == :error - - remote_mirror.update_finish - rescue UpdateAlreadyInProgressError - raise - rescue UpdateError => ex - fail_remote_mirror(remote_mirror, ex.message) - raise - rescue => ex - return unless remote_mirror + result = Projects::UpdateRemoteMirrorService.new(project, current_user).execute(mirror, tries) - fail_remote_mirror(remote_mirror, ex.message) - raise UpdateError, "#{ex.class}: #{ex.message}" + if result[:status] == :error && mirror.to_retry? + schedule_retry(mirror, scheduled_time, tries) + end end - private - - def fail_remote_mirror(remote_mirror, message) - remote_mirror.mark_as_failed(message) + def remote_mirror_update_lock(mirror_id) + [self.class.name, mirror_id].join(':') + end - Rails.logger.error(message) # rubocop:disable Gitlab/RailsLogger + def schedule_retry(mirror, scheduled_time, tries) + self.class.perform_in(mirror.backoff_delay, mirror.id, scheduled_time, tries + 1) end end diff --git a/changelogs/unreleased/34414-update-personal-access-token-scope-descriptions-to-reflect-registry-permissions.yml b/changelogs/unreleased/34414-update-personal-access-token-scope-descriptions-to-reflect-registry-permissions.yml new file mode 100644 index 00000000000..f0cc7fe9b6d --- /dev/null +++ b/changelogs/unreleased/34414-update-personal-access-token-scope-descriptions-to-reflect-registry-permissions.yml @@ -0,0 +1,6 @@ +--- +title: Updated the personal access token api scope description to reflect the permissions + it grants +merge_request: 31759 +author: +type: other diff --git a/changelogs/unreleased/65803-invalidate-branches-cache-on-refresh.yml b/changelogs/unreleased/65803-invalidate-branches-cache-on-refresh.yml new file mode 100644 index 00000000000..217db8aa05a --- /dev/null +++ b/changelogs/unreleased/65803-invalidate-branches-cache-on-refresh.yml @@ -0,0 +1,5 @@ +--- +title: Invalidate branches cache on PostReceive +merge_request: 31653 +author: +type: fixed diff --git a/changelogs/unreleased/66008-fix-project-image-in-slack-notifications.yml b/changelogs/unreleased/66008-fix-project-image-in-slack-notifications.yml new file mode 100644 index 00000000000..df0ac649ac1 --- /dev/null +++ b/changelogs/unreleased/66008-fix-project-image-in-slack-notifications.yml @@ -0,0 +1,5 @@ +--- +title: Fix project avatar image in Slack pipeline notifications +merge_request: 31788 +author: +type: fixed diff --git a/changelogs/unreleased/bump_helm_kubectl_gitlab.yml b/changelogs/unreleased/bump_helm_kubectl_gitlab.yml new file mode 100644 index 00000000000..d768462e130 --- /dev/null +++ b/changelogs/unreleased/bump_helm_kubectl_gitlab.yml @@ -0,0 +1,5 @@ +--- +title: Bump Helm to 2.14.3 and kubectl to 1.11.10 for Kubernetes integration +merge_request: 31716 +author: +type: other diff --git a/changelogs/unreleased/bvl-remote-mirror-exception-handling.yml b/changelogs/unreleased/bvl-remote-mirror-exception-handling.yml new file mode 100644 index 00000000000..962376086b0 --- /dev/null +++ b/changelogs/unreleased/bvl-remote-mirror-exception-handling.yml @@ -0,0 +1,6 @@ +--- +title: Retry push mirrors faster when running concurrently, improve error handling + when push mirrors fail +merge_request: 31247 +author: +type: changed diff --git a/changelogs/unreleased/id-source-code-smau.yml b/changelogs/unreleased/id-source-code-smau.yml new file mode 100644 index 00000000000..6ba5068544e --- /dev/null +++ b/changelogs/unreleased/id-source-code-smau.yml @@ -0,0 +1,5 @@ +--- +title: Add usage pings for source code pushes +merge_request: 31734 +author: +type: added diff --git a/changelogs/unreleased/issue-61873-no-error-message-for-general-settings.yml b/changelogs/unreleased/issue-61873-no-error-message-for-general-settings.yml new file mode 100644 index 00000000000..606c60721b8 --- /dev/null +++ b/changelogs/unreleased/issue-61873-no-error-message-for-general-settings.yml @@ -0,0 +1,5 @@ +--- +title: error message for general settings +merge_request: 31636 +author: Mesut Güneş +type: fixed diff --git a/changelogs/unreleased/sh-only-flush-tags-once-per-push.yml b/changelogs/unreleased/sh-only-flush-tags-once-per-push.yml new file mode 100644 index 00000000000..502fc22ebbd --- /dev/null +++ b/changelogs/unreleased/sh-only-flush-tags-once-per-push.yml @@ -0,0 +1,5 @@ +--- +title: Only expire tag cache once per push +merge_request: 31641 +author: +type: performance diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index a8234263275..258d8a99986 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -69,7 +69,7 @@ en: email: Allows read-only access to the user's primary email address using OpenID Connect scope_desc: api: - Grants complete read/write access to the API, including all groups and projects. + Grants complete read/write access to the API, including all groups and projects, the container registry, and the package registry. read_user: Grants read-only access to the authenticated user's profile through the /user API endpoint, which includes username, public email, and full name. Also grants access to read-only API endpoints under /users. read_repository: diff --git a/db/migrate/20190715173819_add_object_storage_flag_to_geo_node.rb b/db/migrate/20190715173819_add_object_storage_flag_to_geo_node.rb new file mode 100644 index 00000000000..2d3243f3357 --- /dev/null +++ b/db/migrate/20190715173819_add_object_storage_flag_to_geo_node.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddObjectStorageFlagToGeoNode < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :geo_nodes, :sync_object_storage, :boolean, default: false + end + + def down + remove_column :geo_nodes, :sync_object_storage + end +end diff --git a/db/schema.rb b/db/schema.rb index 591758af0e4..003f12b5171 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1456,6 +1456,7 @@ ActiveRecord::Schema.define(version: 2019_08_06_071559) do t.integer "container_repositories_max_capacity", default: 10, null: false t.datetime_with_timezone "created_at" t.datetime_with_timezone "updated_at" + t.boolean "sync_object_storage", default: false, null: false t.index ["access_key"], name: "index_geo_nodes_on_access_key" t.index ["name"], name: "index_geo_nodes_on_name", unique: true t.index ["primary"], name: "index_geo_nodes_on_primary" diff --git a/doc/api/geo_nodes.md b/doc/api/geo_nodes.md index e3af5d60533..d0b33ab467f 100644 --- a/doc/api/geo_nodes.md +++ b/doc/api/geo_nodes.md @@ -10,7 +10,7 @@ GET /geo_nodes ``` ```bash -curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/geo_nodes +curl --header "PRIVATE-TOKEN: <your_access_token>" https://primary.example.com/api/v4/geo_nodes ``` Example response: @@ -27,8 +27,15 @@ Example response: "current": true, "files_max_capacity": 10, "repos_max_capacity": 25, + "container_repositories_max_capacity": 10, "verification_max_capacity": 100, - "clone_protocol": "http" + "clone_protocol": "http", + "web_edit_url": "https://primary.example.com/admin/geo/nodes/1/edit", + "_links": { + "self": "https://primary.example.com/api/v4/geo_nodes/1", + "status":"https://primary.example.com/api/v4/geo_nodes/1/status", + "repair":"https://primary.example.com/api/v4/geo_nodes/1/repair" + } }, { "id": 2, @@ -40,8 +47,17 @@ Example response: "current": false, "files_max_capacity": 10, "repos_max_capacity": 25, + "container_repositories_max_capacity": 10, "verification_max_capacity": 100, - "clone_protocol": "http" + "sync_object_storage": true, + "clone_protocol": "http", + "web_edit_url": "https://primary.example.com/admin/geo/nodes/2/edit", + "web_geo_projects_url": "https://secondary.example.com/admin/geo/projects", + "_links": { + "self":"https://primary.example.com/api/v4/geo_nodes/2", + "status":"https://primary.example.com/api/v4/geo_nodes/2/status", + "repair":"https://primary.example.com/api/v4/geo_nodes/2/repair" + } } ] ``` @@ -53,7 +69,7 @@ GET /geo_nodes/:id ``` ```bash -curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/geo_nodes/1 +curl --header "PRIVATE-TOKEN: <your_access_token>" https://primary.example.com/api/v4/geo_nodes/1 ``` Example response: @@ -69,8 +85,15 @@ Example response: "current": true, "files_max_capacity": 10, "repos_max_capacity": 25, + "container_repositories_max_capacity": 10, "verification_max_capacity": 100, - "clone_protocol": "http" + "clone_protocol": "http", + "web_edit_url": "https://primary.example.com/admin/geo/nodes/1/edit", + "_links": { + "self": "https://primary.example.com/api/v4/geo_nodes/1", + "status":"https://primary.example.com/api/v4/geo_nodes/1/status", + "repair":"https://primary.example.com/api/v4/geo_nodes/1/repair" + } } ``` @@ -84,16 +107,18 @@ _This can only be run against a primary Geo node._ PUT /geo_nodes/:id ``` -| Attribute | Type | Required | Description | -|----------------------|---------|-----------|---------------------------------------------------------------------------| -| `id` | integer | yes | The ID of the Geo node. | -| `enabled` | boolean | no | Flag indicating if the Geo node is enabled. | -| `name` | string | yes | The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url`. | -| `url` | string | yes | The user-facing URL of the Geo node. | -| `internal_url` | string | no | The URL defined on the primary node that secondary nodes should use to contact it. Returns `url` if not set.| -| `files_max_capacity` | integer | no | Control the maximum concurrency of LFS/attachment backfill for this secondary node. | -| `repos_max_capacity` | integer | no | Control the maximum concurrency of repository backfill for this secondary node. | -| `verification_max_capacity` | integer | no | Control the maximum concurrency of verification for this node. | +| Attribute | Type | Required | Description | +|-----------------------------|---------|-----------|---------------------------------------------------------------------------| +| `id` | integer | yes | The ID of the Geo node. | +| `enabled` | boolean | no | Flag indicating if the Geo node is enabled. | +| `name` | string | yes | The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url`. | +| `url` | string | yes | The user-facing URL of the Geo node. | +| `internal_url` | string | no | The URL defined on the primary node that secondary nodes should use to contact it. Returns `url` if not set.| +| `files_max_capacity` | integer | no | Control the maximum concurrency of LFS/attachment backfill for this secondary node. | +| `repos_max_capacity` | integer | no | Control the maximum concurrency of repository backfill for this secondary node. | +| `verification_max_capacity` | integer | no | Control the maximum concurrency of verification for this node. | +| `container_repositories_max_capacity` | integer | no | Control the maximum concurrency of container repository sync for this node. | +| `sync_object_storage` | boolean | no | Flag indicating if the secondary Geo node will replicate blobs in Object Storage. | Example response: @@ -108,8 +133,17 @@ Example response: "current": true, "files_max_capacity": 10, "repos_max_capacity": 25, + "container_repositories_max_capacity": 10, "verification_max_capacity": 100, - "clone_protocol": "http" + "sync_object_storage": true, + "clone_protocol": "http", + "web_edit_url": "https://primary.example.com/admin/geo/nodes/2/edit", + "web_geo_projects_url": "https://secondary.example.com/admin/geo/projects", + "_links": { + "self":"https://primary.example.com/api/v4/geo_nodes/2", + "status":"https://primary.example.com/api/v4/geo_nodes/2/status", + "repair":"https://primary.example.com/api/v4/geo_nodes/2/repair" + } } ``` @@ -151,8 +185,15 @@ Example response: "current": true, "files_max_capacity": 10, "repos_max_capacity": 25, + "container_repositories_max_capacity": 10, "verification_max_capacity": 100, - "clone_protocol": "http" + "clone_protocol": "http", + "web_edit_url": "https://primary.example.com/admin/geo/nodes/1/edit", + "_links": { + "self": "https://primary.example.com/api/v4/geo_nodes/1", + "status":"https://primary.example.com/api/v4/geo_nodes/1/status", + "repair":"https://primary.example.com/api/v4/geo_nodes/1/repair" + } } ``` @@ -163,7 +204,7 @@ GET /geo_nodes/status ``` ```bash -curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/geo_nodes/status +curl --header "PRIVATE-TOKEN: <your_access_token>" https://primary.example.com/api/v4/geo_nodes/status ``` Example response: @@ -314,7 +355,7 @@ GET /geo_nodes/:id/status ``` ```bash -curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/geo_nodes/2/status +curl --header "PRIVATE-TOKEN: <your_access_token>" https://primary.example.com/api/v4/geo_nodes/2/status ``` Example response: @@ -388,7 +429,7 @@ GET /geo_nodes/current/failures This endpoint uses [Pagination](README.md#pagination). ```bash -curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/geo_nodes/current/failures +curl --header "PRIVATE-TOKEN: <your_access_token>" https://primary.example.com/api/v4/geo_nodes/current/failures ``` Example response: diff --git a/doc/api/releases/links.md b/doc/api/releases/links.md index 9c91264ed65..b1b75abe32f 100644 --- a/doc/api/releases/links.md +++ b/doc/api/releases/links.md @@ -21,7 +21,7 @@ GET /projects/:id/releases/:tag_name/assets/links Example request: ```sh -curl --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "http://localhost:3000/api/v4/projects/24/releases/v0.1/assets/links" +curl --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "https://gitlab.example.com/api/v4/projects/24/releases/v0.1/assets/links" ``` Example response: @@ -60,7 +60,7 @@ GET /projects/:id/releases/:tag_name/assets/links/:link_id Example request: ```sh -curl --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "http://localhost:3000/api/v4/projects/24/releases/v0.1/assets/links/1" +curl --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "https://gitlab.example.com/api/v4/projects/24/releases/v0.1/assets/links/1" ``` Example response: @@ -96,7 +96,7 @@ curl --request POST \ --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" \ --data name="awesome-v0.2.dmg" \ --data url="http://192.168.10.15:3000" \ - "http://localhost:3000/api/v4/projects/24/releases/v0.1/assets/links" + "https://gitlab.example.com/api/v4/projects/24/releases/v0.1/assets/links" ``` Example response: @@ -132,7 +132,7 @@ You have to specify at least one of `name` or `url` Example request: ```sh -curl --request PUT --data name="new name" --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "http://localhost:3000/api/v4/projects/24/releases/v0.1/assets/links/1" +curl --request PUT --data name="new name" --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "https://gitlab.example.com/api/v4/projects/24/releases/v0.1/assets/links/1" ``` Example response: @@ -163,7 +163,7 @@ DELETE /projects/:id/releases/:tag_name/assets/links/:link_id Example request: ```sh -curl --request DELETE --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "http://localhost:3000/api/v4/projects/24/releases/v0.1/assets/links/1" +curl --request DELETE --header "PRIVATE-TOKEN: gDybLx3yrUK_HLp3qPjS" "https://gitlab.example.com/api/v4/projects/24/releases/v0.1/assets/links/1" ``` Example response: diff --git a/doc/ci/environments.md b/doc/ci/environments.md index 9e1a62bae71..f6c47a99712 100644 --- a/doc/ci/environments.md +++ b/doc/ci/environments.md @@ -619,6 +619,10 @@ versions of the app, all without leaving GitLab. Add a [button to the Monitoring dashboard](../user/project/operations/linking_to_an_external_dashboard.md) linking directly to your existing external dashboards. +#### Embedding metrics in GitLab Flavored Markdown + +Metric charts can be embedded within GitLab Flavored Markdown. See [Embedding Metrics within GitLab Flavored Markdown](../user/project/integrations/prometheus.md#embedding-metric-charts-within-gitlab-flavored-markdown) for more details. + ### Web terminals > Web terminals were added in GitLab 8.15 and are only available to project Maintainers and Owners. diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 6bf6113dd81..deaef8e768b 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -17,6 +17,8 @@ Working with our frontend assets requires Node (v8.10.0 or greater) and Yarn For our currently-supported browsers, see our [requirements][requirements]. +Use [BrowserStack](https://www.browserstack.com/) to test with our supported browsers. Login to BrowserStack with the credentials saved in GitLab's [shared 1Password account](https://about.gitlab.com/handbook/security/#1password-for-teams). + ## Initiatives Current high-level frontend goals are listed on [Frontend Epics](https://gitlab.com/groups/gitlab-org/-/epics?label_name%5B%5D=frontend). diff --git a/doc/install/installation.md b/doc/install/installation.md index 72a3514e2d5..df6c485b1cb 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -172,7 +172,7 @@ sudo make install # Download and compile from source cd /tmp curl --remote-name --location --progress https://www.kernel.org/pub/software/scm/git/git-2.22.0.tar.gz -echo 'a4b7e4365bee43caa12a38d646d2c93743d755d1cea5eab448ffb40906c9da0b' git-2.22.0.tar.gz' | shasum -a256 -c - && tar -xzf git-2.22.0.tar.gz +echo 'a4b7e4365bee43caa12a38d646d2c93743d755d1cea5eab448ffb40906c9da0b git-2.22.0.tar.gz' | shasum -a256 -c - && tar -xzf git-2.22.0.tar.gz cd git-2.22.0/ ./configure --with-libpcre make prefix=/usr/local all diff --git a/doc/user/markdown.md b/doc/user/markdown.md index edb1e904f2b..6cfc8b6429b 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -533,6 +533,10 @@ This snippet links to `<wiki_root>/miscellaneous.md`: [Link to Related Page](/miscellaneous.md) ``` +### Embedding metrics in GitLab Flavored Markdown + +Metric charts can be embedded within GitLab Flavored Markdown. See [Embedding Metrics within GitLab flavored Markdown](../user/project/integrations/prometheus.md#embedding-metric-charts-within-gitlab-flavored-markdown) for more details. + ## Standard markdown and extensions in GitLab All standard markdown formatting should work as expected within GitLab. Some standard diff --git a/doc/user/profile/personal_access_tokens.md b/doc/user/profile/personal_access_tokens.md index 248188a6457..d556daa3460 100644 --- a/doc/user/profile/personal_access_tokens.md +++ b/doc/user/profile/personal_access_tokens.md @@ -44,7 +44,7 @@ the following table. | Scope | Introduced in | Description | | ------------------ | ------------- | ----------- | | `read_user` | [GitLab 8.15](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5951) | Allows access to the read-only endpoints under `/users`. Essentially, any of the `GET` requests in the [Users API][users] are allowed. | -| `api` | [GitLab 8.15](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5951) | Grants complete access to the API and Container Registry (read/write). | +| `api` | [GitLab 8.15](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5951) | Grants complete read/write access to the API, including all groups and projects, the container registry, and the package registry. | | `read_registry` | [GitLab 9.3](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11845) | Allows to read (pull) [container registry] images if a project is private and authorization is required. | | `sudo` | [GitLab 10.2](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14838) | Allows performing API actions as any user in the system (if the authenticated user is an admin). | | `read_repository` | [GitLab 10.7](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17894) | Allows read-only access (pull) to the repository through git clone. | diff --git a/doc/user/project/integrations/img/embed_metrics.png b/doc/user/project/integrations/img/embed_metrics.png Binary files differnew file mode 100644 index 00000000000..6f9660c9aec --- /dev/null +++ b/doc/user/project/integrations/img/embed_metrics.png diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index 44439b59e77..aa7db97c413 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -354,6 +354,27 @@ Prometheus server.  +## Embedding metric charts within Gitlab Flavored Markdown + +> [Introduced][ce-29691] in GitLab 12.2. +> Requires [Kubernetes](prometheus_library/kubernetes.md) metrics. + +It is possible to display metrics charts within [GitLab Flavored Markdown](../../markdown.md#gitlab-flavored-markdown-gfm). + +To display a metric chart, include a link of the form `https://<root_url>/<project>/environments/<environment_id>/metrics`. + +The following requirements must be met for the metric to unfurl: + +- The `<environment_id>` must correspond to a real environment. +- Prometheus must be monitoring the environment. +- The GitLab instance must be configured to receive data from the environment. +- The user must be allowed access to the monitoring dashboard for the environment ([Reporter or higher](../../permissions.md)). +- The dashboard must have data within the last 8 hours. + + If all of the above are true, then the metric will unfurl as seen below: + + + ## Troubleshooting If the "No data found" screen continues to appear, it could be due to: @@ -376,4 +397,5 @@ If the "No data found" screen continues to appear, it could be due to: [ci-environment-slug]: ../../../ci/variables/#predefined-environment-variables [ce-8935]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8935 [ce-10408]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10408 +[ce-29691]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29691 [promgldocs]: ../../../administration/monitoring/prometheus/index.md diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index 7ff30d1b813..7637e30dfb4 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -287,6 +287,8 @@ as pushing changes: - Set the target of the merge request to a particular branch. - Set the merge request to merge when its pipeline succeeds. - Set the merge request to remove the source branch when it's merged. +- Set the title of the merge request to a particular title. +- Set the description of the merge request to a particular description. ### Create a new merge request using git push options diff --git a/doc/user/project/pipelines/schedules.md b/doc/user/project/pipelines/schedules.md index 4e25d8545e9..f3e9c950efd 100644 --- a/doc/user/project/pipelines/schedules.md +++ b/doc/user/project/pipelines/schedules.md @@ -6,7 +6,9 @@ type: reference, howto > - Introduced in GitLab 9.1 as [Trigger Schedule](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10533). > - [Renamed to Pipeline Schedule](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10853) in GitLab 9.2. -> - Cron notation is parsed by [Fugit](https://github.com/floraison/fugit). + +NOTE: **Note:** +Cron notation is parsed by [Fugit](https://github.com/floraison/fugit). Pipelines are normally run based on certain conditions being met. For example, when a branch is pushed to repository. diff --git a/lib/expand_variables.rb b/lib/expand_variables.rb index c83cec9dc4a..45af30f46dc 100644 --- a/lib/expand_variables.rb +++ b/lib/expand_variables.rb @@ -3,6 +3,20 @@ module ExpandVariables class << self def expand(value, variables) + variables_hash = nil + + value.gsub(/\$([a-zA-Z_][a-zA-Z0-9_]*)|\${\g<1>}|%\g<1>%/) do + variables_hash ||= transform_variables(variables) + variables_hash[$1 || $2] + end + end + + private + + def transform_variables(variables) + # Lazily initialise variables + variables = variables.call if variables.is_a?(Proc) + # Convert hash array to variables if variables.is_a?(Array) variables = variables.reduce({}) do |hash, variable| @@ -11,9 +25,7 @@ module ExpandVariables end end - value.gsub(/\$([a-zA-Z_][a-zA-Z0-9_]*)|\${\g<1>}|%\g<1>%/) do - variables[$1 || $2] - end + variables end end end diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb index d98b85fecc4..24d752b8a4b 100644 --- a/lib/gitlab/git_post_receive.rb +++ b/lib/gitlab/git_post_receive.rb @@ -27,6 +27,18 @@ module Gitlab end end + def includes_branches? + enum_for(:changes_refs).any? do |_oldrev, _newrev, ref| + Gitlab::Git.branch_ref?(ref) + end + end + + def includes_tags? + enum_for(:changes_refs).any? do |_oldrev, _newrev, ref| + Gitlab::Git.tag_ref?(ref) + end + end + private def deserialize_changes(changes) diff --git a/lib/gitlab/kubernetes/helm.rb b/lib/gitlab/kubernetes/helm.rb index 42c4745ff98..6e4286589c1 100644 --- a/lib/gitlab/kubernetes/helm.rb +++ b/lib/gitlab/kubernetes/helm.rb @@ -3,8 +3,8 @@ module Gitlab module Kubernetes module Helm - HELM_VERSION = '2.12.3'.freeze - KUBECTL_VERSION = '1.11.7'.freeze + HELM_VERSION = '2.14.3'.freeze + KUBECTL_VERSION = '1.11.10'.freeze NAMESPACE = 'gitlab-managed-apps'.freeze SERVICE_ACCOUNT = 'tiller'.freeze CLUSTER_ROLE_BINDING = 'tiller-admin'.freeze diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 7e3a695e52a..16a076a8811 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -142,7 +142,8 @@ module Gitlab Gitlab::UsageDataCounters::WebIdeCounter, Gitlab::UsageDataCounters::NoteCounter, Gitlab::UsageDataCounters::SnippetCounter, - Gitlab::UsageDataCounters::SearchCounter + Gitlab::UsageDataCounters::SearchCounter, + Gitlab::UsageDataCounters::SourceCodeCounter ] end diff --git a/lib/gitlab/usage_data_counters/source_code_counter.rb b/lib/gitlab/usage_data_counters/source_code_counter.rb new file mode 100644 index 00000000000..8a1771a7bd1 --- /dev/null +++ b/lib/gitlab/usage_data_counters/source_code_counter.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Gitlab::UsageDataCounters + class SourceCodeCounter < BaseCounter + KNOWN_EVENTS = %w[pushes].freeze + PREFIX = 'source_code' + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d33c62031c4..27435576495 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6419,9 +6419,15 @@ msgstr "" msgid "Last seen" msgstr "" +msgid "Last successful update" +msgstr "" + msgid "Last update" msgstr "" +msgid "Last update attempt" +msgstr "" + msgid "Last updated" msgstr "" diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index 8ee3168273f..b958f419a19 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -60,7 +60,7 @@ describe Projects::RawController do execute_raw_requests(requests: 6, project: project, file_path: file_path) expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.') - expect(response).to redirect_to(project_blob_path(project, file_path)) + expect(response).to have_gitlab_http_status(429) end it 'logs the event on auth.log' do @@ -92,7 +92,7 @@ describe Projects::RawController do execute_raw_requests(requests: 3, project: project, file_path: modified_path) expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.') - expect(response).to redirect_to(project_blob_path(project, modified_path)) + expect(response).to have_gitlab_http_status(429) end end @@ -120,7 +120,7 @@ describe Projects::RawController do execute_raw_requests(requests: 6, project: project, file_path: file_path) expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.') - expect(response).to redirect_to(project_blob_path(project, file_path)) + expect(response).to have_gitlab_http_status(429) # Accessing upcase version of readme file_path = "#{commit_sha}/README.md" diff --git a/spec/features/projects/raw/user_interacts_with_raw_endpoint_spec.rb b/spec/features/projects/raw/user_interacts_with_raw_endpoint_spec.rb new file mode 100644 index 00000000000..6d587053b4f --- /dev/null +++ b/spec/features/projects/raw/user_interacts_with_raw_endpoint_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Projects > Raw > User interacts with raw endpoint' do + include RepoHelpers + + let(:user) { create(:user) } + let(:project) { create(:project, :repository, :public) } + let(:file_path) { 'master/README.md' } + + before do + stub_application_setting(raw_blob_request_limit: 3) + project.add_developer(user) + create_file_in_repo(project, 'master', 'master', 'README.md', 'readme content') + + sign_in(user) + end + + context 'when user access a raw file' do + it 'renders the page successfully' do + visit project_raw_url(project, file_path) + + expect(source).to eq('') # Body is filled in by gitlab-workhorse + end + end + + context 'when user goes over the rate requests limit' do + it 'returns too many requests' do + 4.times do + visit project_raw_url(project, file_path) + end + + expect(source).to have_content('You are being redirected') + click_link('redirected') + expect(page).to have_content('You cannot access the raw file. Please wait a minute.') + end + end +end diff --git a/spec/lib/expand_variables_spec.rb b/spec/lib/expand_variables_spec.rb index 099d7b6b67c..394efa85701 100644 --- a/spec/lib/expand_variables_spec.rb +++ b/spec/lib/expand_variables_spec.rb @@ -4,62 +4,131 @@ require 'spec_helper' describe ExpandVariables do describe '#expand' do - subject { described_class.expand(value, variables) } + context 'table tests' do + using RSpec::Parameterized::TableSyntax - tests = [ - { value: 'key', - result: 'key', - variables: [] }, - { value: 'key$variable', - result: 'key', - variables: [] }, - { value: 'key$variable', - result: 'keyvalue', - variables: [ - { key: 'variable', value: 'value' } - ] }, - { value: 'key${variable}', - result: 'keyvalue', - variables: [ - { key: 'variable', value: 'value' } - ] }, - { value: 'key$variable$variable2', - result: 'keyvalueresult', - variables: [ - { key: 'variable', value: 'value' }, - { key: 'variable2', value: 'result' } - ] }, - { value: 'key${variable}${variable2}', - result: 'keyvalueresult', - variables: [ - { key: 'variable', value: 'value' }, - { key: 'variable2', value: 'result' } - ] }, - { value: 'key$variable2$variable', - result: 'keyresultvalue', - variables: [ - { key: 'variable', value: 'value' }, - { key: 'variable2', value: 'result' } - ] }, - { value: 'key${variable2}${variable}', - result: 'keyresultvalue', - variables: [ - { key: 'variable', value: 'value' }, - { key: 'variable2', value: 'result' } - ] }, - { value: 'review/$CI_COMMIT_REF_NAME', - result: 'review/feature/add-review-apps', - variables: [ - { key: 'CI_COMMIT_REF_NAME', value: 'feature/add-review-apps' } - ] } - ] + where do + { + "no expansion": { + value: 'key', + result: 'key', + variables: [] + }, + "missing variable": { + value: 'key$variable', + result: 'key', + variables: [] + }, + "simple expansion": { + value: 'key$variable', + result: 'keyvalue', + variables: [ + { key: 'variable', value: 'value' } + ] + }, + "simple with hash of variables": { + value: 'key$variable', + result: 'keyvalue', + variables: { + 'variable' => 'value' + } + }, + "complex expansion": { + value: 'key${variable}', + result: 'keyvalue', + variables: [ + { key: 'variable', value: 'value' } + ] + }, + "simple expansions": { + value: 'key$variable$variable2', + result: 'keyvalueresult', + variables: [ + { key: 'variable', value: 'value' }, + { key: 'variable2', value: 'result' } + ] + }, + "complex expansions": { + value: 'key${variable}${variable2}', + result: 'keyvalueresult', + variables: [ + { key: 'variable', value: 'value' }, + { key: 'variable2', value: 'result' } + ] + }, + "complex expansions with missing variable": { + value: 'key${variable}${variable2}', + result: 'keyvalue', + variables: [ + { key: 'variable', value: 'value' } + ] + }, + "out-of-order expansion": { + value: 'key$variable2$variable', + result: 'keyresultvalue', + variables: [ + { key: 'variable', value: 'value' }, + { key: 'variable2', value: 'result' } + ] + }, + "out-of-order complex expansion": { + value: 'key${variable2}${variable}', + result: 'keyresultvalue', + variables: [ + { key: 'variable', value: 'value' }, + { key: 'variable2', value: 'result' } + ] + }, + "review-apps expansion": { + value: 'review/$CI_COMMIT_REF_NAME', + result: 'review/feature/add-review-apps', + variables: [ + { key: 'CI_COMMIT_REF_NAME', value: 'feature/add-review-apps' } + ] + }, + "do not lazily access variables when no expansion": { + value: 'key', + result: 'key', + variables: -> { raise NotImplementedError } + }, + "lazily access variables": { + value: 'key$variable', + result: 'keyvalue', + variables: -> { [{ key: 'variable', value: 'value' }] } + } + } + end + + with_them do + subject { ExpandVariables.expand(value, variables) } # rubocop:disable RSpec/DescribedClass + + it { is_expected.to eq(result) } + end + end + + context 'lazily inits variables' do + let(:variables) { -> { [{ key: 'variable', value: 'result' }] } } + + subject { described_class.expand(value, variables) } + + context 'when expanding variable' do + let(:value) { 'key$variable$variable2' } + + it 'calls block at most once' do + expect(variables).to receive(:call).once.and_call_original + + is_expected.to eq('keyresult') + end + end + + context 'when no expansion is needed' do + let(:value) { 'key' } - tests.each do |test| - context "#{test[:value]} resolves to #{test[:result]}" do - let(:value) { test[:value] } - let(:variables) { test[:variables] } + it 'does not call block' do + expect(variables).not_to receive(:call) - it { is_expected.to eq(test[:result]) } + is_expected.to eq('key') + end end end end diff --git a/spec/lib/gitlab/git_post_receive_spec.rb b/spec/lib/gitlab/git_post_receive_spec.rb new file mode 100644 index 00000000000..4c20d945585 --- /dev/null +++ b/spec/lib/gitlab/git_post_receive_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ::Gitlab::GitPostReceive do + let(:project) { create(:project) } + + subject { described_class.new(project, "project-#{project.id}", changes.dup, {}) } + + describe '#includes_branches?' do + context 'with no branches' do + let(:changes) do + <<~EOF + 654321 210987 refs/nobranches/tag1 + 654322 210986 refs/tags/test1 + 654323 210985 refs/merge-requests/mr1 + EOF + end + + it 'returns false' do + expect(subject.includes_branches?).to be_falsey + end + end + + context 'with branches' do + let(:changes) do + <<~EOF + 654322 210986 refs/heads/test1 + 654321 210987 refs/tags/tag1 + 654323 210985 refs/merge-requests/mr1 + EOF + end + + it 'returns true' do + expect(subject.includes_branches?).to be_truthy + end + end + + context 'with malformed changes' do + let(:changes) do + <<~EOF + ref/heads/1 a + somebranch refs/heads/2 + EOF + end + + it 'returns false' do + expect(subject.includes_branches?).to be_falsey + end + end + end + + describe '#includes_tags?' do + context 'with no tags' do + let(:changes) do + <<~EOF + 654321 210987 refs/notags/tag1 + 654322 210986 refs/heads/test1 + 654323 210985 refs/merge-requests/mr1 + EOF + end + + it 'returns false' do + expect(subject.includes_tags?).to be_falsey + end + end + + context 'with tags' do + let(:changes) do + <<~EOF + 654322 210986 refs/heads/test1 + 654321 210987 refs/tags/tag1 + 654323 210985 refs/merge-requests/mr1 + EOF + end + + it 'returns true' do + expect(subject.includes_tags?).to be_truthy + end + end + + context 'with malformed changes' do + let(:changes) do + <<~EOF + ref/tags/1 a + sometag refs/tags/2 + EOF + end + + it 'returns false' do + expect(subject.includes_tags?).to be_falsey + end + end + end +end diff --git a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb index 06c8d127951..fce2aded786 100644 --- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::Kubernetes::Helm::Pod do it 'generates the appropriate specifications for the container' do container = subject.generate.spec.containers.first expect(container.name).to eq('helm') - expect(container.image).to eq('registry.gitlab.com/gitlab-org/cluster-integration/helm-install-image/releases/2.12.3-kube-1.11.7') + expect(container.image).to eq('registry.gitlab.com/gitlab-org/cluster-integration/helm-install-image/releases/2.14.3-kube-1.11.10') expect(container.env.count).to eq(3) expect(container.env.map(&:name)).to match_array([:HELM_VERSION, :TILLER_NAMESPACE, :COMMAND_SCRIPT]) expect(container.command).to match_array(["/bin/sh"]) diff --git a/spec/lib/gitlab/usage_data_counters/source_code_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/source_code_counter_spec.rb new file mode 100644 index 00000000000..47077345e0c --- /dev/null +++ b/spec/lib/gitlab/usage_data_counters/source_code_counter_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::UsageDataCounters::SourceCodeCounter do + it_behaves_like 'a redis usage counter', 'Source Code', :pushes + + it_behaves_like 'a redis usage counter with totals', :source_code, pushes: 5 +end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index bf36273251b..f63f3b454e7 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -71,7 +71,8 @@ describe Gitlab::UsageData do web_ide_views: a_kind_of(Integer), web_ide_commits: a_kind_of(Integer), web_ide_merge_requests: a_kind_of(Integer), - navbar_searches: a_kind_of(Integer) + navbar_searches: a_kind_of(Integer), + source_code_pushes: a_kind_of(Integer) ) end diff --git a/spec/models/project_services/chat_message/pipeline_message_spec.rb b/spec/models/project_services/chat_message/pipeline_message_spec.rb index 619ab96af94..cf7c7bf7e61 100644 --- a/spec/models/project_services/chat_message/pipeline_message_spec.rb +++ b/spec/models/project_services/chat_message/pipeline_message_spec.rb @@ -42,9 +42,9 @@ describe ChatMessage::PipelineMessage do before do test_commit = double("A test commit", committer: args[:user], title: "A test commit message") - test_project = double("A test project", - commit_by: test_commit, name: args[:project][:name], - web_url: args[:project][:web_url], avatar_url: args[:project][:avatar_url]) + test_project = double("A test project", commit_by: test_commit, name: args[:project][:name], web_url: args[:project][:web_url]) + allow(test_project).to receive(:avatar_url).with(no_args).and_return("/avatar") + allow(test_project).to receive(:avatar_url).with(only_path: false).and_return(args[:project][:avatar_url]) allow(Project).to receive(:find) { test_project } test_pipeline = double("A test pipeline", has_yaml_errors?: has_yaml_errors, diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 29a589eba20..83ce58235dd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2252,6 +2252,21 @@ describe Project do end end + describe '#mark_stuck_remote_mirrors_as_failed!' do + it 'fails stuck remote mirrors' do + project = create(:project, :repository, :remote_mirror) + + project.remote_mirrors.first.update( + update_status: :started, + last_update_started_at: 2.days.ago + ) + + expect do + project.mark_stuck_remote_mirrors_as_failed! + end.to change { project.remote_mirrors.stuck.count }.from(1).to(0) + end + end + describe '#ancestors_upto' do let(:parent) { create(:group) } let(:child) { create(:group, parent: parent) } diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 687b0935c55..7edeb56efe2 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -153,14 +153,14 @@ describe RemoteMirror, :mailer do end end - describe '#mark_as_failed' do + describe '#mark_as_failed!' do let(:remote_mirror) { create(:remote_mirror) } let(:error_message) { 'http://user:pass@test.com/root/repoC.git/' } let(:sanitized_error_message) { 'http://*****:*****@test.com/root/repoC.git/' } subject do remote_mirror.update_start - remote_mirror.mark_as_failed(error_message) + remote_mirror.mark_as_failed!(error_message) end it 'sets the update_status to failed' do @@ -204,8 +204,8 @@ describe RemoteMirror, :mailer do it 'includes mirrors that were started over an hour ago' do mirror = create_mirror(url: 'http://cantbeblank', update_status: 'started', - last_update_at: 3.hours.ago, - updated_at: 2.hours.ago) + last_update_started_at: 3.hours.ago, + last_update_at: 2.hours.ago) expect(described_class.stuck.last).to eq(mirror) end @@ -214,7 +214,7 @@ describe RemoteMirror, :mailer do mirror = create_mirror(url: 'http://cantbeblank', update_status: 'started', last_update_at: nil, - updated_at: 4.hours.ago) + last_update_started_at: 4.hours.ago) expect(described_class.stuck.last).to eq(mirror) end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 12dff440ce2..e68de2e73a8 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1744,12 +1744,23 @@ describe Repository do end end - describe '#before_push_tag' do + describe '#expires_caches_for_tags' do it 'flushes the cache' do expect(repository).to receive(:expire_statistics_caches) expect(repository).to receive(:expire_emptiness_caches) expect(repository).to receive(:expire_tags_cache) + repository.expire_caches_for_tags + end + end + + describe '#before_push_tag' do + it 'logs an event' do + expect(repository).not_to receive(:expire_statistics_caches) + expect(repository).not_to receive(:expire_emptiness_caches) + expect(repository).not_to receive(:expire_tags_cache) + expect(repository).to receive(:repository_event).with(:push_tag) + repository.before_push_tag end end @@ -1781,6 +1792,12 @@ describe Repository do repository.after_create_branch end + + it 'does not expire the branch caches when specified' do + expect(repository).not_to receive(:expire_branches_cache) + + repository.after_create_branch(expire_cache: false) + end end describe '#after_remove_branch' do @@ -1789,6 +1806,12 @@ describe Repository do repository.after_remove_branch end + + it 'does not expire the branch caches when specified' do + expect(repository).not_to receive(:expire_branches_cache) + + repository.after_remove_branch(expire_cache: false) + end end describe '#after_create' do diff --git a/spec/services/git/tag_push_service_spec.rb b/spec/services/git/tag_push_service_spec.rb index 418952b52da..7e008637182 100644 --- a/spec/services/git/tag_push_service_spec.rb +++ b/spec/services/git/tag_push_service_spec.rb @@ -26,8 +26,8 @@ describe Git::TagPushService do subject end - it 'flushes the tags cache' do - expect(project.repository).to receive(:expire_tags_cache) + it 'does not flush the tags cache' do + expect(project.repository).not_to receive(:expire_tags_cache) subject end diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index be2811ab1e7..4396ccab584 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -10,49 +10,91 @@ describe Projects::UpdateRemoteMirrorService do subject(:service) { described_class.new(project, project.creator) } - describe "#execute" do + describe '#execute' do + subject(:execute!) { service.execute(remote_mirror, 0) } + before do project.repository.add_branch(project.owner, 'existing-branch', 'master') allow(remote_mirror).to receive(:update_repository).and_return(true) end - it "ensures the remote exists" do + it 'ensures the remote exists' do stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) expect(remote_mirror).to receive(:ensure_remote!) - service.execute(remote_mirror) + execute! end - it "fetches the remote repository" do + it 'fetches the remote repository' do expect(project.repository) .to receive(:fetch_remote) - .with(remote_mirror.remote_name, no_tags: true, ssh_auth: remote_mirror) + .with(remote_mirror.remote_name, no_tags: true, ssh_auth: remote_mirror) - service.execute(remote_mirror) + execute! end - it "returns success when updated succeeds" do + it 'marks the mirror as started when beginning' do + expect(remote_mirror).to receive(:update_start!).and_call_original + + execute! + end + + it 'marks the mirror as successfully finished' do stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) - result = service.execute(remote_mirror) + result = execute! expect(result[:status]).to eq(:success) + expect(remote_mirror).to be_finished + end + + it 'marks the mirror as failed and raises the error when an unexpected error occurs' do + allow(project.repository).to receive(:fetch_remote).and_raise('Badly broken') + + expect { execute! }.to raise_error /Badly broken/ + + expect(remote_mirror).to be_failed + expect(remote_mirror.last_error).to include('Badly broken') + end + + context 'when the update fails because of a `Gitlab::Git::CommandError`' do + before do + allow(project.repository).to receive(:fetch_remote).and_raise(Gitlab::Git::CommandError.new('fetch failed')) + end + + it 'wraps `Gitlab::Git::CommandError`s in a service error' do + expect(execute!).to eq(status: :error, message: 'fetch failed') + end + + it 'marks the mirror as to be retried' do + execute! + + expect(remote_mirror).to be_to_retry + expect(remote_mirror.last_error).to include('fetch failed') + end + + it "marks the mirror as failed after #{described_class::MAX_TRIES} tries" do + service.execute(remote_mirror, described_class::MAX_TRIES) + + expect(remote_mirror).to be_failed + expect(remote_mirror.last_error).to include('fetch failed') + end end context 'when syncing all branches' do - it "push all the branches the first time" do + it 'push all the branches the first time' do stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) expect(remote_mirror).to receive(:update_repository).with({}) - service.execute(remote_mirror) + execute! end end context 'when only syncing protected branches' do - it "sync updated protected branches" do + it 'sync updated protected branches' do stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) protected_branch = create_protected_branch(project) remote_mirror.only_protected_branches = true @@ -61,7 +103,7 @@ describe Projects::UpdateRemoteMirrorService do .to receive(:update_repository) .with(only_branches_matching: [protected_branch.name]) - service.execute(remote_mirror) + execute! end def create_protected_branch(project) diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 39f1beb4efa..4c9fc0e3a24 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -14,6 +14,10 @@ describe PostReceive do create(:project, :repository, auto_cancel_pending_pipelines: 'disabled') end + def perform(changes: base64_changes) + described_class.new.perform(gl_repository, key_id, changes) + end + context "as a sidekiq worker" do it "responds to #perform" do expect(described_class.new).to respond_to(:perform) @@ -28,7 +32,7 @@ describe PostReceive do it "returns false and logs an error" do expect(Gitlab::GitLogger).to receive(:error).with("POST-RECEIVE: #{error_message}") - expect(described_class.new.perform(gl_repository, key_id, base64_changes)).to be(false) + expect(perform).to be(false) end end @@ -39,7 +43,7 @@ describe PostReceive do expect(Git::TagPushService).not_to receive(:new) expect_next_instance_of(SystemHooksService) { |service| expect(service).to receive(:execute_hooks) } - described_class.new.perform(gl_repository, key_id, "") + perform(changes: "") end end @@ -50,40 +54,80 @@ describe PostReceive do expect(Git::BranchPushService).not_to receive(:new) expect(Git::TagPushService).not_to receive(:new) - expect(described_class.new.perform(gl_repository, key_id, base64_changes)).to be false + expect(perform).to be false end end context 'with changes' do before do allow_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(project.owner) + allow(Gitlab::GlRepository).to receive(:parse).and_return([project, Gitlab::GlRepository::PROJECT]) end context "branches" do - let(:changes) { "123456 789012 refs/heads/tést" } + let(:changes) do + <<~EOF + '123456 789012 refs/heads/tést1' + '123456 789012 refs/heads/tést2' + EOF + end - it "calls Git::BranchPushService" do - expect_next_instance_of(Git::BranchPushService) do |service| + it 'expires the branches cache' do + expect(project.repository).to receive(:expire_branches_cache).once + + described_class.new.perform(gl_repository, key_id, base64_changes) + end + + it 'calls Git::BranchPushService' do + expect_any_instance_of(Git::BranchPushService) do |service| expect(service).to receive(:execute).and_return(true) end expect(Git::TagPushService).not_to receive(:new) - described_class.new.perform(gl_repository, key_id, base64_changes) + perform end end context "tags" do - let(:changes) { "123456 789012 refs/tags/tag" } + let(:changes) do + <<~EOF + 654321 210987 refs/tags/tag1 + 654322 210986 refs/tags/tag2 + 654323 210985 refs/tags/tag3 + 654324 210984 refs/tags/tag4 + 654325 210983 refs/tags/tag5 + EOF + end + + before do + expect(Gitlab::GlRepository).to receive(:parse).and_return([project, Gitlab::GlRepository::PROJECT]) + end + + it 'does not expire branches cache' do + expect(project.repository).not_to receive(:expire_branches_cache) + + described_class.new.perform(gl_repository, key_id, base64_changes) + end + + it "only invalidates tags once" do + expect(project.repository).to receive(:repository_event).exactly(5).times.with(:push_tag).and_call_original + expect(project.repository).to receive(:expire_caches_for_tags).once.and_call_original + expect(project.repository).to receive(:expire_tags_cache).once.and_call_original + + described_class.new.perform(gl_repository, key_id, base64_changes) + end it "calls Git::TagPushService" do - expect(Git::BranchPushService).not_to receive(:execute) + expect(Git::BranchPushService).not_to receive(:new) - expect_next_instance_of(Git::TagPushService) do |service| + expect_any_instance_of(Git::TagPushService) do |service| expect(service).to receive(:execute).and_return(true) end - described_class.new.perform(gl_repository, key_id, base64_changes) + expect(Git::BranchPushService).not_to receive(:new) + + perform end end @@ -94,7 +138,7 @@ describe PostReceive do expect(Git::BranchPushService).not_to receive(:new) expect(Git::TagPushService).not_to receive(:new) - described_class.new.perform(gl_repository, key_id, base64_changes) + perform end end @@ -111,7 +155,7 @@ describe PostReceive do let(:changes_count) { changes.lines.count } - subject { described_class.new.perform(gl_repository, key_id, base64_changes) } + subject { perform } context "with valid .gitlab-ci.yml" do before do @@ -180,7 +224,13 @@ describe PostReceive do it 'calls SystemHooksService' do expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true) - described_class.new.perform(gl_repository, key_id, base64_changes) + perform + end + + it 'increments the usage data counter of pushes event' do + counter = Gitlab::UsageDataCounters::SourceCodeCounter + + expect { perform }.to change { counter.read(:pushes) }.by(1) end end end @@ -197,7 +247,7 @@ describe PostReceive do # a second to ensure we see changes. Timecop.freeze(1.second.from_now) do expect do - described_class.new.perform(gl_repository, key_id, base64_changes) + perform project.reload end.to change(project, :last_activity_at) .and change(project, :last_repository_updated_at) @@ -208,7 +258,8 @@ describe PostReceive do context "webhook" do it "fetches the correct project" do expect(Project).to receive(:find_by).with(id: project.id.to_s) - described_class.new.perform(gl_repository, key_id, base64_changes) + + perform end it "does not run if the author is not in the project" do @@ -218,7 +269,7 @@ describe PostReceive do expect(project).not_to receive(:execute_hooks) - expect(described_class.new.perform(gl_repository, key_id, base64_changes)).to be_falsey + expect(perform).to be_falsey end it "asks the project to trigger all hooks" do @@ -227,7 +278,7 @@ describe PostReceive do expect(project).to receive(:execute_hooks).twice expect(project).to receive(:execute_services).twice - described_class.new.perform(gl_repository, key_id, base64_changes) + perform end it "enqueues a UpdateMergeRequestsWorker job" do @@ -235,7 +286,7 @@ describe PostReceive do expect(UpdateMergeRequestsWorker).to receive(:perform_async).with(project.id, project.owner.id, any_args) - described_class.new.perform(gl_repository, key_id, base64_changes) + perform end end end diff --git a/spec/workers/repository_update_remote_mirror_worker_spec.rb b/spec/workers/repository_update_remote_mirror_worker_spec.rb index 4de51ecb3e9..66d517332ba 100644 --- a/spec/workers/repository_update_remote_mirror_worker_spec.rb +++ b/spec/workers/repository_update_remote_mirror_worker_spec.rb @@ -2,99 +2,70 @@ require 'rails_helper' -describe RepositoryUpdateRemoteMirrorWorker do +describe RepositoryUpdateRemoteMirrorWorker, :clean_gitlab_redis_shared_state do subject { described_class.new } - let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } + let(:remote_mirror) { create(:remote_mirror) } let(:scheduled_time) { Time.now - 5.minutes } around do |example| Timecop.freeze(Time.now) { example.run } end - describe '#perform' do - context 'with status none' do - before do - remote_mirror.update(update_status: 'none') - end - - it 'sets status as finished when update remote mirror service executes successfully' do - expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success) - - expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.update_status }.to('finished') - end - - it 'resets the notification flag upon success' do - expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success) - remote_mirror.update_column(:error_notification_sent, true) - - expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.error_notification_sent }.to(false) - end - - it 'sets status as failed when update remote mirror service executes with errors' do - error_message = 'fail!' - - expect_next_instance_of(Projects::UpdateRemoteMirrorService) do |service| - expect(service).to receive(:execute).with(remote_mirror).and_return(status: :error, message: error_message) - end + def expect_mirror_service_to_return(mirror, result, tries = 0) + expect_next_instance_of(Projects::UpdateRemoteMirrorService) do |service| + expect(service).to receive(:execute).with(mirror, tries).and_return(result) + end + end - # Mock the finder so that it returns an object we can set expectations on - expect_next_instance_of(RemoteMirrorFinder) do |finder| - expect(finder).to receive(:execute).and_return(remote_mirror) - end - expect(remote_mirror).to receive(:mark_as_failed).with(error_message) + describe '#perform' do + it 'calls out to the service to perform the update' do + expect_mirror_service_to_return(remote_mirror, status: :success) - expect do - subject.perform(remote_mirror.id, Time.now) - end.to raise_error(RepositoryUpdateRemoteMirrorWorker::UpdateError, error_message) - end + subject.perform(remote_mirror.id, scheduled_time) + end - it 'does nothing if last_update_started_at is higher than the time the job was scheduled in' do - remote_mirror.update(last_update_started_at: Time.now) + it 'does not do anything if the mirror was already updated' do + remote_mirror.update(last_update_started_at: Time.now, update_status: :finished) - expect_any_instance_of(RemoteMirror).to receive(:updated_since?).with(scheduled_time).and_return(true) - expect_any_instance_of(Projects::UpdateRemoteMirrorService).not_to receive(:execute).with(remote_mirror) + expect(Projects::UpdateRemoteMirrorService).not_to receive(:new) - expect(subject.perform(remote_mirror.id, scheduled_time)).to be_nil - end + subject.perform(remote_mirror.id, scheduled_time) end - context 'with unexpected error' do - it 'marks mirror as failed' do - allow_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_raise(RuntimeError) + it 'schedules a retry when the mirror is marked for retrying' do + remote_mirror = create(:remote_mirror, update_status: :to_retry) + expect_mirror_service_to_return(remote_mirror, status: :error, message: 'Retry!') - expect do - subject.perform(remote_mirror.id, Time.now) - end.to raise_error(RepositoryUpdateRemoteMirrorWorker::UpdateError) - expect(remote_mirror.reload.update_status).to eq('failed') - end - end + expect(described_class) + .to receive(:perform_in) + .with(remote_mirror.backoff_delay, remote_mirror.id, scheduled_time, 1) - context 'with another worker already running' do - before do - remote_mirror.update(update_status: 'started') - end - - it 'raises RemoteMirrorUpdateAlreadyInProgressError' do - expect do - subject.perform(remote_mirror.id, Time.now) - end.to raise_error(RepositoryUpdateRemoteMirrorWorker::UpdateAlreadyInProgressError) - end + subject.perform(remote_mirror.id, scheduled_time) end - context 'with status failed' do - before do - remote_mirror.update(update_status: 'failed') + it 'clears the lease if there was an unexpected exception' do + expect_next_instance_of(Projects::UpdateRemoteMirrorService) do |service| + expect(service).to receive(:execute).with(remote_mirror, 1).and_raise('Unexpected!') end + expect { subject.perform(remote_mirror.id, Time.now, 1) }.to raise_error('Unexpected!') - it 'sets status as finished if last_update_started_at is higher than the time the job was scheduled in' do - remote_mirror.update(last_update_started_at: Time.now) + lease = Gitlab::ExclusiveLease.new("#{described_class.name}:#{remote_mirror.id}", timeout: 1.second) - expect_any_instance_of(RemoteMirror).to receive(:updated_since?).with(scheduled_time).and_return(false) - expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success) + expect(lease.try_obtain).not_to be_nil + end - expect { subject.perform(remote_mirror.id, scheduled_time) }.to change { remote_mirror.reload.update_status }.to('finished') - end + it 'retries 3 times for the worker to finish before rescheduling' do + expect(subject).to receive(:in_lock) + .with("#{described_class.name}:#{remote_mirror.id}", + retries: 3, + ttl: remote_mirror.max_runtime, + sleep_sec: described_class::LOCK_WAIT_TIME) + .and_raise(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError) + expect(described_class).to receive(:perform_in) + .with(remote_mirror.backoff_delay, remote_mirror.id, scheduled_time, 0) + + subject.perform(remote_mirror.id, scheduled_time) end end end |
