diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-13 12:09:50 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-13 12:09:50 +0000 |
commit | 5605efec12c99adf88f641391cb879dedf8fa05e (patch) | |
tree | 4aea188ef160dec9346f0bcf61ecbe9cb7fa6661 | |
parent | 4e33606f0114c39e07f0151465299f75bfe00c3e (diff) | |
download | gitlab-ce-fix-master-merge-train-helper.tar.gz |
Add latest changes from gitlab-org/gitlab@masterfix-master-merge-train-helper
60 files changed, 522 insertions, 289 deletions
diff --git a/app/assets/javascripts/alert_management/components/alert_details.vue b/app/assets/javascripts/alert_management/components/alert_details.vue index f9795a18774..8c3ec527ef6 100644 --- a/app/assets/javascripts/alert_management/components/alert_details.vue +++ b/app/assets/javascripts/alert_management/components/alert_details.vue @@ -35,13 +35,24 @@ export default { errorMsg: s__( 'AlertManagement|There was an error displaying the alert. Please refresh the page to try again.', ), - fullAlertDetailsTitle: s__('AlertManagement|Alert details'), - overviewTitle: s__('AlertManagement|Overview'), - metricsTitle: s__('AlertManagement|Metrics'), reportedAt: s__('AlertManagement|Reported %{when}'), reportedAtWithTool: s__('AlertManagement|Reported %{when} by %{tool}'), }, severityLabels: ALERTS_SEVERITY_LABELS, + tabsConfig: [ + { + id: 'overview', + title: s__('AlertManagement|Overview'), + }, + { + id: 'fullDetails', + title: s__('AlertManagement|Alert details'), + }, + { + id: 'metrics', + title: s__('AlertManagement|Metrics'), + }, + ], components: { GlBadge, GlAlert, @@ -119,6 +130,18 @@ export default { showErrorMsg() { return this.errored && !this.isErrorDismissed; }, + activeTab() { + return this.$route.params.tabId || this.$options.tabsConfig[0].id; + }, + currentTabIndex: { + get() { + return this.$options.tabsConfig.findIndex(tab => tab.id === this.activeTab); + }, + set(tabIdx) { + const tabId = this.$options.tabsConfig[tabIdx].id; + this.$router.replace({ name: 'tab', params: { tabId } }); + }, + }, }, mounted() { this.trackPageViews(); @@ -257,8 +280,8 @@ export default { > <h2 data-testid="title">{{ alert.title }}</h2> </div> - <gl-tabs v-if="alert" data-testid="alertDetailsTabs"> - <gl-tab data-testid="overviewTab" :title="$options.i18n.overviewTitle"> + <gl-tabs v-if="alert" v-model="currentTabIndex" data-testid="alertDetailsTabs"> + <gl-tab :data-testid="$options.tabsConfig[0].id" :title="$options.tabsConfig[0].title"> <div v-if="alert.severity" class="gl-mt-3 gl-mb-5 gl-display-flex"> <div class="gl-font-weight-bold gl-w-13 gl-text-right gl-pr-3"> {{ s__('AlertManagement|Severity') }}: @@ -309,7 +332,7 @@ export default { </div> </template> </gl-tab> - <gl-tab data-testid="fullDetailsTab" :title="$options.i18n.fullAlertDetailsTitle"> + <gl-tab :data-testid="$options.tabsConfig[1].id" :title="$options.tabsConfig[1].title"> <gl-table class="alert-management-details-table" :items="[{ key: 'Value', ...alert }]" @@ -325,7 +348,7 @@ export default { </template> </gl-table> </gl-tab> - <gl-tab data-testId="metricsTab" :title="$options.i18n.metricsTitle"> + <gl-tab :data-testid="$options.tabsConfig[2].id" :title="$options.tabsConfig[2].title"> <alert-metrics :dashboard-url="alert.metricsDashboardUrl" /> </gl-tab> </gl-tabs> diff --git a/app/assets/javascripts/alert_management/details.js b/app/assets/javascripts/alert_management/details.js index 2820bcb9665..5133b1b84ce 100644 --- a/app/assets/javascripts/alert_management/details.js +++ b/app/assets/javascripts/alert_management/details.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import createDefaultClient from '~/lib/graphql'; +import createRouter from './router'; import { defaultDataIdFromObject } from 'apollo-cache-inmemory'; import AlertDetails from './components/alert_details.vue'; import sidebarStatusQuery from './graphql/queries/sidebar_status.query.graphql'; @@ -10,6 +11,7 @@ Vue.use(VueApollo); export default selector => { const domEl = document.querySelector(selector); const { alertId, projectPath, projectIssuesPath, projectId } = domEl.dataset; + const router = createRouter(); const resolvers = { Mutation: { @@ -54,6 +56,7 @@ export default selector => { components: { AlertDetails, }, + router, render(createElement) { return createElement('alert-details', {}); }, diff --git a/app/assets/javascripts/alert_management/router.js b/app/assets/javascripts/alert_management/router.js new file mode 100644 index 00000000000..5687fe4e0f5 --- /dev/null +++ b/app/assets/javascripts/alert_management/router.js @@ -0,0 +1,13 @@ +import Vue from 'vue'; +import VueRouter from 'vue-router'; +import { joinPaths } from '~/lib/utils/url_utility'; + +Vue.use(VueRouter); + +export default function createRouter(base) { + return new VueRouter({ + mode: 'hash', + base: joinPaths(gon.relative_url_root || '', base), + routes: [{ path: '/:tabId', name: 'tab' }], + }); +} diff --git a/app/assets/javascripts/import_projects/components/imported_project_table_row.vue b/app/assets/javascripts/import_projects/components/imported_project_table_row.vue index ab2bd87ee9f..a1c54b11276 100644 --- a/app/assets/javascripts/import_projects/components/imported_project_table_row.vue +++ b/app/assets/javascripts/import_projects/components/imported_project_table_row.vue @@ -1,4 +1,5 @@ <script> +import { GlIcon } from '@gitlab/ui'; import ImportStatus from './import_status.vue'; import { STATUSES } from '../constants'; @@ -6,6 +7,7 @@ export default { name: 'ImportedProjectTableRow', components: { ImportStatus, + GlIcon, }, props: { project: { @@ -36,6 +38,7 @@ export default { class="js-provider-link" > {{ project.importSource }} + <gl-icon v-if="project.providerLink" name="external-link" /> </a> </td> <td class="js-full-path">{{ displayFullPath }}</td> diff --git a/app/assets/javascripts/import_projects/components/incompatible_repo_table_row.vue b/app/assets/javascripts/import_projects/components/incompatible_repo_table_row.vue index fa2fb439eac..d44c155a84e 100644 --- a/app/assets/javascripts/import_projects/components/incompatible_repo_table_row.vue +++ b/app/assets/javascripts/import_projects/components/incompatible_repo_table_row.vue @@ -1,9 +1,10 @@ <script> -import { GlBadge } from '@gitlab/ui'; +import { GlIcon, GlBadge } from '@gitlab/ui'; export default { components: { GlBadge, + GlIcon, }, props: { repo: { @@ -19,6 +20,7 @@ export default { <td> <a :href="repo.providerLink" rel="noreferrer noopener" target="_blank"> {{ repo.fullName }} + <gl-icon v-if="repo.providerLink" name="external-link" /> </a> </td> <td></td> diff --git a/app/assets/javascripts/import_projects/components/provider_repo_table_row.vue b/app/assets/javascripts/import_projects/components/provider_repo_table_row.vue index 63524d61146..a03e3d50135 100644 --- a/app/assets/javascripts/import_projects/components/provider_repo_table_row.vue +++ b/app/assets/javascripts/import_projects/components/provider_repo_table_row.vue @@ -1,5 +1,6 @@ <script> import { mapState, mapGetters, mapActions } from 'vuex'; +import { GlIcon } from '@gitlab/ui'; import Select2Select from '~/vue_shared/components/select2_select.vue'; import { __ } from '~/locale'; import eventHub from '../event_hub'; @@ -11,6 +12,7 @@ export default { components: { Select2Select, ImportStatus, + GlIcon, }, props: { repo: { @@ -84,6 +86,7 @@ export default { class="js-provider-link" > {{ repo.fullName }} + <gl-icon v-if="repo.providerLink" name="external-link" /> </a> </td> <td class="d-flex flex-wrap flex-lg-nowrap"> diff --git a/app/assets/javascripts/lib/graphql.js b/app/assets/javascripts/lib/graphql.js index b6c41ffa7ab..4fed121779e 100644 --- a/app/assets/javascripts/lib/graphql.js +++ b/app/assets/javascripts/lib/graphql.js @@ -14,7 +14,7 @@ export const fetchPolicies = { }; export default (resolvers = {}, config = {}) => { - let uri = `${gon.relative_url_root}/api/graphql`; + let uri = `${gon.relative_url_root || ''}/api/graphql`; if (config.baseUrl) { // Prepend baseUrl and ensure that `///` are replaced with `/` diff --git a/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue b/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue index d82b8e0992e..943cee9b504 100644 --- a/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue +++ b/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue @@ -11,7 +11,6 @@ import { GlTooltipDirective, } from '@gitlab/ui'; import CustomMetricsFormFields from '~/custom_metrics/components/custom_metrics_form_fields.vue'; -import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { PANEL_NEW_PAGE } from '../router/constants'; import DuplicateDashboardModal from './duplicate_dashboard_modal.vue'; import CreateDashboardModal from './create_dashboard_modal.vue'; @@ -38,7 +37,6 @@ export default { GlTooltip: GlTooltipDirective, TrackEvent: TrackEventDirective, }, - mixins: [glFeatureFlagsMixin()], props: { addingMetricsAvailable: { type: Boolean, @@ -86,7 +84,6 @@ export default { }, isMenuItemShown() { return { - addPanel: this.glFeatures.metricsDashboardNewPanelPage, duplicateDashboard: this.isOutOfTheBoxDashboard, }; }, @@ -192,31 +189,29 @@ export default { </gl-modal> </template> - <template v-if="isMenuItemShown.addPanel"> + <gl-new-dropdown-item + v-if="isMenuItemEnabled.addPanel" + data-testid="add-panel-item-enabled" + :to="newPanelPageLocation" + > + {{ $options.i18n.addPanel }} + </gl-new-dropdown-item> + + <!-- + wrapper for tooltip as button can be `disabled` + https://bootstrap-vue.org/docs/components/tooltip#disabled-elements + --> + <div v-else v-gl-tooltip :title="$options.i18n.addPanelInfo"> <gl-new-dropdown-item - v-if="isMenuItemEnabled.addPanel" - data-testid="add-panel-item-enabled" + :alt="$options.i18n.addPanelInfo" :to="newPanelPageLocation" + data-testid="add-panel-item-disabled" + disabled + class="gl-cursor-not-allowed" > - {{ $options.i18n.addPanel }} + <span class="gl-text-gray-400">{{ $options.i18n.addPanel }}</span> </gl-new-dropdown-item> - - <!-- - wrapper for tooltip as button can be `disabled` - https://bootstrap-vue.org/docs/components/tooltip#disabled-elements - --> - <div v-else v-gl-tooltip :title="$options.i18n.addPanelInfo"> - <gl-new-dropdown-item - :alt="$options.i18n.addPanelInfo" - :to="newPanelPageLocation" - data-testid="add-panel-item-disabled" - disabled - class="gl-cursor-not-allowed" - > - <span class="gl-text-gray-400">{{ $options.i18n.addPanel }}</span> - </gl-new-dropdown-item> - </div> - </template> + </div> <gl-new-dropdown-item v-if="isMenuItemEnabled.editDashboard" @@ -230,7 +225,7 @@ export default { <!-- wrapper for tooltip as button can be `disabled` https://bootstrap-vue.org/docs/components/tooltip#disabled-elements - --> + --> <div v-else v-gl-tooltip :title="$options.i18n.editDashboardInfo"> <gl-new-dropdown-item :alt="$options.i18n.editDashboardInfo" diff --git a/app/assets/javascripts/registry/explorer/pages/details.vue b/app/assets/javascripts/registry/explorer/pages/details.vue index cf811156704..b697bca6259 100644 --- a/app/assets/javascripts/registry/explorer/pages/details.vue +++ b/app/assets/javascripts/registry/explorer/pages/details.vue @@ -112,7 +112,7 @@ export default { </script> <template> - <div v-gl-resize-observer="handleResize" class="gl-my-3 gl-w-full slide-enter-to-element"> + <div v-gl-resize-observer="handleResize" class="gl-my-3"> <delete-alert v-model="deleteAlertType" :garbage-collection-help-page-path="config.garbageCollectionHelpPagePath" diff --git a/app/assets/javascripts/registry/explorer/pages/index.vue b/app/assets/javascripts/registry/explorer/pages/index.vue index 709a163d56d..4ac0bca84c1 100644 --- a/app/assets/javascripts/registry/explorer/pages/index.vue +++ b/app/assets/javascripts/registry/explorer/pages/index.vue @@ -4,8 +4,6 @@ export default {}; <template> <div> - <transition name="slide"> - <router-view ref="router-view" /> - </transition> + <router-view ref="router-view" /> </div> </template> diff --git a/app/assets/javascripts/registry/explorer/pages/list.vue b/app/assets/javascripts/registry/explorer/pages/list.vue index 1d353651c38..81e47073fe9 100644 --- a/app/assets/javascripts/registry/explorer/pages/list.vue +++ b/app/assets/javascripts/registry/explorer/pages/list.vue @@ -130,7 +130,7 @@ export default { </script> <template> - <div class="w-100 slide-enter-from-element"> + <div> <gl-alert v-if="showDeleteAlert" :variant="deleteAlertType" diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index cc43135f50a..3d7fde2b4c8 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -15,7 +15,16 @@ import SquashBeforeMerge from './squash_before_merge.vue'; import CommitsHeader from './commits_header.vue'; import CommitEdit from './commit_edit.vue'; import CommitMessageDropdown from './commit_message_dropdown.vue'; -import { AUTO_MERGE_STRATEGIES } from '../../constants'; +import { AUTO_MERGE_STRATEGIES, DANGER, INFO, WARNING } from '../../constants'; + +const PIPELINE_RUNNING_STATE = 'running'; +const PIPELINE_FAILED_STATE = 'failed'; +const PIPELINE_PENDING_STATE = 'pending'; +const PIPELINE_SUCCESS_STATE = 'success'; + +const MERGE_FAILED_STATUS = 'failed'; +const MERGE_SUCCESS_STATUS = 'success'; +const MERGE_HOOK_VALIDATION_ERROR_STATUS = 'hook_validation_error'; export default { name: 'ReadyToMerge', @@ -29,6 +38,8 @@ export default { GlSprintf, GlLink, GlDeprecatedButton, + MergeTrainHelperText: () => + import('ee_component/vue_merge_request_widget/components/merge_train_helper_text.vue'), MergeImmediatelyConfirmationDialog: () => import( 'ee_component/vue_merge_request_widget/components/merge_immediately_confirmation_dialog.vue' @@ -60,35 +71,45 @@ export default { const { pipeline, isPipelineFailed, hasCI, ciStatus } = this.mr; if ((hasCI && !ciStatus) || this.hasPipelineMustSucceedConflict) { - return 'failed'; - } else if (this.isAutoMergeAvailable) { - return 'pending'; - } else if (!pipeline) { - return 'success'; - } else if (isPipelineFailed) { - return 'failed'; + return PIPELINE_FAILED_STATE; + } + + if (this.isAutoMergeAvailable) { + return PIPELINE_PENDING_STATE; + } + + if (pipeline && isPipelineFailed) { + return PIPELINE_FAILED_STATE; } - return 'success'; + return PIPELINE_SUCCESS_STATE; }, mergeButtonVariant() { - if (this.status === 'failed') { - return 'danger'; - } else if (this.status === 'pending') { - return 'info'; + if (this.status === PIPELINE_FAILED_STATE) { + return DANGER; } - return 'success'; + + if (this.status === PIPELINE_PENDING_STATE) { + return INFO; + } + + return PIPELINE_SUCCESS_STATE; }, iconClass() { + if (this.shouldRenderMergeTrainHelperText && !this.mr.preventMerge) { + return PIPELINE_RUNNING_STATE; + } + if ( - this.status === 'failed' || + this.status === PIPELINE_FAILED_STATE || !this.commitMessage.length || !this.mr.isMergeAllowed || this.mr.preventMerge ) { - return 'warning'; + return WARNING; } - return 'success'; + + return PIPELINE_SUCCESS_STATE; }, mergeButtonText() { if (this.isMergingImmediately) { @@ -167,11 +188,13 @@ export default { .merge(options) .then(res => res.data) .then(data => { - const hasError = data.status === 'failed' || data.status === 'hook_validation_error'; + const hasError = + data.status === MERGE_FAILED_STATUS || + data.status === MERGE_HOOK_VALIDATION_ERROR_STATUS; if (AUTO_MERGE_STRATEGIES.includes(data.status)) { eventHub.$emit('MRWidgetUpdateRequested'); - } else if (data.status === 'success') { + } else if (data.status === MERGE_SUCCESS_STATUS) { this.initiateMergePolling(); } else if (hasError) { eventHub.$emit('FailedToMerge', data.merge_error); @@ -269,7 +292,7 @@ export default { <template> <div> - <div class="mr-widget-body media"> + <div class="mr-widget-body media" :class="{ 'gl-pb-3': shouldRenderMergeTrainHelperText }"> <status-icon :status="iconClass" /> <div class="media-body"> <div class="mr-widget-body-controls media space-children"> @@ -358,6 +381,7 @@ export default { <div v-if="hasPipelineMustSucceedConflict" class="gl-display-flex gl-align-items-center" + data-testid="pipeline-succeed-conflict" > <gl-sprintf :message="pipelineMustSucceedConflictText" /> <gl-link @@ -379,6 +403,13 @@ export default { </div> </div> </div> + <merge-train-helper-text + v-if="shouldRenderMergeTrainHelperText" + :pipeline-id="mr.pipeline.id" + :pipeline-link="mr.pipeline.path" + :merge-train-length="mr.mergeTrainsCount" + :merge-train-when-pipeline-succeeds-docs-path="mr.mergeTrainWhenPipelineSucceedsDocsPath" + /> <template v-if="shouldShowMergeControls"> <div v-if="mr.ffOnlyEnabled" class="mr-fast-forward-message"> {{ __('Fast-forward merge without a merge commit') }} diff --git a/app/assets/javascripts/vue_merge_request_widget/constants.js b/app/assets/javascripts/vue_merge_request_widget/constants.js index 1c43db23832..77dfbf9d385 100644 --- a/app/assets/javascripts/vue_merge_request_widget/constants.js +++ b/app/assets/javascripts/vue_merge_request_widget/constants.js @@ -3,6 +3,7 @@ import { s__ } from '~/locale'; export const SUCCESS = 'success'; export const WARNING = 'warning'; export const DANGER = 'danger'; +export const INFO = 'info'; export const WARNING_MESSAGE_CLASS = 'warning_message'; export const DANGER_MESSAGE_CLASS = 'danger_message'; diff --git a/app/assets/stylesheets/fontawesome_custom.scss b/app/assets/stylesheets/fontawesome_custom.scss index 9b490491ee4..a96a002b879 100644 --- a/app/assets/stylesheets/fontawesome_custom.scss +++ b/app/assets/stylesheets/fontawesome_custom.scss @@ -243,10 +243,6 @@ content: '\f187'; } -.fa-sign-out::before { - content: '\f08b'; -} - .fa-thumb-tack::before { content: '\f08d'; } diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 2e7de058e3a..71195fdb892 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -14,7 +14,6 @@ class Projects::EnvironmentsController < Projects::ApplicationController push_frontend_feature_flag(:prometheus_computed_alerts) push_frontend_feature_flag(:disable_metric_dashboard_refresh_rate) - push_frontend_feature_flag(:metrics_dashboard_new_panel_page) end before_action :authorize_read_environment!, except: [:metrics, :additional_metrics, :metrics_dashboard, :metrics_redirect] before_action :authorize_create_environment!, only: [:new, :create] diff --git a/app/controllers/projects/metrics/dashboards/builder_controller.rb b/app/controllers/projects/metrics/dashboards/builder_controller.rb index a7832aa363e..2ab574d7d10 100644 --- a/app/controllers/projects/metrics/dashboards/builder_controller.rb +++ b/app/controllers/projects/metrics/dashboards/builder_controller.rb @@ -4,7 +4,6 @@ module Projects module Metrics module Dashboards class BuilderController < Projects::ApplicationController - before_action :ensure_feature_flags before_action :authorize_metrics_dashboard! def panel_preview @@ -21,10 +20,6 @@ module Projects private - def ensure_feature_flags - render_404 unless Feature.enabled?(:metrics_dashboard_new_panel_page, project) - end - def rendered_panel @panel_preview ||= ::Metrics::Dashboard::PanelPreviewService.new(project, panel_yaml, environment).execute end diff --git a/app/controllers/projects/metrics_dashboard_controller.rb b/app/controllers/projects/metrics_dashboard_controller.rb index 7c964dc44f0..51307c3665c 100644 --- a/app/controllers/projects/metrics_dashboard_controller.rb +++ b/app/controllers/projects/metrics_dashboard_controller.rb @@ -10,14 +10,9 @@ module Projects before_action do push_frontend_feature_flag(:prometheus_computed_alerts) push_frontend_feature_flag(:disable_metric_dashboard_refresh_rate) - push_frontend_feature_flag(:metrics_dashboard_new_panel_page) end def show - if params[:page].present? && !Feature.enabled?(:metrics_dashboard_new_panel_page, project) - return render_404 - end - if environment render 'projects/environments/metrics' else diff --git a/app/models/project_repository_storage_move.rb b/app/models/project_repository_storage_move.rb index b18d9765a57..2b74d9ccd88 100644 --- a/app/models/project_repository_storage_move.rb +++ b/app/models/project_repository_storage_move.rb @@ -29,12 +29,17 @@ class ProjectRepositoryStorageMove < ApplicationRecord transition scheduled: :started end - event :finish do - transition started: :finished + event :finish_replication do + transition started: :replicated + end + + event :finish_cleanup do + transition replicated: :finished end event :do_fail do transition [:initial, :scheduled, :started] => :failed + transition replicated: :cleanup_failed end after_transition initial: :scheduled do |storage_move| @@ -49,7 +54,7 @@ class ProjectRepositoryStorageMove < ApplicationRecord end end - after_transition started: :finished do |storage_move| + after_transition started: :replicated do |storage_move| storage_move.project.update_columns( repository_read_only: false, repository_storage: storage_move.destination_storage_name @@ -65,6 +70,8 @@ class ProjectRepositoryStorageMove < ApplicationRecord state :started, value: 3 state :finished, value: 4 state :failed, value: 5 + state :replicated, value: 6 + state :cleanup_failed, value: 7 end scope :order_created_at_desc, -> { order(created_at: :desc) } diff --git a/app/services/projects/update_repository_storage_service.rb b/app/services/projects/update_repository_storage_service.rb index 7b346c09635..a479d53a43a 100644 --- a/app/services/projects/update_repository_storage_service.rb +++ b/app/services/projects/update_repository_storage_service.rb @@ -6,8 +6,7 @@ module Projects SameFilesystemError = Class.new(Error) attr_reader :repository_storage_move - delegate :project, :destination_storage_name, to: :repository_storage_move - delegate :repository, to: :project + delegate :project, :source_storage_name, :destination_storage_name, to: :repository_storage_move def initialize(repository_storage_move) @repository_storage_move = repository_storage_move @@ -20,21 +19,22 @@ module Projects repository_storage_move.start! end - raise SameFilesystemError if same_filesystem?(repository.storage, destination_storage_name) + raise SameFilesystemError if same_filesystem?(source_storage_name, destination_storage_name) mirror_repositories - project.transaction do - mark_old_paths_for_archive - - repository_storage_move.finish! + repository_storage_move.transaction do + repository_storage_move.finish_replication! project.leave_pool_repository project.track_project_repository end + remove_old_paths enqueue_housekeeping + repository_storage_move.finish_cleanup! + ServiceResponse.success rescue StandardError => e @@ -91,36 +91,31 @@ module Projects end end - def mark_old_paths_for_archive - old_repository_storage = project.repository_storage - new_project_path = moved_path(project.disk_path) - - # Notice that the block passed to `run_after_commit` will run with `repository_storage_move` - # as its context - repository_storage_move.run_after_commit do - GitlabShellWorker.perform_async(:mv_repository, - old_repository_storage, - project.disk_path, - new_project_path) - - if project.wiki.repository_exists? - GitlabShellWorker.perform_async(:mv_repository, - old_repository_storage, - project.wiki.disk_path, - "#{new_project_path}.wiki") - end - - if project.design_repository.exists? - GitlabShellWorker.perform_async(:mv_repository, - old_repository_storage, - project.design_repository.disk_path, - "#{new_project_path}.design") - end + def remove_old_paths + Gitlab::Git::Repository.new( + source_storage_name, + "#{project.disk_path}.git", + nil, + nil + ).remove + + if project.wiki.repository_exists? + Gitlab::Git::Repository.new( + source_storage_name, + "#{project.wiki.disk_path}.git", + nil, + nil + ).remove end - end - def moved_path(path) - "#{path}+#{project.id}+moved+#{Time.current.to_i}" + if project.design_repository.exists? + Gitlab::Git::Repository.new( + source_storage_name, + "#{project.design_repository.disk_path}.git", + nil, + nil + ).remove + end end # The underlying FetchInternalRemote call uses a `git fetch` to move data diff --git a/app/views/projects/_merge_request_settings.html.haml b/app/views/projects/_merge_request_settings.html.haml index 5ffdeef3558..e69972e8163 100644 --- a/app/views/projects/_merge_request_settings.html.haml +++ b/app/views/projects/_merge_request_settings.html.haml @@ -4,7 +4,7 @@ = render 'projects/merge_request_merge_options_settings', project: @project, form: form -- if Feature.enabled?(:squash_options, @project) +- if Feature.enabled?(:squash_options, @project, default_enabled: true) = render 'projects/merge_request_squash_options_settings', form: form = render 'projects/merge_request_merge_checks_settings', project: @project, form: form diff --git a/changelogs/unreleased/225661-preserve-active-tab-alerts-details.yml b/changelogs/unreleased/225661-preserve-active-tab-alerts-details.yml new file mode 100644 index 00000000000..499c58359ed --- /dev/null +++ b/changelogs/unreleased/225661-preserve-active-tab-alerts-details.yml @@ -0,0 +1,5 @@ +--- +title: Preserve active tab on alert details page reload +merge_request: 39369 +author: +type: added diff --git a/changelogs/unreleased/231413-remove-feature-flag.yml b/changelogs/unreleased/231413-remove-feature-flag.yml new file mode 100644 index 00000000000..aca22e48354 --- /dev/null +++ b/changelogs/unreleased/231413-remove-feature-flag.yml @@ -0,0 +1,5 @@ +--- +title: Make available new UI for adding a panel to a metrics dashboard +merge_request: 39124 +author: +type: added diff --git a/changelogs/unreleased/232786-respect-stop-query-failures.yml b/changelogs/unreleased/232786-respect-stop-query-failures.yml new file mode 100644 index 00000000000..51466606375 --- /dev/null +++ b/changelogs/unreleased/232786-respect-stop-query-failures.yml @@ -0,0 +1,5 @@ +--- +title: Skip subsequent topology Prometheus queries if timeout occur +merge_request: 38293 +author: +type: performance diff --git a/changelogs/unreleased/235507-remove-transition-animation-from-the-container-registry-ui.yml b/changelogs/unreleased/235507-remove-transition-animation-from-the-container-registry-ui.yml new file mode 100644 index 00000000000..e5723e44216 --- /dev/null +++ b/changelogs/unreleased/235507-remove-transition-animation-from-the-container-registry-ui.yml @@ -0,0 +1,5 @@ +--- +title: Remove transition animation from the Container Registry UI +merge_request: 39337 +author: +type: changed diff --git a/changelogs/unreleased/storage_move_cleanup.yml b/changelogs/unreleased/storage_move_cleanup.yml new file mode 100644 index 00000000000..229b60efc08 --- /dev/null +++ b/changelogs/unreleased/storage_move_cleanup.yml @@ -0,0 +1,5 @@ +--- +title: Remove repositories from previous storage when storage move succeeds +merge_request: 38547 +author: +type: changed diff --git a/changelogs/unreleased/xanf-add-external-link-icon-for-importers.yml b/changelogs/unreleased/xanf-add-external-link-icon-for-importers.yml new file mode 100644 index 00000000000..5b1cbaa66a0 --- /dev/null +++ b/changelogs/unreleased/xanf-add-external-link-icon-for-importers.yml @@ -0,0 +1,5 @@ +--- +title: Add external link icon to list of repositories in importer +merge_request: 39442 +author: +type: added diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index 3cbbf498db6..e7954fa910b 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -56,6 +56,12 @@ This works because for every path that is present in CE's eager-load/auto-load paths, we add the same `ee/`-prepended path in [`config/application.rb`](https://gitlab.com/gitlab-org/gitlab/blob/925d3d4ebc7a2c72964ce97623ae41b8af12538d/config/application.rb#L42-52). This also applies to views. +#### Testing EE-only features + +To test an EE class that doesn't exist in CE, create the spec file as you normally +would in the `ee/spec` directory, but without the second `ee/` subdirectory. +For example, a class `ee/app/models/vulnerability.rb` would have its tests in `ee/spec/models/vulnerability_spec.rb`. + ### EE features based on CE features For features that build on existing CE features, write a module in the `EE` @@ -96,6 +102,21 @@ This is also not just applied to models. Here's a list of other examples: - `ee/app/validators/ee/foo_attr_validator.rb` - `ee/app/workers/ee/foo_worker.rb` +#### Testing EE features based on CE features + +To test an `EE` namespaced module that extends a CE class with EE features, +create the spec file as you normally would in the `ee/spec` directory, including the second `ee/` subdirectory. +For example, an extension `ee/app/models/ee/user.rb` would have its tests in `ee/app/models/ee/user_spec.rb`. + +In the `RSpec.describe` call, use the CE class name where the EE module would be used. +For example, in `ee/app/models/ee/user_spec.rb`, the test would start with: + +```ruby +RSpec.describe User do + describe 'ee feature added through extension' +end +``` + #### Overriding CE methods To override a method present in the CE codebase, use `prepend`. It diff --git a/doc/development/telemetry/usage_ping.md b/doc/development/telemetry/usage_ping.md index 4f2d8327b17..db470891a19 100644 --- a/doc/development/telemetry/usage_ping.md +++ b/doc/development/telemetry/usage_ping.md @@ -379,6 +379,10 @@ Ensure you comply with the [Changelog entries guide](../changelog.md). On GitLab.com, we have DangerBot setup to monitor Telemetry related files and DangerBot will recommend a Telemetry review. Mention `@gitlab-org/growth/telemetry/engineers` in your MR for a review. +### 9. Verify your metric + +On GitLab.com, the Product Analytics team regularly monitors Usage Ping. They may alert you that your metrics need further optimization to run quicker and with greater success. You may also use the [Usage Ping QA dashboard](https://app.periscopedata.com/app/gitlab/632033/Usage-Ping-QA) to check how well your metric performs. The dashboard allows filtering by GitLab version, by "Self-managed" & "Saas" and shows you how many failures have occurred for each metric. Whenever you notice a high failure rate, you may re-optimize your metric. + ### Optional: Test Prometheus based Usage Ping If the data submitted includes metrics [queried from Prometheus](#prometheus-queries) that you would like to inspect and verify, diff --git a/doc/install/aws/index.md b/doc/install/aws/index.md index 1ffeebce711..92a4ce860c3 100644 --- a/doc/install/aws/index.md +++ b/doc/install/aws/index.md @@ -246,10 +246,7 @@ On the EC2 dashboard, look for Load Balancer in the left navigation bar: 1. For **Ping Protocol**, select HTTP. 1. For **Ping Port**, enter 80. 1. For **Ping Path**, enter `/users/sign_in`. (We use `/users/sign_in` as it's a public endpoint that does - not require authorization.) - - NOTE: **Note:** - When booting a fresh GitLab instance for the first time, GitLab redirects you to `/users/password/` to change the admin password. Temporarily change the health check to this URL (or to the TCP protocol) and change it back to `/users/sign_in` after setting the admin password. + not require authentication.) 1. Keep the default **Advanced Details** or adjust them according to your needs. 1. Click **Add EC2 Instances** - don't add anything as we will create an Auto Scaling Group later to manage instances for us. 1. Click **Add Tags** and add any tags you need. @@ -646,6 +643,13 @@ to eliminate the need for NFS to support GitLab Pages. That concludes the configuration changes for our GitLab instance. Next, we'll create a custom AMI based on this instance to use for our launch configuration and auto scaling group. +### Log in for the first time + +Using the domain name you used when setting up [DNS for the load balancer](#configure-dns-for-load-balancer), you should now be able to visit GitLab in your browser. You will be asked to set up a password +for the `root` user which has admin privileges on the GitLab instance. This password will be stored in the database. + +When our [auto scaling group](#create-an-auto-scaling-group) spins up new instances, we'll be able to log in with username `root` and the newly created password. + ### Create custom AMI On the EC2 dashboard: @@ -700,13 +704,6 @@ As the auto scaling group is created, you'll see your new instances spinning up Since our instances are created by the auto scaling group, go back to your instances and terminate the [instance we created manually above](#install-gitlab). We only needed this instance to create our custom AMI. -### Log in for the first time - -Using the domain name you used when setting up [DNS for the load balancer](#configure-dns-for-load-balancer), you should now be able to visit GitLab in your browser. The very first time you will be asked to set up a password -for the `root` user which has admin privileges on the GitLab instance. - -After you set it up, login with username `root` and the newly created password. - ## Health check and monitoring with Prometheus Apart from Amazon's Cloudwatch which you can enable on various services, diff --git a/doc/user/analytics/img/new_value_stream_v13_3.png b/doc/user/analytics/img/new_value_stream_v13_3.png Binary files differnew file mode 100644 index 00000000000..4284b8ab194 --- /dev/null +++ b/doc/user/analytics/img/new_value_stream_v13_3.png diff --git a/doc/user/analytics/value_stream_analytics.md b/doc/user/analytics/value_stream_analytics.md index f78d381ea79..9114b6de6bc 100644 --- a/doc/user/analytics/value_stream_analytics.md +++ b/doc/user/analytics/value_stream_analytics.md @@ -188,7 +188,7 @@ A few notes: cycles, calculate their median time and the result is what the dashboard of Value Stream Analytics is showing. -## Customizable Value Stream Analytics +## Customizable Value Stream Analytics **(PREMIUM)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12196) in GitLab 12.9. @@ -196,8 +196,7 @@ The default stages are designed to work straight out of the box, but they might all teams. Different teams use different approaches to building software, so some teams might want to customize their Value Stream Analytics. -GitLab allows users to hide default stages and create custom stages that align better to their -development workflow. +GitLab allows users to create multiple value streams, hide default stages and create custom stages that align better to their development workflow. NOTE: **Note:** Customizability is [only available for group-level](https://gitlab.com/gitlab-org/gitlab/-/issues/35823#note_272558950) Value Stream Analytics. @@ -295,6 +294,34 @@ To recover a default stage that was previously hidden: 1. In the top right corner open the **Recover hidden stage** dropdown. 1. Select a stage. +### Creating a value stream + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/221202) in GitLab 13.3 + +A default value stream is readily available for each group. You can create additional value streams based on the different areas of work that you would like to measure. + +Once created, a new value stream includes the [seven stages](#overview) that follow +[GitLab workflow](../../topics/gitlab_flow.md) +best practices. You can customize this flow by adding, hiding or re-ordering stages. + +To create a value stream: + +1. Navigate to your group's **Analytics > Value Stream**. +1. Click the Value stream dropdown and select **Create new Value Stream** +1. Fill in a name for the new Value Stream +1. Click the **Create Value Stream** button. + +![New value stream](img/new_value_stream_v13_3.png "Creating a new value stream") + +### Disabling custom value streams + +Custom value streams are enabled by default. If you have a self-managed instance, an +administrator can open a Rails console and disable them with the following command: + +```ruby +Feature.disable(:value_stream_analytics_create_multiple_value_streams) +``` + ## Days to completion chart > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/21631) in GitLab 12.6. diff --git a/lib/gitlab/ci/features.rb b/lib/gitlab/ci/features.rb index d4b3c58ec56..472f0f36231 100644 --- a/lib/gitlab/ci/features.rb +++ b/lib/gitlab/ci/features.rb @@ -77,7 +77,7 @@ module Gitlab end def self.lint_creates_pipeline_with_dry_run?(project) - ::Feature.enabled?(:ci_lint_creates_pipeline_with_dry_run, project) + ::Feature.enabled?(:ci_lint_creates_pipeline_with_dry_run, project, default_enabled: true) end def self.reset_ci_minutes_for_all_namespaces? diff --git a/lib/gitlab/usage_data/topology.rb b/lib/gitlab/usage_data/topology.rb index 960b8f74eb1..edc4dc75750 100644 --- a/lib/gitlab/usage_data/topology.rb +++ b/lib/gitlab/usage_data/topology.rb @@ -17,6 +17,9 @@ module Gitlab 'registry' => 'registry' }.freeze + # If these errors occur, all subsequent queries are likely to fail for the same error + TIMEOUT_ERRORS = [Errno::ETIMEDOUT, Net::OpenTimeout, Net::ReadTimeout].freeze + CollectionFailure = Struct.new(:query, :error) do def to_h { query => error } @@ -158,6 +161,11 @@ module Gitlab end def query_safely(query, query_name, fallback:) + if timeout_error_exists? + @failures << CollectionFailure.new(query_name, 'timeout_cancellation') + return fallback + end + result = yield query return result if result.present? @@ -169,6 +177,14 @@ module Gitlab fallback end + def timeout_error_exists? + timeout_error_names = TIMEOUT_ERRORS.map(&:to_s).to_set + + @failures.any? do |failure| + timeout_error_names.include?(failure.error) + end + end + def topology_node_services(instance, all_process_counts, all_process_memory, all_server_types) # returns all node service data grouped by service name as the key instance_service_data = diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9e26d182feb..b259ca8ebe1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -29333,19 +29333,19 @@ msgstr "" msgid "mrWidget|There are merge conflicts" msgstr "" -msgid "mrWidget|This feature merges changes from the target branch to the source branch. You cannot use this feature since the source branch is protected." +msgid "mrWidget|This action will add the merge request to the merge train when pipeline %{pipelineLink} succeeds." msgstr "" -msgid "mrWidget|This merge request failed to be merged automatically" +msgid "mrWidget|This action will start a merge train when pipeline %{pipelineLink} succeeds." msgstr "" -msgid "mrWidget|This merge request is in the process of being merged" +msgid "mrWidget|This feature merges changes from the target branch to the source branch. You cannot use this feature since the source branch is protected." msgstr "" -msgid "mrWidget|This merge request will be added to the merge train when pipeline %{linkStart}#%{pipelineId}%{linkEnd} succeeds." +msgid "mrWidget|This merge request failed to be merged automatically" msgstr "" -msgid "mrWidget|This merge request will start a merge train when pipeline %{linkStart}#%{pipelineId}%{linkEnd} succeeds." +msgid "mrWidget|This merge request is in the process of being merged" msgstr "" msgid "mrWidget|This project is archived, write access has been disabled" diff --git a/package.json b/package.json index 01da8fe7c51..1bf5e229ccd 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "@babel/preset-env": "^7.10.1", "@gitlab/at.js": "1.5.5", "@gitlab/svgs": "1.158.0", - "@gitlab/ui": "18.6.2", + "@gitlab/ui": "18.7.0", "@gitlab/visual-review-tools": "1.6.1", "@rails/actioncable": "^6.0.3-1", "@sentry/browser": "^5.10.2", diff --git a/spec/factories/project_repository_storage_moves.rb b/spec/factories/project_repository_storage_moves.rb index ea0b34e0338..69fb3af45e6 100644 --- a/spec/factories/project_repository_storage_moves.rb +++ b/spec/factories/project_repository_storage_moves.rb @@ -15,6 +15,10 @@ FactoryBot.define do state { ProjectRepositoryStorageMove.state_machines[:state].states[:started].value } end + trait :replicated do + state { ProjectRepositoryStorageMove.state_machines[:state].states[:replicated].value } + end + trait :finished do state { ProjectRepositoryStorageMove.state_machines[:state].states[:finished].value } end diff --git a/spec/features/projects/commit/mini_pipeline_graph_spec.rb b/spec/features/projects/commit/mini_pipeline_graph_spec.rb index cf9b86f16bb..7bd3bce85d5 100644 --- a/spec/features/projects/commit/mini_pipeline_graph_spec.rb +++ b/spec/features/projects/commit/mini_pipeline_graph_spec.rb @@ -26,6 +26,8 @@ RSpec.describe 'Mini Pipeline Graph in Commit View', :js do build.run visit project_commit_path(project, project.commit.id) + wait_for_all_requests + expect(page).to have_selector('.mr-widget-pipeline-graph') first('.mini-pipeline-graph-dropdown-toggle').click diff --git a/spec/finders/todos_finder_spec.rb b/spec/finders/todos_finder_spec.rb index 648d727cb59..83f33a95c2e 100644 --- a/spec/finders/todos_finder_spec.rb +++ b/spec/finders/todos_finder_spec.rb @@ -235,15 +235,18 @@ RSpec.describe TodosFinder do context 'when filtering by target id' do it 'returns the expected todos for the target' do - todos = finder.new(user, { target_id: issue.id }).execute + todos = finder.new(user, { type: 'Issue', target_id: issue.id }).execute expect(todos).to match_array([todo1]) end it 'returns the expected todos for multiple target ids' do - todos = finder.new(user, { target_id: [issue.id, merge_request.id] }).execute + another_issue = create(:issue, project: project) + todo3 = create(:todo, user: user, project: project, target: another_issue) - expect(todos).to match_array([todo1, todo2]) + todos = finder.new(user, { type: 'Issue', target_id: [issue.id, another_issue.id] }).execute + + expect(todos).to match_array([todo1, todo3]) end it 'returns the expected todos for empty target id collection' do diff --git a/spec/frontend/alert_management/components/alert_management_detail_spec.js b/spec/frontend/alert_management/components/alert_details_spec.js index daa730d3b9f..5364d662591 100644 --- a/spec/frontend/alert_management/components/alert_management_detail_spec.js +++ b/spec/frontend/alert_management/components/alert_details_spec.js @@ -20,6 +20,7 @@ describe('AlertDetails', () => { const projectPath = 'root/alerts'; const projectIssuesPath = 'root/alerts/-/issues'; const projectId = '1'; + const $router = { replace: jest.fn() }; const findDetailsTable = () => wrapper.find(GlTable); @@ -44,6 +45,8 @@ describe('AlertDetails', () => { sidebarStatus: {}, }, }, + $router, + $route: { params: {} }, }, stubs, }); @@ -81,11 +84,11 @@ describe('AlertDetails', () => { }); it('renders a tab with overview information', () => { - expect(wrapper.find('[data-testid="overviewTab"]').exists()).toBe(true); + expect(wrapper.find('[data-testid="overview"]').exists()).toBe(true); }); it('renders a tab with full alert information', () => { - expect(wrapper.find('[data-testid="fullDetailsTab"]').exists()).toBe(true); + expect(wrapper.find('[data-testid="fullDetails"]').exists()).toBe(true); }); it('renders severity', () => { @@ -191,7 +194,7 @@ describe('AlertDetails', () => { mountComponent({ data: { alert: mockAlert } }); }); it('should display a table of raw alert details data', () => { - wrapper.find('[data-testid="fullDetailsTab"]').trigger('click'); + wrapper.find('[data-testid="fullDetails"]').trigger('click'); expect(findDetailsTable().exists()).toBe(true); }); }); @@ -252,6 +255,22 @@ describe('AlertDetails', () => { ); }); }); + + describe('tab navigation', () => { + beforeEach(() => { + mountComponent({ data: { alert: mockAlert } }); + }); + + it.each` + index | tabId + ${0} | ${'overview'} + ${1} | ${'fullDetails'} + ${2} | ${'metrics'} + `('will navigate to the correct tab via $tabId', ({ index, tabId }) => { + wrapper.setData({ currentTabIndex: index }); + expect($router.replace).toHaveBeenCalledWith({ name: 'tab', params: { tabId } }); + }); + }); }); describe('Snowplow tracking', () => { diff --git a/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js b/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js index d05e9d8786d..024b2cbd7f1 100644 --- a/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js +++ b/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js @@ -42,9 +42,6 @@ describe('Actions menu', () => { const createShallowWrapper = (props = {}, options = {}) => { wrapper = shallowMount(ActionsMenu, { propsData: { ...dashboardActionsMenuProps, ...props }, - provide: { - glFeatures: { metricsDashboardNewPanelPage: true }, - }, store, ...options, }); diff --git a/spec/lib/gitlab/usage_data/topology_spec.rb b/spec/lib/gitlab/usage_data/topology_spec.rb index 1c2bb1faaed..7f4a25297e6 100644 --- a/spec/lib/gitlab/usage_data/topology_spec.rb +++ b/spec/lib/gitlab/usage_data/topology_spec.rb @@ -402,28 +402,61 @@ RSpec.describe Gitlab::UsageData::Topology do end context 'and an error is raised when querying Prometheus' do - it 'returns empty result with failures' do - expect_prometheus_api_to receive(:query) - .at_least(:once) - .and_raise(Gitlab::PrometheusClient::ConnectionError) + context 'without timeout failures' do + it 'returns empty result and executes subsequent queries as usual' do + expect_prometheus_api_to receive(:query) + .at_least(:once) + .and_raise(Gitlab::PrometheusClient::ConnectionError) + + expect(subject[:topology]).to eq({ + duration_s: 0, + failures: [ + { 'app_requests' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'node_memory' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'node_memory_utilization' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'node_cpus' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'node_cpu_utilization' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'node_uname_info' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'service_rss' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'service_uss' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'service_pss' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'service_process_count' => 'Gitlab::PrometheusClient::ConnectionError' }, + { 'service_workers' => 'Gitlab::PrometheusClient::ConnectionError' } + ], + nodes: [] + }) + end + end - expect(subject[:topology]).to eq({ - duration_s: 0, - failures: [ - { 'app_requests' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'node_memory' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'node_memory_utilization' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'node_cpus' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'node_cpu_utilization' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'node_uname_info' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'service_rss' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'service_uss' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'service_pss' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'service_process_count' => 'Gitlab::PrometheusClient::ConnectionError' }, - { 'service_workers' => 'Gitlab::PrometheusClient::ConnectionError' } - ], - nodes: [] - }) + context 'with timeout failures' do + where(:exception) do + described_class::TIMEOUT_ERRORS + end + + with_them do + it 'returns empty result and cancelled subsequent queries' do + expect_prometheus_api_to receive(:query) + .and_raise(exception) + + expect(subject[:topology]).to eq({ + duration_s: 0, + failures: [ + { 'app_requests' => exception.to_s }, + { 'node_memory' => 'timeout_cancellation' }, + { 'node_memory_utilization' => 'timeout_cancellation' }, + { 'node_cpus' => 'timeout_cancellation' }, + { 'node_cpu_utilization' => 'timeout_cancellation' }, + { 'node_uname_info' => 'timeout_cancellation' }, + { 'service_rss' => 'timeout_cancellation' }, + { 'service_uss' => 'timeout_cancellation' }, + { 'service_pss' => 'timeout_cancellation' }, + { 'service_process_count' => 'timeout_cancellation' }, + { 'service_workers' => 'timeout_cancellation' } + ], + nodes: [] + }) + end + end end end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index f155c240fb2..a3ed39abfb3 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -113,9 +113,10 @@ RSpec.describe Member do end describe 'Scopes & finders' do - before do - project = create(:project, :public) - group = create(:group) + let_it_be(:project) { create(:project, :public) } + let_it_be(:group) { create(:group) } + + before_all do @owner_user = create(:user).tap { |u| group.add_owner(u) } @owner = group.members.find_by(user_id: @owner_user.id) @@ -252,9 +253,9 @@ RSpec.describe Member do describe '.add_user' do %w[project group].each do |source_type| context "when source is a #{source_type}" do - let!(:source) { create(source_type, :public) } - let!(:user) { create(:user) } - let!(:admin) { create(:admin) } + let_it_be(:source, reload: true) { create(source_type, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:admin) } it 'returns a <Source>Member object' do member = described_class.add_user(source, user, :maintainer) @@ -322,7 +323,7 @@ RSpec.describe Member do it 'adds the user as a member' do expect(source.users).not_to include(user) - described_class.add_user(source, 42, :maintainer) + described_class.add_user(source, non_existing_record_id, :maintainer) expect(source.users.reload).not_to include(user) end @@ -482,10 +483,10 @@ RSpec.describe Member do describe '.add_users' do %w[project group].each do |source_type| context "when source is a #{source_type}" do - let!(:source) { create(source_type, :public) } - let!(:admin) { create(:admin) } - let(:user1) { create(:user) } - let(:user2) { create(:user) } + let_it_be(:source) { create(source_type, :public) } + let_it_be(:admin) { create(:admin) } + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } it 'returns a <Source>Member objects' do members = described_class.add_users(source, [user1, user2], :maintainer) diff --git a/spec/models/project_repository_storage_move_spec.rb b/spec/models/project_repository_storage_move_spec.rb index 83711085c92..3e679c8af4d 100644 --- a/spec/models/project_repository_storage_move_spec.rb +++ b/spec/models/project_repository_storage_move_spec.rb @@ -74,9 +74,9 @@ RSpec.describe ProjectRepositoryStorageMove, type: :model do context 'when started' do subject(:storage_move) { create(:project_repository_storage_move, :started, project: project, destination_storage_name: 'test_second_storage') } - context 'and transits to finished' do + context 'and transits to replicated' do it 'sets the repository storage and marks the project as writable' do - storage_move.finish! + storage_move.finish_replication! expect(project.repository_storage).to eq('test_second_storage') expect(project).not_to be_repository_read_only diff --git a/spec/requests/api/composer_packages_spec.rb b/spec/requests/api/composer_packages_spec.rb index d5e23a91da0..f5b8ebb545b 100644 --- a/spec/requests/api/composer_packages_spec.rb +++ b/spec/requests/api/composer_packages_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' RSpec.describe API::ComposerPackages do - include PackagesManagerApiSpecHelpers + include HttpBasicAuthHelpers let_it_be(:user) { create(:user) } let_it_be(:group, reload: true) { create(:group, :public) } @@ -224,7 +224,7 @@ RSpec.describe API::ComposerPackages do end context 'with no tag or branch params' do - let(:headers) { build_basic_auth_header(user.username, personal_access_token.token) } + let(:headers) { basic_auth_header(user.username, personal_access_token.token) } it_behaves_like 'process Composer api request', :developer, :bad_request end @@ -238,7 +238,7 @@ RSpec.describe API::ComposerPackages do context 'with a non existing tag' do let(:params) { { tag: 'non-existing-tag' } } - let(:headers) { build_basic_auth_header(user.username, personal_access_token.token) } + let(:headers) { basic_auth_header(user.username, personal_access_token.token) } it_behaves_like 'process Composer api request', :developer, :not_found end @@ -253,7 +253,7 @@ RSpec.describe API::ComposerPackages do context 'with a non existing branch' do let(:params) { { branch: 'non-existing-branch' } } - let(:headers) { build_basic_auth_header(user.username, personal_access_token.token) } + let(:headers) { basic_auth_header(user.username, personal_access_token.token) } it_behaves_like 'process Composer api request', :developer, :not_found end @@ -311,7 +311,7 @@ RSpec.describe API::ComposerPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } before do project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility_level, false)) diff --git a/spec/requests/api/conan_packages_spec.rb b/spec/requests/api/conan_packages_spec.rb index 1d88eaef79c..ac6a4171751 100644 --- a/spec/requests/api/conan_packages_spec.rb +++ b/spec/requests/api/conan_packages_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe API::ConanPackages do include WorkhorseHelpers + include HttpBasicAuthHelpers include PackagesManagerApiSpecHelpers let(:package) { create(:conan_package) } diff --git a/spec/requests/api/go_proxy_spec.rb b/spec/requests/api/go_proxy_spec.rb index faffb6bd192..2d7e319b0be 100644 --- a/spec/requests/api/go_proxy_spec.rb +++ b/spec/requests/api/go_proxy_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe API::GoProxy do include PackagesManagerApiSpecHelpers + include HttpBasicAuthHelpers let_it_be(:user) { create :user } let_it_be(:project) { create :project_empty_repo, creator: user, path: 'my-go-lib' } @@ -387,7 +388,7 @@ RSpec.describe API::GoProxy do end it 'returns ok with a personal access token and basic authentication' do - get_resource(headers: build_basic_auth_header(user.username, pa_token.token)) + get_resource(headers: basic_auth_header(user.username, pa_token.token)) expect(response).to have_gitlab_http_status(:ok) end diff --git a/spec/requests/api/npm_packages_spec.rb b/spec/requests/api/npm_packages_spec.rb index 214518f5c4b..94647123df0 100644 --- a/spec/requests/api/npm_packages_spec.rb +++ b/spec/requests/api/npm_packages_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe API::NpmPackages do include PackagesManagerApiSpecHelpers + include HttpBasicAuthHelpers let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } diff --git a/spec/requests/api/nuget_packages_spec.rb b/spec/requests/api/nuget_packages_spec.rb index a2441ad83b5..ab537a61058 100644 --- a/spec/requests/api/nuget_packages_spec.rb +++ b/spec/requests/api/nuget_packages_spec.rb @@ -45,7 +45,7 @@ RSpec.describe API::NugetPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } subject { get api(url), headers: headers } @@ -137,7 +137,7 @@ RSpec.describe API::NugetPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:user_headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } let(:headers) { user_headers.merge(workhorse_header) } before do @@ -204,7 +204,7 @@ RSpec.describe API::NugetPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:user_headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } let(:headers) { user_headers.merge(workhorse_header) } before do @@ -264,7 +264,7 @@ RSpec.describe API::NugetPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } subject { get api(url), headers: headers } @@ -325,7 +325,7 @@ RSpec.describe API::NugetPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } subject { get api(url), headers: headers } @@ -381,7 +381,7 @@ RSpec.describe API::NugetPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } subject { get api(url), headers: headers } @@ -436,7 +436,7 @@ RSpec.describe API::NugetPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } subject { get api(url), headers: headers } @@ -499,7 +499,7 @@ RSpec.describe API::NugetPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } subject { get api(url), headers: headers } diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 2af97c4b984..e2cfd87b507 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe API::PypiPackages do include WorkhorseHelpers include PackagesManagerApiSpecHelpers + include HttpBasicAuthHelpers let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project, :public) } @@ -43,7 +44,7 @@ RSpec.describe API::PypiPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } subject { get api(url), headers: headers } @@ -94,7 +95,7 @@ RSpec.describe API::PypiPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:user_headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } let(:headers) { user_headers.merge(workhorse_header) } before do @@ -157,7 +158,7 @@ RSpec.describe API::PypiPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:user_headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } let(:headers) { user_headers.merge(workhorse_header) } before do @@ -170,7 +171,7 @@ RSpec.describe API::PypiPackages do context 'with an invalid package' do let(:token) { personal_access_token.token } - let(:user_headers) { build_basic_auth_header(user.username, token) } + let(:user_headers) { basic_auth_header(user.username, token) } let(:headers) { user_headers.merge(workhorse_header) } before do @@ -220,7 +221,7 @@ RSpec.describe API::PypiPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } subject { get api(url), headers: headers } @@ -233,14 +234,14 @@ RSpec.describe API::PypiPackages do end context 'with deploy token headers' do - let(:headers) { build_basic_auth_header(deploy_token.username, deploy_token.token) } + let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token) } context 'valid token' do it_behaves_like 'returning response status', :success end context 'invalid token' do - let(:headers) { build_basic_auth_header('foo', 'bar') } + let(:headers) { basic_auth_header('foo', 'bar') } it_behaves_like 'returning response status', :success end diff --git a/spec/requests/projects/metrics/dashboards/builder_spec.rb b/spec/requests/projects/metrics/dashboards/builder_spec.rb index 5f85788cdb6..e59ed591f63 100644 --- a/spec/requests/projects/metrics/dashboards/builder_spec.rb +++ b/spec/requests/projects/metrics/dashboards/builder_spec.rb @@ -49,10 +49,6 @@ RSpec.describe 'Projects::Metrics::Dashboards::BuilderController' do describe 'POST /:namespace/:project/-/metrics/dashboards/builder' do context 'as anonymous user' do - before do - stub_feature_flags(metrics_dashboard_new_panel_page: true) - end - it 'redirects user to sign in page' do send_request @@ -62,7 +58,6 @@ RSpec.describe 'Projects::Metrics::Dashboards::BuilderController' do context 'as user with guest access' do before do - stub_feature_flags(metrics_dashboard_new_panel_page: true) project.add_guest(user) login_as(user) end @@ -80,48 +75,30 @@ RSpec.describe 'Projects::Metrics::Dashboards::BuilderController' do login_as(user) end - context 'metrics_dashboard_new_panel_page is enabled' do - before do - stub_feature_flags(metrics_dashboard_new_panel_page: true) - end - - context 'valid yaml panel is supplied' do - it 'returns success' do - send_request(panel_yaml: valid_panel_yml) + context 'valid yaml panel is supplied' do + it 'returns success' do + send_request(panel_yaml: valid_panel_yml) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include('title' => 'Super Chart A1', 'type' => 'area-chart') - end - end - - context 'invalid yaml panel is supplied' do - it 'returns unprocessable entity' do - send_request(panel_yaml: invalid_panel_yml) - - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Each "panel" must define an array :metrics') - end + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include('title' => 'Super Chart A1', 'type' => 'area-chart') end + end - context 'invalid panel_yaml is not a yaml string' do - it 'returns unprocessable entity' do - send_request(panel_yaml: 1) + context 'invalid yaml panel is supplied' do + it 'returns unprocessable entity' do + send_request(panel_yaml: invalid_panel_yml) - expect(response).to have_gitlab_http_status(:unprocessable_entity) - expect(json_response['message']).to eq('Invalid configuration format') - end + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Each "panel" must define an array :metrics') end end - context 'metrics_dashboard_new_panel_page is disabled' do - before do - stub_feature_flags(metrics_dashboard_new_panel_page: false) - end - - it 'returns not found' do - send_request + context 'invalid panel_yaml is not a yaml string' do + it 'returns unprocessable entity' do + send_request(panel_yaml: 1) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Invalid configuration format') end end end diff --git a/spec/requests/projects/metrics_dashboard_spec.rb b/spec/requests/projects/metrics_dashboard_spec.rb index b061d499171..f571e4a4309 100644 --- a/spec/requests/projects/metrics_dashboard_spec.rb +++ b/spec/requests/projects/metrics_dashboard_spec.rb @@ -96,26 +96,26 @@ RSpec.describe 'metrics dashboard page' do end describe 'GET :/namespace/:project/-/metrics/:page' do - it 'returns 200 with path param page and feature flag enabled' do - stub_feature_flags(metrics_dashboard_new_panel_page: true) - + it 'returns 200 with path param page' do # send_request(page: 'panel/new') cannot be used because it encodes '/' - get "/#{project.namespace.to_param}/#{project.to_param}/-/metrics/panel/new" + get "#{dashboard_route}/panel/new" expect(response).to have_gitlab_http_status(:ok) end - it 'returns 404 with path param page and feature flag disabled' do - stub_feature_flags(metrics_dashboard_new_panel_page: false) - + it 'returns 200 with dashboard and path param page' do # send_request(page: 'panel/new') cannot be used because it encodes '/' - get "/#{project.namespace.to_param}/#{project.to_param}/-/metrics/panel/new" + get "#{dashboard_route(dashboard_path: 'dashboard.yml')}/panel/new" - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:ok) end end def send_request(params = {}) - get namespace_project_metrics_dashboard_path(namespace_id: project.namespace, project_id: project, **params) + get dashboard_route(params) + end + + def dashboard_route(params = {}) + namespace_project_metrics_dashboard_path(namespace_id: project.namespace, project_id: project, **params) end end diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index 57e02c26b71..0fcd14f3bc9 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -21,6 +21,7 @@ RSpec.describe Projects::UpdateRepositoryStorageService do let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) } let!(:checksum) { project.repository.checksum } let(:project_repository_double) { double(:repository) } + let(:original_project_repository_double) { double(:repository) } before do allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original @@ -29,6 +30,9 @@ RSpec.describe Projects::UpdateRepositoryStorageService do allow(Gitlab::Git::Repository).to receive(:new) .with('test_second_storage', project.repository.raw.relative_path, project.repository.gl_repository, project.repository.full_path) .and_return(project_repository_double) + allow(Gitlab::Git::Repository).to receive(:new) + .with('default', project.repository.raw.relative_path, nil, nil) + .and_return(original_project_repository_double) end context 'when the move succeeds' do @@ -41,8 +45,7 @@ RSpec.describe Projects::UpdateRepositoryStorageService do .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) .and_return(checksum) - expect(GitlabShellWorker).to receive(:perform_async).with(:mv_repository, 'default', anything, anything) - .and_call_original + expect(original_project_repository_double).to receive(:remove) result = subject.execute project.reload @@ -74,13 +77,29 @@ RSpec.describe Projects::UpdateRepositoryStorageService do expect(project_repository_double).to receive(:replicate) .with(project.repository.raw) .and_raise(Gitlab::Git::CommandError) - expect(GitlabShellWorker).not_to receive(:perform_async) result = subject.execute expect(result).to be_error expect(project).not_to be_repository_read_only expect(project.repository_storage).to eq('default') + expect(repository_storage_move).to be_failed + end + end + + context 'when the cleanup fails' do + it 'sets the correct state' do + expect(project_repository_double).to receive(:replicate) + .with(project.repository.raw) + expect(project_repository_double).to receive(:checksum) + .and_return(checksum) + expect(original_project_repository_double).to receive(:remove) + .and_raise(Gitlab::Git::CommandError) + + result = subject.execute + + expect(result).to be_error + expect(repository_storage_move).to be_cleanup_failed end end @@ -93,7 +112,6 @@ RSpec.describe Projects::UpdateRepositoryStorageService do .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) .and_return('not matching checksum') - expect(GitlabShellWorker).not_to receive(:perform_async) result = subject.execute @@ -114,6 +132,7 @@ RSpec.describe Projects::UpdateRepositoryStorageService do .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) .and_return(checksum) + expect(original_project_repository_double).to receive(:remove) result = subject.execute project.reload diff --git a/spec/support/helpers/http_basic_auth_helpers.rb b/spec/support/helpers/http_basic_auth_helpers.rb index ba1491048c4..bc34e073f9f 100644 --- a/spec/support/helpers/http_basic_auth_helpers.rb +++ b/spec/support/helpers/http_basic_auth_helpers.rb @@ -15,12 +15,15 @@ module HttpBasicAuthHelpers basic_auth_header(client.uid, client.secret) end + def build_auth_headers(value) + { 'HTTP_AUTHORIZATION' => value } + end + + def build_token_auth_header(token) + build_auth_headers("Bearer #{token}") + end + def basic_auth_header(username, password) - { - 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials( - username, - password - ) - } + build_auth_headers(ActionController::HttpAuthentication::Basic.encode_credentials(username, password)) end end diff --git a/spec/support/helpers/packages_manager_api_spec_helper.rb b/spec/support/helpers/packages_manager_api_spec_helper.rb index e5a690e1680..34e92c0595c 100644 --- a/spec/support/helpers/packages_manager_api_spec_helper.rb +++ b/spec/support/helpers/packages_manager_api_spec_helper.rb @@ -1,18 +1,6 @@ # frozen_string_literal: true module PackagesManagerApiSpecHelpers - def build_auth_headers(value) - { 'HTTP_AUTHORIZATION' => value } - end - - def build_basic_auth_header(username, password) - build_auth_headers(ActionController::HttpAuthentication::Basic.encode_credentials(username, password)) - end - - def build_token_auth_header(token) - build_auth_headers("Bearer #{token}") - end - def build_jwt(personal_access_token, secret: jwt_secret, user_id: nil) JSONWebToken::HMACToken.new(secret).tap do |jwt| jwt['access_token'] = personal_access_token.id diff --git a/spec/support/shared_examples/requests/api/composer_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/composer_packages_shared_examples.rb index 5ebf4e350e3..6ef4e852428 100644 --- a/spec/support/shared_examples/requests/api/composer_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/composer_packages_shared_examples.rb @@ -89,7 +89,7 @@ end RSpec.shared_context 'Composer auth headers' do |user_role, user_token| let(:token) { user_token ? personal_access_token.token : 'wrong' } - let(:headers) { user_role == :anonymous ? {} : build_basic_auth_header(user.username, token) } + let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } end RSpec.shared_context 'Composer api project access' do |project_visibility_level, user_role, user_token| @@ -118,7 +118,7 @@ RSpec.shared_examples 'rejects Composer access with unknown group id' do end context 'as authenticated user' do - subject { get api(url), headers: build_basic_auth_header(user.username, personal_access_token.token) } + subject { get api(url), headers: basic_auth_header(user.username, personal_access_token.token) } it_behaves_like 'process Composer api request', :anonymous, :not_found end @@ -134,7 +134,7 @@ RSpec.shared_examples 'rejects Composer access with unknown project id' do end context 'as authenticated user' do - subject { get api(url), headers: build_basic_auth_header(user.username, personal_access_token.token) } + subject { get api(url), headers: basic_auth_header(user.username, personal_access_token.token) } it_behaves_like 'process Composer api request', :anonymous, :not_found end diff --git a/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb index dab0ed82883..fcdc594f258 100644 --- a/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb @@ -122,7 +122,7 @@ RSpec.shared_examples 'process nuget workhorse authorization' do |user_type, sta context 'with a request that bypassed gitlab-workhorse' do let(:headers) do - build_basic_auth_header(user.username, personal_access_token.token) + basic_auth_header(user.username, personal_access_token.token) .merge(workhorse_header) .tap { |h| h.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) } end @@ -401,7 +401,7 @@ RSpec.shared_examples 'rejects nuget access with unknown project id' do end context 'as authenticated user' do - subject { get api(url), headers: build_basic_auth_header(user.username, personal_access_token.token) } + subject { get api(url), headers: basic_auth_header(user.username, personal_access_token.token) } it_behaves_like 'rejects nuget packages access', :anonymous, :not_found end diff --git a/spec/support/shared_examples/requests/api/packages_shared_examples.rb b/spec/support/shared_examples/requests/api/packages_shared_examples.rb index ec15d7a4d2e..6f4a0236b66 100644 --- a/spec/support/shared_examples/requests/api/packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/packages_shared_examples.rb @@ -2,7 +2,7 @@ RSpec.shared_examples 'deploy token for package GET requests' do context 'with deploy token headers' do - let(:headers) { build_basic_auth_header(deploy_token.username, deploy_token.token) } + let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token) } subject { get api(url), headers: headers } @@ -15,7 +15,7 @@ RSpec.shared_examples 'deploy token for package GET requests' do end context 'invalid token' do - let(:headers) { build_basic_auth_header(deploy_token.username, 'bar') } + let(:headers) { basic_auth_header(deploy_token.username, 'bar') } it_behaves_like 'returning response status', :unauthorized end @@ -24,7 +24,7 @@ end RSpec.shared_examples 'deploy token for package uploads' do context 'with deploy token headers' do - let(:headers) { build_basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_header) } + let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_header) } before do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) @@ -35,7 +35,7 @@ RSpec.shared_examples 'deploy token for package uploads' do end context 'invalid token' do - let(:headers) { build_basic_auth_header(deploy_token.username, 'bar').merge(workhorse_header) } + let(:headers) { basic_auth_header(deploy_token.username, 'bar').merge(workhorse_header) } it_behaves_like 'returning response status', :unauthorized end diff --git a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb index 67666b6cf34..4954151b93b 100644 --- a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb @@ -159,7 +159,7 @@ RSpec.shared_examples 'rejects PyPI access with unknown project id' do end context 'as authenticated user' do - subject { get api(url), headers: build_basic_auth_header(user.username, personal_access_token.token) } + subject { get api(url), headers: basic_auth_header(user.username, personal_access_token.token) } it_behaves_like 'process PyPi api request', :anonymous, :not_found end diff --git a/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb b/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb index 2ddbdebdb97..f201c7b1780 100644 --- a/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb +++ b/spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb @@ -2,9 +2,11 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| let(:project_repository_double) { double(:repository) } + let(:original_project_repository_double) { double(:repository) } let!(:project_repository_checksum) { project.repository.checksum } let(:repository_double) { double(:repository) } + let(:original_repository_double) { double(:repository) } let(:repository_checksum) { repository.checksum } before do @@ -14,10 +16,16 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| allow(Gitlab::Git::Repository).to receive(:new) .with('test_second_storage', project.repository.raw.relative_path, project.repository.gl_repository, project.repository.full_path) .and_return(project_repository_double) + allow(Gitlab::Git::Repository).to receive(:new) + .with('default', project.repository.raw.relative_path, nil, nil) + .and_return(original_project_repository_double) allow(Gitlab::Git::Repository).to receive(:new) .with('test_second_storage', repository.raw.relative_path, repository.gl_repository, repository.full_path) .and_return(repository_double) + allow(Gitlab::Git::Repository).to receive(:new) + .with('default', repository.raw.relative_path, nil, nil) + .and_return(original_repository_double) end context 'when the move succeeds', :clean_gitlab_redis_shared_state do @@ -35,8 +43,8 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| allow(repository_double).to receive(:checksum) .and_return(repository_checksum) - expect(GitlabShellWorker).to receive(:perform_async).with(:mv_repository, 'default', anything, anything) - .twice.and_call_original + expect(original_project_repository_double).to receive(:remove) + expect(original_repository_double).to receive(:remove) end it "moves the project and its #{repository_type} repository to the new storage and unmarks the repository as read only" do @@ -110,13 +118,36 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| .with(repository.raw) .and_raise(Gitlab::Git::CommandError) - expect(GitlabShellWorker).not_to receive(:perform_async) - result = subject.execute expect(result).to be_error expect(project).not_to be_repository_read_only expect(project.repository_storage).to eq('default') + expect(repository_storage_move).to be_failed + end + end + + context "when the cleanup of the #{repository_type} repository fails" do + it 'sets the correct state' do + allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original + allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('test_second_storage').and_return(SecureRandom.uuid) + allow(project_repository_double).to receive(:replicate) + .with(project.repository.raw) + allow(project_repository_double).to receive(:checksum) + .and_return(project_repository_checksum) + allow(original_project_repository_double).to receive(:remove) + allow(repository_double).to receive(:replicate) + .with(repository.raw) + allow(repository_double).to receive(:checksum) + .and_return(repository_checksum) + + expect(original_repository_double).to receive(:remove) + .and_raise(Gitlab::Git::CommandError) + + result = subject.execute + + expect(result).to be_error + expect(repository_storage_move).to be_cleanup_failed end end @@ -134,8 +165,6 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type| allow(repository_double).to receive(:checksum) .and_return('not matching checksum') - expect(GitlabShellWorker).not_to receive(:perform_async) - result = subject.execute expect(result).to be_error diff --git a/yarn.lock b/yarn.lock index 4552b82c9d5..506fd57dc64 100644 --- a/yarn.lock +++ b/yarn.lock @@ -848,10 +848,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.158.0.tgz#300d416184a2b0e05f15a96547f726e1825b08a1" integrity sha512-5OJl+7TsXN9PJhY6/uwi+mTwmDZa9n/6119rf77orQ/joFYUypaYhBmy/1TcKVPsy5Zs6KCxE1kmGsfoXc1TYA== -"@gitlab/ui@18.6.2": - version "18.6.2" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-18.6.2.tgz#92d18dd36482ae412500820835e9d62f30f108d4" - integrity sha512-3g22Q9RM1rmexipsZdroETJXd20+Fam1CHsC1h8vOWV4Fad5u3lgQp3KIQQlbmROIGTJ4PbiwE1Qldg+XAMsUw== +"@gitlab/ui@18.7.0": + version "18.7.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-18.7.0.tgz#aee0054d50e50aaf9e7c4ea4b9e36ca4b97102bf" + integrity sha512-y1Gix1aCHvVO+zh6TCDmsCr97nLLHFnfEZRtg69EBnLBCLgwBcucC3mNeR4Q2EHTWjy/5U035UkyW6LDRX05mA== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.3.0" |