diff --git a/app/assets/javascripts/monitoring/constants.js b/app/assets/javascripts/monitoring/constants.js index 9e5d0d0fd289eeaa3edf43011a1db6025dd8728e..e97320fd68272403165144afe883c7a29bc9227c 100644 --- a/app/assets/javascripts/monitoring/constants.js +++ b/app/assets/javascripts/monitoring/constants.js @@ -18,5 +18,3 @@ export const timeWindows = { threeDays: __('3 days'), oneWeek: __('1 week'), }; - -export const msPerMinute = 60000; diff --git a/app/assets/javascripts/monitoring/utils.js b/app/assets/javascripts/monitoring/utils.js index e379827b769ea905052d64d9048a743cfcf35116..ef309c8a398fed9ef49ed226f34c1f27be5ce894 100644 --- a/app/assets/javascripts/monitoring/utils.js +++ b/app/assets/javascripts/monitoring/utils.js @@ -1,4 +1,4 @@ -import { timeWindows, msPerMinute } from './constants'; +import { timeWindows } from './constants'; /** * method that converts a predetermined time window to minutes @@ -6,27 +6,26 @@ import { timeWindows, msPerMinute } from './constants'; * @param {String} timeWindow - The time window to convert to minutes * @returns {number} The time window in minutes */ -const getTimeDifferenceMinutes = timeWindow => { +const getTimeDifferenceSeconds = timeWindow => { switch (timeWindow) { case timeWindows.thirtyMinutes: - return 30; + return 60 * 30; case timeWindows.threeHours: - return 60 * 3; + return 60 * 60 * 3; case timeWindows.oneDay: - return 60 * 24 * 1; + return 60 * 60 * 24 * 1; case timeWindows.threeDays: - return 60 * 24 * 3; + return 60 * 60 * 24 * 3; case timeWindows.oneWeek: - return 60 * 24 * 7 * 1; + return 60 * 60 * 24 * 7 * 1; default: - return 60 * 8; + return 60 * 60 * 8; } }; export const getTimeDiff = selectedTimeWindow => { - const end = Date.now(); - const timeDifferenceMinutes = getTimeDifferenceMinutes(selectedTimeWindow); - const start = new Date(end - timeDifferenceMinutes * msPerMinute).getTime(); + const end = Date.now() / 1000; // convert milliseconds to seconds + const start = end - getTimeDifferenceSeconds(selectedTimeWindow); return { start, end }; }; diff --git a/app/assets/stylesheets/components/related_items_list.scss b/app/assets/stylesheets/components/related_items_list.scss index 5a5601f2fa3dd7aa557798daa79a6ecc8d56e744..628dffc39f166166df8a91c94a6763b874c5bccb 100644 --- a/app/assets/stylesheets/components/related_items_list.scss +++ b/app/assets/stylesheets/components/related_items_list.scss @@ -25,6 +25,18 @@ $item-weight-max-width: 48px; flex-grow: 1; } + .issue-token-state-icon-open { + color: $green-500; + } + + .issue-token-state-icon-closed { + color: $blue-500; + } + + .merge-request-status.closed { + color: $red-500; + } + .issue-token-state-icon-open, .issue-token-state-icon-closed, .confidential-icon, diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 301449cfa90acd7fb41b81eab04f04b6b05512cf..e35f34be23c2cdc1cf773e93b6950dccf28abf27 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -193,7 +193,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController return unless Feature.enabled?(:metrics_time_window, project) return unless params[:start].present? || params[:end].present? - params.require([:start, :end]).values_at(:start, :end) + params.require([:start, :end]) end def search_environment_names diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 0319e95d439d43b7d1238a8ab59f9a3dc08de91e..93d3c991846463c6b9f2960ced89a4415c1c44be 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -81,7 +81,7 @@ class ProjectsFinder < UnionFinder if private_only? current_user.authorized_projects else - Project.public_or_visible_to_user(current_user, params[:visibility_level]) + Project.public_or_visible_to_user(current_user) end end end diff --git a/app/models/project.rb b/app/models/project.rb index e2869fc2ad5280a7905b32af60606e9798daf2d0..65e8c5b4191c584965f0a13d1d5868f0212f9132 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -459,41 +459,14 @@ class Project < ApplicationRecord # Returns a collection of projects that is either public or visible to the # logged in user. - # - # requested_visiblity_levels: Normally all projects that are visible - # to the user (e.g. internal and public) are queried, but this - # parameter allows the caller to narrow the search space to optimize - # database queries. For instance, a caller may only want to see - # internal projects. Instead of querying for internal and public - # projects and throwing away public projects, this parameter allows - # the query to be targeted for only internal projects. - def self.public_or_visible_to_user(user = nil, requested_visibility_levels = []) - return public_to_user unless user - - visible_levels = Gitlab::VisibilityLevel.levels_for_user(user) - include_private = true - requested_visibility_levels = Array(requested_visibility_levels) - - if requested_visibility_levels.present? - visible_levels &= requested_visibility_levels - include_private = requested_visibility_levels.include?(Gitlab::VisibilityLevel::PRIVATE) - end - - public_or_internal_rel = - if visible_levels.present? - where('projects.visibility_level IN (?)', visible_levels) - else - Project.none - end - - private_rel = - if include_private - where('EXISTS (?)', user.authorizations_for_projects) - else - Project.none - end - - public_or_internal_rel.or(private_rel) + def self.public_or_visible_to_user(user = nil) + if user + where('EXISTS (?) OR projects.visibility_level IN (?)', + user.authorizations_for_projects, + Gitlab::VisibilityLevel.levels_for_user(user)) + else + public_to_user + end end # project features may be "disabled", "internal", "enabled" or "public". If "internal", diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 51d2767378762ca8bb4d55b94bf4545889162f31..e0460f7081cb0191f14ffceaf00d5ff19c0c77ac 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -21,6 +21,7 @@ module MergeRequests post_merge_manually_merged reload_merge_requests outdate_suggestions + refresh_pipelines_on_merge_requests reset_merge_when_pipeline_succeeds mark_pending_todos_done cache_merge_requests_closing_issues @@ -107,8 +108,6 @@ module MergeRequests end merge_request.mark_as_unchecked - create_pipeline_for(merge_request, current_user) - UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) end # Upcoming method calls need the refreshed version of @@ -134,6 +133,13 @@ module MergeRequests end end + def refresh_pipelines_on_merge_requests + merge_requests_for_source_branch.each do |merge_request| + create_pipeline_for(merge_request, current_user) + UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) + end + end + def reset_merge_when_pipeline_succeeds merge_requests_for_source_branch.each(&:reset_merge_when_pipeline_succeeds) end diff --git a/changelogs/unreleased/prevent-running-mr-pipelines-when-target-updated.yml b/changelogs/unreleased/prevent-running-mr-pipelines-when-target-updated.yml new file mode 100644 index 0000000000000000000000000000000000000000..d003ca55febf2d057a8f144868e7dfd4adb1c07d --- /dev/null +++ b/changelogs/unreleased/prevent-running-mr-pipelines-when-target-updated.yml @@ -0,0 +1,5 @@ +--- +title: Create pipelines for merge requests only when source branch is updated +merge_request: 26921 +author: +type: fixed diff --git a/changelogs/unreleased/sh-optimize-projects-api.yml b/changelogs/unreleased/sh-optimize-projects-api.yml deleted file mode 100644 index 2f2459be77f2ddcda6ff9255983521557448f114..0000000000000000000000000000000000000000 --- a/changelogs/unreleased/sh-optimize-projects-api.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Optimize /api/v4/projects endpoint for visibility level -merge_request: 26481 -author: -type: performance diff --git a/doc/ci/README.md b/doc/ci/README.md index 06c6b8839092c3c86933029899c0b5b72f8b5a4a..a7ad2018b095c3897a51429933be1aa409bc43c5 100644 --- a/doc/ci/README.md +++ b/doc/ci/README.md @@ -62,7 +62,7 @@ into more features: | [ChatOps](chatops/README.md) | Trigger CI jobs from chat, with results sent back to the channel. | | [Interactive web terminals](interactive_web_terminal/index.md) | Open an interactive web terminal to debug the running jobs. | | [Review Apps](review_apps/index.md) | Configure GitLab CI/CD to preview code changes in a per-branch basis. | -| [Optimising GitLab for large repositories](large_repositories/index.md) | Useful tips on how to optimise GitLab and GitLab Runner for big repositories. | +| [Optimizing GitLab for large repositories](large_repositories/index.md) | Useful tips on how to optimize GitLab and GitLab Runner for big repositories. | | [Deploy Boards](https://docs.gitlab.com/ee/user/project/deploy_boards.html) **[PREMIUM]** | Check the current health and status of each CI/CD environment running on Kubernetes. | | [GitLab CI/CD for external repositories](https://docs.gitlab.com/ee/ci/ci_cd_for_external_repos/index.html) **[PREMIUM]** | Get the benefits of GitLab CI/CD combined with repositories in GitHub and BitBucket Cloud. | diff --git a/doc/ci/large_repositories/index.md b/doc/ci/large_repositories/index.md index 576914e9dc380b7bd51c4670c0a0062c4821f293..cfe638c0a2281225082feec1a59ab22123c1bad4 100644 --- a/doc/ci/large_repositories/index.md +++ b/doc/ci/large_repositories/index.md @@ -1,11 +1,11 @@ -# Optimising GitLab for large repositories +# Optimizing GitLab for large repositories Large repositories consisting of more than 50k files in a worktree often require special consideration because of the time required to clone and check out. GitLab and GitLab Runner handle this scenario well -but require optimised configuration to efficiently perform its +but require optimized configuration to efficiently perform its set of operations. The general guidelines for handling big repositories are simple. @@ -15,7 +15,7 @@ Each guideline is described in more detail in the sections below: - Always use shallow clone to reduce data transfer. Be aware that this puts more burden on GitLab instance due to higher CPU impact. - Control the clone directory if you heavily use a fork-based workflow. -- Optimise `git clean` flags to ensure that you remove or keep data that might affect or speed-up your build. +- Optimize `git clean` flags to ensure that you remove or keep data that might affect or speed-up your build. ## Shallow cloning @@ -76,7 +76,7 @@ done by GitLab, requiring you to do them. This can have implications if you heavily use big repositories with fork workflow. Fork workflow from GitLab Runner's perspective is stored as a separate repository -with separate worktree. That means that GitLab Runner cannot optimise the usage +with separate worktree. That means that GitLab Runner cannot optimize the usage of worktrees and you might have to instruct GitLab Runner to use that. In such cases, ideally you want to make the GitLab Runner executor be used only used only @@ -113,7 +113,7 @@ available parameters are dependent on Git version. Following the guidelines above, lets imagine that we want to: -- Optimise for a big project (more than 50k files in directory). +- Optimize for a big project (more than 50k files in directory). - Use forks-based workflow for contributing. - Reuse existing worktrees. Have preconfigured runners that are pre-cloned with repositories. - Runner assigned only to project and all forks. diff --git a/qa/qa/specs/features/browser_ui/4_verify/ci_variable/add_ci_variable_spec.rb b/qa/qa/specs/features/browser_ui/4_verify/ci_variable/add_ci_variable_spec.rb index 33f342edb08d6508e6cd57cc1e16d65000aed0dc..cb273ec40547a87e0bcabd752639ca8df33251cc 100644 --- a/qa/qa/specs/features/browser_ui/4_verify/ci_variable/add_ci_variable_spec.rb +++ b/qa/qa/specs/features/browser_ui/4_verify/ci_variable/add_ci_variable_spec.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true module QA - context 'Verify' do + # Failure issue: https://gitlab.com/gitlab-org/quality/nightly/issues/91 + context 'Verify', :quarantine do describe 'CI variable support' do it 'user adds a CI variable' do Runtime::Browser.visit(:gitlab, Page::Main::Login) diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 436398752656d3df7b7a00284f0ae2207aebdad8..168c0168bba126d005765fc4e4c86948f14a7216 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -419,6 +419,17 @@ describe Projects::EnvironmentsController do expect(json_response['data']).to eq({}) expect(json_response['last_update']).to eq(42) end + + context 'when time params are provided' do + it 'returns a metrics JSON document' do + additional_metrics(start: '1554702993.5398998', end: '1554717396.996232') + + expect(response).to be_ok + expect(json_response['success']).to be(true) + expect(json_response['data']).to eq({}) + expect(json_response['last_update']).to eq(42) + end + end end context 'when only one time param is provided' do diff --git a/spec/javascripts/monitoring/utils_spec.js b/spec/javascripts/monitoring/utils_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..e3c455d168625d2b79f090e12634898f05c1bfcc --- /dev/null +++ b/spec/javascripts/monitoring/utils_spec.js @@ -0,0 +1,29 @@ +import { getTimeDiff } from '~/monitoring/utils'; +import { timeWindows } from '~/monitoring/constants'; + +describe('getTimeDiff', () => { + it('defaults to an 8 hour (28800s) difference', () => { + const params = getTimeDiff(); + + expect(params.end - params.start).toEqual(28800); + }); + + it('accepts time window as an argument', () => { + const params = getTimeDiff(timeWindows.thirtyMinutes); + + expect(params.end - params.start).not.toEqual(28800); + }); + + it('returns a value for every defined time window', () => { + const nonDefaultWindows = Object.keys(timeWindows).filter(window => window !== 'eightHours'); + + nonDefaultWindows.forEach(window => { + const params = getTimeDiff(timeWindows[window]); + const diff = params.end - params.start; + + // Ensure we're not returning the default, 28800 (the # of seconds in 8 hrs) + expect(diff).not.toEqual(28800); + expect(typeof diff).toEqual('number'); + }); + }); +}); diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5eb31430ccd5f5b0c1f1852e626833b0f84ff226..dc8f8519339ac92c901b8d2ca9a206309862c331 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2721,7 +2721,7 @@ describe Project do end describe '#any_lfs_file_locks?', :request_store do - let!(:project) { create(:project) } + set(:project) { create(:project) } it 'returns false when there are no LFS file locks' do expect(project.any_lfs_file_locks?).to be_falsey @@ -3159,53 +3159,6 @@ describe Project do expect(projects).to eq([public_project]) end end - - context 'with requested visibility levels' do - set(:internal_project) { create(:project, :internal, :repository) } - set(:private_project_2) { create(:project, :private) } - - context 'with admin user' do - set(:admin) { create(:admin) } - - it 'returns all projects' do - projects = described_class.all.public_or_visible_to_user(admin, []) - - expect(projects).to match_array([public_project, private_project, private_project_2, internal_project]) - end - - it 'returns all public and private projects' do - projects = described_class.all.public_or_visible_to_user(admin, [Gitlab::VisibilityLevel::PUBLIC, Gitlab::VisibilityLevel::PRIVATE]) - - expect(projects).to match_array([public_project, private_project, private_project_2]) - end - - it 'returns all private projects' do - projects = described_class.all.public_or_visible_to_user(admin, [Gitlab::VisibilityLevel::PRIVATE]) - - expect(projects).to match_array([private_project, private_project_2]) - end - end - - context 'with regular user' do - it 'returns authorized projects' do - projects = described_class.all.public_or_visible_to_user(user, []) - - expect(projects).to match_array([public_project, private_project, internal_project]) - end - - it "returns user's public and private projects" do - projects = described_class.all.public_or_visible_to_user(user, [Gitlab::VisibilityLevel::PUBLIC, Gitlab::VisibilityLevel::PRIVATE]) - - expect(projects).to match_array([public_project, private_project]) - end - - it 'returns one private project' do - projects = described_class.all.public_or_visible_to_user(user, [Gitlab::VisibilityLevel::PRIVATE]) - - expect(projects).to eq([private_project]) - end - end - end end describe '.with_feature_available_for_user' do diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index bd10523bc94f854104eb431a156a9c75a6a4ba4a..5ed06df707280bc2b65888dc9cd10c74900a6551 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -146,7 +146,10 @@ describe MergeRequests::RefreshService do stub_ci_pipeline_yaml_file(YAML.dump(config)) end - subject { service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/master') } + subject { service.new(project, @user).execute(@oldrev, @newrev, ref) } + + let(:ref) { 'refs/heads/master' } + let(:project) { @project } context "when .gitlab-ci.yml has merge_requests keywords" do let(:config) do @@ -162,14 +165,17 @@ describe MergeRequests::RefreshService do it 'create detached merge request pipeline with commits' do expect { subject } .to change { @merge_request.merge_request_pipelines.count }.by(1) - .and change { @fork_merge_request.merge_request_pipelines.count }.by(1) .and change { @another_merge_request.merge_request_pipelines.count }.by(0) expect(@merge_request.has_commits?).to be_truthy - expect(@fork_merge_request.has_commits?).to be_truthy expect(@another_merge_request.has_commits?).to be_falsy end + it 'does not create detached merge request pipeline for forked project' do + expect { subject } + .not_to change { @fork_merge_request.merge_request_pipelines.count } + end + it 'create detached merge request pipeline for non-fork merge request' do subject @@ -177,11 +183,25 @@ describe MergeRequests::RefreshService do .to be_detached_merge_request_pipeline end - it 'create legacy detached merge request pipeline for fork merge request' do - subject + context 'when service is hooked by target branch' do + let(:ref) { 'refs/heads/feature' } - expect(@fork_merge_request.merge_request_pipelines.first) - .to be_legacy_detached_merge_request_pipeline + it 'does not create detached merge request pipeline' do + expect { subject } + .not_to change { @merge_request.merge_request_pipelines.count } + end + end + + context 'when service runs on forked project' do + let(:project) { @fork_project } + + it 'creates legacy detached merge request pipeline for fork merge request' do + expect { subject } + .to change { @fork_merge_request.merge_request_pipelines.count }.by(1) + + expect(@fork_merge_request.merge_request_pipelines.first) + .to be_legacy_detached_merge_request_pipeline + end end context 'when ci_use_merge_request_ref feature flag is false' do