diff options
author | Steve Azzopardi <sazzopardi@gitlab.com> | 2018-11-12 19:22:54 +0300 |
---|---|---|
committer | Steve Azzopardi <sazzopardi@gitlab.com> | 2018-11-12 19:22:54 +0300 |
commit | b239c181aefa3dcb22ec98e82cc093ef364fa572 (patch) | |
tree | df17799f94e01c6cec571ce4706ff852218912d3 | |
parent | 62ccf75e9e22d75a6596b6876908eabc72534b02 (diff) | |
parent | 8e6a32d05b65b0d0d1e3e8b95c25d96f31baa6b9 (diff) |
Merge branch '11-5-stable-prepare-rc5' into '11-5-stable'
Prepare 11.5 RC5 release
See merge request gitlab-org/gitlab-ce!22981
12 files changed, 81 insertions, 25 deletions
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index a153edd0476..c1dfa036678 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -433,7 +433,7 @@ Please check your network connection and try again.`; <div class="discussion-with-resolve-btn"> <button type="button" - class="js-vue-discussion-reply btn btn-text-field mr-2 qa-discussion-reply" + class="js-vue-discussion-reply btn btn-text-field mr-sm-2 qa-discussion-reply" title="Add a reply" @click="showReplyForm" > @@ -442,7 +442,7 @@ Please check your network connection and try again.`; <div v-if="discussion.resolvable"> <button type="button" - class="btn btn-default mx-sm-2" + class="btn btn-default mr-sm-2" @click="resolveHandler()" > <i @@ -455,7 +455,7 @@ Please check your network connection and try again.`; </div> <div v-if="discussion.resolvable" - class="btn-group discussion-actions" + class="btn-group discussion-actions ml-sm-2" role="group" > <div diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index ed5ac112dc0..e555279a6ac 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -50,6 +50,7 @@ export default { }, data() { return { + isFetching: false, currentFilter: null, }; }, @@ -141,6 +142,10 @@ export default { return discussion.individual_note ? { note: discussion.notes[0] } : { discussion }; }, fetchNotes() { + if (this.isFetching) return null; + + this.isFetching = true; + return this.fetchDiscussions({ path: this.getNotesDataByProp('discussionsPath') }) .then(() => { this.initPolling(); @@ -149,6 +154,7 @@ export default { this.setLoadingState(false); this.setNotesFetchedState(true); eventHub.$emit('fetchedNotesData'); + this.isFetching = false; }) .then(() => this.$nextTick()) .then(() => this.checkLocationHash()) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index dcb1275d182..c57c1eee350 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -595,7 +595,6 @@ $note-form-margin-left: 70px; .discussion-actions { float: right; - margin-left: 10px; color: $gray-darkest; @include media-breakpoint-down(xs) { diff --git a/app/controllers/concerns/send_file_upload.rb b/app/controllers/concerns/send_file_upload.rb index 0bb7b7efed0..515a9eede8e 100644 --- a/app/controllers/concerns/send_file_upload.rb +++ b/app/controllers/concerns/send_file_upload.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module SendFileUpload - def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, disposition: 'attachment') + def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, proxy: false, disposition: 'attachment') if attachment # Response-Content-Type will not override an existing Content-Type in # Google Cloud Storage, so the metadata needs to be cleared on GCS for @@ -17,7 +17,7 @@ module SendFileUpload if file_upload.file_storage? send_file file_upload.path, send_params - elsif file_upload.class.proxy_download_enabled? + elsif file_upload.class.proxy_download_enabled? || proxy headers.store(*Gitlab::Workhorse.send_url(file_upload.url(**redirect_params))) head :ok else diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 312e256ea6c..ae9c17802b9 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -16,7 +16,7 @@ class Projects::ArtifactsController < Projects::ApplicationController def download return render_404 unless artifacts_file - send_upload(artifacts_file, attachment: artifacts_file.filename) + send_upload(artifacts_file, attachment: artifacts_file.filename, proxy: params[:proxy]) end def browse diff --git a/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb b/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb index a888fab2789..2b607681082 100644 --- a/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb +++ b/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb @@ -16,8 +16,6 @@ module Clusters configure_kubernetes_token kubernetes_namespace.save! - rescue ::Kubeclient::HttpError => err - raise err unless err.error_code = 404 end private diff --git a/app/workers/cluster_platform_configure_worker.rb b/app/workers/cluster_platform_configure_worker.rb index 68e8335a09d..8f3689f0166 100644 --- a/app/workers/cluster_platform_configure_worker.rb +++ b/app/workers/cluster_platform_configure_worker.rb @@ -17,6 +17,6 @@ class ClusterPlatformConfigureWorker end rescue ::Kubeclient::HttpError => err - Rails.logger.error "Failed to create/update Kubernetes Namespace. id: #{kubernetes_namespace.id} message: #{err.message}" + Rails.logger.error "Failed to create/update Kubernetes namespace for cluster_id: #{cluster_id} with error: #{err.message}" end end diff --git a/changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml b/changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml new file mode 100644 index 00000000000..f2a117fe63f --- /dev/null +++ b/changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml @@ -0,0 +1,5 @@ +--- +title: Fix error handling bugs in kubernetes integration +merge_request: 22922 +author: +type: fixed diff --git a/doc/administration/custom_hooks.md b/doc/administration/custom_hooks.md index c58ced7d520..60ad4bf4e8f 100644 --- a/doc/administration/custom_hooks.md +++ b/doc/administration/custom_hooks.md @@ -60,7 +60,7 @@ installations, this can be set in `gitlab-shell/config.yml`. The hooks are searched and executed in this order: -1. `<project>.git/hooks/` - symlink to `gitlab-shell/hooks` global dir +1. `gitlab-shell/hooks` directory as known to Gitaly 1. `<project>.git/hooks/<hook_name>` - executed by `git` itself, this is `gitlab-shell/hooks/<hook_name>` 1. `<project>.git/custom_hooks/<hook_name>` - per project hook (this is already existing behavior) 1. `<project>.git/custom_hooks/<hook_name>.d/*` - per project hooks diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 767fba7fd58..4f1f6bb31f3 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -28,8 +28,9 @@ describe SendFileUpload do describe '#send_upload' do let(:controller) { controller_class.new } let(:temp_file) { Tempfile.new('test') } + let(:params) { {} } - subject { controller.send_upload(uploader) } + subject { controller.send_upload(uploader, **params) } before do FileUtils.touch(temp_file) @@ -52,7 +53,7 @@ describe SendFileUpload do end context 'with attachment' do - let(:send_attachment) { controller.send_upload(uploader, attachment: 'test.js') } + let(:params) { { attachment: 'test.js' } } it 'sends a file with content-type of text/plain' do expected_params = { @@ -62,7 +63,7 @@ describe SendFileUpload do } expect(controller).to receive(:send_file).with(uploader.path, expected_params) - send_attachment + subject end context 'with a proxied file in object storage' do @@ -83,7 +84,7 @@ describe SendFileUpload do expect(controller).to receive(:headers) { headers } expect(controller).to receive(:head).with(:ok) - send_attachment + subject end end end @@ -95,11 +96,7 @@ describe SendFileUpload do uploader.store!(temp_file) end - context 'and proxying is enabled' do - before do - allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { true } - end - + shared_examples 'proxied file' do it 'sends a file' do headers = double expect(Gitlab::Workhorse).not_to receive(:send_url).with(/response-content-disposition/) @@ -115,6 +112,14 @@ describe SendFileUpload do end end + context 'and proxying is enabled' do + before do + allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { true } + end + + it_behaves_like 'proxied file' + end + context 'and proxying is disabled' do before do allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { false } @@ -125,6 +130,12 @@ describe SendFileUpload do subject end + + context 'with proxy requested' do + let(:params) { { proxy: true } } + + it_behaves_like 'proxied file' + end end end end diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index 6091185e252..b3c8d6a954e 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -47,14 +47,37 @@ describe Projects::ArtifactsController do context 'when codequality file type is supplied' do let(:file_type) { 'codequality' } - before do - create(:ci_job_artifact, :codequality, job: job) + context 'when file is stored locally' do + before do + create(:ci_job_artifact, :codequality, job: job) + end + + it 'sends the codequality report' do + expect(controller).to receive(:send_file).with(job.job_artifacts_codequality.file.path, hash_including(disposition: 'attachment')).and_call_original + + download_artifact(file_type: file_type) + end end - it 'sends the codequality report' do - expect(controller).to receive(:send_file).with(job.job_artifacts_codequality.file.path, hash_including(disposition: 'attachment')).and_call_original + context 'when file is stored remotely' do + before do + stub_artifacts_object_storage + create(:ci_job_artifact, :remote_store, :codequality, job: job) + end + + it 'sends the codequality report' do + expect(controller).to receive(:redirect_to).and_call_original - download_artifact(file_type: file_type) + download_artifact(file_type: file_type) + end + + context 'when proxied' do + it 'sends the codequality report' do + expect(Gitlab::Workhorse).to receive(:send_url).and_call_original + + download_artifact(file_type: file_type, proxy: true) + end + end end end end diff --git a/spec/workers/cluster_platform_configure_worker_spec.rb b/spec/workers/cluster_platform_configure_worker_spec.rb index 1a7ad8923f6..b51f6e07c6a 100644 --- a/spec/workers/cluster_platform_configure_worker_spec.rb +++ b/spec/workers/cluster_platform_configure_worker_spec.rb @@ -30,4 +30,18 @@ describe ClusterPlatformConfigureWorker, '#execute' do described_class.new.perform(123) end end + + context 'when kubeclient raises error' do + let(:cluster) { create(:cluster, :project) } + + it 'rescues and logs the error' do + allow_any_instance_of(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService).to receive(:execute).and_raise(::Kubeclient::HttpError.new(500, 'something baaaad happened', '')) + + expect(Rails.logger) + .to receive(:error) + .with("Failed to create/update Kubernetes namespace for cluster_id: #{cluster.id} with error: something baaaad happened") + + described_class.new.perform(cluster.id) + end + end end |