From a3eda49d426e8e599b2f47929c8f9b9203ae33d5 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Tue, 6 Mar 2018 11:01:37 +0000
Subject: Merge branch '43299-revert-bad-merge' into 'master'

Resolve "Group Leave action is broken on Groups Dashboard and Homepage"

Closes #43299

See merge request gitlab-org/gitlab-ce!17183
---
 app/assets/javascripts/groups/components/app.vue | 10 ++--
 spec/javascripts/groups/components/app_spec.js   | 59 +++++++++++++-----------
 2 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/app/assets/javascripts/groups/components/app.vue b/app/assets/javascripts/groups/components/app.vue
index b8f0566f48c..0578f43d5af 100644
--- a/app/assets/javascripts/groups/components/app.vue
+++ b/app/assets/javascripts/groups/components/app.vue
@@ -152,14 +152,14 @@ export default {
     showLeaveGroupModal(group, parentGroup) {
       this.targetGroup = group;
       this.targetParentGroup = parentGroup;
-      this.updateModal = true;
+      this.showModal = true;
       this.groupLeaveConfirmationMessage = s__(`GroupsTree|Are you sure you want to leave the "${group.fullName}" group?`);
     },
     hideLeaveGroupModal() {
-      this.updateModal = false;
+      this.showModal = false;
     },
     leaveGroup() {
-      this.updateModal = false;
+      this.showModal = false;
       this.targetGroup.isBeingRemoved = true;
       this.service.leaveGroup(this.targetGroup.leavePath)
         .then(res => res.json())
@@ -208,9 +208,9 @@ export default {
       :page-info="pageInfo"
     />
     <modal
-      v-show="showModal"
-      :primary-button-label="__('Leave')"
+      v-if="showModal"
       kind="warning"
+      :primary-button-label="__('Leave')"
       :title="__('Are you sure?')"
       :text="groupLeaveConfirmationMessage"
       @cancel="hideLeaveGroupModal"
diff --git a/spec/javascripts/groups/components/app_spec.js b/spec/javascripts/groups/components/app_spec.js
index 3adc29262f3..46c7b9f54f2 100644
--- a/spec/javascripts/groups/components/app_spec.js
+++ b/spec/javascripts/groups/components/app_spec.js
@@ -129,7 +129,7 @@ describe('AppComponent', () => {
 
         vm.fetchGroups({});
         setTimeout(() => {
-          expect(vm.isLoading).toBeFalsy();
+          expect(vm.isLoading).toBe(false);
           expect($.scrollTo).toHaveBeenCalledWith(0);
           expect(window.Flash).toHaveBeenCalledWith('An error occurred. Please try again.');
           done();
@@ -144,10 +144,10 @@ describe('AppComponent', () => {
         spyOn(vm, 'updateGroups').and.callThrough();
 
         vm.fetchAllGroups();
-        expect(vm.isLoading).toBeTruthy();
+        expect(vm.isLoading).toBe(true);
         expect(vm.fetchGroups).toHaveBeenCalled();
         setTimeout(() => {
-          expect(vm.isLoading).toBeFalsy();
+          expect(vm.isLoading).toBe(false);
           expect(vm.updateGroups).toHaveBeenCalled();
           done();
         }, 0);
@@ -181,7 +181,7 @@ describe('AppComponent', () => {
         spyOn($, 'scrollTo');
 
         vm.fetchPage(2, null, null, true);
-        expect(vm.isLoading).toBeTruthy();
+        expect(vm.isLoading).toBe(true);
         expect(vm.fetchGroups).toHaveBeenCalledWith({
           page: 2,
           filterGroupsBy: null,
@@ -190,7 +190,7 @@ describe('AppComponent', () => {
           archived: true,
         });
         setTimeout(() => {
-          expect(vm.isLoading).toBeFalsy();
+          expect(vm.isLoading).toBe(false);
           expect($.scrollTo).toHaveBeenCalledWith(0);
           expect(utils.mergeUrlParams).toHaveBeenCalledWith({ page: 2 }, jasmine.any(String));
           expect(window.history.replaceState).toHaveBeenCalledWith({
@@ -216,7 +216,7 @@ describe('AppComponent', () => {
         spyOn(vm.store, 'setGroupChildren');
 
         vm.toggleChildren(groupItem);
-        expect(groupItem.isChildrenLoading).toBeTruthy();
+        expect(groupItem.isChildrenLoading).toBe(true);
         expect(vm.fetchGroups).toHaveBeenCalledWith({
           parentId: groupItem.id,
         });
@@ -232,7 +232,7 @@ describe('AppComponent', () => {
 
         vm.toggleChildren(groupItem);
         expect(vm.fetchGroups).not.toHaveBeenCalled();
-        expect(groupItem.isOpen).toBeTruthy();
+        expect(groupItem.isOpen).toBe(true);
       });
 
       it('should collapse group if it is already expanded', () => {
@@ -241,16 +241,16 @@ describe('AppComponent', () => {
 
         vm.toggleChildren(groupItem);
         expect(vm.fetchGroups).not.toHaveBeenCalled();
-        expect(groupItem.isOpen).toBeFalsy();
+        expect(groupItem.isOpen).toBe(false);
       });
 
       it('should set `isChildrenLoading` back to `false` if load request fails', (done) => {
         spyOn(vm, 'fetchGroups').and.returnValue(returnServicePromise({}, true));
 
         vm.toggleChildren(groupItem);
-        expect(groupItem.isChildrenLoading).toBeTruthy();
+        expect(groupItem.isChildrenLoading).toBe(true);
         setTimeout(() => {
-          expect(groupItem.isChildrenLoading).toBeFalsy();
+          expect(groupItem.isChildrenLoading).toBe(false);
           done();
         }, 0);
       });
@@ -268,10 +268,10 @@ describe('AppComponent', () => {
 
       it('updates props which show modal confirmation dialog', () => {
         const group = Object.assign({}, mockParentGroupItem);
-        expect(vm.updateModal).toBeFalsy();
+        expect(vm.showModal).toBe(false);
         expect(vm.groupLeaveConfirmationMessage).toBe('');
         vm.showLeaveGroupModal(group, mockParentGroupItem);
-        expect(vm.updateModal).toBeTruthy();
+        expect(vm.showModal).toBe(true);
         expect(vm.groupLeaveConfirmationMessage).toBe(`Are you sure you want to leave the "${group.fullName}" group?`);
       });
     });
@@ -280,9 +280,9 @@ describe('AppComponent', () => {
       it('hides modal confirmation which is shown before leaving the group', () => {
         const group = Object.assign({}, mockParentGroupItem);
         vm.showLeaveGroupModal(group, mockParentGroupItem);
-        expect(vm.updateModal).toBeTruthy();
+        expect(vm.showModal).toBe(true);
         vm.hideLeaveGroupModal();
-        expect(vm.updateModal).toBeFalsy();
+        expect(vm.showModal).toBe(false);
       });
     });
 
@@ -307,8 +307,8 @@ describe('AppComponent', () => {
         spyOn($, 'scrollTo');
 
         vm.leaveGroup();
-        expect(vm.updateModal).toBeFalsy();
-        expect(vm.targetGroup.isBeingRemoved).toBeTruthy();
+        expect(vm.showModal).toBe(false);
+        expect(vm.targetGroup.isBeingRemoved).toBe(true);
         expect(vm.service.leaveGroup).toHaveBeenCalledWith(vm.targetGroup.leavePath);
         setTimeout(() => {
           expect($.scrollTo).toHaveBeenCalledWith(0);
@@ -325,12 +325,12 @@ describe('AppComponent', () => {
         spyOn(window, 'Flash');
 
         vm.leaveGroup();
-        expect(vm.targetGroup.isBeingRemoved).toBeTruthy();
+        expect(vm.targetGroup.isBeingRemoved).toBe(true);
         expect(vm.service.leaveGroup).toHaveBeenCalledWith(childGroupItem.leavePath);
         setTimeout(() => {
           expect(vm.store.removeGroup).not.toHaveBeenCalled();
           expect(window.Flash).toHaveBeenCalledWith(message);
-          expect(vm.targetGroup.isBeingRemoved).toBeFalsy();
+          expect(vm.targetGroup.isBeingRemoved).toBe(false);
           done();
         }, 0);
       });
@@ -342,12 +342,12 @@ describe('AppComponent', () => {
         spyOn(window, 'Flash');
 
         vm.leaveGroup(childGroupItem, groupItem);
-        expect(vm.targetGroup.isBeingRemoved).toBeTruthy();
+        expect(vm.targetGroup.isBeingRemoved).toBe(true);
         expect(vm.service.leaveGroup).toHaveBeenCalledWith(childGroupItem.leavePath);
         setTimeout(() => {
           expect(vm.store.removeGroup).not.toHaveBeenCalled();
           expect(window.Flash).toHaveBeenCalledWith(message);
-          expect(vm.targetGroup.isBeingRemoved).toBeFalsy();
+          expect(vm.targetGroup.isBeingRemoved).toBe(false);
           done();
         }, 0);
       });
@@ -379,10 +379,10 @@ describe('AppComponent', () => {
 
       it('should set `isSearchEmpty` prop based on groups count', () => {
         vm.updateGroups(mockGroups);
-        expect(vm.isSearchEmpty).toBeFalsy();
+        expect(vm.isSearchEmpty).toBe(false);
 
         vm.updateGroups([]);
-        expect(vm.isSearchEmpty).toBeTruthy();
+        expect(vm.isSearchEmpty).toBe(true);
       });
     });
   });
@@ -473,13 +473,16 @@ describe('AppComponent', () => {
       });
     });
 
-    it('renders modal confirmation dialog', () => {
+    it('renders modal confirmation dialog', (done) => {
       vm.groupLeaveConfirmationMessage = 'Are you sure you want to leave the "foo" group?';
-      vm.updateModal = true;
-      const modalDialogEl = vm.$el.querySelector('.modal');
-      expect(modalDialogEl).not.toBe(null);
-      expect(modalDialogEl.querySelector('.modal-title').innerText.trim()).toBe('Are you sure?');
-      expect(modalDialogEl.querySelector('.btn.btn-warning').innerText.trim()).toBe('Leave');
+      vm.showModal = true;
+      Vue.nextTick(() => {
+        const modalDialogEl = vm.$el.querySelector('.modal');
+        expect(modalDialogEl).not.toBe(null);
+        expect(modalDialogEl.querySelector('.modal-title').innerText.trim()).toBe('Are you sure?');
+        expect(modalDialogEl.querySelector('.btn.btn-warning').innerText.trim()).toBe('Leave');
+        done();
+      });
     });
   });
 });
-- 
cgit v1.2.1


From c092acdba54236be71727f877fd95a7fa35372a2 Mon Sep 17 00:00:00 2001
From: Robert Speicher <robert@gitlab.com>
Date: Thu, 1 Mar 2018 15:23:27 +0000
Subject: Merge branch 'zj-gitaly-encoding-issue' into 'master'

Encode revision for gitattributes ref

Closes gitaly#1032 and #43278

See merge request gitlab-org/gitlab-ce!17291
---
 changelogs/unreleased/zj-gitaly-encoding-issue.yml |   5 +
 lib/gitlab/gitaly_client/repository_service.rb     |   2 +-
 spec/lib/gitlab/git/repository_spec.rb             | 118 ++++++++++++---------
 3 files changed, 73 insertions(+), 52 deletions(-)
 create mode 100644 changelogs/unreleased/zj-gitaly-encoding-issue.yml

diff --git a/changelogs/unreleased/zj-gitaly-encoding-issue.yml b/changelogs/unreleased/zj-gitaly-encoding-issue.yml
new file mode 100644
index 00000000000..073d8f38e4b
--- /dev/null
+++ b/changelogs/unreleased/zj-gitaly-encoding-issue.yml
@@ -0,0 +1,5 @@
+---
+title: Encode branch name as binary before creating a RPC request to copy attributes
+merge_request: 17291
+author:
+type: fixed
diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb
index 60706b4f0d8..35c6cfe5817 100644
--- a/lib/gitlab/gitaly_client/repository_service.rb
+++ b/lib/gitlab/gitaly_client/repository_service.rb
@@ -41,7 +41,7 @@ module Gitlab
       end
 
       def apply_gitattributes(revision)
-        request = Gitaly::ApplyGitattributesRequest.new(repository: @gitaly_repo, revision: revision)
+        request = Gitaly::ApplyGitattributesRequest.new(repository: @gitaly_repo, revision: encode_binary(revision))
         GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request)
       end
 
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 9feaaec3f3b..420672ccf12 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1406,79 +1406,95 @@ describe Gitlab::Git::Repository, seed_helper: true do
   end
 
   describe "#copy_gitattributes" do
-    let(:attributes_path) { File.join(SEED_STORAGE_PATH, TEST_REPO_PATH, 'info/attributes') }
+    shared_examples 'applying git attributes' do
+      let(:attributes_path) { File.join(SEED_STORAGE_PATH, TEST_REPO_PATH, 'info/attributes') }
 
-    it "raises an error with invalid ref" do
-      expect { repository.copy_gitattributes("invalid") }.to raise_error(Gitlab::Git::Repository::InvalidRef)
-    end
-
-    context "with no .gitattrbutes" do
-      before do
-        repository.copy_gitattributes("master")
+      after do
+        FileUtils.rm_rf(attributes_path) if Dir.exist?(attributes_path)
       end
 
-      it "does not have an info/attributes" do
-        expect(File.exist?(attributes_path)).to be_falsey
+      it "raises an error with invalid ref" do
+        expect { repository.copy_gitattributes("invalid") }.to raise_error(Gitlab::Git::Repository::InvalidRef)
       end
 
-      after do
-        FileUtils.rm_rf(attributes_path)
-      end
-    end
+      context 'when forcing encoding issues' do
+        let(:branch_name) { "ʕ•ᴥ•ʔ" }
 
-    context "with .gitattrbutes" do
-      before do
-        repository.copy_gitattributes("gitattributes")
-      end
+        before do
+          repository.create_branch(branch_name, "master")
+        end
 
-      it "has an info/attributes" do
-        expect(File.exist?(attributes_path)).to be_truthy
-      end
+        after do
+          repository.rm_branch(branch_name, user: build(:admin))
+        end
 
-      it "has the same content in info/attributes as .gitattributes" do
-        contents = File.open(attributes_path, "rb") { |f| f.read }
-        expect(contents).to eq("*.md binary\n")
-      end
+        it "doesn't raise with a valid unicode ref" do
+          expect { repository.copy_gitattributes(branch_name) }.not_to raise_error
 
-      after do
-        FileUtils.rm_rf(attributes_path)
+          repository
+        end
       end
-    end
 
-    context "with updated .gitattrbutes" do
-      before do
-        repository.copy_gitattributes("gitattributes")
-        repository.copy_gitattributes("gitattributes-updated")
-      end
+      context "with no .gitattrbutes" do
+        before do
+          repository.copy_gitattributes("master")
+        end
 
-      it "has an info/attributes" do
-        expect(File.exist?(attributes_path)).to be_truthy
+        it "does not have an info/attributes" do
+          expect(File.exist?(attributes_path)).to be_falsey
+        end
       end
 
-      it "has the updated content in info/attributes" do
-        contents = File.read(attributes_path)
-        expect(contents).to eq("*.txt binary\n")
-      end
+      context "with .gitattrbutes" do
+        before do
+          repository.copy_gitattributes("gitattributes")
+        end
 
-      after do
-        FileUtils.rm_rf(attributes_path)
-      end
-    end
+        it "has an info/attributes" do
+          expect(File.exist?(attributes_path)).to be_truthy
+        end
 
-    context "with no .gitattrbutes in HEAD but with previous info/attributes" do
-      before do
-        repository.copy_gitattributes("gitattributes")
-        repository.copy_gitattributes("master")
+        it "has the same content in info/attributes as .gitattributes" do
+          contents = File.open(attributes_path, "rb") { |f| f.read }
+          expect(contents).to eq("*.md binary\n")
+        end
       end
 
-      it "does not have an info/attributes" do
-        expect(File.exist?(attributes_path)).to be_falsey
+      context "with updated .gitattrbutes" do
+        before do
+          repository.copy_gitattributes("gitattributes")
+          repository.copy_gitattributes("gitattributes-updated")
+        end
+
+        it "has an info/attributes" do
+          expect(File.exist?(attributes_path)).to be_truthy
+        end
+
+        it "has the updated content in info/attributes" do
+          contents = File.read(attributes_path)
+          expect(contents).to eq("*.txt binary\n")
+        end
       end
 
-      after do
-        FileUtils.rm_rf(attributes_path)
+      context "with no .gitattrbutes in HEAD but with previous info/attributes" do
+        before do
+          repository.copy_gitattributes("gitattributes")
+          repository.copy_gitattributes("master")
+        end
+
+        it "does not have an info/attributes" do
+          expect(File.exist?(attributes_path)).to be_falsey
+        end
       end
     end
+
+    context 'when gitaly is enabled' do
+      it_behaves_like 'applying git attributes'
+    end
+
+    context 'when gitaly is disabled', :disable_gitaly do
+      it_behaves_like 'applying git attributes'
+    end
   end
 
   describe '#ref_exists?' do
-- 
cgit v1.2.1


From f750e0b3583cbdf587c16a51ef1f4e99fb5378e1 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Mon, 26 Feb 2018 09:54:20 +0000
Subject: Merge branch 'grpc-unavailable-restart' into 'master'

Restart Unicorn and Sidekiq when GRPC throws 14:Endpoint read failed

Closes gitaly#1036

See merge request gitlab-org/gitlab-ce!17293
---
 changelogs/unreleased/grpc-unavailable-restart.yml |   5 +
 config/initializers/sidekiq.rb                     |   2 +-
 lib/gitlab/gitaly_client.rb                        |  23 ++++
 lib/gitlab/sidekiq_middleware/memory_killer.rb     |  67 -----------
 lib/gitlab/sidekiq_middleware/shutdown.rb          | 133 +++++++++++++++++++++
 .../sidekiq_middleware/memory_killer_spec.rb       |  63 ----------
 .../lib/gitlab/sidekiq_middleware/shutdown_spec.rb |  88 ++++++++++++++
 7 files changed, 250 insertions(+), 131 deletions(-)
 create mode 100644 changelogs/unreleased/grpc-unavailable-restart.yml
 delete mode 100644 lib/gitlab/sidekiq_middleware/memory_killer.rb
 create mode 100644 lib/gitlab/sidekiq_middleware/shutdown.rb
 delete mode 100644 spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb
 create mode 100644 spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb

diff --git a/changelogs/unreleased/grpc-unavailable-restart.yml b/changelogs/unreleased/grpc-unavailable-restart.yml
new file mode 100644
index 00000000000..5ce08d66004
--- /dev/null
+++ b/changelogs/unreleased/grpc-unavailable-restart.yml
@@ -0,0 +1,5 @@
+---
+title: Restart Unicorn and Sidekiq when GRPC throws 14:Endpoint read failed
+merge_request: 17293
+author:
+type: fixed
diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb
index 0f164e628f9..161fb185c9b 100644
--- a/config/initializers/sidekiq.rb
+++ b/config/initializers/sidekiq.rb
@@ -10,7 +10,7 @@ Sidekiq.configure_server do |config|
 
   config.server_middleware do |chain|
     chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS']
-    chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS']
+    chain.add Gitlab::SidekiqMiddleware::Shutdown
     chain.add Gitlab::SidekiqMiddleware::RequestStoreMiddleware unless ENV['SIDEKIQ_REQUEST_STORE'] == '0'
     chain.add Gitlab::SidekiqStatus::ServerMiddleware
   end
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index c5d3e944f7d..9cd76630484 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -125,6 +125,8 @@ module Gitlab
       kwargs = yield(kwargs) if block_given?
 
       stub(service, storage).__send__(rpc, request, kwargs) # rubocop:disable GitlabSecurity/PublicSend
+    rescue GRPC::Unavailable => ex
+      handle_grpc_unavailable!(ex)
     ensure
       duration = Gitlab::Metrics::System.monotonic_time - start
 
@@ -135,6 +137,27 @@ module Gitlab
         duration)
     end
 
+    def self.handle_grpc_unavailable!(ex)
+      status = ex.to_status
+      raise ex unless status.details == 'Endpoint read failed'
+
+      # There is a bug in grpc 1.8.x that causes a client process to get stuck
+      # always raising '14:Endpoint read failed'. The only thing that we can
+      # do to recover is to restart the process.
+      #
+      # See https://gitlab.com/gitlab-org/gitaly/issues/1029
+
+      if Sidekiq.server?
+        raise Gitlab::SidekiqMiddleware::Shutdown::WantShutdown.new(ex.to_s)
+      else
+        # SIGQUIT requests a Unicorn worker to shut down gracefully after the current request.
+        Process.kill('QUIT', Process.pid)
+      end
+
+      raise ex
+    end
+    private_class_method :handle_grpc_unavailable!
+
     def self.current_transaction_labels
       Gitlab::Metrics::Transaction.current&.labels || {}
     end
diff --git a/lib/gitlab/sidekiq_middleware/memory_killer.rb b/lib/gitlab/sidekiq_middleware/memory_killer.rb
deleted file mode 100644
index b89ae2505c9..00000000000
--- a/lib/gitlab/sidekiq_middleware/memory_killer.rb
+++ /dev/null
@@ -1,67 +0,0 @@
-module Gitlab
-  module SidekiqMiddleware
-    class MemoryKiller
-      # Default the RSS limit to 0, meaning the MemoryKiller is disabled
-      MAX_RSS = (ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] || 0).to_s.to_i
-      # Give Sidekiq 15 minutes of grace time after exceeding the RSS limit
-      GRACE_TIME = (ENV['SIDEKIQ_MEMORY_KILLER_GRACE_TIME'] || 15 * 60).to_s.to_i
-      # Wait 30 seconds for running jobs to finish during graceful shutdown
-      SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i
-
-      # Create a mutex used to ensure there will be only one thread waiting to
-      # shut Sidekiq down
-      MUTEX = Mutex.new
-
-      def call(worker, job, queue)
-        yield
-
-        current_rss = get_rss
-
-        return unless MAX_RSS > 0 && current_rss > MAX_RSS
-
-        Thread.new do
-          # Return if another thread is already waiting to shut Sidekiq down
-          return unless MUTEX.try_lock
-
-          Sidekiq.logger.warn "Sidekiq worker PID-#{pid} current RSS #{current_rss}"\
-            " exceeds maximum RSS #{MAX_RSS} after finishing job #{worker.class} JID-#{job['jid']}"
-          Sidekiq.logger.warn "Sidekiq worker PID-#{pid} will stop fetching new jobs in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later"
-
-          # Wait `GRACE_TIME` to give the memory intensive job time to finish.
-          # Then, tell Sidekiq to stop fetching new jobs.
-          wait_and_signal(GRACE_TIME, 'SIGSTP', 'stop fetching new jobs')
-
-          # Wait `SHUTDOWN_WAIT` to give already fetched jobs time to finish.
-          # Then, tell Sidekiq to gracefully shut down by giving jobs a few more
-          # moments to finish, killing and requeuing them if they didn't, and
-          # then terminating itself.
-          wait_and_signal(SHUTDOWN_WAIT, 'SIGTERM', 'gracefully shut down')
-
-          # Wait for Sidekiq to shutdown gracefully, and kill it if it didn't.
-          wait_and_signal(Sidekiq.options[:timeout] + 2, 'SIGKILL', 'die')
-        end
-      end
-
-      private
-
-      def get_rss
-        output, status = Gitlab::Popen.popen(%W(ps -o rss= -p #{pid}), Rails.root.to_s)
-        return 0 unless status.zero?
-
-        output.to_i
-      end
-
-      def wait_and_signal(time, signal, explanation)
-        Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})"
-        sleep(time)
-
-        Sidekiq.logger.warn "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})"
-        Process.kill(signal, pid)
-      end
-
-      def pid
-        Process.pid
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/sidekiq_middleware/shutdown.rb b/lib/gitlab/sidekiq_middleware/shutdown.rb
new file mode 100644
index 00000000000..c2b8d6de66e
--- /dev/null
+++ b/lib/gitlab/sidekiq_middleware/shutdown.rb
@@ -0,0 +1,133 @@
+require 'mutex_m'
+
+module Gitlab
+  module SidekiqMiddleware
+    class Shutdown
+      extend Mutex_m
+
+      # Default the RSS limit to 0, meaning the MemoryKiller is disabled
+      MAX_RSS = (ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'] || 0).to_s.to_i
+      # Give Sidekiq 15 minutes of grace time after exceeding the RSS limit
+      GRACE_TIME = (ENV['SIDEKIQ_MEMORY_KILLER_GRACE_TIME'] || 15 * 60).to_s.to_i
+      # Wait 30 seconds for running jobs to finish during graceful shutdown
+      SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i
+
+      # This exception can be used to request that the middleware start shutting down Sidekiq
+      WantShutdown = Class.new(StandardError)
+
+      ShutdownWithoutRaise = Class.new(WantShutdown)
+      private_constant :ShutdownWithoutRaise
+
+      # For testing only, to avoid race conditions (?) in Rspec mocks.
+      attr_reader :trace
+
+      # We store the shutdown thread in a class variable to ensure that there
+      # can be only one shutdown thread in the process.
+      def self.create_shutdown_thread
+        mu_synchronize do
+          return unless @shutdown_thread.nil?
+
+          @shutdown_thread = Thread.new { yield }
+        end
+      end
+
+      # For testing only: so we can wait for the shutdown thread to finish.
+      def self.shutdown_thread
+        mu_synchronize { @shutdown_thread }
+      end
+
+      # For testing only: so that we can reset the global state before each test.
+      def self.clear_shutdown_thread
+        mu_synchronize { @shutdown_thread = nil }
+      end
+
+      def initialize
+        @trace = Queue.new if Rails.env.test?
+      end
+
+      def call(worker, job, queue)
+        shutdown_exception = nil
+
+        begin
+          yield
+          check_rss!
+        rescue WantShutdown => ex
+          shutdown_exception = ex
+        end
+
+        return unless shutdown_exception
+
+        self.class.create_shutdown_thread do
+          do_shutdown(worker, job, shutdown_exception)
+        end
+
+        raise shutdown_exception unless shutdown_exception.is_a?(ShutdownWithoutRaise)
+      end
+
+      private
+
+      def do_shutdown(worker, job, shutdown_exception)
+        Sidekiq.logger.warn "Sidekiq worker PID-#{pid} shutting down because of #{shutdown_exception} after job "\
+          "#{worker.class} JID-#{job['jid']}"
+        Sidekiq.logger.warn "Sidekiq worker PID-#{pid} will stop fetching new jobs in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later"
+
+        # Wait `GRACE_TIME` to give the memory intensive job time to finish.
+        # Then, tell Sidekiq to stop fetching new jobs.
+        wait_and_signal(GRACE_TIME, 'SIGTSTP', 'stop fetching new jobs')
+
+        # Wait `SHUTDOWN_WAIT` to give already fetched jobs time to finish.
+        # Then, tell Sidekiq to gracefully shut down by giving jobs a few more
+        # moments to finish, killing and requeuing them if they didn't, and
+        # then terminating itself.
+        wait_and_signal(SHUTDOWN_WAIT, 'SIGTERM', 'gracefully shut down')
+
+        # Wait for Sidekiq to shutdown gracefully, and kill it if it didn't.
+        wait_and_signal(Sidekiq.options[:timeout] + 2, 'SIGKILL', 'die')
+      end
+
+      def check_rss!
+        return unless MAX_RSS > 0
+
+        current_rss = get_rss
+        return unless current_rss > MAX_RSS
+
+        raise ShutdownWithoutRaise.new("current RSS #{current_rss} exceeds maximum RSS #{MAX_RSS}")
+      end
+
+      def get_rss
+        output, status = Gitlab::Popen.popen(%W(ps -o rss= -p #{pid}), Rails.root.to_s)
+        return 0 unless status.zero?
+
+        output.to_i
+      end
+
+      def wait_and_signal(time, signal, explanation)
+        Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})"
+        sleep(time)
+
+        Sidekiq.logger.warn "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})"
+        kill(signal, pid)
+      end
+
+      def pid
+        Process.pid
+      end
+
+      def sleep(time)
+        if Rails.env.test?
+          @trace << [:sleep, time]
+        else
+          Kernel.sleep(time)
+        end
+      end
+
+      def kill(signal, pid)
+        if Rails.env.test?
+          @trace << [:kill, signal, pid]
+        else
+          Process.kill(signal, pid)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb
deleted file mode 100644
index 8fdbbacd04d..00000000000
--- a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb
+++ /dev/null
@@ -1,63 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::SidekiqMiddleware::MemoryKiller do
-  subject { described_class.new }
-  let(:pid) { 999 }
-
-  let(:worker) { double(:worker, class: 'TestWorker') }
-  let(:job) { { 'jid' => 123 } }
-  let(:queue) { 'test_queue' }
-
-  def run
-    thread = subject.call(worker, job, queue) { nil }
-    thread&.join
-  end
-
-  before do
-    allow(subject).to receive(:get_rss).and_return(10.kilobytes)
-    allow(subject).to receive(:pid).and_return(pid)
-  end
-
-  context 'when MAX_RSS is set to 0' do
-    before do
-      stub_const("#{described_class}::MAX_RSS", 0)
-    end
-
-    it 'does nothing' do
-      expect(subject).not_to receive(:sleep)
-
-      run
-    end
-  end
-
-  context 'when MAX_RSS is exceeded' do
-    before do
-      stub_const("#{described_class}::MAX_RSS", 5.kilobytes)
-    end
-
-    it 'sends the STP, TERM and KILL signals at expected times' do
-      expect(subject).to receive(:sleep).with(15 * 60).ordered
-      expect(Process).to receive(:kill).with('SIGSTP', pid).ordered
-
-      expect(subject).to receive(:sleep).with(30).ordered
-      expect(Process).to receive(:kill).with('SIGTERM', pid).ordered
-
-      expect(subject).to receive(:sleep).with(10).ordered
-      expect(Process).to receive(:kill).with('SIGKILL', pid).ordered
-
-      run
-    end
-  end
-
-  context 'when MAX_RSS is not exceeded' do
-    before do
-      stub_const("#{described_class}::MAX_RSS", 15.kilobytes)
-    end
-
-    it 'does nothing' do
-      expect(subject).not_to receive(:sleep)
-
-      run
-    end
-  end
-end
diff --git a/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb b/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb
new file mode 100644
index 00000000000..0001795c3f0
--- /dev/null
+++ b/spec/lib/gitlab/sidekiq_middleware/shutdown_spec.rb
@@ -0,0 +1,88 @@
+require 'spec_helper'
+
+describe Gitlab::SidekiqMiddleware::Shutdown do
+  subject { described_class.new }
+
+  let(:pid) { Process.pid }
+  let(:worker) { double(:worker, class: 'TestWorker') }
+  let(:job) { { 'jid' => 123 } }
+  let(:queue) { 'test_queue' }
+  let(:block) { proc { nil } }
+
+  def run
+    subject.call(worker, job, queue) { block.call }
+    described_class.shutdown_thread&.join
+  end
+
+  def pop_trace
+    subject.trace.pop(true)
+  end
+
+  before do
+    allow(subject).to receive(:get_rss).and_return(10.kilobytes)
+    described_class.clear_shutdown_thread
+  end
+
+  context 'when MAX_RSS is set to 0' do
+    before do
+      stub_const("#{described_class}::MAX_RSS", 0)
+    end
+
+    it 'does nothing' do
+      expect(subject).not_to receive(:sleep)
+
+      run
+    end
+  end
+
+  def expect_shutdown_sequence
+    expect(pop_trace).to eq([:sleep, 15 * 60])
+    expect(pop_trace).to eq([:kill, 'SIGTSTP', pid])
+
+    expect(pop_trace).to eq([:sleep, 30])
+    expect(pop_trace).to eq([:kill, 'SIGTERM', pid])
+
+    expect(pop_trace).to eq([:sleep, 10])
+    expect(pop_trace).to eq([:kill, 'SIGKILL', pid])
+  end
+
+  context 'when MAX_RSS is exceeded' do
+    before do
+      stub_const("#{described_class}::MAX_RSS", 5.kilobytes)
+    end
+
+    it 'sends the TSTP, TERM and KILL signals at expected times' do
+      run
+
+      expect_shutdown_sequence
+    end
+  end
+
+  context 'when MAX_RSS is not exceeded' do
+    before do
+      stub_const("#{described_class}::MAX_RSS", 15.kilobytes)
+    end
+
+    it 'does nothing' do
+      expect(subject).not_to receive(:sleep)
+
+      run
+    end
+  end
+
+  context 'when WantShutdown is raised' do
+    let(:block) { proc { raise described_class::WantShutdown } }
+
+    it 'starts the shutdown sequence and re-raises the exception' do
+      expect { run }.to raise_exception(described_class::WantShutdown)
+
+      # We can't expect 'run' to have joined on the shutdown thread, because
+      # it hit an exception.
+      shutdown_thread = described_class.shutdown_thread
+      expect(shutdown_thread).not_to be_nil
+      shutdown_thread.join
+
+      expect_shutdown_sequence
+    end
+  end
+end
-- 
cgit v1.2.1


From 29c3f5e7e685f18156efd00aa88e5d204694ceae Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz@gitlab.com>
Date: Tue, 27 Feb 2018 11:23:50 +0000
Subject: Merge branch
 '43510-merge-requests-and-issues-don-t-show-for-all-subgroups' into 'master'

Resolve "Merge requests and issues don't show for all subgroups"

Closes #43510

See merge request gitlab-org/gitlab-ce!17312
---
 app/controllers/groups/application_controller.rb   |   4 -
 app/controllers/groups_controller.rb               |   1 -
 app/helpers/groups_helper.rb                       |  18 ++-
 app/views/groups/issues.html.haml                  |   7 +-
 app/views/groups/merge_requests.html.haml          |   2 +-
 app/views/layouts/nav/sidebar/_group.html.haml     |   5 +-
 ...sts-and-issues-don-t-show-for-all-subgroups.yml |   6 +
 spec/features/groups/empty_states_spec.rb          | 124 ++++++++++++++-------
 8 files changed, 108 insertions(+), 59 deletions(-)
 create mode 100644 changelogs/unreleased/43510-merge-requests-and-issues-don-t-show-for-all-subgroups.yml

diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb
index 96ce686c989..43abed7a934 100644
--- a/app/controllers/groups/application_controller.rb
+++ b/app/controllers/groups/application_controller.rb
@@ -16,10 +16,6 @@ class Groups::ApplicationController < ApplicationController
     @projects ||= GroupProjectsFinder.new(group: group, current_user: current_user).execute
   end
 
-  def group_merge_requests
-    @group_merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id).execute
-  end
-
   def authorize_admin_group!
     unless can?(current_user, :admin_group, group)
       return render_404
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 7d129c5dece..4c8afebb028 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -14,7 +14,6 @@ class GroupsController < Groups::ApplicationController
   before_action :authorize_create_group!, only: [:new]
 
   before_action :group_projects, only: [:projects, :activity, :issues, :merge_requests]
-  before_action :group_merge_requests, only: [:merge_requests]
   before_action :event_filter, only: [:activity]
 
   before_action :user_actions, only: [:show, :subgroups]
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index 23de3590b93..c4558c6150b 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -11,6 +11,20 @@ module GroupsHelper
     can?(current_user, :change_share_with_group_lock, group)
   end
 
+  def group_issues_count(state:)
+    IssuesFinder
+      .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true)
+      .execute
+      .count
+  end
+
+  def group_merge_requests_count(state:)
+    MergeRequestsFinder
+      .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true)
+      .execute
+      .count
+  end
+
   def group_icon(group, options = {})
     img_path = group_icon_url(group, options)
     image_tag img_path, options
@@ -69,10 +83,6 @@ module GroupsHelper
     end
   end
 
-  def group_issues(group)
-    IssuesFinder.new(current_user, group_id: group.id).execute
-  end
-
   def remove_group_message(group)
     _("You are going to remove %{group_name}. Removed groups CANNOT be restored! Are you ABSOLUTELY sure?") %
       { group_name: group.name }
diff --git a/app/views/groups/issues.html.haml b/app/views/groups/issues.html.haml
index 00909982d59..70c830915d0 100644
--- a/app/views/groups/issues.html.haml
+++ b/app/views/groups/issues.html.haml
@@ -1,5 +1,4 @@
 - page_title "Issues"
-- group_issues_exists = group_issues(@group).exists?
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@group.name} issues")
 
@@ -7,7 +6,9 @@
   = webpack_bundle_tag 'common_vue'
   = webpack_bundle_tag 'filtered_search'
 
-- if group_issues_exists
+- if group_issues_count(state: 'all').zero?
+  = render 'shared/empty_states/issues', project_select_button: true
+- else
   .top-area
     = render 'shared/issuable/nav', type: :issues
     .nav-controls
@@ -20,5 +21,3 @@
   = render 'shared/issuable/search_bar', type: :issues
 
   = render 'shared/issues'
-- else
-  = render 'shared/empty_states/issues', project_select_button: true
diff --git a/app/views/groups/merge_requests.html.haml b/app/views/groups/merge_requests.html.haml
index 694292aa7c1..4326c60bd80 100644
--- a/app/views/groups/merge_requests.html.haml
+++ b/app/views/groups/merge_requests.html.haml
@@ -4,7 +4,7 @@
   = webpack_bundle_tag 'common_vue'
   = webpack_bundle_tag 'filtered_search'
 
-- if @group_merge_requests.empty?
+- if group_merge_requests_count(state: 'all').zero?
   = render 'shared/empty_states/merge_requests', project_select_button: true
 - else
   .top-area
diff --git a/app/views/layouts/nav/sidebar/_group.html.haml b/app/views/layouts/nav/sidebar/_group.html.haml
index 09a43a2cac5..56e6d108fd6 100644
--- a/app/views/layouts/nav/sidebar/_group.html.haml
+++ b/app/views/layouts/nav/sidebar/_group.html.haml
@@ -1,5 +1,6 @@
-- issues_count = IssuesFinder.new(current_user, group_id: @group.id, state: 'opened').execute.count
-- merge_requests_count = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened', non_archived: true).execute.count
+- issues_count = group_issues_count(state: 'opened')
+- merge_requests_count = group_merge_requests_count(state: 'opened')
+- issues_sub_menu_items = ['groups#issues', 'labels#index', 'milestones#index']
 
 .nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) }
   .nav-sidebar-inner-scroll
diff --git a/changelogs/unreleased/43510-merge-requests-and-issues-don-t-show-for-all-subgroups.yml b/changelogs/unreleased/43510-merge-requests-and-issues-don-t-show-for-all-subgroups.yml
new file mode 100644
index 00000000000..e163c04f430
--- /dev/null
+++ b/changelogs/unreleased/43510-merge-requests-and-issues-don-t-show-for-all-subgroups.yml
@@ -0,0 +1,6 @@
+---
+title: Ensure group issues and merge requests pages show results from subgroups when
+  there are no results from the current group
+merge_request: 17312
+author:
+type: fixed
diff --git a/spec/features/groups/empty_states_spec.rb b/spec/features/groups/empty_states_spec.rb
index 243e8536168..04217fec06c 100644
--- a/spec/features/groups/empty_states_spec.rb
+++ b/spec/features/groups/empty_states_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-feature 'Groups Merge Requests Empty States' do
+feature 'Group empty states' do
   let(:group) { create(:group) }
   let(:user) { create(:group_member, :developer, user: create(:user), group: group ).user }
 
@@ -8,62 +8,100 @@ feature 'Groups Merge Requests Empty States' do
     sign_in(user)
   end
 
-  context 'group has a project' do
-    let(:project) { create(:project, namespace: group) }
+  [:issue, :merge_request].each do |issuable|
+    issuable_name = issuable.to_s.humanize.downcase
+    project_relation = issuable == :issue ? :project : :source_project
 
-    before do
-      project.add_master(user)
-    end
+    context "for #{issuable_name}s" do
+      let(:path) { public_send(:"#{issuable}s_group_path", group) }
 
-    context 'the project has a merge request' do
-      before do
-        create(:merge_request, source_project: project)
+      context 'group has a project' do
+        let(:project) { create(:project, namespace: group) }
 
-        visit merge_requests_group_path(group)
-      end
+        before do
+          project.add_master(user)
+        end
 
-      it 'should not display an empty state' do
-        expect(page).not_to have_selector('.empty-state')
-      end
-    end
+        context "the project has #{issuable_name}s" do
+          before do
+            create(issuable, project_relation => project)
 
-    context 'the project has no merge requests', :js do
-      before do
-        visit merge_requests_group_path(group)
-      end
+            visit path
+          end
 
-      it 'should display an empty state' do
-        expect(page).to have_selector('.empty-state')
-      end
+          it 'does not display an empty state' do
+            expect(page).not_to have_selector('.empty-state')
+          end
+        end
+
+        context "the project has no #{issuable_name}s", :js do
+          before do
+            visit path
+          end
+
+          it 'displays an empty state' do
+            expect(page).to have_selector('.empty-state')
+          end
+
+          it "shows a new #{issuable_name} button" do
+            within '.empty-state' do
+              expect(page).to have_content("create #{issuable_name}")
+            end
+          end
+
+          it "the new #{issuable_name} button opens a project dropdown" do
+            within '.empty-state' do
+              find('.new-project-item-select-button').click
+            end
 
-      it 'should show a new merge request button' do
-        within '.empty-state' do
-          expect(page).to have_content('create merge request')
+            expect(page).to have_selector('.ajax-project-dropdown')
+          end
         end
       end
 
-      it 'the new merge request button opens a project dropdown' do
-        within '.empty-state' do
-          find('.new-project-item-select-button').click
-        end
+      context 'group without a project' do
+        context 'group has a subgroup', :nested_groups do
+          let(:subgroup) { create(:group, parent: group) }
+          let(:subgroup_project) { create(:project, namespace: subgroup) }
 
-        expect(page).to have_selector('.ajax-project-dropdown')
-      end
-    end
-  end
+          context "the project has #{issuable_name}s" do
+            before do
+              create(issuable, project_relation => subgroup_project)
 
-  context 'group without a project' do
-    before do
-      visit merge_requests_group_path(group)
-    end
+              visit path
+            end
 
-    it 'should display an empty state' do
-      expect(page).to have_selector('.empty-state')
-    end
+            it 'does not display an empty state' do
+              expect(page).not_to have_selector('.empty-state')
+            end
+          end
 
-    it 'should not show a new merge request button' do
-      within '.empty-state' do
-        expect(page).not_to have_link('create merge request')
+          context "the project has no #{issuable_name}s" do
+            before do
+              visit path
+            end
+
+            it 'displays an empty state' do
+              expect(page).to have_selector('.empty-state')
+            end
+          end
+        end
+
+        context 'group has no subgroups' do
+          before do
+            visit path
+          end
+
+          it 'displays an empty state' do
+            expect(page).to have_selector('.empty-state')
+          end
+
+          it "shows a new #{issuable_name} button" do
+            within '.empty-state' do
+              expect(page).not_to have_link("create #{issuable_name}")
+            end
+          end
+        end
       end
     end
   end
-- 
cgit v1.2.1


From 09f48199b3c721d54605a55d2ccdf85fc6ebad2f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz@gitlab.com>
Date: Sat, 24 Feb 2018 11:07:19 +0000
Subject: Merge branch 'minimal-fix-for-artifacts-service' into 'master'

Minimal fix for artifacts service

Closes #43022

See merge request gitlab-org/gitlab-ce!17313
---
 app/services/ci/create_trace_artifact_service.rb   | 30 +++++++++++---
 .../minimal-fix-for-artifacts-service.yml          |  5 +++
 .../ci/create_trace_artifact_service_spec.rb       | 46 ++++++++++++++++------
 3 files changed, 63 insertions(+), 18 deletions(-)
 create mode 100644 changelogs/unreleased/minimal-fix-for-artifacts-service.yml

diff --git a/app/services/ci/create_trace_artifact_service.rb b/app/services/ci/create_trace_artifact_service.rb
index 280a2c3afa4..ffde824972c 100644
--- a/app/services/ci/create_trace_artifact_service.rb
+++ b/app/services/ci/create_trace_artifact_service.rb
@@ -4,13 +4,33 @@ module Ci
       return if job.job_artifacts_trace
 
       job.trace.read do |stream|
-        if stream.file?
-          job.create_job_artifacts_trace!(
-            project: job.project,
-            file_type: :trace,
-            file: stream)
+        break unless stream.file?
+
+        clone_file!(stream.path, JobArtifactUploader.workhorse_upload_path) do |clone_path|
+          create_job_trace!(job, clone_path)
+          FileUtils.rm(stream.path)
         end
       end
     end
+
+    private
+
+    def create_job_trace!(job, path)
+      File.open(path) do |stream|
+        job.create_job_artifacts_trace!(
+          project: job.project,
+          file_type: :trace,
+          file: stream)
+      end
+    end
+
+    def clone_file!(src_path, temp_dir)
+      FileUtils.mkdir_p(temp_dir)
+      Dir.mktmpdir('tmp-trace', temp_dir) do |dir_path|
+        temp_path = File.join(dir_path, "job.log")
+        FileUtils.copy(src_path, temp_path)
+        yield(temp_path)
+      end
+    end
   end
 end
diff --git a/changelogs/unreleased/minimal-fix-for-artifacts-service.yml b/changelogs/unreleased/minimal-fix-for-artifacts-service.yml
new file mode 100644
index 00000000000..11f5bc17759
--- /dev/null
+++ b/changelogs/unreleased/minimal-fix-for-artifacts-service.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent trace artifact migration to incur data loss
+merge_request: 17313
+author:
+type: fixed
diff --git a/spec/services/ci/create_trace_artifact_service_spec.rb b/spec/services/ci/create_trace_artifact_service_spec.rb
index 847a88920fe..8c5e8e438c7 100644
--- a/spec/services/ci/create_trace_artifact_service_spec.rb
+++ b/spec/services/ci/create_trace_artifact_service_spec.rb
@@ -4,40 +4,60 @@ describe Ci::CreateTraceArtifactService do
   describe '#execute' do
     subject { described_class.new(nil, nil).execute(job) }
 
-    let(:job) { create(:ci_build) }
-
     context 'when the job does not have trace artifact' do
       context 'when the job has a trace file' do
-        before do
-          allow_any_instance_of(Gitlab::Ci::Trace)
-            .to receive(:default_path) { expand_fixture_path('trace/sample_trace') }
+        let!(:job) { create(:ci_build, :trace_live) }
+        let!(:legacy_path) { job.trace.read { |stream| return stream.path } }
+        let!(:legacy_checksum) { Digest::SHA256.file(legacy_path).hexdigest }
+        let(:new_path) { job.job_artifacts_trace.file.path }
+        let(:new_checksum) { Digest::SHA256.file(new_path).hexdigest }
 
-          allow_any_instance_of(JobArtifactUploader).to receive(:move_to_cache) { false }
-          allow_any_instance_of(JobArtifactUploader).to receive(:move_to_store) { false }
-        end
+        it { expect(File.exist?(legacy_path)).to be_truthy }
 
         it 'creates trace artifact' do
           expect { subject }.to change { Ci::JobArtifact.count }.by(1)
 
-          expect(job.job_artifacts_trace.read_attribute(:file)).to eq('sample_trace')
+          expect(File.exist?(legacy_path)).to be_falsy
+          expect(File.exist?(new_path)).to be_truthy
+          expect(new_checksum).to eq(legacy_checksum)
+          expect(job.job_artifacts_trace.file.exists?).to be_truthy
+          expect(job.job_artifacts_trace.file.filename).to eq('job.log')
         end
 
-        context 'when the job has already had trace artifact' do
+        context 'when failed to create trace artifact record' do
           before do
-            create(:ci_job_artifact, :trace, job: job)
+            # When ActiveRecord error happens
+            allow_any_instance_of(Ci::JobArtifact).to receive(:save).and_return(false)
+            allow_any_instance_of(Ci::JobArtifact).to receive_message_chain(:errors, :full_messages)
+              .and_return("Error")
+
+            subject rescue nil
+
+            job.reload
           end
 
-          it 'does not create trace artifact' do
-            expect { subject }.not_to change { Ci::JobArtifact.count }
+          it 'keeps legacy trace and removes trace artifact' do
+            expect(File.exist?(legacy_path)).to be_truthy
+            expect(job.job_artifacts_trace).to be_nil
           end
         end
       end
 
       context 'when the job does not have a trace file' do
+        let!(:job) { create(:ci_build) }
+
         it 'does not create trace artifact' do
           expect { subject }.not_to change { Ci::JobArtifact.count }
         end
       end
     end
+
+    context 'when the job has already had trace artifact' do
+      let!(:job) { create(:ci_build, :trace_artifact) }
+
+      it 'does not create trace artifact' do
+        expect { subject }.not_to change { Ci::JobArtifact.count }
+      end
+    end
   end
 end
-- 
cgit v1.2.1


From 76cec37071a92d5a77665703aa42cc7810c5c0c5 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Mon, 26 Feb 2018 09:57:11 +0000
Subject: Merge branch 'mk/fix-error-code-for-repo-does-not-exist' into
 'master'

Respond with more appropriate HTTP status code when repo does not exist

See merge request gitlab-org/gitlab-ce!17341
---
 .../mk-fix-error-code-for-repo-does-not-exist.yml           |  5 +++++
 lib/gitlab/git_access.rb                                    |  2 +-
 spec/lib/gitlab/git_access_spec.rb                          | 13 +++++++++++++
 spec/lib/gitlab/git_access_wiki_spec.rb                     |  2 +-
 spec/requests/git_http_spec.rb                              |  6 +++---
 5 files changed, 23 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml

diff --git a/changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml b/changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml
new file mode 100644
index 00000000000..a761d610da1
--- /dev/null
+++ b/changelogs/unreleased/mk-fix-error-code-for-repo-does-not-exist.yml
@@ -0,0 +1,5 @@
+---
+title: Return a 404 instead of 403 if the repository does not exist on disk
+merge_request: 17341
+author:
+type: fixed
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index bbdb593d4e2..6400089a22f 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -199,7 +199,7 @@ module Gitlab
 
     def check_repository_existence!
       unless repository.exists?
-        raise UnauthorizedError, ERROR_MESSAGES[:no_repo]
+        raise NotFoundError, ERROR_MESSAGES[:no_repo]
       end
     end
 
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 19d3f55501e..6f07e423c1b 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -534,6 +534,19 @@ describe Gitlab::GitAccess do
       expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.')
     end
 
+    context 'when the project repository does not exist' do
+      it 'returns not found' do
+        project.add_guest(user)
+        repo = project.repository
+        FileUtils.rm_rf(repo.path)
+
+        # Sanity check for rm_rf
+        expect(repo.exists?).to eq(false)
+
+        expect { pull_access_check }.to raise_error(Gitlab::GitAccess::NotFoundError, 'A repository for this project does not exist yet.')
+      end
+    end
+
     describe 'without access to project' do
       context 'pull code' do
         it { expect { pull_access_check }.to raise_not_found }
diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb
index 215f1ecc9c5..730ede99fc9 100644
--- a/spec/lib/gitlab/git_access_wiki_spec.rb
+++ b/spec/lib/gitlab/git_access_wiki_spec.rb
@@ -57,7 +57,7 @@ describe Gitlab::GitAccessWiki do
           # Sanity check for rm_rf
           expect(wiki_repo.exists?).to eq(false)
 
-          expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'A repository for this project does not exist yet.')
+          expect { subject }.to raise_error(Gitlab::GitAccess::NotFoundError, 'A repository for this project does not exist yet.')
         end
       end
     end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 3deca0ac076..381d5198115 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -597,7 +597,7 @@ describe 'Git HTTP requests' do
         context "when a gitlab ci token is provided" do
           let(:project) { create(:project, :repository) }
           let(:build) { create(:ci_build, :running) }
-          let(:other_project) { create(:project) }
+          let(:other_project) { create(:project, :repository) }
 
           before do
             build.update!(project: project) # can't associate it on factory create
@@ -648,10 +648,10 @@ describe 'Git HTTP requests' do
               context 'when the repo does not exist' do
                 let(:project) { create(:project) }
 
-                it 'rejects pulls with 403 Forbidden' do
+                it 'rejects pulls with 404 Not Found' do
                   clone_get path, env
 
-                  expect(response).to have_gitlab_http_status(:forbidden)
+                  expect(response).to have_gitlab_http_status(:not_found)
                   expect(response.body).to eq(git_access_error(:no_repo))
                 end
               end
-- 
cgit v1.2.1


From 9240f84b59d21166840b55abac40311e31232f30 Mon Sep 17 00:00:00 2001
From: Filipa Lacerda <filipa@gitlab.com>
Date: Mon, 26 Feb 2018 16:22:02 +0000
Subject: Merge branch 'issue-edit-shortcut' into 'master'

Fixed issue edit shortcut not working

Closes #43560

See merge request gitlab-org/gitlab-ce!17360
---
 app/assets/javascripts/shortcuts_issuable.js  |  7 +++----
 changelogs/unreleased/issue-edit-shortcut.yml |  5 +++++
 spec/features/issues/form_spec.rb             | 12 ++++++++++++
 3 files changed, 20 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/unreleased/issue-edit-shortcut.yml

diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js
index 689befc742e..14545824e74 100644
--- a/app/assets/javascripts/shortcuts_issuable.js
+++ b/app/assets/javascripts/shortcuts_issuable.js
@@ -9,13 +9,12 @@ export default class ShortcutsIssuable extends Shortcuts {
     super();
 
     this.$replyField = isMergeRequest ? $('.js-main-target-form #note_note') : $('.js-main-target-form .js-vue-comment-form');
-    this.editBtn = document.querySelector('.js-issuable-edit');
 
     Mousetrap.bind('a', () => ShortcutsIssuable.openSidebarDropdown('assignee'));
     Mousetrap.bind('m', () => ShortcutsIssuable.openSidebarDropdown('milestone'));
     Mousetrap.bind('l', () => ShortcutsIssuable.openSidebarDropdown('labels'));
     Mousetrap.bind('r', this.replyWithSelectedText.bind(this));
-    Mousetrap.bind('e', this.editIssue.bind(this));
+    Mousetrap.bind('e', ShortcutsIssuable.editIssue);
 
     if (isMergeRequest) {
       this.enabledHelp.push('.hidden-shortcut.merge_requests');
@@ -58,10 +57,10 @@ export default class ShortcutsIssuable extends Shortcuts {
     return false;
   }
 
-  editIssue() {
+  static editIssue() {
     // Need to click the element as on issues, editing is inline
     // on merge request, editing is on a different page
-    this.editBtn.click();
+    document.querySelector('.js-issuable-edit').click();
 
     return false;
   }
diff --git a/changelogs/unreleased/issue-edit-shortcut.yml b/changelogs/unreleased/issue-edit-shortcut.yml
new file mode 100644
index 00000000000..2b29b2bc03f
--- /dev/null
+++ b/changelogs/unreleased/issue-edit-shortcut.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed issue edit shortcut not opening edit form
+merge_request:
+author:
+type: fixed
diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb
index faf14be4818..c2c4b479a8a 100644
--- a/spec/features/issues/form_spec.rb
+++ b/spec/features/issues/form_spec.rb
@@ -271,6 +271,18 @@ describe 'New/edit issue', :js do
     end
   end
 
+  context 'inline edit' do
+    before do
+      visit project_issue_path(project, issue)
+    end
+
+    it 'opens inline edit form with shortcut' do
+      find('body').send_keys('e')
+
+      expect(page).to have_selector('.detail-page-description form')
+    end
+  end
+
   describe 'sub-group project' do
     let(:group) { create(:group) }
     let(:nested_group_1) { create(:group, parent: group) }
-- 
cgit v1.2.1


From efefd65c809f92ae5880f4c9c5e2666137f65958 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Tue, 27 Feb 2018 09:49:59 +0000
Subject: Merge branch '43261-fix-prometheus-installation' into 'master'

Disable RBAC on Prometheus values.yml

Closes #43621

See merge request gitlab-org/gitlab-ce!17372
---
 changelogs/unreleased/43261-fix-prometheus-installation.yml | 5 +++++
 vendor/prometheus/values.yaml                               | 3 +++
 2 files changed, 8 insertions(+)
 create mode 100644 changelogs/unreleased/43261-fix-prometheus-installation.yml

diff --git a/changelogs/unreleased/43261-fix-prometheus-installation.yml b/changelogs/unreleased/43261-fix-prometheus-installation.yml
new file mode 100644
index 00000000000..b5fc7980390
--- /dev/null
+++ b/changelogs/unreleased/43261-fix-prometheus-installation.yml
@@ -0,0 +1,5 @@
+---
+title: Allow Prometheus application to be installed from Cluster applications
+merge_request: 17372
+author:
+type: fixed
diff --git a/vendor/prometheus/values.yaml b/vendor/prometheus/values.yaml
index db967514be7..859f2ad82a4 100644
--- a/vendor/prometheus/values.yaml
+++ b/vendor/prometheus/values.yaml
@@ -10,6 +10,9 @@ nodeExporter:
 pushgateway:
   enabled: false
 
+rbac:
+  create: false
+
 server:
   image:
     tag: v2.1.0
-- 
cgit v1.2.1


From d77353b8b12345c1d10e427f2fc2e5a3ba03eef3 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Mon, 26 Feb 2018 22:07:17 +0000
Subject: Merge branch '43532-error-on-admin-applications-prometheus-template'
 into 'master'

Resolve "Error 500 on route "/admin/application_settings/services/1882/edit" -> edit Prometheus Service Template"

Closes #43532

See merge request gitlab-org/gitlab-ce!17377
---
 .../prometheus/_configuration_banner.html.haml     | 26 ++++++++++++++++++++
 .../projects/services/prometheus/_help.html.haml   | 28 ++--------------------
 ...r-on-admin-applications-prometheus-template.yml |  5 ++++
 .../services/admin_activates_prometheus_spec.rb    | 21 ++++++++++++++++
 .../services/user_activates_prometheus_spec.rb     | 23 ++++++++++++++++++
 5 files changed, 77 insertions(+), 26 deletions(-)
 create mode 100644 app/views/projects/services/prometheus/_configuration_banner.html.haml
 create mode 100644 changelogs/unreleased/43532-error-on-admin-applications-prometheus-template.yml
 create mode 100644 spec/features/admin/services/admin_activates_prometheus_spec.rb
 create mode 100644 spec/features/projects/services/user_activates_prometheus_spec.rb

diff --git a/app/views/projects/services/prometheus/_configuration_banner.html.haml b/app/views/projects/services/prometheus/_configuration_banner.html.haml
new file mode 100644
index 00000000000..2cc2a6b2b5b
--- /dev/null
+++ b/app/views/projects/services/prometheus/_configuration_banner.html.haml
@@ -0,0 +1,26 @@
+%h4
+  = s_('PrometheusService|Auto configuration')
+
+- if service.manual_configuration?
+  .well
+    = s_('PrometheusService|To enable the installation of Prometheus on your clusters, deactivate the manual configuration below')
+- else
+  .container-fluid
+    .row
+      - if service.prometheus_installed?
+        .col-sm-2
+          .svg-container
+            = image_tag 'illustrations/monitoring/getting_started.svg'
+        .col-sm-10
+          %p.text-success.prepend-top-default
+            = s_('PrometheusService|Prometheus is being automatically managed on your clusters')
+          = link_to s_('PrometheusService|Manage clusters'), project_clusters_path(project), class: 'btn'
+      - else
+        .col-sm-2
+          = image_tag 'illustrations/monitoring/loading.svg'
+        .col-sm-10
+          %p.prepend-top-default
+            = s_('PrometheusService|Automatically deploy and configure Prometheus on your clusters to monitor your project’s environments')
+          = link_to s_('PrometheusService|Install Prometheus on clusters'), project_clusters_path(project), class: 'btn btn-success'
+
+%hr
diff --git a/app/views/projects/services/prometheus/_help.html.haml b/app/views/projects/services/prometheus/_help.html.haml
index 5e320a252d8..88acb824ba7 100644
--- a/app/views/projects/services/prometheus/_help.html.haml
+++ b/app/views/projects/services/prometheus/_help.html.haml
@@ -1,29 +1,5 @@
-%h4
-  = s_('PrometheusService|Auto configuration')
-
-- if @service.manual_configuration?
-  .well
-    = s_('PrometheusService|To enable the installation of Prometheus on your clusters, deactivate the manual configuration below')
-- else
-  .container-fluid
-    .row
-      - if @service.prometheus_installed?
-        .col-sm-2
-          .svg-container
-            = image_tag 'illustrations/monitoring/getting_started.svg'
-        .col-sm-10
-          %p.text-success.prepend-top-default
-            = s_('PrometheusService|Prometheus is being automatically managed on your clusters')
-          = link_to s_('PrometheusService|Manage clusters'), project_clusters_path(@project), class: 'btn'
-      - else
-        .col-sm-2
-          = image_tag 'illustrations/monitoring/loading.svg'
-        .col-sm-10
-          %p.prepend-top-default
-            = s_('PrometheusService|Automatically deploy and configure Prometheus on your clusters to monitor your project’s environments')
-          = link_to s_('PrometheusService|Install Prometheus on clusters'), project_clusters_path(@project), class: 'btn btn-success'
-
-%hr
+- if @project
+  = render 'projects/services/prometheus/configuration_banner', project: @project, service: @service
 
 %h4.append-bottom-default
   = s_('PrometheusService|Manual configuration')
diff --git a/changelogs/unreleased/43532-error-on-admin-applications-prometheus-template.yml b/changelogs/unreleased/43532-error-on-admin-applications-prometheus-template.yml
new file mode 100644
index 00000000000..25bcbf2fbab
--- /dev/null
+++ b/changelogs/unreleased/43532-error-on-admin-applications-prometheus-template.yml
@@ -0,0 +1,5 @@
+---
+title: Fixes Prometheus admin configuration page
+merge_request: 17377
+author:
+type: fixed
diff --git a/spec/features/admin/services/admin_activates_prometheus_spec.rb b/spec/features/admin/services/admin_activates_prometheus_spec.rb
new file mode 100644
index 00000000000..904fe5b406b
--- /dev/null
+++ b/spec/features/admin/services/admin_activates_prometheus_spec.rb
@@ -0,0 +1,21 @@
+require 'spec_helper'
+
+describe 'Admin activates Prometheus' do
+  let(:admin) { create(:user, :admin) }
+
+  before do
+    sign_in(admin)
+
+    visit(admin_application_settings_services_path)
+
+    click_link('Prometheus')
+  end
+
+  it 'activates service' do
+    check('Active')
+    fill_in('API URL', with: 'http://prometheus.example.com')
+    click_button('Save')
+
+    expect(page).to have_content('Application settings saved successfully')
+  end
+end
diff --git a/spec/features/projects/services/user_activates_prometheus_spec.rb b/spec/features/projects/services/user_activates_prometheus_spec.rb
new file mode 100644
index 00000000000..33f884eb148
--- /dev/null
+++ b/spec/features/projects/services/user_activates_prometheus_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe 'User activates Prometheus' do
+  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+
+  before do
+    project.add_master(user)
+    sign_in(user)
+
+    visit(project_settings_integrations_path(project))
+
+    click_link('Prometheus')
+  end
+
+  it 'activates service' do
+    check('Active')
+    fill_in('API URL', with: 'http://prometheus.example.com')
+    click_button('Save changes')
+
+    expect(page).to have_content('Prometheus activated.')
+  end
+end
-- 
cgit v1.2.1


From b67bac5699a8b48f2406ef7d480af5b40db1294f Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Wed, 28 Feb 2018 13:07:55 +0000
Subject: Merge branch
 '43531-500-error-searching-wiki-incompatible-character-encodings-utf-8-and-ascii-8bit'
 into 'master'

Resolve "500 Error searching wiki: incompatible character encodings: UTF-8 and ASCII-8BIT"

Closes #43531

See merge request gitlab-org/gitlab-ce!17413
---
 ...le-character-encodings-utf-8-and-ascii-8bit.yml |  5 ++++
 lib/gitlab/search_results.rb                       |  4 +++-
 spec/lib/gitlab/project_search_results_spec.rb     | 27 ++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/43531-500-error-searching-wiki-incompatible-character-encodings-utf-8-and-ascii-8bit.yml

diff --git a/changelogs/unreleased/43531-500-error-searching-wiki-incompatible-character-encodings-utf-8-and-ascii-8bit.yml b/changelogs/unreleased/43531-500-error-searching-wiki-incompatible-character-encodings-utf-8-and-ascii-8bit.yml
new file mode 100644
index 00000000000..173710412a5
--- /dev/null
+++ b/changelogs/unreleased/43531-500-error-searching-wiki-incompatible-character-encodings-utf-8-and-ascii-8bit.yml
@@ -0,0 +1,5 @@
+---
+title: Fix code and wiki search results pages when non-ASCII text is displayed
+merge_request: 17413
+author:
+type: fixed
diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb
index 5a5ae7f19d4..781783f4d97 100644
--- a/lib/gitlab/search_results.rb
+++ b/lib/gitlab/search_results.rb
@@ -1,6 +1,8 @@
 module Gitlab
   class SearchResults
     class FoundBlob
+      include EncodingHelper
+
       attr_reader :id, :filename, :basename, :ref, :startline, :data, :project_id
 
       def initialize(opts = {})
@@ -9,7 +11,7 @@ module Gitlab
         @basename = opts.fetch(:basename, nil)
         @ref = opts.fetch(:ref, nil)
         @startline = opts.fetch(:startline, nil)
-        @data = opts.fetch(:data, nil)
+        @data = encode_utf8(opts.fetch(:data, nil))
         @per_page = opts.fetch(:per_page, 20)
         @project_id = opts.fetch(:project_id, nil)
       end
diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb
index 1ebb0105cf5..d8250e4b4c6 100644
--- a/spec/lib/gitlab/project_search_results_spec.rb
+++ b/spec/lib/gitlab/project_search_results_spec.rb
@@ -1,3 +1,4 @@
+# coding: utf-8
 require 'spec_helper'
 
 describe Gitlab::ProjectSearchResults do
@@ -105,6 +106,32 @@ describe Gitlab::ProjectSearchResults do
         end
       end
 
+      context 'when the search returns non-ASCII data' do
+        context 'with UTF-8' do
+          let(:results) { project.repository.search_files_by_content("файл", 'master') }
+
+          it 'returns results as UTF-8' do
+            expect(subject.filename).to eq('encoding/russian.rb')
+            expect(subject.basename).to eq('encoding/russian')
+            expect(subject.ref).to eq('master')
+            expect(subject.startline).to eq(1)
+            expect(subject.data).to eq("Хороший файл")
+          end
+        end
+
+        context 'with ISO-8859-1' do
+          let(:search_result) { "master:encoding/iso8859.txt\x001\x00\xC4\xFC\nmaster:encoding/iso8859.txt\x002\x00\nmaster:encoding/iso8859.txt\x003\x00foo\n".force_encoding(Encoding::ASCII_8BIT) }
+
+          it 'returns results as UTF-8' do
+            expect(subject.filename).to eq('encoding/iso8859.txt')
+            expect(subject.basename).to eq('encoding/iso8859')
+            expect(subject.ref).to eq('master')
+            expect(subject.startline).to eq(1)
+            expect(subject.data).to eq("Äü\n\nfoo")
+          end
+        end
+      end
+
       context "when filename has extension" do
         let(:search_result) { "master:CONTRIBUTE.md\x005\x00- [Contribute to GitLab](#contribute-to-gitlab)\n" }
 
-- 
cgit v1.2.1


From d00802fa1ed1729e8ef0a485e62a5ef9ac84e990 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Wed, 28 Feb 2018 15:45:42 +0000
Subject: Merge branch 'an/lograge-fix' into 'master'

Fix for open-ended parameter's in lograge causing elastic memory issues

See merge request gitlab-org/gitlab-ce!17419
---
 config/initializers/lograge.rb | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb
index 8560d24526f..114c1cb512f 100644
--- a/config/initializers/lograge.rb
+++ b/config/initializers/lograge.rb
@@ -12,9 +12,14 @@ unless Sidekiq.server?
     config.lograge.logger = ActiveSupport::Logger.new(filename)
     # Add request parameters to log output
     config.lograge.custom_options = lambda do |event|
+      params = event.payload[:params]
+        .except(*%w(controller action format))
+        .each_pair
+        .map { |k, v| { key: k, value: v } }
+
       payload = {
         time: event.time.utc.iso8601(3),
-        params: event.payload[:params].except(*%w(controller action format)),
+        params: params,
         remote_ip: event.payload[:remote_ip],
         user_id: event.payload[:user_id],
         username: event.payload[:username]
-- 
cgit v1.2.1


From 6b94f83df2fd467a1141e1be841998ed01f1c2ad Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@mcgivern.me.uk>
Date: Mon, 5 Mar 2018 11:05:54 +0000
Subject: Merge branch 'remove-projects-finder-from-todos-finder' into 'master'

Don't use ProjectsFinder in TodosFinder

Closes #43767

See merge request gitlab-org/gitlab-ce!17462
---
 app/finders/todos_finder.rb                               | 15 ++++++---------
 .../remove-projects-finder-from-todos-finder.yml          |  5 +++++
 2 files changed, 11 insertions(+), 9 deletions(-)
 create mode 100644 changelogs/unreleased/remove-projects-finder-from-todos-finder.yml

diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index 3502bf08971..f07fb1c064d 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -105,10 +105,6 @@ class TodosFinder
     ids
   end
 
-  def projects(items)
-    ProjectsFinder.new(current_user: current_user, project_ids_relation: project_ids(items)).execute
-  end
-
   def type?
     type.present? && %w(Issue MergeRequest).include?(type)
   end
@@ -147,13 +143,14 @@ class TodosFinder
 
   def by_project(items)
     if project?
-      items = items.where(project: project)
+      items.where(project: project)
     else
-      item_projects = projects(items)
-      items = items.merge(item_projects).joins(:project)
-    end
+      projects = Project
+        .public_or_visible_to_user(current_user)
+        .order_id_desc
 
-    items
+      items.joins(:project).merge(projects)
+    end
   end
 
   def by_state(items)
diff --git a/changelogs/unreleased/remove-projects-finder-from-todos-finder.yml b/changelogs/unreleased/remove-projects-finder-from-todos-finder.yml
new file mode 100644
index 00000000000..0a3fc751edb
--- /dev/null
+++ b/changelogs/unreleased/remove-projects-finder-from-todos-finder.yml
@@ -0,0 +1,5 @@
+---
+title: Don't use ProjectsFinder in TodosFinder
+merge_request:
+author:
+type: performance
-- 
cgit v1.2.1


From 5cdc15b9121b42d6b28c9083e340a3e1f74870c3 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Fri, 2 Mar 2018 20:41:00 +0000
Subject: Merge branch '42877-fix-visibility-change-performance' into 'master'

Revert Project.public_or_visible_to_user changes but apply change to SnippetsFinder

Closes #42877

See merge request gitlab-org/gitlab-ce!17476
---
 app/finders/snippets_finder.rb                     | 31 ++++++++++++++-
 app/models/project.rb                              | 46 ++++------------------
 app/models/user.rb                                 |  9 +++++
 .../revert-project-visibility-changes.yml          |  5 +++
 spec/models/user_spec.rb                           | 26 ++++++++++++
 5 files changed, 77 insertions(+), 40 deletions(-)
 create mode 100644 changelogs/unreleased/revert-project-visibility-changes.yml

diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb
index ec61fe1892e..8deef84555e 100644
--- a/app/finders/snippets_finder.rb
+++ b/app/finders/snippets_finder.rb
@@ -55,8 +55,37 @@ class SnippetsFinder < UnionFinder
     Snippet.where(feature_available_projects.or(not_project_related)).public_or_visible_to_user(current_user)
   end
 
+  # Returns a collection of projects that is either public or visible to the
+  # logged in user.
+  #
+  # A caller must pass in a block to modify individual parts of
+  # the query, e.g. to apply .with_feature_available_for_user on top of it.
+  # This is useful for performance as we can stick those additional filters
+  # at the bottom of e.g. the UNION.
+  def projects_for_user
+    return yield(Project.public_to_user) unless current_user
+
+    # If the current_user is allowed to see all projects,
+    # we can shortcut and just return.
+    return yield(Project.all) if current_user.full_private_access?
+
+    authorized_projects = yield(Project.where('EXISTS (?)', current_user.authorizations_for_projects))
+
+    levels = Gitlab::VisibilityLevel.levels_for_user(current_user)
+    visible_projects = yield(Project.where(visibility_level: levels))
+
+    # We use a UNION here instead of OR clauses since this results in better
+    # performance.
+    union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')])
+
+    Project.from("(#{union.to_sql}) AS #{Project.table_name}")
+  end
+
   def feature_available_projects
-    projects = Project.public_or_visible_to_user(current_user, use_where_in: false) do |part|
+    # Don't return any project related snippets if the user cannot read cross project
+    return table[:id].eq(nil) unless Ability.allowed?(current_user, :read_cross_project)
+
+    projects = projects_for_user do |part|
       part.with_feature_available_for_user(:snippets, current_user)
     end.select(:id)
 
diff --git a/app/models/project.rb b/app/models/project.rb
index 79058d51af8..194e9a2593b 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -316,42 +316,13 @@ class Project < ActiveRecord::Base
 
   # Returns a collection of projects that is either public or visible to the
   # logged in user.
-  #
-  # A caller may pass in a block to modify individual parts of
-  # the query, e.g. to apply .with_feature_available_for_user on top of it.
-  # This is useful for performance as we can stick those additional filters
-  # at the bottom of e.g. the UNION.
-  #
-  # Optionally, turning `use_where_in` off leads to returning a
-  # relation using #from instead of #where. This can perform much better
-  # but leads to trouble when used in conjunction with AR's #merge method.
-  def self.public_or_visible_to_user(user = nil, use_where_in: true, &block)
-    # If we don't get a block passed, use identity to avoid if/else repetitions
-    block = ->(part) { part } unless block_given?
-
-    return block.call(public_to_user) unless user
-
-    # If the user is allowed to see all projects,
-    # we can shortcut and just return.
-    return block.call(all) if user.full_private_access?
-
-    authorized = user
-      .project_authorizations
-      .select(1)
-      .where('project_authorizations.project_id = projects.id')
-    authorized_projects = block.call(where('EXISTS (?)', authorized))
-
-    levels = Gitlab::VisibilityLevel.levels_for_user(user)
-    visible_projects = block.call(where(visibility_level: levels))
-
-    # We use a UNION here instead of OR clauses since this results in better
-    # performance.
-    union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')])
-
-    if use_where_in
-      where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
+  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
-      from("(#{union.to_sql}) AS #{table_name}")
+      public_to_user
     end
   end
 
@@ -370,14 +341,11 @@ class Project < ActiveRecord::Base
     elsif user
       column = ProjectFeature.quoted_access_level_column(feature)
 
-      authorized = user.project_authorizations.select(1)
-        .where('project_authorizations.project_id = projects.id')
-
       with_project_feature
         .where("#{column} IN (?) OR (#{column} = ? AND EXISTS (?))",
               visible,
               ProjectFeature::PRIVATE,
-              authorized)
+              user.authorizations_for_projects)
     else
       with_feature_access_level(feature, visible)
     end
diff --git a/app/models/user.rb b/app/models/user.rb
index e81e555c090..9093ba2a04f 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -596,6 +596,15 @@ class User < ActiveRecord::Base
     authorized_projects(min_access_level).exists?({ id: project.id })
   end
 
+  # Typically used in conjunction with projects table to get projects
+  # a user has been given access to.
+  #
+  # Example use:
+  # `Project.where('EXISTS(?)', user.authorizations_for_projects)`
+  def authorizations_for_projects
+    project_authorizations.select(1).where('project_authorizations.project_id = projects.id')
+  end
+
   # Returns the projects this user has reporter (or greater) access to, limited
   # to at most the given projects.
   #
diff --git a/changelogs/unreleased/revert-project-visibility-changes.yml b/changelogs/unreleased/revert-project-visibility-changes.yml
new file mode 100644
index 00000000000..df44fdb79b1
--- /dev/null
+++ b/changelogs/unreleased/revert-project-visibility-changes.yml
@@ -0,0 +1,5 @@
+---
+title: Revert Project.public_or_visible_to_user changes and only apply to snippets
+merge_request:
+author:
+type: fixed
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index dd1bfba1421..970a5ed056e 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1604,6 +1604,32 @@ describe User do
     it { is_expected.to eq([private_group]) }
   end
 
+  describe '#authorizations_for_projects' do
+    let!(:user) { create(:user) }
+    subject { Project.where("EXISTS (?)", user.authorizations_for_projects) }
+
+    it 'includes projects that belong to a user, but no other projects' do
+      owned = create(:project, :private, namespace: user.namespace)
+      member = create(:project, :private).tap { |p| p.add_master(user) }
+      other = create(:project)
+
+      expect(subject).to include(owned)
+      expect(subject).to include(member)
+      expect(subject).not_to include(other)
+    end
+
+    it 'includes projects a user has access to, but no other projects' do
+      other_user = create(:user)
+      accessible = create(:project, :private, namespace: other_user.namespace) do |project|
+        project.add_developer(user)
+      end
+      other = create(:project)
+
+      expect(subject).to include(accessible)
+      expect(subject).not_to include(other)
+    end
+  end
+
   describe '#authorized_projects', :delete do
     context 'with a minimum access level' do
       it 'includes projects for which the user is an owner' do
-- 
cgit v1.2.1


From 6846dad292bd7693859012a5b27158d1a995ddae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Tue, 6 Mar 2018 12:11:00 +0000
Subject: Merge branch 'fix/sm/fix_pages_worker' into 'master'

Reload stable object to prevent erase artifacts with old state (Ver 2)

Closes #43641

See merge request gitlab-org/gitlab-ce!17522
---
 app/services/projects/update_pages_service.rb      | 35 +++++++++------
 app/workers/pages_worker.rb                        |  2 +-
 changelogs/unreleased/fix-sm-fix_pages_worker.yml  |  5 +++
 spec/features/projects/pages_spec.rb               | 50 ++++++++++++++++++++++
 .../services/projects/update_pages_service_spec.rb | 16 +++++++
 5 files changed, 94 insertions(+), 14 deletions(-)
 create mode 100644 changelogs/unreleased/fix-sm-fix_pages_worker.yml

diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb
index c760bd3b626..00fdd047208 100644
--- a/app/services/projects/update_pages_service.rb
+++ b/app/services/projects/update_pages_service.rb
@@ -1,5 +1,8 @@
 module Projects
   class UpdatePagesService < BaseService
+    InvaildStateError = Class.new(StandardError)
+    FailedToExtractError = Class.new(StandardError)
+
     BLOCK_SIZE = 32.kilobytes
     MAX_SIZE = 1.terabyte
     SITE_PATH = 'public/'.freeze
@@ -11,13 +14,15 @@ module Projects
     end
 
     def execute
+      register_attempt
+
       # Create status notifying the deployment of pages
       @status = create_status
       @status.enqueue!
       @status.run!
 
-      raise 'missing pages artifacts' unless build.artifacts?
-      raise 'pages are outdated' unless latest?
+      raise InvaildStateError, 'missing pages artifacts' unless build.artifacts?
+      raise InvaildStateError, 'pages are outdated' unless latest?
 
       # Create temporary directory in which we will extract the artifacts
       FileUtils.mkdir_p(tmp_path)
@@ -26,24 +31,22 @@ module Projects
 
         # Check if we did extract public directory
         archive_public_path = File.join(archive_path, 'public')
-        raise 'pages miss the public folder' unless Dir.exist?(archive_public_path)
-        raise 'pages are outdated' unless latest?
+        raise FailedToExtractError, 'pages miss the public folder' unless Dir.exist?(archive_public_path)
+        raise InvaildStateError, 'pages are outdated' unless latest?
 
         deploy_page!(archive_public_path)
         success
       end
-    rescue => e
+    rescue InvaildStateError, FailedToExtractError => e
       register_failure
       error(e.message)
-    ensure
-      register_attempt
-      build.erase_artifacts! unless build.has_expiring_artifacts?
     end
 
     private
 
     def success
       @status.success
+      delete_artifact!
       super
     end
 
@@ -52,6 +55,7 @@ module Projects
       @status.allow_failure = !latest?
       @status.description = message
       @status.drop(:script_failure)
+      delete_artifact!
       super
     end
 
@@ -72,7 +76,7 @@ module Projects
       elsif artifacts.ends_with?('.zip')
         extract_zip_archive!(temp_path)
       else
-        raise 'unsupported artifacts format'
+        raise FailedToExtractError, 'unsupported artifacts format'
       end
     end
 
@@ -81,17 +85,17 @@ module Projects
                                %W(dd bs=#{BLOCK_SIZE} count=#{blocks}),
                                %W(tar -x -C #{temp_path} #{SITE_PATH}),
                                err: '/dev/null')
-      raise 'pages failed to extract' unless results.compact.all?(&:success?)
+      raise FailedToExtractError, 'pages failed to extract' unless results.compact.all?(&:success?)
     end
 
     def extract_zip_archive!(temp_path)
-      raise 'missing artifacts metadata' unless build.artifacts_metadata?
+      raise FailedToExtractError, 'missing artifacts metadata' unless build.artifacts_metadata?
 
       # Calculate page size after extract
       public_entry = build.artifacts_metadata_entry(SITE_PATH, recursive: true)
 
       if public_entry.total_size > max_size
-        raise "artifacts for pages are too large: #{public_entry.total_size}"
+        raise FailedToExtractError, "artifacts for pages are too large: #{public_entry.total_size}"
       end
 
       # Requires UnZip at least 6.00 Info-ZIP.
@@ -100,7 +104,7 @@ module Projects
       # We add * to end of SITE_PATH, because we want to extract SITE_PATH and all subdirectories
       site_path = File.join(SITE_PATH, '*')
       unless system(*%W(unzip -qq -n #{artifacts} #{site_path} -d #{temp_path}))
-        raise 'pages failed to extract'
+        raise FailedToExtractError, 'pages failed to extract'
       end
     end
 
@@ -163,6 +167,11 @@ module Projects
       build.artifacts_file.path
     end
 
+    def delete_artifact!
+      build.reload # Reload stable object to prevent erase artifacts with old state
+      build.erase_artifacts! unless build.has_expiring_artifacts?
+    end
+
     def latest_sha
       project.commit(build.ref).try(:sha).to_s
     end
diff --git a/app/workers/pages_worker.rb b/app/workers/pages_worker.rb
index d3b95009364..66a0ff83bef 100644
--- a/app/workers/pages_worker.rb
+++ b/app/workers/pages_worker.rb
@@ -1,7 +1,7 @@
 class PagesWorker
   include ApplicationWorker
 
-  sidekiq_options retry: false
+  sidekiq_options retry: 3
 
   def perform(action, *arg)
     send(action, *arg) # rubocop:disable GitlabSecurity/PublicSend
diff --git a/changelogs/unreleased/fix-sm-fix_pages_worker.yml b/changelogs/unreleased/fix-sm-fix_pages_worker.yml
new file mode 100644
index 00000000000..190c7d3e83e
--- /dev/null
+++ b/changelogs/unreleased/fix-sm-fix_pages_worker.yml
@@ -0,0 +1,5 @@
+---
+title: Fix pages flaky failure by reloading stale object
+merge_request: 17522
+author:
+type: fixed
diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb
index 2e334caa98f..73fdb449557 100644
--- a/spec/features/projects/pages_spec.rb
+++ b/spec/features/projects/pages_spec.rb
@@ -57,4 +57,54 @@ feature 'Pages' do
 
     it_behaves_like 'no pages deployed'
   end
+
+  describe 'Remove page' do
+    context 'when user is the owner' do
+      let(:project) { create :project, :repository }
+
+      before do
+        project.namespace.update(owner: user)
+      end
+
+      context 'when pages are deployed' do
+        let(:pipeline) do
+          commit_sha = project.commit('HEAD').sha
+
+          project.pipelines.create(
+            ref: 'HEAD',
+            sha: commit_sha,
+            source: :push,
+            protected: false
+          )
+        end
+
+        let(:ci_build) do
+          create(
+            :ci_build,
+            project: project,
+            pipeline: pipeline,
+            ref: 'HEAD',
+            legacy_artifacts_file: fixture_file_upload(Rails.root.join('spec/fixtures/pages.zip')),
+            legacy_artifacts_metadata: fixture_file_upload(Rails.root.join('spec/fixtures/pages.zip.meta'))
+          )
+        end
+
+        before do
+          result = Projects::UpdatePagesService.new(project, ci_build).execute
+          expect(result[:status]).to eq(:success)
+          expect(project).to be_pages_deployed
+        end
+
+        it 'removes the pages' do
+          visit project_pages_path(project)
+
+          expect(page).to have_link('Remove pages')
+
+          click_link 'Remove pages'
+
+          expect(project.pages_deployed?).to be_falsey
+        end
+      end
+    end
+  end
 end
diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb
index bfb86284d86..934106627a9 100644
--- a/spec/services/projects/update_pages_service_spec.rb
+++ b/spec/services/projects/update_pages_service_spec.rb
@@ -34,6 +34,7 @@ describe Projects::UpdatePagesService do
           context 'with expiry date' do
             before do
               build.artifacts_expire_in = "2 days"
+              build.save!
             end
 
             it "doesn't delete artifacts" do
@@ -105,6 +106,7 @@ describe Projects::UpdatePagesService do
         context 'with expiry date' do
           before do
             build.artifacts_expire_in = "2 days"
+            build.save!
           end
 
           it "doesn't delete artifacts" do
@@ -159,6 +161,20 @@ describe Projects::UpdatePagesService do
 
         expect(execute).not_to eq(:success)
       end
+
+      context 'when timeout happens by DNS error' do
+        before do
+          allow_any_instance_of(described_class)
+            .to receive(:extract_zip_archive!).and_raise(SocketError)
+        end
+
+        it 'raises an error' do
+          expect { execute }.to raise_error(SocketError)
+
+          build.reload
+          expect(build.artifacts?).to eq(true)
+        end
+      end
     end
   end
 
-- 
cgit v1.2.1


From 43304ec08c54aa00075815026130afed573233e6 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@gitlab.com>
Date: Mon, 5 Mar 2018 14:24:09 +0000
Subject: Merge branch 'backport-ee-5049-to-ce' into 'master'

Ports https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/4747 to CE

See merge request gitlab-org/gitlab-ce!17520
---
 config/initializers/forbid_sidekiq_in_transactions.rb | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/config/initializers/forbid_sidekiq_in_transactions.rb b/config/initializers/forbid_sidekiq_in_transactions.rb
index cb611aa21df..4cf1d455eb4 100644
--- a/config/initializers/forbid_sidekiq_in_transactions.rb
+++ b/config/initializers/forbid_sidekiq_in_transactions.rb
@@ -18,13 +18,26 @@ module Sidekiq
         %i(perform_async perform_at perform_in).each do |name|
           define_method(name) do |*args|
             if !Sidekiq::Worker.skip_transaction_check && AfterCommitQueue.inside_transaction?
-              raise Sidekiq::Worker::EnqueueFromTransactionError, <<~MSG
+              begin
+                raise Sidekiq::Worker::EnqueueFromTransactionError, <<~MSG
                 `#{self}.#{name}` cannot be called inside a transaction as this can lead to
                 race conditions when the worker runs before the transaction is committed and
                 tries to access a model that has not been saved yet.
 
                 Use an `after_commit` hook, or include `AfterCommitQueue` and use a `run_after_commit` block instead.
-              MSG
+                MSG
+              rescue Sidekiq::Worker::EnqueueFromTransactionError => e
+                if Rails.env.production?
+                  Rails.logger.error(e.message)
+
+                  if Gitlab::Sentry.enabled?
+                    Gitlab::Sentry.context
+                    Raven.capture_exception(e)
+                  end
+                else
+                  raise
+                end
+              end
             end
 
             super(*args)
-- 
cgit v1.2.1


From b20417299820d51f8c6a0c8d21a7eaf66acaeb33 Mon Sep 17 00:00:00 2001
From: Ian Baum <ibaum@gitlab.com>
Date: Wed, 7 Mar 2018 13:32:08 -0600
Subject: Revert "Merge branch 'fix/sm/fix_pages_worker' into 'master'"

This reverts commit 6846dad292bd7693859012a5b27158d1a995ddae.
---
 app/services/projects/update_pages_service.rb      | 35 ++++++---------
 app/workers/pages_worker.rb                        |  2 +-
 changelogs/unreleased/fix-sm-fix_pages_worker.yml  |  5 ---
 spec/features/projects/pages_spec.rb               | 50 ----------------------
 .../services/projects/update_pages_service_spec.rb | 16 -------
 5 files changed, 14 insertions(+), 94 deletions(-)
 delete mode 100644 changelogs/unreleased/fix-sm-fix_pages_worker.yml

diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb
index 00fdd047208..c760bd3b626 100644
--- a/app/services/projects/update_pages_service.rb
+++ b/app/services/projects/update_pages_service.rb
@@ -1,8 +1,5 @@
 module Projects
   class UpdatePagesService < BaseService
-    InvaildStateError = Class.new(StandardError)
-    FailedToExtractError = Class.new(StandardError)
-
     BLOCK_SIZE = 32.kilobytes
     MAX_SIZE = 1.terabyte
     SITE_PATH = 'public/'.freeze
@@ -14,15 +11,13 @@ module Projects
     end
 
     def execute
-      register_attempt
-
       # Create status notifying the deployment of pages
       @status = create_status
       @status.enqueue!
       @status.run!
 
-      raise InvaildStateError, 'missing pages artifacts' unless build.artifacts?
-      raise InvaildStateError, 'pages are outdated' unless latest?
+      raise 'missing pages artifacts' unless build.artifacts?
+      raise 'pages are outdated' unless latest?
 
       # Create temporary directory in which we will extract the artifacts
       FileUtils.mkdir_p(tmp_path)
@@ -31,22 +26,24 @@ module Projects
 
         # Check if we did extract public directory
         archive_public_path = File.join(archive_path, 'public')
-        raise FailedToExtractError, 'pages miss the public folder' unless Dir.exist?(archive_public_path)
-        raise InvaildStateError, 'pages are outdated' unless latest?
+        raise 'pages miss the public folder' unless Dir.exist?(archive_public_path)
+        raise 'pages are outdated' unless latest?
 
         deploy_page!(archive_public_path)
         success
       end
-    rescue InvaildStateError, FailedToExtractError => e
+    rescue => e
       register_failure
       error(e.message)
+    ensure
+      register_attempt
+      build.erase_artifacts! unless build.has_expiring_artifacts?
     end
 
     private
 
     def success
       @status.success
-      delete_artifact!
       super
     end
 
@@ -55,7 +52,6 @@ module Projects
       @status.allow_failure = !latest?
       @status.description = message
       @status.drop(:script_failure)
-      delete_artifact!
       super
     end
 
@@ -76,7 +72,7 @@ module Projects
       elsif artifacts.ends_with?('.zip')
         extract_zip_archive!(temp_path)
       else
-        raise FailedToExtractError, 'unsupported artifacts format'
+        raise 'unsupported artifacts format'
       end
     end
 
@@ -85,17 +81,17 @@ module Projects
                                %W(dd bs=#{BLOCK_SIZE} count=#{blocks}),
                                %W(tar -x -C #{temp_path} #{SITE_PATH}),
                                err: '/dev/null')
-      raise FailedToExtractError, 'pages failed to extract' unless results.compact.all?(&:success?)
+      raise 'pages failed to extract' unless results.compact.all?(&:success?)
     end
 
     def extract_zip_archive!(temp_path)
-      raise FailedToExtractError, 'missing artifacts metadata' unless build.artifacts_metadata?
+      raise 'missing artifacts metadata' unless build.artifacts_metadata?
 
       # Calculate page size after extract
       public_entry = build.artifacts_metadata_entry(SITE_PATH, recursive: true)
 
       if public_entry.total_size > max_size
-        raise FailedToExtractError, "artifacts for pages are too large: #{public_entry.total_size}"
+        raise "artifacts for pages are too large: #{public_entry.total_size}"
       end
 
       # Requires UnZip at least 6.00 Info-ZIP.
@@ -104,7 +100,7 @@ module Projects
       # We add * to end of SITE_PATH, because we want to extract SITE_PATH and all subdirectories
       site_path = File.join(SITE_PATH, '*')
       unless system(*%W(unzip -qq -n #{artifacts} #{site_path} -d #{temp_path}))
-        raise FailedToExtractError, 'pages failed to extract'
+        raise 'pages failed to extract'
       end
     end
 
@@ -167,11 +163,6 @@ module Projects
       build.artifacts_file.path
     end
 
-    def delete_artifact!
-      build.reload # Reload stable object to prevent erase artifacts with old state
-      build.erase_artifacts! unless build.has_expiring_artifacts?
-    end
-
     def latest_sha
       project.commit(build.ref).try(:sha).to_s
     end
diff --git a/app/workers/pages_worker.rb b/app/workers/pages_worker.rb
index 66a0ff83bef..d3b95009364 100644
--- a/app/workers/pages_worker.rb
+++ b/app/workers/pages_worker.rb
@@ -1,7 +1,7 @@
 class PagesWorker
   include ApplicationWorker
 
-  sidekiq_options retry: 3
+  sidekiq_options retry: false
 
   def perform(action, *arg)
     send(action, *arg) # rubocop:disable GitlabSecurity/PublicSend
diff --git a/changelogs/unreleased/fix-sm-fix_pages_worker.yml b/changelogs/unreleased/fix-sm-fix_pages_worker.yml
deleted file mode 100644
index 190c7d3e83e..00000000000
--- a/changelogs/unreleased/fix-sm-fix_pages_worker.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: Fix pages flaky failure by reloading stale object
-merge_request: 17522
-author:
-type: fixed
diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb
index 73fdb449557..2e334caa98f 100644
--- a/spec/features/projects/pages_spec.rb
+++ b/spec/features/projects/pages_spec.rb
@@ -57,54 +57,4 @@ feature 'Pages' do
 
     it_behaves_like 'no pages deployed'
   end
-
-  describe 'Remove page' do
-    context 'when user is the owner' do
-      let(:project) { create :project, :repository }
-
-      before do
-        project.namespace.update(owner: user)
-      end
-
-      context 'when pages are deployed' do
-        let(:pipeline) do
-          commit_sha = project.commit('HEAD').sha
-
-          project.pipelines.create(
-            ref: 'HEAD',
-            sha: commit_sha,
-            source: :push,
-            protected: false
-          )
-        end
-
-        let(:ci_build) do
-          create(
-            :ci_build,
-            project: project,
-            pipeline: pipeline,
-            ref: 'HEAD',
-            legacy_artifacts_file: fixture_file_upload(Rails.root.join('spec/fixtures/pages.zip')),
-            legacy_artifacts_metadata: fixture_file_upload(Rails.root.join('spec/fixtures/pages.zip.meta'))
-          )
-        end
-
-        before do
-          result = Projects::UpdatePagesService.new(project, ci_build).execute
-          expect(result[:status]).to eq(:success)
-          expect(project).to be_pages_deployed
-        end
-
-        it 'removes the pages' do
-          visit project_pages_path(project)
-
-          expect(page).to have_link('Remove pages')
-
-          click_link 'Remove pages'
-
-          expect(project.pages_deployed?).to be_falsey
-        end
-      end
-    end
-  end
 end
diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb
index 934106627a9..bfb86284d86 100644
--- a/spec/services/projects/update_pages_service_spec.rb
+++ b/spec/services/projects/update_pages_service_spec.rb
@@ -34,7 +34,6 @@ describe Projects::UpdatePagesService do
           context 'with expiry date' do
             before do
               build.artifacts_expire_in = "2 days"
-              build.save!
             end
 
             it "doesn't delete artifacts" do
@@ -106,7 +105,6 @@ describe Projects::UpdatePagesService do
         context 'with expiry date' do
           before do
             build.artifacts_expire_in = "2 days"
-            build.save!
           end
 
           it "doesn't delete artifacts" do
@@ -161,20 +159,6 @@ describe Projects::UpdatePagesService do
 
         expect(execute).not_to eq(:success)
       end
-
-      context 'when timeout happens by DNS error' do
-        before do
-          allow_any_instance_of(described_class)
-            .to receive(:extract_zip_archive!).and_raise(SocketError)
-        end
-
-        it 'raises an error' do
-          expect { execute }.to raise_error(SocketError)
-
-          build.reload
-          expect(build.artifacts?).to eq(true)
-        end
-      end
     end
   end
 
-- 
cgit v1.2.1


From 790bcf704bfda33942ee616764b5741ae1cb2e9e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Tue, 6 Mar 2018 12:11:00 +0000
Subject: Merge branch 'fix/sm/fix_pages_worker' into 'master'

Reload stable object to prevent erase artifacts with old state (Ver 2)

Closes #43641

See merge request gitlab-org/gitlab-ce!17522
---
 app/services/projects/update_pages_service.rb      | 35 ++++++++++++++--------
 app/workers/pages_worker.rb                        |  2 +-
 changelogs/unreleased/fix-sm-fix_pages_worker.yml  |  5 ++++
 .../services/projects/update_pages_service_spec.rb | 16 ++++++++++
 4 files changed, 44 insertions(+), 14 deletions(-)
 create mode 100644 changelogs/unreleased/fix-sm-fix_pages_worker.yml

diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb
index c760bd3b626..00fdd047208 100644
--- a/app/services/projects/update_pages_service.rb
+++ b/app/services/projects/update_pages_service.rb
@@ -1,5 +1,8 @@
 module Projects
   class UpdatePagesService < BaseService
+    InvaildStateError = Class.new(StandardError)
+    FailedToExtractError = Class.new(StandardError)
+
     BLOCK_SIZE = 32.kilobytes
     MAX_SIZE = 1.terabyte
     SITE_PATH = 'public/'.freeze
@@ -11,13 +14,15 @@ module Projects
     end
 
     def execute
+      register_attempt
+
       # Create status notifying the deployment of pages
       @status = create_status
       @status.enqueue!
       @status.run!
 
-      raise 'missing pages artifacts' unless build.artifacts?
-      raise 'pages are outdated' unless latest?
+      raise InvaildStateError, 'missing pages artifacts' unless build.artifacts?
+      raise InvaildStateError, 'pages are outdated' unless latest?
 
       # Create temporary directory in which we will extract the artifacts
       FileUtils.mkdir_p(tmp_path)
@@ -26,24 +31,22 @@ module Projects
 
         # Check if we did extract public directory
         archive_public_path = File.join(archive_path, 'public')
-        raise 'pages miss the public folder' unless Dir.exist?(archive_public_path)
-        raise 'pages are outdated' unless latest?
+        raise FailedToExtractError, 'pages miss the public folder' unless Dir.exist?(archive_public_path)
+        raise InvaildStateError, 'pages are outdated' unless latest?
 
         deploy_page!(archive_public_path)
         success
       end
-    rescue => e
+    rescue InvaildStateError, FailedToExtractError => e
       register_failure
       error(e.message)
-    ensure
-      register_attempt
-      build.erase_artifacts! unless build.has_expiring_artifacts?
     end
 
     private
 
     def success
       @status.success
+      delete_artifact!
       super
     end
 
@@ -52,6 +55,7 @@ module Projects
       @status.allow_failure = !latest?
       @status.description = message
       @status.drop(:script_failure)
+      delete_artifact!
       super
     end
 
@@ -72,7 +76,7 @@ module Projects
       elsif artifacts.ends_with?('.zip')
         extract_zip_archive!(temp_path)
       else
-        raise 'unsupported artifacts format'
+        raise FailedToExtractError, 'unsupported artifacts format'
       end
     end
 
@@ -81,17 +85,17 @@ module Projects
                                %W(dd bs=#{BLOCK_SIZE} count=#{blocks}),
                                %W(tar -x -C #{temp_path} #{SITE_PATH}),
                                err: '/dev/null')
-      raise 'pages failed to extract' unless results.compact.all?(&:success?)
+      raise FailedToExtractError, 'pages failed to extract' unless results.compact.all?(&:success?)
     end
 
     def extract_zip_archive!(temp_path)
-      raise 'missing artifacts metadata' unless build.artifacts_metadata?
+      raise FailedToExtractError, 'missing artifacts metadata' unless build.artifacts_metadata?
 
       # Calculate page size after extract
       public_entry = build.artifacts_metadata_entry(SITE_PATH, recursive: true)
 
       if public_entry.total_size > max_size
-        raise "artifacts for pages are too large: #{public_entry.total_size}"
+        raise FailedToExtractError, "artifacts for pages are too large: #{public_entry.total_size}"
       end
 
       # Requires UnZip at least 6.00 Info-ZIP.
@@ -100,7 +104,7 @@ module Projects
       # We add * to end of SITE_PATH, because we want to extract SITE_PATH and all subdirectories
       site_path = File.join(SITE_PATH, '*')
       unless system(*%W(unzip -qq -n #{artifacts} #{site_path} -d #{temp_path}))
-        raise 'pages failed to extract'
+        raise FailedToExtractError, 'pages failed to extract'
       end
     end
 
@@ -163,6 +167,11 @@ module Projects
       build.artifacts_file.path
     end
 
+    def delete_artifact!
+      build.reload # Reload stable object to prevent erase artifacts with old state
+      build.erase_artifacts! unless build.has_expiring_artifacts?
+    end
+
     def latest_sha
       project.commit(build.ref).try(:sha).to_s
     end
diff --git a/app/workers/pages_worker.rb b/app/workers/pages_worker.rb
index d3b95009364..66a0ff83bef 100644
--- a/app/workers/pages_worker.rb
+++ b/app/workers/pages_worker.rb
@@ -1,7 +1,7 @@
 class PagesWorker
   include ApplicationWorker
 
-  sidekiq_options retry: false
+  sidekiq_options retry: 3
 
   def perform(action, *arg)
     send(action, *arg) # rubocop:disable GitlabSecurity/PublicSend
diff --git a/changelogs/unreleased/fix-sm-fix_pages_worker.yml b/changelogs/unreleased/fix-sm-fix_pages_worker.yml
new file mode 100644
index 00000000000..190c7d3e83e
--- /dev/null
+++ b/changelogs/unreleased/fix-sm-fix_pages_worker.yml
@@ -0,0 +1,5 @@
+---
+title: Fix pages flaky failure by reloading stale object
+merge_request: 17522
+author:
+type: fixed
diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb
index bfb86284d86..934106627a9 100644
--- a/spec/services/projects/update_pages_service_spec.rb
+++ b/spec/services/projects/update_pages_service_spec.rb
@@ -34,6 +34,7 @@ describe Projects::UpdatePagesService do
           context 'with expiry date' do
             before do
               build.artifacts_expire_in = "2 days"
+              build.save!
             end
 
             it "doesn't delete artifacts" do
@@ -105,6 +106,7 @@ describe Projects::UpdatePagesService do
         context 'with expiry date' do
           before do
             build.artifacts_expire_in = "2 days"
+            build.save!
           end
 
           it "doesn't delete artifacts" do
@@ -159,6 +161,20 @@ describe Projects::UpdatePagesService do
 
         expect(execute).not_to eq(:success)
       end
+
+      context 'when timeout happens by DNS error' do
+        before do
+          allow_any_instance_of(described_class)
+            .to receive(:extract_zip_archive!).and_raise(SocketError)
+        end
+
+        it 'raises an error' do
+          expect { execute }.to raise_error(SocketError)
+
+          build.reload
+          expect(build.artifacts?).to eq(true)
+        end
+      end
     end
   end
 
-- 
cgit v1.2.1


From ecf1af65ef617c50fe28205840a4cdb5efec32e8 Mon Sep 17 00:00:00 2001
From: Andreas Brandl <abrandl@gitlab.com>
Date: Wed, 7 Mar 2018 20:01:40 +0000
Subject: Fix snippets problem in 10-5-stable-patch-4

---
 app/finders/snippets_finder.rb | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb
index 8deef84555e..0048e684c13 100644
--- a/app/finders/snippets_finder.rb
+++ b/app/finders/snippets_finder.rb
@@ -82,9 +82,6 @@ class SnippetsFinder < UnionFinder
   end
 
   def feature_available_projects
-    # Don't return any project related snippets if the user cannot read cross project
-    return table[:id].eq(nil) unless Ability.allowed?(current_user, :read_cross_project)
-
     projects = projects_for_user do |part|
       part.with_feature_available_for_user(:snippets, current_user)
     end.select(:id)
-- 
cgit v1.2.1


From 3d90b157b2a12c7706900e0cd17defef803157ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Wed, 7 Mar 2018 21:14:36 +0100
Subject: Fix pages spinach failure

---
 features/steps/project/pages.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/features/steps/project/pages.rb b/features/steps/project/pages.rb
index d92b707310c..e626e4c5b17 100644
--- a/features/steps/project/pages.rb
+++ b/features/steps/project/pages.rb
@@ -40,7 +40,7 @@ class Spinach::Features::ProjectPages < Spinach::FeatureSteps
                                          source: :push,
                                          protected: false)
 
-    build = build(:ci_build,
+    build = create(:ci_build,
                   project: @project,
                   pipeline: pipeline,
                   ref: 'HEAD',
-- 
cgit v1.2.1


From 69972b263f45470667366585d24fc0a363e59e55 Mon Sep 17 00:00:00 2001
From: Ian Baum <ibaum@gitlab.com>
Date: Wed, 7 Mar 2018 14:51:31 -0600
Subject: Fix rubocop error with features/steps/project/pages.rb

---
 features/steps/project/pages.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/features/steps/project/pages.rb b/features/steps/project/pages.rb
index e626e4c5b17..d0f8dcbc00a 100644
--- a/features/steps/project/pages.rb
+++ b/features/steps/project/pages.rb
@@ -46,7 +46,7 @@ class Spinach::Features::ProjectPages < Spinach::FeatureSteps
                   ref: 'HEAD',
                   legacy_artifacts_file: fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip'),
                   legacy_artifacts_metadata: fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip.meta')
-                 )
+                  )
 
     result = ::Projects::UpdatePagesService.new(@project, build).execute
     expect(result[:status]).to eq(:success)
-- 
cgit v1.2.1