diff options
26 files changed, 376 insertions, 398 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index edca45f22f9..a8d615dd8f0 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -41,6 +41,11 @@ export default { required: true, }, }, + data() { + return { + assignedDiscussions: false, + }; + }, computed: { ...mapState({ isLoading: state => state.diffs.isLoading, @@ -58,9 +63,9 @@ export default { plainDiffPath: state => state.diffs.plainDiffPath, emailPatchPath: state => state.diffs.emailPatchPath, }), - ...mapState('diffs', ['showTreeList']), + ...mapState('diffs', ['showTreeList', 'isLoading']), ...mapGetters('diffs', ['isParallelView']), - ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), + ...mapGetters(['isNotesFetched', 'getNoteableData']), targetBranch() { return { branchName: this.targetBranchName, @@ -147,11 +152,12 @@ export default { } }, setDiscussions() { - if (this.isNotesFetched) { + if (this.isNotesFetched && !this.assignedDiscussions && !this.isLoading) { requestIdleCallback( - () => { - this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); - }, + () => + this.assignDiscussionsToDiff().then(() => { + this.assignedDiscussions = true; + }), { timeout: 1000 }, ); } diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index f72c7a84e5c..958e57c5652 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -29,7 +29,7 @@ export default { }, computed: { ...mapState('diffs', ['currentDiffFileId']), - ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']), + ...mapGetters(['isNotesFetched']), isCollapsed() { return this.file.collapsed || false; }, @@ -79,7 +79,7 @@ export default { .then(() => { requestIdleCallback( () => { - this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode); + this.assignDiscussionsToDiff(); }, { timeout: 1000 }, ); diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 1e0b27b538d..ca8ae605cb4 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -5,7 +5,6 @@ import createFlash from '~/flash'; import { s__ } from '~/locale'; import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils'; import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility'; -import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils'; import { getDiffPositionByLineCode, getNoteFormData } from './utils'; import * as types from './mutation_types'; import { @@ -36,18 +35,17 @@ export const fetchDiffFiles = ({ state, commit }) => { // This is adding line discussions to the actual lines in the diff tree // once for parallel and once for inline mode -export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => { +export const assignDiscussionsToDiff = ( + { commit, state, rootState }, + discussions = rootState.notes.discussions, +) => { const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles); - Object.values(allLineDiscussions).forEach(discussions => { - if (discussions.length > 0) { - const { fileHash } = discussions[0]; - commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { - fileHash, - discussions, - diffPositionByLineCode, - }); - } + discussions.filter(discussion => discussion.diff_discussion).forEach(discussion => { + commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { + discussion, + diffPositionByLineCode, + }); }); }; @@ -190,9 +188,7 @@ export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { return dispatch('saveNote', postData, { root: true }) .then(result => dispatch('updateDiscussion', result.discussion, { root: true })) - .then(discussion => - dispatch('assignDiscussionsToDiff', reduceDiscussionsToLineCodes([discussion])), - ) + .then(discussion => dispatch('assignDiscussionsToDiff', [discussion])) .catch(() => createFlash(s__('MergeRequests|Saving the comment failed'))); }; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 0b4485ecdb5..5a8aebd2086 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -90,53 +90,67 @@ export default { })); }, - [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) { - const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); - const firstDiscussion = discussions[0]; - const isDiffDiscussion = firstDiscussion.diff_discussion; - const hasLineCode = firstDiscussion.line_code; - const diffPosition = diffPositionByLineCode[firstDiscussion.line_code]; - - if ( - selectedFile && - isDiffDiscussion && - hasLineCode && - diffPosition && + [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode }) { + const { latestDiff } = state; + + const discussionLineCode = discussion.line_code; + const fileHash = discussion.diff_file.file_hash; + const lineCheck = ({ lineCode }) => + lineCode === discussionLineCode && isDiscussionApplicableToLine({ - discussion: firstDiscussion, - diffPosition, - latestDiff: state.latestDiff, - }) - ) { - const targetLine = selectedFile.parallelDiffLines.find( - line => - (line.left && line.left.lineCode === firstDiscussion.line_code) || - (line.right && line.right.lineCode === firstDiscussion.line_code), - ); - if (targetLine) { - if (targetLine.left && targetLine.left.lineCode === firstDiscussion.line_code) { - Object.assign(targetLine.left, { - discussions, - }); - } else { - Object.assign(targetLine.right, { - discussions, + discussion, + diffPosition: diffPositionByLineCode[lineCode], + latestDiff, + }); + + state.diffFiles = state.diffFiles.map(diffFile => { + if (diffFile.fileHash === fileHash) { + const file = { ...diffFile }; + + if (file.highlightedDiffLines) { + file.highlightedDiffLines = file.highlightedDiffLines.map(line => { + if (lineCheck(line)) { + return { + ...line, + discussions: line.discussions.concat(discussion), + }; + } + + return line; }); } - } - - if (selectedFile.highlightedDiffLines) { - const targetInlineLine = selectedFile.highlightedDiffLines.find( - line => line.lineCode === firstDiscussion.line_code, - ); - if (targetInlineLine) { - Object.assign(targetInlineLine, { - discussions, + if (file.parallelDiffLines) { + file.parallelDiffLines = file.parallelDiffLines.map(line => { + const left = line.left && lineCheck(line.left); + const right = line.right && lineCheck(line.right); + + if (left || right) { + return { + left: { + ...line.left, + discussions: left ? line.left.discussions.concat(discussion) : [], + }, + right: { + ...line.right, + discussions: right ? line.right.discussions.concat(discussion) : [], + }, + }; + } + + return line; }); } + + if (!file.parallelDiffLines || !file.highlightedDiffLines) { + file.discussions = file.discussions.concat(discussion); + } + + return file; } - } + + return diffFile; + }); }, [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) { diff --git a/app/assets/javascripts/jobs/components/job_app.vue b/app/assets/javascripts/jobs/components/job_app.vue index ba14aaeed2c..ac19034f69d 100644 --- a/app/assets/javascripts/jobs/components/job_app.vue +++ b/app/assets/javascripts/jobs/components/job_app.vue @@ -77,11 +77,11 @@ 'shouldRenderCalloutMessage', 'shouldRenderTriggeredLabel', 'hasEnvironment', - 'isJobStuck', 'hasTrace', 'emptyStateIllustration', 'isScrollingDown', 'emptyStateAction', + 'hasRunnersForProject', ]), shouldRenderContent() { @@ -195,9 +195,9 @@ <!-- Body Section --> <stuck-block - v-if="isJobStuck" + v-if="job.stuck" class="js-job-stuck" - :has-no-runners-for-project="job.runners.available" + :has-no-runners-for-project="hasRunnersForProject" :tags="job.tags" :runners-path="runnerSettingsUrl" /> diff --git a/app/assets/javascripts/jobs/components/stuck_block.vue b/app/assets/javascripts/jobs/components/stuck_block.vue index a60643b2c65..1d5789b175a 100644 --- a/app/assets/javascripts/jobs/components/stuck_block.vue +++ b/app/assets/javascripts/jobs/components/stuck_block.vue @@ -23,14 +23,7 @@ export default { <template> <div class="bs-callout bs-callout-warning"> <p - v-if="hasNoRunnersForProject" - class="js-stuck-no-runners append-bottom-0" - > - {{ s__(`Job|This job is stuck, because the project - doesn't have any runners online assigned to it.`) }} - </p> - <p - v-else-if="tags.length" + v-if="tags.length" class="js-stuck-with-tags append-bottom-0" > {{ s__(`This job is stuck, because you don't have @@ -44,6 +37,13 @@ export default { </span> </p> <p + v-else-if="hasNoRunnersForProject" + class="js-stuck-no-runners append-bottom-0" + > + {{ s__(`Job|This job is stuck, because the project + doesn't have any runners online assigned to it.`) }} + </p> + <p v-else class="js-stuck-no-active-runner append-bottom-0" > diff --git a/app/assets/javascripts/jobs/store/getters.js b/app/assets/javascripts/jobs/store/getters.js index 4ce395a9106..4de01f8e532 100644 --- a/app/assets/javascripts/jobs/store/getters.js +++ b/app/assets/javascripts/jobs/store/getters.js @@ -41,17 +41,10 @@ export const emptyStateIllustration = state => (state.job && state.job.status && state.job.status.illustration) || {}; export const emptyStateAction = state => (state.job && state.job.status && state.job.status.action) || {}; -/** - * When the job is pending and there are no available runners - * we need to render the stuck block; - * - * @returns {Boolean} - */ -export const isJobStuck = state => - (!_.isEmpty(state.job.status) && state.job.status.group === 'pending') && - (!_.isEmpty(state.job.runners) && state.job.runners.available === false); export const isScrollingDown = state => isScrolledToBottom() && !state.isTraceComplete; +export const hasRunnersForProject = state => state.job.runners.available && !state.job.runners.online; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index 21c334a9d33..e4f36154fcd 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -1,6 +1,5 @@ import _ from 'underscore'; import * as constants from '../constants'; -import { reduceDiscussionsToLineCodes } from './utils'; import { collapseSystemNotes } from './collapse_utils'; export const discussions = state => collapseSystemNotes(state.discussions); @@ -31,9 +30,6 @@ export const notesById = state => return acc; }, {}); -export const discussionsStructuredByLineCode = state => - reduceDiscussionsToLineCodes(state.discussions); - export const noteableType = state => { const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE, EPIC_NOTEABLE_TYPE } = constants; diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js index 0e41ff03d67..dd57539e4d8 100644 --- a/app/assets/javascripts/notes/stores/utils.js +++ b/app/assets/javascripts/notes/stores/utils.js @@ -25,18 +25,6 @@ export const getQuickActionText = note => { return text; }; -export const reduceDiscussionsToLineCodes = selectedDiscussions => - selectedDiscussions.reduce((acc, note) => { - if (note.diff_discussion && note.line_code) { - // For context about line notes: there might be multiple notes with the same line code - const items = acc[note.line_code] || []; - items.push(note); - - Object.assign(acc, { [note.line_code]: items }); - } - return acc; - }, {}); - export const hasQuickActions = note => REGEX_QUICK_ACTIONS.test(note); export const stripQuickActions = note => note.replace(REGEX_QUICK_ACTIONS, '').trim(); diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index e8e943872de..f0f791742f4 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -107,7 +107,7 @@ module Clusters end def kubeclient - @kubeclient ||= build_kube_client!(api_groups: ['api', 'apis/rbac.authorization.k8s.io']) + @kubeclient ||= build_kube_client! end private @@ -136,7 +136,7 @@ module Clusters Gitlab::NamespaceSanitizer.sanitize(slug) end - def build_kube_client!(api_groups: ['api'], api_version: 'v1') + def build_kube_client! raise "Incomplete settings" unless api_url && actual_namespace unless (username && password) || token @@ -145,8 +145,6 @@ module Clusters Gitlab::Kubernetes::KubeClient.new( api_url, - api_groups, - api_version, auth_options: kubeclient_auth_options, ssl_options: kubeclient_ssl_options, http_proxy_uri: ENV['http_proxy'] diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index f119555f16b..798944d0c06 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -144,7 +144,7 @@ class KubernetesService < DeploymentService end def kubeclient - @kubeclient ||= build_kube_client!(api_groups: ['api', 'apis/rbac.authorization.k8s.io']) + @kubeclient ||= build_kube_client! end def deprecated? @@ -182,13 +182,11 @@ class KubernetesService < DeploymentService slug.gsub(/[^-a-z0-9]/, '-').gsub(/^-+/, '') end - def build_kube_client!(api_groups: ['api'], api_version: 'v1') + def build_kube_client! raise "Incomplete settings" unless api_url && actual_namespace && token Gitlab::Kubernetes::KubeClient.new( api_url, - api_groups, - api_version, auth_options: kubeclient_auth_options, ssl_options: kubeclient_ssl_options, http_proxy_uri: ENV['http_proxy'] diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 066a5b1885c..9ddce0d2c80 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -5,6 +5,7 @@ class BuildDetailsEntity < JobEntity expose :tag_list, as: :tags expose :has_trace?, as: :has_trace expose :stage + expose :stuck?, as: :stuck expose :user, using: UserEntity expose :runner, using: RunnerEntity expose :pipeline, using: PipelineEntity diff --git a/app/services/clusters/gcp/finalize_creation_service.rb b/app/services/clusters/gcp/finalize_creation_service.rb index 3ae0a4a19d0..6ee63db8eb9 100644 --- a/app/services/clusters/gcp/finalize_creation_service.rb +++ b/app/services/clusters/gcp/finalize_creation_service.rb @@ -60,18 +60,15 @@ module Clusters 'https://' + gke_cluster.endpoint, Base64.decode64(gke_cluster.master_auth.cluster_ca_certificate), gke_cluster.master_auth.username, - gke_cluster.master_auth.password, - api_groups: ['api', 'apis/rbac.authorization.k8s.io'] + gke_cluster.master_auth.password ) end - def build_kube_client!(api_url, ca_pem, username, password, api_groups: ['api'], api_version: 'v1') + def build_kube_client!(api_url, ca_pem, username, password) raise "Incomplete settings" unless api_url && username && password Gitlab::Kubernetes::KubeClient.new( api_url, - api_groups, - api_version, auth_options: { username: username, password: password }, ssl_options: kubeclient_ssl_options(ca_pem), http_proxy_uri: ENV['http_proxy'] diff --git a/changelogs/unreleased/52202-consider-moving-isjobstuck-verification-to-backend.yml b/changelogs/unreleased/52202-consider-moving-isjobstuck-verification-to-backend.yml new file mode 100644 index 00000000000..0efd97d91b8 --- /dev/null +++ b/changelogs/unreleased/52202-consider-moving-isjobstuck-verification-to-backend.yml @@ -0,0 +1,5 @@ +--- +title: Renders stuck block when runners are stuck +merge_request: +author: +type: fixed diff --git a/lib/gitlab/ci/templates/Android.gitlab-ci.yml b/lib/gitlab/ci/templates/Android.gitlab-ci.yml index bf7831b937c..6e138639b71 100644 --- a/lib/gitlab/ci/templates/Android.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Android.gitlab-ci.yml @@ -1,51 +1,45 @@ -# Read more about this script on this blog post https://about.gitlab.com/2016/11/30/setting-up-gitlab-ci-for-android-projects/, by Greyson Parrelli +# Read more about this script on this blog post https://about.gitlab.com/2018/10/24/setting-up-gitlab-ci-for-android-projects/, by Jason Lenny image: openjdk:8-jdk variables: ANDROID_COMPILE_SDK: "28" - ANDROID_BUILD_TOOLS: "28.0.3" - ANDROID_SDK_TOOLS: "26.1.1" + ANDROID_BUILD_TOOLS: "28.0.2" + ANDROID_SDK_TOOLS: "4333796" before_script: -- apt-get --quiet update --yes -- apt-get --quiet install --yes wget tar unzip lib32stdc++6 lib32z1 -- wget --quiet --output-document=android-sdk.zip https://dl.google.com/android/repository/sdk-tools-linux-4333796.zip -- unzip android-sdk.zip -d android-sdk-linux -- echo y | android-sdk-linux/tools/bin/sdkmanager "platforms;android-${ANDROID_COMPILE_SDK}" > /dev/null -- echo y | android-sdk-linux/tools/bin/sdkmanager platform-tools > /dev/null -- echo y | android-sdk-linux/tools/bin/sdkmanager "build-tools;${ANDROID_BUILD_TOOLS}" > /dev/null -- echo y | android-sdk-linux/tools/bin/sdkmanager "extras;google;google_play_services" > /dev/null -- echo y | android-sdk-linux/tools/bin/sdkmanager "extras;google;m2repository" > /dev/null -- export ANDROID_HOME=$PWD/android-sdk-linux -- export PATH=$PATH:$PWD/android-sdk-linux/platform-tools/ -- yes | android-sdk-linux/tools/bin/sdkmanager --licenses & -- chmod +x ./gradlew + - apt-get --quiet update --yes + - apt-get --quiet install --yes wget tar unzip lib32stdc++6 lib32z1 + - wget --quiet --output-document=android-sdk.zip https://dl.google.com/android/repository/sdk-tools-linux-${ANDROID_SDK_TOOLS}.zip + - unzip -d android-sdk-linux android-sdk.zip + - echo y | android-sdk-linux/tools/bin/sdkmanager "platforms;android-${ANDROID_COMPILE_SDK}" >/dev/null + - echo y | android-sdk-linux/tools/bin/sdkmanager "platform-tools" >/dev/null + - echo y | android-sdk-linux/tools/bin/sdkmanager "build-tools;${ANDROID_BUILD_TOOLS}" >/dev/null + - export ANDROID_HOME=$PWD/android-sdk-linux + - export PATH=$PATH:$PWD/android-sdk-linux/platform-tools/ + - chmod +x ./gradlew + # temporarily disable checking for EPIPE error and use yes to accept all licenses + - set +o pipefail + - yes | android-sdk-linux/tools/bin/sdkmanager --licenses + - set -o pipefail stages: -- build -- test + - build + - test -build: +lintDebug: stage: build script: - - ./gradlew assembleDebug + - ./gradlew -Pci --console=plain :app:lintDebug -PbuildDir=lint + +assembleDebug: + stage: build + script: + - ./gradlew assembleDebug artifacts: paths: - app/build/outputs/ -unitTests: - stage: test - script: - - ./gradlew test - -functionalTests: +debugTests: stage: test script: - - wget --quiet --output-document=android-wait-for-emulator https://raw.githubusercontent.com/travis-ci/travis-cookbooks/0f497eb71291b52a703143c5cd63a217c8766dc9/community-cookbooks/android-sdk/files/default/android-wait-for-emulator - - chmod +x android-wait-for-emulator - - echo y | android-sdk-linux/tools/android --silent update sdk --no-ui --all --filter sys-img-x86-google_apis-${ANDROID_COMPILE_SDK} - - echo no | android-sdk-linux/tools/android create avd -n test -t android-${ANDROID_COMPILE_SDK} --abi google_apis/x86 - - android-sdk-linux/tools/emulator64-x86 -avd test -no-window -no-audio & - - ./android-wait-for-emulator - - adb shell input keyevent 82 - - ./gradlew cAT + - ./gradlew -Pci --console=plain :app:testDebug diff --git a/lib/gitlab/kubernetes/kube_client.rb b/lib/gitlab/kubernetes/kube_client.rb index e88a15b8acd..f266177bec1 100644 --- a/lib/gitlab/kubernetes/kube_client.rb +++ b/lib/gitlab/kubernetes/kube_client.rb @@ -13,11 +13,21 @@ module Gitlab class KubeClient include Gitlab::Utils::StrongMemoize - SUPPORTED_API_GROUPS = [ - 'api', - 'apis/rbac.authorization.k8s.io', - 'apis/extensions' - ].freeze + SUPPORTED_API_GROUPS = { + core: { group: 'api', version: 'v1' }, + rbac: { group: 'apis/rbac.authorization.k8s.io', version: 'v1' }, + extensions: { group: 'apis/extensions', version: 'v1beta1' } + }.freeze + + SUPPORTED_API_GROUPS.each do |name, params| + client_method_name = "#{name}_client".to_sym + + define_method(client_method_name) do + strong_memoize(client_method_name) do + build_kubeclient(params[:group], params[:version]) + end + end + end # Core API methods delegates to the core api group client delegate :get_pods, @@ -62,48 +72,21 @@ module Gitlab :watch_pod_log, to: :core_client - def initialize(api_prefix, api_groups = ['api'], api_version = 'v1', **kubeclient_options) - raise ArgumentError unless check_api_groups_supported?(api_groups) + attr_reader :api_prefix, :kubeclient_options + def initialize(api_prefix, **kubeclient_options) @api_prefix = api_prefix - @api_groups = api_groups - @api_version = api_version @kubeclient_options = kubeclient_options end - def discover! - clients.each(&:discover) - end - - def clients - hashed_clients.values - end - - def core_client - hashed_clients['api'] - end - - def rbac_client - hashed_clients['apis/rbac.authorization.k8s.io'] - end - - def extensions_client - hashed_clients['apis/extensions'] - end - - def hashed_clients - strong_memoize(:hashed_clients) do - @api_groups.map do |api_group| - api_url = join_api_url(@api_prefix, api_group) - [api_group, ::Kubeclient::Client.new(api_url, @api_version, **@kubeclient_options)] - end.to_h - end - end - private - def check_api_groups_supported?(api_groups) - api_groups.all? {|api_group| SUPPORTED_API_GROUPS.include?(api_group) } + def build_kubeclient(api_group, api_version) + ::Kubeclient::Client.new( + join_api_url(api_prefix, api_group), + api_version, + **kubeclient_options + ) end def join_api_url(api_prefix, api_path) diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 1484676eea3..2023d4b0bd0 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -297,6 +297,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(response).to match_response_schema('job/job_details') expect(json_response['runners']['online']).to be false expect(json_response['runners']['available']).to be false + expect(json_response['stuck']).to be true end end @@ -309,6 +310,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(response).to match_response_schema('job/job_details') expect(json_response['runners']['online']).to be false expect(json_response['runners']['available']).to be true + expect(json_response['stuck']).to be true end end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index c3902ecdd17..b3bea92e635 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -721,6 +721,62 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do expect(page).not_to have_css('.js-job-sidebar.right-sidebar-collpased') end end + + context 'stuck', :js do + before do + visit project_job_path(project, job) + wait_for_requests + end + + context 'without active runners available' do + let(:runner) { create(:ci_runner, :instance, active: false) } + let(:job) { create(:ci_build, :pending, pipeline: pipeline, runner: runner) } + + it 'renders message about job being stuck because no runners are active' do + expect(page).to have_css('.js-stuck-no-active-runner') + expect(page).to have_content("This job is stuck, because you don't have any active runners that can run this job.") + end + end + + context 'when available runners can not run specified tag' do + let(:runner) { create(:ci_runner, :instance, active: false) } + let(:job) { create(:ci_build, :pending, pipeline: pipeline, runner: runner, tag_list: %w(docker linux)) } + + it 'renders message about job being stuck because of no runners with the specified tags' do + expect(page).to have_css('.js-stuck-with-tags') + expect(page).to have_content("This job is stuck, because you don't have any active runners online with any of these tags assigned to them:") + end + end + + context 'when runners are offline and build has tags' do + let(:runner) { create(:ci_runner, :instance, active: true) } + let(:job) { create(:ci_build, :pending, pipeline: pipeline, runner: runner, tag_list: %w(docker linux)) } + + it 'renders message about job being stuck because of no runners with the specified tags' do + expect(page).to have_css('.js-stuck-with-tags') + expect(page).to have_content("This job is stuck, because you don't have any active runners online with any of these tags assigned to them:") + end + end + + context 'without any runners available' do + let(:job) { create(:ci_build, :pending, pipeline: pipeline) } + + it 'renders message about job being stuck because not runners are available' do + expect(page).to have_css('.js-stuck-no-active-runner') + expect(page).to have_content("This job is stuck, because you don't have any active runners that can run this job.") + end + end + + context 'without available runners online' do + let(:runner) { create(:ci_runner, :instance, active: true) } + let(:job) { create(:ci_build, :pending, pipeline: pipeline, runner: runner) } + + it 'renders message about job being stuck because runners are offline' do + expect(page).to have_css('.js-stuck-no-runners') + expect(page).to have_content("This job is stuck, because the project doesn't have any runners online assigned to it.") + end + end + end end describe "POST /:project/jobs/:id/cancel", :js do diff --git a/spec/fixtures/api/schemas/job/job_details.json b/spec/fixtures/api/schemas/job/job_details.json index 8218474705c..cdf7b049ab6 100644 --- a/spec/fixtures/api/schemas/job/job_details.json +++ b/spec/fixtures/api/schemas/job/job_details.json @@ -18,6 +18,7 @@ "runner": { "$ref": "runner.json" }, "runners": { "$ref": "runners.json" }, "has_trace": { "type": "boolean" }, - "stage": { "type": "string" } + "stage": { "type": "string" }, + "stuck": { "type": "boolean" } } } diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index 85c1926fcb1..bb623953710 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -27,7 +27,6 @@ import actions, { toggleShowTreeList, } from '~/diffs/store/actions'; import * as types from '~/diffs/store/mutation_types'; -import { reduceDiscussionsToLineCodes } from '~/notes/stores/utils'; import axios from '~/lib/utils/axios_utils'; import testAction from '../../helpers/vuex_action_helper'; @@ -152,7 +151,7 @@ describe('DiffsStoreActions', () => { original_position: diffPosition, }; - const discussions = reduceDiscussionsToLineCodes([singleDiscussion]); + const discussions = [singleDiscussion]; testAction( assignDiscussionsToDiff, @@ -162,8 +161,7 @@ describe('DiffsStoreActions', () => { { type: types.SET_LINE_DISCUSSIONS_FOR_FILE, payload: { - fileHash: 'ABC', - discussions: [singleDiscussion], + discussion: singleDiscussion, diffPositionByLineCode: { ABC_1_1: { baseSha: 'abc', @@ -581,7 +579,6 @@ describe('DiffsStoreActions', () => { describe('saveDiffDiscussion', () => { beforeEach(() => { spyOnDependency(actions, 'getNoteFormData').and.returnValue('testData'); - spyOnDependency(actions, 'reduceDiscussionsToLineCodes').and.returnValue('discussions'); }); it('dispatches actions', done => { @@ -602,7 +599,7 @@ describe('DiffsStoreActions', () => { .then(() => { expect(dispatch.calls.argsFor(0)).toEqual(['saveNote', 'testData', { root: true }]); expect(dispatch.calls.argsFor(1)).toEqual(['updateDiscussion', 'test', { root: true }]); - expect(dispatch.calls.argsFor(2)).toEqual(['assignDiscussionsToDiff', 'discussions']); + expect(dispatch.calls.argsFor(2)).toEqual(['assignDiscussionsToDiff', ['discussion']]); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index b7e28391419..4b6d3d5bcba 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -198,40 +198,32 @@ describe('DiffsStoreMutations', () => { }, ], }; - const discussions = [ - { - id: 1, - line_code: 'ABC_1', - diff_discussion: true, - resolvable: true, - original_position: diffPosition, - position: diffPosition, + const discussion = { + id: 1, + line_code: 'ABC_1', + diff_discussion: true, + resolvable: true, + original_position: diffPosition, + position: diffPosition, + diff_file: { + file_hash: state.diffFiles[0].fileHash, }, - { - id: 2, - line_code: 'ABC_1', - diff_discussion: true, - resolvable: true, - original_position: diffPosition, - position: diffPosition, - }, - ]; + }; const diffPositionByLineCode = { ABC_1: diffPosition, }; mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { - fileHash: 'ABC', - discussions, + discussion, diffPositionByLineCode, }); - expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(2); - expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[1].id).toEqual(2); + expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(1); + expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[0].id).toEqual(1); - expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(2); - expect(state.diffFiles[0].highlightedDiffLines[0].discussions[1].id).toEqual(2); + expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(1); + expect(state.diffFiles[0].highlightedDiffLines[0].discussions[0].id).toEqual(1); }); it('should add legacy discussions to the given line', () => { @@ -272,36 +264,30 @@ describe('DiffsStoreMutations', () => { }, ], }; - const discussions = [ - { - id: 1, - line_code: 'ABC_1', - diff_discussion: true, - active: true, + const discussion = { + id: 1, + line_code: 'ABC_1', + diff_discussion: true, + active: true, + diff_file: { + file_hash: state.diffFiles[0].fileHash, }, - { - id: 2, - line_code: 'ABC_1', - diff_discussion: true, - active: true, - }, - ]; + }; const diffPositionByLineCode = { ABC_1: diffPosition, }; mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { - fileHash: 'ABC', - discussions, + discussion, diffPositionByLineCode, }); - expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(2); - expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[1].id).toEqual(2); + expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(1); + expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[0].id).toEqual(1); - expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(2); - expect(state.diffFiles[0].highlightedDiffLines[0].discussions[1].id).toEqual(2); + expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(1); + expect(state.diffFiles[0].highlightedDiffLines[0].discussions[0].id).toEqual(1); }); }); diff --git a/spec/javascripts/jobs/components/job_app_spec.js b/spec/javascripts/jobs/components/job_app_spec.js index e6d403dc826..288c06d6615 100644 --- a/spec/javascripts/jobs/components/job_app_spec.js +++ b/spec/javascripts/jobs/components/job_app_spec.js @@ -88,7 +88,9 @@ describe('Job App ', () => { describe('triggered job', () => { beforeEach(() => { - mock.onGet(props.endpoint).replyOnce(200, Object.assign({}, job, { started: '2017-05-24T10:59:52.000+01:00' })); + mock + .onGet(props.endpoint) + .replyOnce(200, Object.assign({}, job, { started: '2017-05-24T10:59:52.000+01:00' })); vm = mountComponentWithStore(Component, { props, store }); }); @@ -133,57 +135,106 @@ describe('Job App ', () => { }); describe('stuck block', () => { - it('renders stuck block when there are no runners', done => { - mock.onGet(props.endpoint).replyOnce( - 200, - Object.assign({}, job, { - status: { - group: 'pending', - icon: 'status_pending', - label: 'pending', - text: 'pending', - details_path: 'path', - }, - runners: { - available: false, - }, - }), - ); - vm = mountComponentWithStore(Component, { props, store }); - - setTimeout(() => { - expect(vm.$el.querySelector('.js-job-stuck')).not.toBeNull(); + describe('without active runners availabl', () => { + it('renders stuck block when there are no runners', done => { + mock.onGet(props.endpoint).replyOnce( + 200, + Object.assign({}, job, { + status: { + group: 'pending', + icon: 'status_pending', + label: 'pending', + text: 'pending', + details_path: 'path', + }, + stuck: true, + runners: { + available: false, + online: false, + }, + tags: [], + }), + ); + vm = mountComponentWithStore(Component, { props, store }); - done(); - }, 0); + setTimeout(() => { + expect(vm.$el.querySelector('.js-job-stuck')).not.toBeNull(); + expect(vm.$el.querySelector('.js-job-stuck').textContent).toContain( + "This job is stuck, because you don't have any active runners that can run this job.", + ); + done(); + }, 0); + }); }); - it('renders tags in stuck block when there are no runners', done => { - mock.onGet(props.endpoint).replyOnce( - 200, - Object.assign({}, job, { - status: { - group: 'pending', - icon: 'status_pending', - label: 'pending', - text: 'pending', - details_path: 'path', - }, - runners: { - available: false, - }, - }), - ); + describe('when available runners can not run specified tag', () => { + it('renders tags in stuck block when there are no runners', done => { + mock.onGet(props.endpoint).replyOnce( + 200, + Object.assign({}, job, { + status: { + group: 'pending', + icon: 'status_pending', + label: 'pending', + text: 'pending', + details_path: 'path', + }, + stuck: true, + runners: { + available: false, + online: false, + }, + }), + ); - vm = mountComponentWithStore(Component, { - props, - store, + vm = mountComponentWithStore(Component, { + props, + store, + }); + + setTimeout(() => { + expect(vm.$el.querySelector('.js-job-stuck').textContent).toContain(job.tags[0]); + expect(vm.$el.querySelector('.js-job-stuck').textContent).toContain( + "This job is stuck, because you don't have any active runners online with any of these tags assigned to them:", + ); + done(); + }, 0); }); + }); - setTimeout(() => { - expect(vm.$el.querySelector('.js-job-stuck').textContent).toContain(job.tags[0]); - done(); - }, 0); + describe('when runners are offline and build has tags', () => { + it('renders message about job being stuck because of no runners with the specified tags', done => { + mock.onGet(props.endpoint).replyOnce( + 200, + Object.assign({}, job, { + status: { + group: 'pending', + icon: 'status_pending', + label: 'pending', + text: 'pending', + details_path: 'path', + }, + stuck: true, + runners: { + available: true, + online: true, + }, + }), + ); + + vm = mountComponentWithStore(Component, { + props, + store, + }); + + setTimeout(() => { + expect(vm.$el.querySelector('.js-job-stuck').textContent).toContain(job.tags[0]); + expect(vm.$el.querySelector('.js-job-stuck').textContent).toContain( + "This job is stuck, because you don't have any active runners online with any of these tags assigned to them:", + ); + done(); + }, 0); + }) }); it('does not renders stuck block when there are no runners', done => { @@ -418,10 +469,11 @@ describe('Job App ', () => { vm.$store.state.trace = 'Update'; setTimeout(() => { - expect(vm.$el.querySelector('.js-build-trace').textContent.trim()).not.toContain('Update'); - expect(vm.$el.querySelector('.js-build-trace').textContent.trim()).toContain( - 'Different', + expect(vm.$el.querySelector('.js-build-trace').textContent.trim()).not.toContain( + 'Update', ); + + expect(vm.$el.querySelector('.js-build-trace').textContent.trim()).toContain('Different'); done(); }, 0); }); diff --git a/spec/javascripts/jobs/store/getters_spec.js b/spec/javascripts/jobs/store/getters_spec.js index 46a20122ec8..34e9707eadd 100644 --- a/spec/javascripts/jobs/store/getters_spec.js +++ b/spec/javascripts/jobs/store/getters_spec.js @@ -175,43 +175,37 @@ describe('Job Store Getters', () => { }); }); - describe('isJobStuck', () => { - describe('when job is pending and runners are not available', () => { + describe('hasRunnersForProject', () => { + describe('with available and offline runners', () => { it('returns true', () => { - localState.job.status = { - group: 'pending', - }; localState.job.runners = { - available: false, + available: true, + online: false }; - expect(getters.isJobStuck(localState)).toEqual(true); + expect(getters.hasRunnersForProject(localState)).toEqual(true); }); }); - describe('when job is not pending', () => { + describe('with non available runners', () => { it('returns false', () => { - localState.job.status = { - group: 'running', - }; localState.job.runners = { available: false, + online: false }; - expect(getters.isJobStuck(localState)).toEqual(false); + expect(getters.hasRunnersForProject(localState)).toEqual(false); }); }); - describe('when runners are available', () => { + describe('with online runners', () => { it('returns false', () => { - localState.job.status = { - group: 'pending', - }; localState.job.runners = { - available: true, + available: false, + online: true }; - expect(getters.isJobStuck(localState)).toEqual(false); + expect(getters.hasRunnersForProject(localState)).toEqual(false); }); }); }); diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb index 53c5a4e7c94..eed4135d8a2 100644 --- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb +++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb @@ -6,104 +6,63 @@ describe Gitlab::Kubernetes::KubeClient do include KubernetesHelpers let(:api_url) { 'https://kubernetes.example.com/prefix' } - let(:api_groups) { ['api', 'apis/rbac.authorization.k8s.io'] } - let(:api_version) { 'v1' } let(:kubeclient_options) { { auth_options: { bearer_token: 'xyz' } } } - let(:client) { described_class.new(api_url, api_groups, api_version, kubeclient_options) } + let(:client) { described_class.new(api_url, kubeclient_options) } before do stub_kubeclient_discover(api_url) end - describe '#hashed_clients' do - subject { client.hashed_clients } - - it 'has keys from api groups' do - expect(subject.keys).to match_array api_groups - end - - it 'has values of Kubeclient::Client' do - expect(subject.values).to all(be_an_instance_of Kubeclient::Client) - end - end - - describe '#clients' do - subject { client.clients } - - it 'is not empty' do - is_expected.to be_present - end - - it 'is an array of Kubeclient::Client objects' do - is_expected.to all(be_an_instance_of Kubeclient::Client) - end - - it 'has each API group url' do - expected_urls = api_groups.map { |group| "#{api_url}/#{group}" } - - expect(subject.map(&:api_endpoint).map(&:to_s)).to match_array(expected_urls) + shared_examples 'a Kubeclient' do + it 'is a Kubeclient::Client' do + is_expected.to be_an_instance_of Kubeclient::Client end it 'has the kubeclient options' do - subject.each do |client| - expect(client.auth_options).to eq({ bearer_token: 'xyz' }) - end - end - - it 'has the api_version' do - subject.each do |client| - expect(client.instance_variable_get(:@api_version)).to eq('v1') - end + expect(subject.auth_options).to eq({ bearer_token: 'xyz' }) end end describe '#core_client' do subject { client.core_client } - it 'is a Kubeclient::Client' do - is_expected.to be_an_instance_of Kubeclient::Client - end + it_behaves_like 'a Kubeclient' it 'has the core API endpoint' do expect(subject.api_endpoint.to_s).to match(%r{\/api\Z}) end + + it 'has the api_version' do + expect(subject.instance_variable_get(:@api_version)).to eq('v1') + end end describe '#rbac_client' do subject { client.rbac_client } - it 'is a Kubeclient::Client' do - is_expected.to be_an_instance_of Kubeclient::Client - end + it_behaves_like 'a Kubeclient' it 'has the RBAC API group endpoint' do expect(subject.api_endpoint.to_s).to match(%r{\/apis\/rbac.authorization.k8s.io\Z}) end + + it 'has the api_version' do + expect(subject.instance_variable_get(:@api_version)).to eq('v1') + end end describe '#extensions_client' do subject { client.extensions_client } - let(:api_groups) { ['apis/extensions'] } - - it 'is a Kubeclient::Client' do - is_expected.to be_an_instance_of Kubeclient::Client - end + it_behaves_like 'a Kubeclient' it 'has the extensions API group endpoint' do expect(subject.api_endpoint.to_s).to match(%r{\/apis\/extensions\Z}) end - end - describe '#discover!' do - it 'makes a discovery request for each API group' do - client.discover! - - api_groups.each do |api_group| - discovery_url = api_url + '/' + api_group + '/v1' - expect(WebMock).to have_requested(:get, discovery_url).once - end + it 'has the api_version' do + expect(subject.instance_variable_get(:@api_version)).to eq('v1beta1') end end @@ -156,21 +115,12 @@ describe Gitlab::Kubernetes::KubeClient do it 'responds to the method' do expect(client).to respond_to method end - - context 'no rbac client' do - let(:api_groups) { ['api'] } - - it 'throws an error' do - expect { client.public_send(method) }.to raise_error(Module::DelegationError) - end - end end end end describe 'extensions API group' do let(:api_groups) { ['apis/extensions'] } - let(:api_version) { 'v1beta1' } let(:extensions_client) { client.extensions_client } describe '#get_deployments' do @@ -181,22 +131,11 @@ describe Gitlab::Kubernetes::KubeClient do it 'responds to the method' do expect(client).to respond_to :get_deployments end - - context 'no extensions client' do - let(:api_groups) { ['api'] } - let(:api_version) { 'v1' } - - it 'throws an error' do - expect { client.get_deployments }.to raise_error(Module::DelegationError) - end - end end end describe 'non-entity methods' do it 'does not proxy for non-entity methods' do - expect(client.clients.first).to respond_to :proxy_url - expect(client).not_to respond_to :proxy_url end @@ -211,14 +150,6 @@ describe Gitlab::Kubernetes::KubeClient do it 'is delegated to the core client' do expect(client).to delegate_method(:get_pod_log).to(:core_client) end - - context 'when no core client' do - let(:api_groups) { ['apis/extensions'] } - - it 'throws an error' do - expect { client.get_pod_log('pod-name') }.to raise_error(Module::DelegationError) - end - end end describe '#watch_pod_log' do @@ -227,14 +158,6 @@ describe Gitlab::Kubernetes::KubeClient do it 'is delegated to the core client' do expect(client).to delegate_method(:watch_pod_log).to(:core_client) end - - context 'when no core client' do - let(:api_groups) { ['apis/extensions'] } - - it 'throws an error' do - expect { client.watch_pod_log('pod-name') }.to raise_error(Module::DelegationError) - end - end end describe 'methods that do not exist on any client' do diff --git a/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb b/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb index 065d021db5e..b096f1fa4fb 100644 --- a/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb +++ b/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb @@ -16,7 +16,6 @@ describe Clusters::Gcp::Kubernetes::CreateServiceAccountService do let(:kubeclient) do Gitlab::Kubernetes::KubeClient.new( api_url, - ['api', 'apis/rbac.authorization.k8s.io'], auth_options: { username: username, password: password } ) end diff --git a/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb b/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb index c543de21d5b..2355827fa5a 100644 --- a/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb +++ b/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb @@ -11,7 +11,6 @@ describe Clusters::Gcp::Kubernetes::FetchKubernetesTokenService do let(:kubeclient) do Gitlab::Kubernetes::KubeClient.new( api_url, - ['api', 'apis/rbac.authorization.k8s.io'], auth_options: { username: username, password: password } ) end |