Automatic merge of gitlab-org/gitlab-ce master

app/services/clusters/applications/base_helm_service.rb

@@ -13,16 +13,20 @@ module Clusters
 
       def log_error(error)
         meta = {
-          exception: error.class.name,
           error_code: error.respond_to?(:error_code) ? error.error_code : nil,
           service: self.class.name,
           app_id: app.id,
           project_ids: app.cluster.project_ids,
-          group_ids: app.cluster.group_ids,
-          message: error.message
+          group_ids: app.cluster.group_ids
         }
 
-        logger.error(meta)
+        logger_meta = meta.merge(
+          exception: error.class.name,
+          message: error.message,
+          backtrace: Gitlab::Profiler.clean_backtrace(error.backtrace)
+        )
+
+        logger.error(logger_meta)
         Gitlab::Sentry.track_acceptable_exception(error, extra: meta)
       end
 

changelogs/unreleased/add_backtrace_to_kubernetes_log.yml

+---
+title: Show error backtrace when logging errors to kubernetes.log
+merge_request: 25726
+author:
+type: other

lib/api/entities.rb

@@ -690,6 +690,10 @@ module API
       # Deprecated
       expose :allow_collaboration, as: :allow_maintainer_to_push, if: -> (merge_request, _) { merge_request.for_fork? }
 
+      expose :reference do |merge_request, options|
+        merge_request.to_reference(options[:project])
+      end
+
       expose :web_url do |merge_request|
         Gitlab::UrlBuilder.build(merge_request)
       end
@@ -726,6 +730,8 @@ module API
         merge_request.metrics&.pipeline
       end
 
+      expose :head_pipeline, using: 'API::Entities::Pipeline'
+
       expose :diff_refs, using: Entities::DiffRefs
 
       # Allow the status of a rebase to be determined
@@ -1267,6 +1273,9 @@ module API
       expose :created_at, :updated_at, :started_at, :finished_at, :committed_at
       expose :duration
       expose :coverage
+      expose :detailed_status, using: DetailedStatusEntity do |pipeline, options|
+        pipeline.detailed_status(options[:current_user])
+      end
     end
 
     class PipelineSchedule < Grape::Entity

lib/api/issues.rb

@@ -310,7 +310,7 @@ module API
           .flatten
 
         present paginate(::Kaminari.paginate_array(merge_requests)),
-          with: Entities::MergeRequestBasic,
+          with: Entities::MergeRequest,
           current_user: current_user,
           project: user_project
       end

spec/fixtures/api/schemas/public_api/v4/merge_request.json

@@ -119,6 +119,12 @@
       "merge_status", "sha", "merge_commit_sha", "user_notes_count",
       "should_remove_source_branch", "force_remove_source_branch",
       "web_url", "squash"
-    ]
+    ],
+    "head_pipeline": {
+      "oneOf": [
+        { "type": "null" },
+        { "$ref": "pipeline/detail.json" }
+      ]
+    }
   }
 }

spec/fixtures/api/schemas/public_api/v4/pipeline/basic.json

@@ -13,6 +13,5 @@
     "ref": { "type": "string" },
     "status": { "type": "string" },
     "web_url": { "type": "string" }
-  },
-  "additionalProperties": false
+  }
 }

spec/fixtures/api/schemas/public_api/v4/pipeline/detail.json

+{
+  "type": "object",
+  "allOf": [
+    { "$ref": "basic.json" },
+    {
+      "properties": {
+        "before_sha": { "type": ["string", "null"] },
+        "tag": { "type": ["boolean"] },
+        "yaml_errors": { "type": ["string", "null"] },
+        "user": {
+          "anyOf": [
+            { "type": ["object", "null"] },
+            { "$ref": "../user/basic.json" }
+          ]
+        },
+        "created_at": { "type": ["date", "null"] },
+        "updated_at": { "type": ["date", "null"] },
+        "started_at": { "type": ["date", "null"] },
+        "finished_at": { "type": ["date", "null"] },
+        "committed_at": { "type": ["date", "null"] },
+        "duration": { "type": ["number", "null"] },
+        "coverage": { "type": ["string", "null"] },
+        "detailed_status": {
+          "oneOf": [
+            { "type": "null" },
+            { "$ref": "../../../status/ci_detailed_status.json" }
+          ]
+        }
+      }
+    }
+  ]
+}

spec/requests/api/merge_requests_spec.rb

@@ -729,6 +729,14 @@ describe API::MergeRequests do
   end
 
   describe "GET /projects/:id/merge_requests/:merge_request_iid" do
+    it 'matches json schema' do
+      merge_request = create(:merge_request, :with_test_reports, milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(response).to match_response_schema('public_api/v4/merge_request')
+    end
+
     it 'exposes known attributes' do
       create(:award_emoji, :downvote, awardable: merge_request)
       create(:award_emoji, :upvote, awardable: merge_request)

spec/requests/api/pipelines_spec.rb

@@ -399,6 +399,13 @@ describe API::Pipelines do
 
   describe 'GET /projects/:id/pipelines/:pipeline_id' do
     context 'authorized user' do
+      it 'exposes known attributes' do
+        get api("/projects/#{project.id}/pipelines/#{pipeline.id}", user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to match_response_schema('public_api/v4/pipeline/detail')
+      end
+
       it 'returns project pipelines' do
         get api("/projects/#{project.id}/pipelines/#{pipeline.id}", user)
 

spec/services/clusters/applications/check_installation_progress_service_spec.rb

@@ -33,14 +33,22 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do
     end
   end
 
-  shared_examples 'error logging' do
+  shared_examples 'error handling' do
     context 'when installation raises a Kubeclient::HttpError' do
       let(:cluster) { create(:cluster, :provided_by_user, :project) }
+      let(:logger) { service.send(:logger) }
+      let(:error) { Kubeclient::HttpError.new(401, 'Unauthorized', nil) }
 
       before do
         application.update!(cluster: cluster)
 
-        expect(service).to receive(:installation_phase).and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil))
+        expect(service).to receive(:installation_phase).and_raise(error)
+      end
+
+      include_examples 'logs kubernetes errors' do
+        let(:error_name) { 'Kubeclient::HttpError' }
+        let(:error_message) { 'Unauthorized' }
+        let(:error_code) { 401 }
       end
 
       it 'shows the response code from the error' do
@@ -49,12 +57,6 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do
         expect(application).to be_errored.or(be_update_errored)
         expect(application.status_reason).to eq('Kubernetes error: 401')
       end
-
-      it 'should log error' do
-        expect(service.send(:logger)).to receive(:error)
-
-        service.execute
-      end
     end
   end
 
@@ -66,7 +68,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do
   context 'when application is updating' do
     let(:application) { create(:clusters_applications_helm, :updating) }
 
-    include_examples 'error logging'
+    include_examples 'error handling'
 
     RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase }
 
@@ -127,7 +129,7 @@ describe Clusters::Applications::CheckInstallationProgressService, '#execute' do
   end
 
   context 'when application is installing' do
-    include_examples 'error logging'
+    include_examples 'error handling'
 
     RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated installation', phase }
 

spec/services/clusters/applications/install_service_spec.rb

@@ -39,51 +39,34 @@ describe Clusters::Applications::InstallService do
         expect(helm_client).to receive(:install).with(install_command).and_raise(error)
       end
 
+      include_examples 'logs kubernetes errors' do
+        let(:error_name) { 'Kubeclient::HttpError' }
+        let(:error_message) { 'system failure' }
+        let(:error_code) { 500 }
+      end
+
       it 'make the application errored' do
         service.execute
 
         expect(application).to be_errored
         expect(application.status_reason).to match('Kubernetes error: 500')
       end
-
-      it 'logs errors' do
-        expect(service.send(:logger)).to receive(:error).with(
-          {
-            exception: 'Kubeclient::HttpError',
-            message: 'system failure',
-            service: 'Clusters::Applications::InstallService',
-            app_id: application.id,
-            project_ids: application.cluster.project_ids,
-            group_ids: [],
-            error_code: 500
-          }
-        )
-
-        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
-          error,
-          extra: {
-            exception: 'Kubeclient::HttpError',
-            message: 'system failure',
-            service: 'Clusters::Applications::InstallService',
-            app_id: application.id,
-            project_ids: application.cluster.project_ids,
-            group_ids: [],
-            error_code: 500
-          }
-        )
-
-        service.execute
-      end
     end
 
     context 'a non kubernetes error happens' do
       let(:application) { create(:clusters_applications_helm, :scheduled) }
-      let(:error) { StandardError.new("something bad happened") }
+      let(:error) { StandardError.new('something bad happened') }
 
       before do
         expect(application).to receive(:make_installing!).once.and_raise(error)
       end
 
+      include_examples 'logs kubernetes errors' do
+        let(:error_name) { 'StandardError' }
+        let(:error_message) { 'something bad happened' }
+        let(:error_code) { nil }
+      end
+
       it 'make the application errored' do
         expect(helm_client).not_to receive(:install)
 
@@ -92,35 +75,6 @@ describe Clusters::Applications::InstallService do
         expect(application).to be_errored
         expect(application.status_reason).to eq("Can't start installation process.")
       end
-
-      it 'logs errors' do
-        expect(service.send(:logger)).to receive(:error).with(
-          {
-            exception: 'StandardError',
-            error_code: nil,
-            message: 'something bad happened',
-            service: 'Clusters::Applications::InstallService',
-            app_id: application.id,
-            project_ids: application.cluster.projects.pluck(:id),
-            group_ids: []
-          }
-        )
-
-        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
-          error,
-          extra: {
-            exception: 'StandardError',
-            error_code: nil,
-            message: 'something bad happened',
-            service: 'Clusters::Applications::InstallService',
-            app_id: application.id,
-            project_ids: application.cluster.projects.pluck(:id),
-            group_ids: []
-          }
-        )
-
-        service.execute
-      end
     end
   end
 end

spec/services/clusters/applications/patch_service_spec.rb

@@ -41,47 +41,30 @@ describe Clusters::Applications::PatchService do
         expect(helm_client).to receive(:update).with(update_command).and_raise(error)
       end
 
+      include_examples 'logs kubernetes errors' do
+        let(:error_name) { 'Kubeclient::HttpError' }
+        let(:error_message) { 'system failure' }
+        let(:error_code) { 500 }
+      end
+
       it 'make the application errored' do
         service.execute
 
         expect(application).to be_update_errored
         expect(application.status_reason).to match('Kubernetes error: 500')
       end
-
-      it 'logs errors' do
-        expect(service.send(:logger)).to receive(:error).with(
-          {
-            exception: 'Kubeclient::HttpError',
-            message: 'system failure',
-            service: 'Clusters::Applications::PatchService',
-            app_id: application.id,
-            project_ids: application.cluster.project_ids,
-            group_ids: [],
-            error_code: 500
-          }
-        )
-
-        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
-          error,
-          extra: {
-            exception: 'Kubeclient::HttpError',
-            message: 'system failure',
-            service: 'Clusters::Applications::PatchService',
-            app_id: application.id,
-            project_ids: application.cluster.project_ids,
-            group_ids: [],
-            error_code: 500
-          }
-        )
-
-        service.execute
-      end
     end
 
     context 'a non kubernetes error happens' do
       let(:application) { create(:clusters_applications_knative, :scheduled) }
       let(:error) { StandardError.new('something bad happened') }
 
+      include_examples 'logs kubernetes errors' do
+        let(:error_name) { 'StandardError' }
+        let(:error_message) { 'something bad happened' }
+        let(:error_code) { nil }
+      end
+
       before do
         expect(application).to receive(:make_updating!).once.and_raise(error)
       end
@@ -94,35 +77,6 @@ describe Clusters::Applications::PatchService do
         expect(application).to be_update_errored
         expect(application.status_reason).to eq("Can't start update process.")
       end
-
-      it 'logs errors' do
-        expect(service.send(:logger)).to receive(:error).with(
-          {
-            exception: 'StandardError',
-            error_code: nil,
-            message: 'something bad happened',
-            service: 'Clusters::Applications::PatchService',
-            app_id: application.id,
-            project_ids: application.cluster.projects.pluck(:id),
-            group_ids: []
-          }
-        )
-
-        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
-          error,
-          extra: {
-            exception: 'StandardError',
-            error_code: nil,
-            message: 'something bad happened',
-            service: 'Clusters::Applications::PatchService',
-            app_id: application.id,
-            project_ids: application.cluster.projects.pluck(:id),
-            group_ids: []
-          }
-        )
-
-        service.execute
-      end
     end
   end
 end

spec/services/clusters/applications/upgrade_service_spec.rb

@@ -41,41 +41,18 @@ describe Clusters::Applications::UpgradeService do
         expect(helm_client).to receive(:update).with(install_command).and_raise(error)
       end
 
+      include_examples 'logs kubernetes errors' do
+        let(:error_name) { 'Kubeclient::HttpError' }
+        let(:error_message) { 'system failure' }
+        let(:error_code) { 500 }
+      end
+
       it 'make the application errored' do
         service.execute
 
         expect(application).to be_update_errored
         expect(application.status_reason).to match('Kubernetes error: 500')
       end
-
-      it 'logs errors' do
-        expect(service.send(:logger)).to receive(:error).with(
-          {
-            exception: 'Kubeclient::HttpError',
-            message: 'system failure',
-            service: 'Clusters::Applications::UpgradeService',
-            app_id: application.id,
-            project_ids: application.cluster.project_ids,
-            group_ids: [],
-            error_code: 500
-          }
-        )
-
-        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
-          error,
-          extra: {
-            exception: 'Kubeclient::HttpError',
-            message: 'system failure',
-            service: 'Clusters::Applications::UpgradeService',
-            app_id: application.id,
-            project_ids: application.cluster.project_ids,
-            group_ids: [],
-            error_code: 500
-          }
-        )
-
-        service.execute
-      end
     end
 
     context 'a non kubernetes error happens' do
@@ -86,6 +63,12 @@ describe Clusters::Applications::UpgradeService do
         expect(application).to receive(:make_updating!).once.and_raise(error)
       end
 
+      include_examples 'logs kubernetes errors' do
+        let(:error_name) { 'StandardError' }
+        let(:error_message) { 'something bad happened' }
+        let(:error_code) { nil }
+      end
+
       it 'make the application errored' do
         expect(helm_client).not_to receive(:update)
 
@@ -94,35 +77,6 @@ describe Clusters::Applications::UpgradeService do
         expect(application).to be_update_errored
         expect(application.status_reason).to eq("Can't start upgrade process.")
       end
-
-      it 'logs errors' do
-        expect(service.send(:logger)).to receive(:error).with(
-          {
-            exception: 'StandardError',
-            error_code: nil,
-            message: 'something bad happened',
-            service: 'Clusters::Applications::UpgradeService',
-            app_id: application.id,
-            project_ids: application.cluster.projects.pluck(:id),
-            group_ids: []
-          }
-        )
-
-        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
-          error,
-          extra: {
-            exception: 'StandardError',
-            error_code: nil,
-            message: 'something bad happened',
-            service: 'Clusters::Applications::UpgradeService',
-            app_id: application.id,
-            project_ids: application.cluster.projects.pluck(:id),
-            group_ids: []
-          }
-        )
-
-        service.execute
-      end
     end
   end
 end

spec/support/shared_examples/services/base_helm_service_shared_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'logs kubernetes errors' do
+  let(:error_hash) do
+    {
+      service: service.class.name,
+      app_id: application.id,
+      project_ids: application.cluster.project_ids,
+      group_ids: [],
+      error_code: error_code
+    }
+  end
+
+  let(:logger_hash) do
+    error_hash.merge(
+      exception: error_name,
+      message: error_message,
+      backtrace: instance_of(Array)
+    )
+  end
+
+  it 'logs into kubernetes.log and Sentry' do
+    expect(service.send(:logger)).to receive(:error).with(logger_hash)
+
+    expect(Gitlab::Sentry).to receive(:track_acceptable_exception).with(
+      error,
+      extra: hash_including(error_hash)
+    )
+
+    service.execute
+  end
+end