Merge branch 'ce-to-ee-2018-11-14' into 'master'

CE upstream - 2018-11-14 11:23 UTC

See merge request gitlab-org/gitlab-ee!8445

app/models/deployment.rb

@@ -160,18 +160,18 @@ class Deployment < ActiveRecord::Base
   end
 
   def has_metrics?
-    prometheus_adapter&.can_query?
+    prometheus_adapter&.can_query? && success?
   end
 
   def metrics
-    return {} unless has_metrics? && success?
+    return {} unless has_metrics?
 
     metrics = prometheus_adapter.query(:deployment, self)
     metrics&.merge(deployment_time: finished_at.to_i) || {}
   end
 
   def additional_metrics
-    return {} unless has_metrics? && success?
+    return {} unless has_metrics?
 
     metrics = prometheus_adapter.query(:additional_metrics_deployment, self)
     metrics&.merge(deployment_time: finished_at.to_i) || {}

app/serializers/environment_status_entity.rb

@@ -11,7 +11,7 @@ class EnvironmentStatusEntity < Grape::Entity
     project_environment_path(es.project, es.environment)
   end
 
-  expose :metrics_url, if: ->(*) { can_read_environment? && environment.has_metrics? } do |es|
+  expose :metrics_url, if: ->(*) { can_read_environment? && deployment.has_metrics? } do |es|
     metrics_project_environment_deployment_path(es.project, es.environment, es.deployment)
   end
 
@@ -45,6 +45,10 @@ class EnvironmentStatusEntity < Grape::Entity
     object.environment
   end
 
+  def deployment
+    object.deployment
+  end
+
   def project
     object.environment.project
   end

app/services/clusters/applications/check_installation_progress_service.rb

@@ -16,6 +16,7 @@ module Clusters
         end
       rescue Kubeclient::HttpError => e
         Rails.logger.error("Kubernetes error: #{e.error_code} #{e.message}")
+        Gitlab::Sentry.track_acceptable_exception(e, extra: { scope: 'kubernetes', app_id: app.id })
         app.make_errored!("Kubernetes error: #{e.error_code}") unless app.errored?
       end
 

app/services/clusters/applications/install_service.rb

@@ -14,9 +14,11 @@ module Clusters
             ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id)
         rescue Kubeclient::HttpError => e
           Rails.logger.error("Kubernetes error: #{e.error_code} #{e.message}")
+          Gitlab::Sentry.track_acceptable_exception(e, extra: { scope: 'kubernetes', app_id: app.id })
           app.make_errored!("Kubernetes error: #{e.error_code}")
         rescue StandardError => e
           Rails.logger.error "Can't start installation process: #{e.class.name} #{e.message}"
+          Gitlab::Sentry.track_acceptable_exception(e, extra: { scope: 'kubernetes', app_id: app.id })
           app.make_errored!("Can't start installation process.")
         end
       end

changelogs/unreleased/drop-gcp-cluster-table.yml

+---
+title: Drop gcp_clusters table
+merge_request: 22713
+author:
+type: other

changelogs/unreleased/fix-deployment-metrics-in-mr-widget.yml

+---
+title: Avoid returning deployment metrics url to MR widget when the deployment is
+  not successful
+merge_request: 23010
+author:
+type: fixed

db/migrate/20181031190558_drop_fk_gcp_clusters_table.rb

+# frozen_string_literal: true
+
+class DropFkGcpClustersTable < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    remove_foreign_key_if_exists :gcp_clusters, column: :project_id
+    remove_foreign_key_if_exists :gcp_clusters, column: :user_id
+    remove_foreign_key_if_exists :gcp_clusters, column: :service_id
+  end
+
+  def down
+    add_foreign_key_if_not_exists :gcp_clusters, :projects, column: :project_id, on_delete: :cascade
+    add_foreign_key_if_not_exists :gcp_clusters, :users, column: :user_id, on_delete: :nullify
+    add_foreign_key_if_not_exists :gcp_clusters, :services, column: :service_id, on_delete: :nullify
+  end
+
+  private
+
+  def add_foreign_key_if_not_exists(source, target, column:, on_delete:)
+    return unless table_exists?(source)
+    return if foreign_key_exists?(source, target, column: column)
+
+    add_concurrent_foreign_key(source, target, column: column, on_delete: on_delete)
+  end
+
+  def remove_foreign_key_if_exists(table, column:)
+    return unless table_exists?(table)
+    return unless foreign_key_exists?(table, column: column)
+
+    remove_foreign_key(table, column: column)
+  end
+end

db/migrate/20181031190559_drop_gcp_clusters_table.rb

+# frozen_string_literal: true
+
+class DropGcpClustersTable < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    drop_table :gcp_clusters
+  end
+
+  def down
+    create_table :gcp_clusters do |t|
+      # Order columns by best align scheme
+      t.references :project, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade }
+      t.references :user, foreign_key: { on_delete: :nullify }
+      t.references :service, foreign_key: { on_delete: :nullify }
+      t.integer :status
+      t.integer :gcp_cluster_size, null: false
+
+      # Timestamps
+      t.datetime_with_timezone :created_at, null: false
+      t.datetime_with_timezone :updated_at, null: false
+
+      # Enable/disable
+      t.boolean :enabled, default: true
+
+      # General
+      t.text :status_reason
+
+      # k8s integration specific
+      t.string :project_namespace
+
+      # Cluster details
+      t.string :endpoint
+      t.text :ca_cert
+      t.text :encrypted_kubernetes_token
+      t.string :encrypted_kubernetes_token_iv
+      t.string :username
+      t.text :encrypted_password
+      t.string :encrypted_password_iv
+
+      # GKE
+      t.string :gcp_project_id, null: false
+      t.string :gcp_cluster_zone, null: false
+      t.string :gcp_cluster_name, null: false
+      t.string :gcp_machine_type
+      t.string :gcp_operation_id
+      t.text :encrypted_gcp_token
+      t.string :encrypted_gcp_token_iv
+    end
+  end
+end

db/schema.rb

@@ -1127,35 +1127,6 @@ ActiveRecord::Schema.define(version: 20181107054254) do
 
   add_index "forked_project_links", ["forked_to_project_id"], name: "index_forked_project_links_on_forked_to_project_id", unique: true, using: :btree
 
-  create_table "gcp_clusters", force: :cascade do |t|
-    t.integer "project_id", null: false
-    t.integer "user_id"
-    t.integer "service_id"
-    t.integer "status"
-    t.integer "gcp_cluster_size", null: false
-    t.datetime_with_timezone "created_at", null: false
-    t.datetime_with_timezone "updated_at", null: false
-    t.boolean "enabled", default: true
-    t.text "status_reason"
-    t.string "project_namespace"
-    t.string "endpoint"
-    t.text "ca_cert"
-    t.text "encrypted_kubernetes_token"
-    t.string "encrypted_kubernetes_token_iv"
-    t.string "username"
-    t.text "encrypted_password"
-    t.string "encrypted_password_iv"
-    t.string "gcp_project_id", null: false
-    t.string "gcp_cluster_zone", null: false
-    t.string "gcp_cluster_name", null: false
-    t.string "gcp_machine_type"
-    t.string "gcp_operation_id"
-    t.text "encrypted_gcp_token"
-    t.string "encrypted_gcp_token_iv"
-  end
-
-  add_index "gcp_clusters", ["project_id"], name: "index_gcp_clusters_on_project_id", unique: true, using: :btree
-
   create_table "geo_cache_invalidation_events", id: :bigserial, force: :cascade do |t|
     t.string "key", null: false
   end
@@ -3333,9 +3304,6 @@ ActiveRecord::Schema.define(version: 20181107054254) do
   add_foreign_key "fork_network_members", "projects", on_delete: :cascade
   add_foreign_key "fork_networks", "projects", column: "root_project_id", name: "fk_e7b436b2b5", on_delete: :nullify
   add_foreign_key "forked_project_links", "projects", column: "forked_to_project_id", name: "fk_434510edb0", on_delete: :cascade
-  add_foreign_key "gcp_clusters", "projects", on_delete: :cascade
-  add_foreign_key "gcp_clusters", "services", on_delete: :nullify
-  add_foreign_key "gcp_clusters", "users", on_delete: :nullify
   add_foreign_key "geo_event_log", "geo_cache_invalidation_events", column: "cache_invalidation_event_id", name: "fk_42c3b54bed", on_delete: :cascade
   add_foreign_key "geo_event_log", "geo_hashed_storage_migrated_events", column: "hashed_storage_migrated_event_id", name: "fk_27548c6db3", on_delete: :cascade
   add_foreign_key "geo_event_log", "geo_job_artifact_deleted_events", column: "job_artifact_deleted_event_id", name: "fk_176d3fbb5d", on_delete: :cascade

doc/topics/git/troubleshooting_git.md

@@ -78,5 +78,20 @@ git push
 In case you're running an older version of Git (< 2.9), consider upgrading
 to >= 2.9 (see [Broken pipe when pushing to Git repository][Broken-Pipe]).
 
+## Timeout during git push/pull
+
+If pulling/pushing from/to your repository ends up taking more than 50 seconds,
+a timeout will be issued with a log of the number of operations performed 
+and their respective timings, like the example below:
+
+```
+remote: Running checks for branch: master
+remote: Scanning for LFS objects... (153ms)
+remote: Calculating new repository size... (cancelled after 729ms)
+```
+
+This could be used to further investigate what operation is performing poorly
+and provide GitLab with more information on how to improve the service.
+
 [SSH troubleshooting]: ../../ssh/README.md#troubleshooting "SSH Troubleshooting"
 [Broken-Pipe]: https://stackoverflow.com/questions/19120120/broken-pipe-when-pushing-to-git-repository/36971469#36971469 "StackOverflow: 'Broken pipe when pushing to Git repository'"

lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml

@@ -27,7 +27,7 @@
 # Continuous deployment to production is enabled by default.
 # If you want to deploy to staging first, set STAGING_ENABLED environment variable.
 # If you want to enable incremental rollout, either manual or time based,
-# set INCREMENTAL_ROLLOUT_TYPE environment variable to "manual" or "timed".
+# set INCREMENTAL_ROLLOUT_MODE environment variable to "manual" or "timed".
 # If you want to use canary deployments, set CANARY_ENABLED environment variable.
 #
 # If Auto DevOps fails to detect the proper buildpack, or if you want to

lib/gitlab/sentry.rb

@@ -7,7 +7,7 @@ module Gitlab
     end
 
     def self.context(current_user = nil)
-      return unless self.enabled?
+      return unless enabled?
 
       Raven.tags_context(locale: I18n.locale)
 
@@ -29,14 +29,22 @@ module Gitlab
     #
     # Provide an issue URL for follow up.
     def self.track_exception(exception, issue_url: nil, extra: {})
+      track_acceptable_exception(exception, issue_url: issue_url, extra: extra)
+
+      raise exception if should_raise?
+    end
+
+    # This should be used when you do not want to raise an exception in
+    # development and test. If you need development and test to behave
+    # just the same as production you can use this instead of
+    # track_exception.
+    def self.track_acceptable_exception(exception, issue_url: nil, extra: {})
       if enabled?
         extra[:issue_url] = issue_url if issue_url
         context # Make sure we've set everything we know in the context
 
         Raven.capture_exception(exception, extra: extra)
       end
-
-      raise exception if should_raise?
     end
 
     def self.program_context

spec/lib/gitlab/sentry_spec.rb

@@ -52,4 +52,28 @@ describe Gitlab::Sentry do
       end
     end
   end
+
+  context '.track_acceptable_exception' do
+    let(:exception) { RuntimeError.new('boom') }
+
+    before do
+      allow(described_class).to receive(:enabled?).and_return(true)
+    end
+
+    it 'calls Raven.capture_exception' do
+      expected_extras = {
+        some_other_info: 'info',
+        issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1'
+      }
+
+      expect(Raven).to receive(:capture_exception)
+                         .with(exception, extra: a_hash_including(expected_extras))
+
+      described_class.track_acceptable_exception(
+        exception,
+        issue_url: 'http://gitlab.com/gitlab-org/gitlab-ce/issues/1',
+        extra: { some_other_info: 'info' }
+      )
+    end
+  end
 end

spec/serializers/environment_status_entity_spec.rb

@@ -40,4 +40,40 @@ describe EnvironmentStatusEntity do
 
     it { is_expected.to include(:stop_url) }
   end
+
+  context 'when deployment has metrics' do
+    let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) }
+
+    let(:simple_metrics) do
+      {
+        success: true,
+        metrics: {},
+        last_update: 42
+      }
+    end
+
+    before do
+      project.add_maintainer(user)
+      allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter)
+      allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics)
+      allow(entity).to receive(:deployment).and_return(deployment)
+    end
+
+    context 'when deployment succeeded' do
+      let(:deployment)    { create(:deployment, :succeed, :review_app) }
+
+      it 'returns metrics url' do
+        expect(subject[:metrics_url])
+          .to eq("/#{project.namespace.name}/#{project.name}/environments/#{environment.id}/deployments/#{deployment.iid}/metrics")
+      end
+    end
+
+    context 'when deployment is running' do
+      let(:deployment)    { create(:deployment, :running, :review_app) }
+
+      it 'does not return metrics url' do
+        expect(subject[:metrics_url]).to be_nil
+      end
+    end
+  end
 end