From f8c584e1e9f1c1d6b2fde01760bd5986ef26a072 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 23 Feb 2018 18:20:19 +0100 Subject: [PATCH 01/49] Moved from CE: Squashed commit of the following: commit 766a42a96393f502d439c1f0beb0b6cfb2c228d4 Author: Pawel Chojnacki Date: Fri Feb 23 17:58:45 2018 +0100 Move prometheus adapter to app/models/concerns commit ca84eed49811cf3064a2e5ea611af1c947c590d7 Merge: 66702099586 cb504cedc2a Author: Pawel Chojnacki Date: Fri Feb 23 15:36:03 2018 +0100 Merge remote-tracking branch 'upstream/backport_custom_metrics_ce_components' into 38783-add-cluster-metrics.yml + fix failing tests # Conflicts: # app/controllers/projects/prometheus_controller.rb # app/models/project_services/prometheus_service.rb # lib/gitlab/prometheus/queries/query_additional_metrics.rb # spec/models/project_services/prometheus_service_spec.rb commit cb504cedc2a6e353ffb56833334681e3da09fc14 Author: Pawel Chojnacki Date: Fri Feb 23 14:46:15 2018 +0100 Fix prometheus_service found by find_or_initialize_service commit 928b84c72c2c4c46e1785b9a943c1822a137de16 Author: Pawel Chojnacki Date: Thu Feb 22 23:15:11 2018 +0100 additional metrics and backported tests commit 41291383b4b6976af94eaf9e1a7b2e2a172310e7 Author: Pawel Chojnacki Date: Thu Feb 22 21:57:45 2018 +0100 Introduce Metrics controller and retire prometheus controller commit 6e7492e4c7ffa9d8621f09198071bc14be875976 Author: Pawel Chojnacki Date: Thu Feb 22 21:46:37 2018 +0100 Backport PrometheusClient::Error and all->common_metrics rename commit 66702099586c864a7f78970a0ee0dd9f3c2beeef Author: Pawel Chojnacki Date: Thu Feb 22 20:23:03 2018 +0100 fix failing tests commit ce921ea723cde61b2441ba98c9aca97606c719c8 Author: Mike Greiling Date: Wed Feb 21 23:47:30 2018 -0600 add labels to cluster metrics commit 993830c6892e4fbf53d3f72da00002d642c7e9b2 Author: Pawel Chojnacki Date: Thu Feb 22 01:20:49 2018 +0100 Fix formatting probelms and few small tests commit 66ee65d8e3f747c90d986bc3056178422156bc8e Author: Pawel Chojnacki Date: Thu Feb 22 01:08:03 2018 +0100 stop using in deployment tests environment.id commit 62c91978d15f0369988521363dae24bd7510d68d Author: Pawel Chojnacki Date: Thu Feb 22 01:06:33 2018 +0100 fix prometheus_controller and adapter tests commit 977b1d34c1d03c7233582e8328f85caf634895ed Author: Pawel Chojnacki Date: Thu Feb 22 00:19:39 2018 +0100 finish up active? -> can_query? rename commit e614f7daee58a9758d83ba3efe1649c8b80bc1e2 Author: Pawel Chojnacki Date: Wed Feb 21 23:51:33 2018 +0100 deployment prometheus adapter tests fix commit ebd726c114a6026fef0adf3eba6ee1972530148a Author: Pawel Chojnacki Date: Wed Feb 21 21:40:40 2018 +0100 Move environment dependant tests to environment commit 6d31311cd3729c29233283dded70e03a4a9a3c97 Author: Pawel Chojnacki Date: Wed Feb 21 20:40:24 2018 +0100 update monitoring service and move adding dpeloyment_time to deployment model commit 60b6bf391ab36846dce122bc6b0c5196a186267c Author: Pawel Chojnacki Date: Wed Feb 21 20:15:46 2018 +0100 adjust deployment spec and prometheus specs commit 6681662cf1c028aff2ff94aa0501732cb7119ba1 Author: Pawel Chojnacki Date: Tue Feb 20 22:30:20 2018 +0100 Revert changes to reactive caching commit e282f86c45a056889f57d3f7fd23a81c88efff6a Merge: 5751c73df59 6844a2df873 Author: Pawel Chojnacki Date: Tue Feb 20 22:08:17 2018 +0100 Merge remote-tracking branch 'upstream/master' into 38783-add-cluster-metrics.yml commit 5751c73df59d0a03840a1b4b71b0637670f971a6 Author: Pawel Chojnacki Date: Tue Feb 20 22:07:43 2018 +0100 rename active? to can_query? and cleanup environment prometheus router commit 3f3c6e1d33dcd9315979daf26a95f2aab83a7de9 Author: Pawel Chojnacki Date: Tue Feb 20 21:51:43 2018 +0100 Fix tests, and only use prometheus service if its active commit 6345838bac584c213b665d334252ccab202cb271 Author: Pawel Chojnacki Date: Tue Feb 20 16:24:27 2018 +0100 Fix typo commit 7a585d32afe8da050b5615b1d036a550e06479f5 Author: Pawel Chojnacki Date: Tue Feb 20 15:29:40 2018 +0100 Cluster id is not required commit e6af62afb11fa380f6aff1c31a81bcc9bab3b1eb Author: Pawel Chojnacki Date: Tue Feb 20 15:15:46 2018 +0100 Result transformation support commit f3b1bd7c67894f44efe33591ddb70093bd620c03 Author: Pawel Chojnacki Date: Tue Feb 20 15:07:07 2018 +0100 Fix rubocop warning and exten cluster query timeframe commit be77947cea64261a4d3dead33c3c57f413a9880c Author: Pawel Chojnacki Date: Tue Feb 20 14:42:47 2018 +0100 Fix additional metrics test commit eb3922e16221abe16f59fae1c38122f227643343 Author: Pawel Chojnacki Date: Tue Feb 20 14:34:44 2018 +0100 rename prometheus adapter methods commit 045476cd08b21593818b274ae8a44d19b705523f Author: Pawel Chojnacki Date: Tue Feb 20 13:19:36 2018 +0100 Make prometheus adapter a module commit f2daf050d8c689f72c4c61207930bc53c331f12e Author: Pawel Chojnacki Date: Tue Feb 20 12:00:44 2018 +0100 refactoring wip commit 52e4ef5587794e811dc10a0f2dca522342a865da Author: Pawel Chojnacki Date: Sun Feb 18 20:25:55 2018 +0100 cleanup prometheus adapter concept commit 3887365faab9dfcd9c00bcfc501d09ac62431a03 Author: Pawel Chojnacki Date: Sun Feb 18 19:22:11 2018 +0100 Refactor out deployment id, Rename PrometheusQuerier to PrometheusAdapter commit aa2fc2df57bd72c9a5e94f66d1f1e23990be6c3f Author: Pawel Chojnacki Date: Fri Feb 16 23:59:54 2018 +0100 Refactor prometheus client commit e43c1ca9d9874d6cf1569f40fa1aca158d9d5d91 Author: Pawel Chojnacki Date: Thu Feb 15 20:24:45 2018 +0100 Use initial version of cluster_metrics.yml commit 867821ce0b2609ebf8994220aa8e3a94d66a01e0 Author: Pawel Chojnacki Date: Thu Feb 15 05:24:14 2018 +0100 Fix querying cluster metrics commit 1601e002a064cbb10ffe110a19433c5662858f1d Author: Pawel Chojnacki Date: Thu Feb 15 04:49:03 2018 +0100 Queues for unicersal querier commit 5db198fdc925c0223be24939b76da1d544dd569c Author: Pawel Chojnacki Date: Thu Feb 15 03:35:17 2018 +0100 refactor reactive caching and prometheus querying commit b0fc00e8c9d21e961ef44b0129103e2a62928b52 Author: Pawel Chojnacki Date: Thu Feb 15 00:12:03 2018 +0100 Add generic query additional metrics commit ffe76e6a9ba196bccff22c4880e0384959ad5e48 Author: Pawel Chojnacki Date: Tue Feb 13 15:40:15 2018 +0100 Cluster Metric yml initial --- .../monitoring/utils/multiple_time_series.js | 2 +- .../projects/clusters_controller.rb | 22 ++++ .../projects/deployments_controller.rb | 2 +- .../projects/prometheus/metrics_controller.rb | 32 +++++ .../projects/prometheus_controller.rb | 8 +- .../clusters/applications/prometheus.rb | 4 +- app/models/clusters/cluster.rb | 1 - app/models/clusters/platforms/kubernetes.rb | 2 +- app/models/concerns/prometheus_adapter.rb | 48 +++++++ app/models/deployment.rb | 17 +-- app/models/environment.rb | 33 +++-- .../project_services/monitoring_service.rb | 4 +- .../project_services/prometheus_service.rb | 85 +------------ .../services/prometheus/_show.html.haml | 2 +- config/prometheus/cluster_metrics.yml | 25 ++++ config/routes/project.rb | 5 +- .../prometheus/additional_metrics_parser.rb | 21 +++- lib/gitlab/prometheus/metric_group.rb | 7 +- .../additional_metrics_deployment_query.rb | 3 +- .../additional_metrics_environment_query.rb | 1 + lib/gitlab/prometheus/queries/base_query.rb | 4 + .../prometheus/queries/cluster_query.rb | 14 +++ .../prometheus/queries/deployment_query.rb | 7 +- .../prometheus/queries/environment_query.rb | 5 + ...trics_query.rb => matched_metric_query.rb} | 4 +- .../queries/query_additional_metrics.rb | 28 +++-- lib/gitlab/prometheus_client.rb | 39 +++--- .../projects/deployments_controller_spec.rb | 4 +- .../metrics_controller_spec.rb} | 22 ++-- ...dditional_metrics_deployment_query_spec.rb | 2 +- .../queries/deployment_query_spec.rb | 4 +- ...y_spec.rb => matched_metric_query_spec.rb} | 8 +- spec/lib/gitlab/prometheus_client_spec.rb | 28 ++--- .../clusters/applications/prometheus_spec.rb | 12 +- spec/models/deployment_spec.rb | 16 +-- spec/models/environment_spec.rb | 118 ++++++++++++------ .../prometheus_service_spec.rb | 99 +++------------ .../additional_metrics_shared_examples.rb | 16 +-- 38 files changed, 435 insertions(+), 319 deletions(-) create mode 100644 app/controllers/projects/prometheus/metrics_controller.rb create mode 100644 app/models/concerns/prometheus_adapter.rb create mode 100644 config/prometheus/cluster_metrics.yml create mode 100644 lib/gitlab/prometheus/queries/cluster_query.rb rename lib/gitlab/prometheus/queries/{matched_metrics_query.rb => matched_metric_query.rb} (96%) rename spec/controllers/projects/{prometheus_controller_spec.rb => prometheus/metrics_controller_spec.rb} (64%) rename spec/lib/gitlab/prometheus/queries/{matched_metrics_query_spec.rb => matched_metric_query_spec.rb} (93%) diff --git a/app/assets/javascripts/monitoring/utils/multiple_time_series.js b/app/assets/javascripts/monitoring/utils/multiple_time_series.js index 4ce3dad440c0..b5b8e3c255df 100644 --- a/app/assets/javascripts/monitoring/utils/multiple_time_series.js +++ b/app/assets/javascripts/monitoring/utils/multiple_time_series.js @@ -76,7 +76,7 @@ function queryTimeSeries(query, graphWidth, graphHeight, graphHeightOffset, xDom metricTag = seriesCustomizationData.value || timeSeriesMetricLabel; [lineColor, areaColor] = pickColor(seriesCustomizationData.color); } else { - metricTag = timeSeriesMetricLabel || `series ${timeSeriesNumber + 1}`; + metricTag = timeSeriesMetricLabel || query.label || `series ${timeSeriesNumber + 1}`; [lineColor, areaColor] = pickColor(); } diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index 6c497c8b10af..ac2db72a38a7 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -63,6 +63,22 @@ def destroy end end + def metrics + render_404 unless prometheus_adapter&.can_query? + + respond_to do |format| + format.json do + metrics = prometheus_adapter.query(:cluster) || {} + + if metrics.any? + render json: metrics + else + head :no_content + end + end + end + end + private def cluster @@ -70,6 +86,12 @@ def cluster .present(current_user: current_user) end + def prometheus_adapter + return unless cluster&.application_prometheus&.installed? + + cluster.application_prometheus + end + def update_params if cluster.managed? params.require(:cluster).permit( diff --git a/app/controllers/projects/deployments_controller.rb b/app/controllers/projects/deployments_controller.rb index 1a418d0f15a1..b68cdc39cb8d 100644 --- a/app/controllers/projects/deployments_controller.rb +++ b/app/controllers/projects/deployments_controller.rb @@ -24,7 +24,7 @@ def metrics end def additional_metrics - return render_404 unless deployment.has_additional_metrics? + return render_404 unless deployment.has_metrics? respond_to do |format| format.json do diff --git a/app/controllers/projects/prometheus/metrics_controller.rb b/app/controllers/projects/prometheus/metrics_controller.rb new file mode 100644 index 000000000000..255fbf254fba --- /dev/null +++ b/app/controllers/projects/prometheus/metrics_controller.rb @@ -0,0 +1,32 @@ +module Projects + module Prometheus + class MetricsController < Projects::ApplicationController + before_action :authorize_admin_project! + before_action :require_prometheus_metrics! + + def active_common + respond_to do |format| + format.json do + matched_metrics = prometheus_adapter.query(:matched_metrics) || {} + + if matched_metrics.any? + render json: matched_metrics + else + head :no_content + end + end + end + end + + private + + def prometheus_adapter + project.prometheus_service + end + + def require_prometheus_metrics! + render_404 unless prometheus_adapter.can_query? + end + end + end +end diff --git a/app/controllers/projects/prometheus_controller.rb b/app/controllers/projects/prometheus_controller.rb index 507468d71023..c36829f7f1e9 100644 --- a/app/controllers/projects/prometheus_controller.rb +++ b/app/controllers/projects/prometheus_controller.rb @@ -5,7 +5,7 @@ class Projects::PrometheusController < Projects::ApplicationController def active_metrics respond_to do |format| format.json do - matched_metrics = project.prometheus_service.matched_metrics || {} + matched_metrics = prometheus_adapter.query(:matched_metrics) || {} if matched_metrics.any? render json: matched_metrics @@ -18,7 +18,11 @@ def active_metrics private + def prometheus_adapter + project.prometheus_service + end + def require_prometheus_metrics! - render_404 unless project.prometheus_service.present? + render_404 unless prometheus_adapter.can_query? end end diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index aa22e9d5d581..ba63d0f3c64f 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -1,6 +1,8 @@ module Clusters module Applications class Prometheus < ActiveRecord::Base + include PrometheusAdapter + VERSION = "2.0.0".freeze self.table_name = 'clusters_applications_prometheus' @@ -38,7 +40,7 @@ def install_command Gitlab::Kubernetes::Helm::InstallCommand.new(name, chart: chart, chart_values_file: chart_values_file) end - def proxy_client + def prometheus_client return unless kube_client proxy_url = kube_client.proxy_url('service', service_name, service_port, Gitlab::Kubernetes::Helm::NAMESPACE) diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 9896c356fbd3..39a72e573eeb 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -52,7 +52,6 @@ class Cluster < ActiveRecord::Base scope :disabled, -> { where(enabled: false) } scope :for_environment, -> (env) { where(environment_scope: ['*', '', env.slug]) } - scope :for_all_environments, -> { where(environment_scope: ['*', '']) } def status_name if provider diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 74e30aa04d55..b9a362bd3081 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -143,7 +143,7 @@ def read_pods end def kubeclient_ssl_options - opts = { verify_ssl: OpenSSL::SSL::VERIFY_PEER } + opts = { verify_ssl: OpenSSL::SSL::VERIFY_NONE } if ca_pem.present? opts[:cert_store] = OpenSSL::X509::Store.new diff --git a/app/models/concerns/prometheus_adapter.rb b/app/models/concerns/prometheus_adapter.rb new file mode 100644 index 000000000000..fea99a7cd35a --- /dev/null +++ b/app/models/concerns/prometheus_adapter.rb @@ -0,0 +1,48 @@ +module PrometheusAdapter + extend ActiveSupport::Concern + + included do + include ReactiveCaching + + self.reactive_cache_key = ->(adapter) { [adapter.class.model_name.singular, adapter.id] } + self.reactive_cache_lease_timeout = 30.seconds + self.reactive_cache_refresh_interval = 30.seconds + self.reactive_cache_lifetime = 1.minute + + def prometheus_client + raise NotImplementedError + end + + def prometheus_client_wrapper + Gitlab::PrometheusClient.new(prometheus_client) + end + + def can_query? + prometheus_client.present? + end + + def query(query_name, *args) + return unless can_query? + + query_class = Gitlab::Prometheus::Queries.const_get("#{query_name.to_s.classify}Query") + + args.map! { |arg| arg.id } + + with_reactive_cache(query_class.name, *args, &query_class.method(:transform_reactive_result)) + end + + # Cache metrics for specific environment + def calculate_reactive_cache(query_class_name, *args) + return unless prometheus_client + + data = Kernel.const_get(query_class_name).new(prometheus_client_wrapper).query(*args) + { + success: true, + data: data, + last_update: Time.now.utc + } + rescue Gitlab::PrometheusClient::Error => err + { success: false, result: err.message } + end + end +end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index b6cf168d60e3..66e61c067654 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -98,28 +98,29 @@ def formatted_deployment_time end def has_metrics? - project.monitoring_service.present? + prometheus_adapter&.can_query? end def metrics return {} unless has_metrics? - project.monitoring_service.deployment_metrics(self) - end - - def has_additional_metrics? - project.prometheus_service.present? + metrics = prometheus_adapter.query(:deployment, self) + metrics&.merge(deployment_time: created_at.to_i) || {} end def additional_metrics - return {} unless project.prometheus_service.present? + return {} unless has_metrics? - metrics = project.prometheus_service.additional_deployment_metrics(self) + metrics = prometheus_adapter.query(:additional_metrics_deployment, self) metrics&.merge(deployment_time: created_at.to_i) || {} end private + def prometheus_adapter + environment.prometheus_adapter + end + def ref_path File.join(environment.ref_path, 'deployments', iid.to_s) end diff --git a/app/models/environment.rb b/app/models/environment.rb index 68ea886f74ea..d15c3933977c 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -154,21 +154,15 @@ def rollout_status end def has_metrics? - project.monitoring_service.present? && available? && last_deployment.present? + prometheus_adapter&.can_query? && available? && last_deployment.present? end def metrics - project.monitoring_service.environment_metrics(self) if has_metrics? - end - - def has_additional_metrics? - project.prometheus_service.present? && available? && last_deployment.present? + prometheus_adapter.query(:environment, self) if has_metrics? end def additional_metrics - if has_additional_metrics? - project.prometheus_service.additional_environment_metrics(self) - end + prometheus_adapter.query(:additional_metrics_environment, self) if has_metrics? end def slug @@ -234,6 +228,27 @@ def folder_name self.environment_type || self.name end + def prometheus_adapter + @prometheus_adapter ||= if service_prometheus_adapter.can_query? + service_prometheus_adapter + else + cluster_prometheus_adapter + end + end + + def service_prometheus_adapter + project.find_or_initialize_service('prometheus') + end + + def cluster_prometheus_adapter + # sort results by descending order based on environment_scope being longer + # thus more closely matching environment slug + clusters = project.clusters.enabled.for_environment(self).sort_by { |c| c.environment_scope&.length }.reverse! + + cluster = clusters&.detect { |cluster| cluster.application_prometheus&.installed? } + cluster&.application_prometheus + end + private # Slugifying a name may remove the uniqueness guarantee afforded by it being diff --git a/app/models/project_services/monitoring_service.rb b/app/models/project_services/monitoring_service.rb index ee9cd78327a8..9af68b4e8210 100644 --- a/app/models/project_services/monitoring_service.rb +++ b/app/models/project_services/monitoring_service.rb @@ -9,11 +9,11 @@ def self.supported_events %w() end - def environment_metrics(environment) + def can_query? raise NotImplementedError end - def deployment_metrics(deployment) + def query(_, *_) raise NotImplementedError end end diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 1bb576ff971e..7a0c9a33e70a 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -1,9 +1,5 @@ class PrometheusService < MonitoringService - include ReactiveService - - self.reactive_cache_lease_timeout = 30.seconds - self.reactive_cache_refresh_interval = 30.seconds - self.reactive_cache_lifetime = 1.minute + include PrometheusAdapter # Access to prometheus is directly through the API prop_accessor :api_url @@ -66,63 +62,15 @@ def fields # Check we can connect to the Prometheus API def test(*args) - client.ping + Gitlab::PrometheusClient.new(prometheus_client).ping { success: true, result: 'Checked API endpoint' } - rescue Gitlab::PrometheusError => err + rescue Gitlab::PrometheusClient::Error => err { success: false, result: err } end - def environment_metrics(environment) - with_reactive_cache(Gitlab::Prometheus::Queries::EnvironmentQuery.name, environment.id, &method(:rename_data_to_metrics)) - end - - def deployment_metrics(deployment) - metrics = with_reactive_cache(Gitlab::Prometheus::Queries::DeploymentQuery.name, deployment.environment.id, deployment.id, &method(:rename_data_to_metrics)) - metrics&.merge(deployment_time: deployment.created_at.to_i) || {} - end - - def additional_environment_metrics(environment) - with_reactive_cache(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, environment.id, &:itself) - end - - def additional_deployment_metrics(deployment) - with_reactive_cache(Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery.name, deployment.environment.id, deployment.id, &:itself) - end - - def matched_metrics - with_reactive_cache(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) - end - - # Cache metrics for specific environment - def calculate_reactive_cache(query_class_name, *args) - return unless active? && project && !project.pending_delete? - - environment_id = args.first - client = client(environment_id) - - data = Kernel.const_get(query_class_name).new(client).query(*args) - { - success: true, - data: data, - last_update: Time.now.utc - } - rescue Gitlab::PrometheusError => err - { success: false, result: err.message } - end - - def client(environment_id = nil) - if manual_configuration? - Gitlab::PrometheusClient.new(RestClient::Resource.new(api_url)) - else - cluster = cluster_with_prometheus(environment_id) - raise Gitlab::PrometheusError, "couldn't find cluster with Prometheus installed" unless cluster - - rest_client = client_from_cluster(cluster) - raise Gitlab::PrometheusError, "couldn't create proxy Prometheus client" unless rest_client - - Gitlab::PrometheusClient.new(rest_client) - end + def prometheus_client + RestClient::Resource.new(api_url) if api_url && manual_configuration? && active? end def prometheus_installed? @@ -134,29 +82,6 @@ def prometheus_installed? private - def cluster_with_prometheus(environment_id = nil) - clusters = if environment_id - ::Environment.find_by(id: environment_id).try do |env| - # sort results by descending order based on environment_scope being longer - # thus more closely matching environment slug - project.clusters.enabled.for_environment(env).sort_by { |c| c.environment_scope&.length }.reverse! - end - else - project.clusters.enabled.for_all_environments - end - - clusters&.detect { |cluster| cluster.application_prometheus&.installed? } - end - - def client_from_cluster(cluster) - cluster.application_prometheus.proxy_client - end - - def rename_data_to_metrics(metrics) - metrics[:metrics] = metrics.delete :data - metrics - end - def synchronize_service_state! self.active = prometheus_installed? || manual_configuration? diff --git a/app/views/projects/services/prometheus/_show.html.haml b/app/views/projects/services/prometheus/_show.html.haml index 5f38ecd68201..6dc2b85fd322 100644 --- a/app/views/projects/services/prometheus/_show.html.haml +++ b/app/views/projects/services/prometheus/_show.html.haml @@ -7,7 +7,7 @@ = link_to s_('PrometheusService|More information'), help_page_path('user/project/integrations/prometheus') .col-lg-9 - .panel.panel-default.js-panel-monitored-metrics{ data: { "active-metrics" => "#{project_prometheus_active_metrics_path(@project, :json)}" } } + .panel.panel-default.js-panel-monitored-metrics{ data: { active_metrics: active_common_project_prometheus_metrics_path(@project, :json) } } .panel-heading %h3.panel-title = s_('PrometheusService|Monitored') diff --git a/config/prometheus/cluster_metrics.yml b/config/prometheus/cluster_metrics.yml new file mode 100644 index 000000000000..3a6836a29dd2 --- /dev/null +++ b/config/prometheus/cluster_metrics.yml @@ -0,0 +1,25 @@ +- group: Cluster Health + priority: 1 + metrics: + - title: "CPU Usage" + y_label: "CPU" + required_metrics: ['container_cpu_usage_seconds_total'] + weight: 1 + queries: + - query_range: 'sum(rate(container_cpu_usage_seconds_total{id="/"}[15m]))' + label: Usage + unit: "cores" + - query_range: 'sum(kube_node_status_capacity_cpu_cores)' + label: Capacity + unit: "cores" + - title: "Memory usage" + y_label: "Memory" + required_metrics: ['container_memory_usage_bytes'] + weight: 1 + queries: + - query_range: 'sum(container_memory_usage_bytes{id="/"})/2^30' + label: Usage + unit: "GiB" + - query_range: 'sum(kube_node_status_capacity_memory_bytes)/2^30' + label: Capacity + unit: "GiB" diff --git a/config/routes/project.rb b/config/routes/project.rb index 362d4bf07806..07efed7b9d8c 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -74,7 +74,9 @@ resource :mattermost, only: [:new, :create] namespace :prometheus do - get :active_metrics + resources :metrics, constraints: { id: %r{[^\/]+} }, only: [] do + get :active_common, on: :collection + end end resources :deploy_keys, constraints: { id: /\d+/ }, only: [:index, :new, :create, :edit, :update] do @@ -237,6 +239,7 @@ member do get :status, format: :json + get :metrics, format: :json scope :applications do post '/:application', to: 'clusters/applications#create', as: :install_applications diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index cb95daf22602..bb1172f82a19 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -1,10 +1,12 @@ module Gitlab module Prometheus module AdditionalMetricsParser + CONFIG_ROOT = 'config/prometheus'.freeze + MUTEX = Mutex.new extend self - def load_groups_from_yaml - additional_metrics_raw.map(&method(:group_from_entry)) + def load_groups_from_yaml(file_name = 'additional_metrics.yml') + yaml_metrics_raw(file_name).map(&method(:group_from_entry)) end private @@ -22,13 +24,20 @@ def group_from_entry(entry) MetricGroup.new(entry).tap(&method(:validate!)) end - def additional_metrics_raw - load_yaml_file&.map(&:deep_symbolize_keys).freeze + def yaml_metrics_raw(file_name) + load_yaml_file(file_name)&.map(&:deep_symbolize_keys).freeze end - def load_yaml_file - @loaded_yaml_file ||= YAML.load_file(Rails.root.join('config/prometheus/additional_metrics.yml')) + # rubocop:disable Gitlab/ModuleWithInstanceVariables + def load_yaml_file(file_name) + return YAML.load_file(Rails.root.join(CONFIG_ROOT, file_name)) if Rails.env.development? + + MUTEX.synchronize do + @loaded_yaml_cache ||= {} + @loaded_yaml_cache[file_name] ||= YAML.load_file(Rails.root.join(CONFIG_ROOT, file_name)) + end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end end end diff --git a/lib/gitlab/prometheus/metric_group.rb b/lib/gitlab/prometheus/metric_group.rb index 729fef34b35e..e91c6fb2e276 100644 --- a/lib/gitlab/prometheus/metric_group.rb +++ b/lib/gitlab/prometheus/metric_group.rb @@ -6,9 +6,14 @@ class MetricGroup attr_accessor :name, :priority, :metrics validates :name, :priority, :metrics, presence: true - def self.all + def self.common_metrics AdditionalMetricsParser.load_groups_from_yaml end + + # EE only + def self.for_project(_) + common_metrics + end end end end diff --git a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb index 294a6ae34caf..e677ec84cd4f 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb @@ -4,9 +4,10 @@ module Queries class AdditionalMetricsDeploymentQuery < BaseQuery include QueryAdditionalMetrics - def query(environment_id, deployment_id) + def query(deployment_id) Deployment.find_by(id: deployment_id).try do |deployment| query_metrics( + deployment.project, common_query_context( deployment.environment, timeframe_start: (deployment.created_at - 30.minutes).to_f, diff --git a/lib/gitlab/prometheus/queries/additional_metrics_environment_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_environment_query.rb index 32fe8201a8dd..9273e69e158a 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_environment_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_environment_query.rb @@ -7,6 +7,7 @@ class AdditionalMetricsEnvironmentQuery < BaseQuery def query(environment_id) ::Environment.find_by(id: environment_id).try do |environment| query_metrics( + environment.project, common_query_context(environment, timeframe_start: 8.hours.ago.to_f, timeframe_end: Time.now.to_f) ) end diff --git a/lib/gitlab/prometheus/queries/base_query.rb b/lib/gitlab/prometheus/queries/base_query.rb index c60828165bd9..29cab6e9c15e 100644 --- a/lib/gitlab/prometheus/queries/base_query.rb +++ b/lib/gitlab/prometheus/queries/base_query.rb @@ -20,6 +20,10 @@ def initialize(client) def query(*args) raise NotImplementedError end + + def self.transform_reactive_result(result) + result + end end end end diff --git a/lib/gitlab/prometheus/queries/cluster_query.rb b/lib/gitlab/prometheus/queries/cluster_query.rb new file mode 100644 index 000000000000..1eff80db1476 --- /dev/null +++ b/lib/gitlab/prometheus/queries/cluster_query.rb @@ -0,0 +1,14 @@ +module Gitlab + module Prometheus + module Queries + class ClusterQuery < BaseQuery + include QueryAdditionalMetrics + + def query + AdditionalMetricsParser.load_groups_from_yaml('cluster_metrics.yml') + .map(&query_group(base_query_context(8.hours.ago, Time.now))) + end + end + end + end +end diff --git a/lib/gitlab/prometheus/queries/deployment_query.rb b/lib/gitlab/prometheus/queries/deployment_query.rb index 6e6da593178e..c2626581897a 100644 --- a/lib/gitlab/prometheus/queries/deployment_query.rb +++ b/lib/gitlab/prometheus/queries/deployment_query.rb @@ -2,7 +2,7 @@ module Gitlab module Prometheus module Queries class DeploymentQuery < BaseQuery - def query(environment_id, deployment_id) + def query(deployment_id) Deployment.find_by(id: deployment_id).try do |deployment| environment_slug = deployment.environment.slug @@ -25,6 +25,11 @@ def query(environment_id, deployment_id) } end end + + def self.transform_reactive_result(result) + result[:metrics] = result.delete :data + result + end end end end diff --git a/lib/gitlab/prometheus/queries/environment_query.rb b/lib/gitlab/prometheus/queries/environment_query.rb index 1d17d3cfd56c..b62910c8de67 100644 --- a/lib/gitlab/prometheus/queries/environment_query.rb +++ b/lib/gitlab/prometheus/queries/environment_query.rb @@ -19,6 +19,11 @@ def query(environment_id) } end end + + def self.transform_reactive_result(result) + result[:metrics] = result.delete :data + result + end end end end diff --git a/lib/gitlab/prometheus/queries/matched_metrics_query.rb b/lib/gitlab/prometheus/queries/matched_metric_query.rb similarity index 96% rename from lib/gitlab/prometheus/queries/matched_metrics_query.rb rename to lib/gitlab/prometheus/queries/matched_metric_query.rb index 4c3edccc71a7..d920e9a749fe 100644 --- a/lib/gitlab/prometheus/queries/matched_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/matched_metric_query.rb @@ -1,7 +1,7 @@ module Gitlab module Prometheus module Queries - class MatchedMetricsQuery < BaseQuery + class MatchedMetricQuery < BaseQuery MAX_QUERY_ITEMS = 40.freeze def query @@ -18,7 +18,7 @@ def query private def groups_data - metrics_groups = groups_with_active_metrics(Gitlab::Prometheus::MetricGroup.all) + metrics_groups = groups_with_active_metrics(Gitlab::Prometheus::MetricGroup.common_metrics) lookup = active_series_lookup(metrics_groups) groups = {} diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb index 79ec8c124426..4b23f392a66b 100644 --- a/lib/gitlab/prometheus/queries/query_additional_metrics.rb +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -2,10 +2,17 @@ module Gitlab module Prometheus module Queries module QueryAdditionalMetrics - def query_metrics(query_context) + def query_metrics(project, query_context) + matched_metrics(project).map(&query_group(query_context)) + .select(&method(:group_with_any_metrics)) + end + + protected + + def query_group(query_context) query_processor = method(:process_query).curry[query_context] - groups = matched_metrics.map do |group| + lambda do |group| metrics = group.metrics.map do |metric| { title: metric.title, @@ -21,8 +28,6 @@ def query_metrics(query_context) metrics: metrics.select(&method(:metric_with_any_queries)) } end - - groups.select(&method(:group_with_any_metrics)) end private @@ -60,8 +65,8 @@ def available_metrics @available_metrics ||= client_label_values || [] end - def matched_metrics - result = Gitlab::Prometheus::MetricGroup.all.map do |group| + def matched_metrics(project) + result = Gitlab::Prometheus::MetricGroup.for_project(project).map do |group| group.metrics.select! do |metric| metric.required_metrics.all?(&available_metrics.method(:include?)) end @@ -72,12 +77,17 @@ def matched_metrics end def common_query_context(environment, timeframe_start:, timeframe_end:) - { - timeframe_start: timeframe_start, - timeframe_end: timeframe_end, + base_query_context(timeframe_start, timeframe_end).merge({ ci_environment_slug: environment.slug, kube_namespace: environment.project.deployment_platform(environment: environment)&.actual_namespace || '', environment_filter: %{container_name!="POD",environment="#{environment.slug}"} + }) + end + + def base_query_context(timeframe_start, timeframe_end) + { + timeframe_start: timeframe_start, + timeframe_end: timeframe_end } end end diff --git a/lib/gitlab/prometheus_client.rb b/lib/gitlab/prometheus_client.rb index 105279726636..b66253a10e0e 100644 --- a/lib/gitlab/prometheus_client.rb +++ b/lib/gitlab/prometheus_client.rb @@ -1,8 +1,9 @@ module Gitlab - PrometheusError = Class.new(StandardError) - # Helper methods to interact with Prometheus network services & resources class PrometheusClient + Error = Class.new(StandardError) + QueryError = Class.new(Gitlab::PrometheusClient::Error) + attr_reader :rest_client, :headers def initialize(rest_client) @@ -22,10 +23,10 @@ def query(query, time: Time.now) def query_range(query, start: 8.hours.ago, stop: Time.now) get_result('matrix') do json_api_get('query_range', - query: query, - start: start.to_f, - end: stop.to_f, - step: 1.minute.to_i) + query: query, + start: start.to_f, + end: stop.to_f, + step: 1.minute.to_i) end end @@ -43,22 +44,26 @@ def json_api_get(type, args = {}) path = ['api', 'v1', type].join('/') get(path, args) rescue JSON::ParserError - raise PrometheusError, 'Parsing response failed' + raise PrometheusClient::Error, 'Parsing response failed' rescue Errno::ECONNREFUSED - raise PrometheusError, 'Connection refused' + raise PrometheusClient::Error, 'Connection refused' end def get(path, args) response = rest_client[path].get(params: args) handle_response(response) rescue SocketError - raise PrometheusError, "Can't connect to #{rest_client.url}" + raise PrometheusClient::Error, "Can't connect to #{rest_client.url}" rescue OpenSSL::SSL::SSLError - raise PrometheusError, "#{rest_client.url} contains invalid SSL data" + raise PrometheusClient::Error, "#{rest_client.url} contains invalid SSL data" rescue RestClient::ExceptionWithResponse => ex - handle_exception_response(ex.response) + if ex.response + handle_exception_response(ex.response) + else + raise PrometheusClient::Error, "Network connection error" + end rescue RestClient::Exception - raise PrometheusError, "Network connection error" + raise PrometheusClient::Error, "Network connection error" end def handle_response(response) @@ -66,16 +71,18 @@ def handle_response(response) if response.code == 200 && json_data['status'] == 'success' json_data['data'] || {} else - raise PrometheusError, "#{response.code} - #{response.body}" + raise PrometheusClient::Error, "#{response.code} - #{response.body}" end end def handle_exception_response(response) - if response.code == 400 + if response.code == 200 && response['status'] == 'success' + response['data'] || {} + elsif response.code == 400 json_data = JSON.parse(response.body) - raise PrometheusError, json_data['error'] || 'Bad data received' + raise PrometheusClient::QueryError, json_data['error'] || 'Bad data received' else - raise PrometheusError, "#{response.code} - #{response.body}" + raise PrometheusClient::Error, "#{response.code} - #{response.body}" end end diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index 73e7921fab78..6c67dfde63ac 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -129,10 +129,10 @@ end context 'when metrics are enabled' do - let(:prometheus_service) { double('prometheus_service') } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do - allow(deployment.project).to receive(:prometheus_service).and_return(prometheus_service) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) end context 'when environment has no metrics' do diff --git a/spec/controllers/projects/prometheus_controller_spec.rb b/spec/controllers/projects/prometheus/metrics_controller_spec.rb similarity index 64% rename from spec/controllers/projects/prometheus_controller_spec.rb rename to spec/controllers/projects/prometheus/metrics_controller_spec.rb index bbfe78d305af..e364e99d8570 100644 --- a/spec/controllers/projects/prometheus_controller_spec.rb +++ b/spec/controllers/projects/prometheus/metrics_controller_spec.rb @@ -1,28 +1,28 @@ -require('spec_helper') +require 'spec_helper' -describe Projects::PrometheusController do +describe Projects::Prometheus::MetricsController do let(:user) { create(:user) } - let!(:project) { create(:project) } + let(:project) { create(:project) } - let(:prometheus_service) { double('prometheus_service') } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do allow(controller).to receive(:project).and_return(project) - allow(project).to receive(:prometheus_service).and_return(prometheus_service) + allow(project).to receive(:prometheus_service).and_return(prometheus_adapter) project.add_master(user) sign_in(user) end - describe 'GET #active_metrics' do + describe 'GET #active_common' do context 'when prometheus metrics are enabled' do context 'when data is not present' do before do - allow(prometheus_service).to receive(:matched_metrics).and_return({}) + allow(prometheus_adapter).to receive(:query).with(:matched_metrics).and_return({}) end it 'returns no content response' do - get :active_metrics, project_params(format: :json) + get :active_common, project_params(format: :json) expect(response).to have_gitlab_http_status(204) end @@ -32,11 +32,11 @@ let(:sample_response) { { some_data: 1 } } before do - allow(prometheus_service).to receive(:matched_metrics).and_return(sample_response) + allow(prometheus_adapter).to receive(:query).with(:matched_metrics).and_return(sample_response) end it 'returns no content response' do - get :active_metrics, project_params(format: :json) + get :active_common, project_params(format: :json) expect(response).to have_gitlab_http_status(200) expect(json_response).to eq(sample_response.deep_stringify_keys) @@ -45,7 +45,7 @@ context 'when requesting non json response' do it 'returns not found response' do - get :active_metrics, project_params + get :active_common, project_params expect(response).to have_gitlab_http_status(404) end diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb index 0697cb2def64..c7169717fc1f 100644 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb @@ -7,7 +7,7 @@ include_examples 'additional metrics query' do let(:deployment) { create(:deployment, environment: environment) } - let(:query_params) { [environment.id, deployment.id] } + let(:query_params) { [deployment.id] } it 'queries using specific time' do expect(client).to receive(:query_range).with(anything, diff --git a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb index 84dc31d97322..ffe3ad85baa3 100644 --- a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb @@ -31,7 +31,7 @@ expect(client).to receive(:query).with('avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="environment-slug"}[30m])) * 100', time: stop_time) - expect(subject.query(environment.id, deployment.id)).to eq(memory_values: nil, memory_before: nil, memory_after: nil, - cpu_values: nil, cpu_before: nil, cpu_after: nil) + expect(subject.query(deployment.id)).to eq(memory_values: nil, memory_before: nil, memory_after: nil, + cpu_values: nil, cpu_before: nil, cpu_after: nil) end end diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metric_query_spec.rb similarity index 93% rename from spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb rename to spec/lib/gitlab/prometheus/queries/matched_metric_query_spec.rb index 2b488101496b..420218a695a5 100644 --- a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/matched_metric_query_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Prometheus::Queries::MatchedMetricsQuery do +describe Gitlab::Prometheus::Queries::MatchedMetricQuery do include Prometheus::MetricBuilders let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } @@ -24,7 +24,7 @@ def series_info_with_environment(*more_metrics) context 'with one group where two metrics is found' do before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group]) allow(client).to receive(:label_values).and_return(metric_names) end @@ -70,7 +70,7 @@ def series_info_with_environment(*more_metrics) context 'with one group where only one metric is found' do before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group]) allow(client).to receive(:label_values).and_return('metric_a') end @@ -99,7 +99,7 @@ def series_info_with_environment(*more_metrics) let(:second_metric_group) { simple_metric_group(name: 'nameb', metrics: simple_metrics(added_metric_name: 'metric_c')) } before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group, second_metric_group]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group, second_metric_group]) allow(client).to receive(:label_values).and_return('metric_c') end diff --git a/spec/lib/gitlab/prometheus_client_spec.rb b/spec/lib/gitlab/prometheus_client_spec.rb index 5d86007f71fe..4c3b8deefb91 100644 --- a/spec/lib/gitlab/prometheus_client_spec.rb +++ b/spec/lib/gitlab/prometheus_client_spec.rb @@ -19,41 +19,41 @@ # - execute_query: A query call shared_examples 'failure response' do context 'when request returns 400 with an error message' do - it 'raises a Gitlab::PrometheusError error' do + it 'raises a Gitlab::PrometheusClient::Error error' do req_stub = stub_prometheus_request(query_url, status: 400, body: { error: 'bar!' }) expect { execute_query } - .to raise_error(Gitlab::PrometheusError, 'bar!') + .to raise_error(Gitlab::PrometheusClient::Error, 'bar!') expect(req_stub).to have_been_requested end end context 'when request returns 400 without an error message' do - it 'raises a Gitlab::PrometheusError error' do + it 'raises a Gitlab::PrometheusClient::Error error' do req_stub = stub_prometheus_request(query_url, status: 400) expect { execute_query } - .to raise_error(Gitlab::PrometheusError, 'Bad data received') + .to raise_error(Gitlab::PrometheusClient::Error, 'Bad data received') expect(req_stub).to have_been_requested end end context 'when request returns 500' do - it 'raises a Gitlab::PrometheusError error' do + it 'raises a Gitlab::PrometheusClient::Error error' do req_stub = stub_prometheus_request(query_url, status: 500, body: { message: 'FAIL!' }) expect { execute_query } - .to raise_error(Gitlab::PrometheusError, '500 - {"message":"FAIL!"}') + .to raise_error(Gitlab::PrometheusClient::Error, '500 - {"message":"FAIL!"}') expect(req_stub).to have_been_requested end end context 'when request returns non json data' do - it 'raises a Gitlab::PrometheusError error' do + it 'raises a Gitlab::PrometheusClient::Error error' do req_stub = stub_prometheus_request(query_url, status: 200, body: 'not json') expect { execute_query } - .to raise_error(Gitlab::PrometheusError, 'Parsing response failed') + .to raise_error(Gitlab::PrometheusClient::Error, 'Parsing response failed') expect(req_stub).to have_been_requested end end @@ -65,27 +65,27 @@ subject { described_class.new(RestClient::Resource.new(prometheus_url)) } context 'exceptions are raised' do - it 'raises a Gitlab::PrometheusError error when a SocketError is rescued' do + it 'raises a Gitlab::PrometheusClient::Error error when a SocketError is rescued' do req_stub = stub_prometheus_request_with_exception(prometheus_url, SocketError) expect { subject.send(:get, '/', {}) } - .to raise_error(Gitlab::PrometheusError, "Can't connect to #{prometheus_url}") + .to raise_error(Gitlab::PrometheusClient::Error, "Can't connect to #{prometheus_url}") expect(req_stub).to have_been_requested end - it 'raises a Gitlab::PrometheusError error when a SSLError is rescued' do + it 'raises a Gitlab::PrometheusClient::Error error when a SSLError is rescued' do req_stub = stub_prometheus_request_with_exception(prometheus_url, OpenSSL::SSL::SSLError) expect { subject.send(:get, '/', {}) } - .to raise_error(Gitlab::PrometheusError, "#{prometheus_url} contains invalid SSL data") + .to raise_error(Gitlab::PrometheusClient::Error, "#{prometheus_url} contains invalid SSL data") expect(req_stub).to have_been_requested end - it 'raises a Gitlab::PrometheusError error when a RestClient::Exception is rescued' do + it 'raises a Gitlab::PrometheusClient::Error error when a RestClient::Exception is rescued' do req_stub = stub_prometheus_request_with_exception(prometheus_url, RestClient::Exception) expect { subject.send(:get, '/', {}) } - .to raise_error(Gitlab::PrometheusError, "Network connection error") + .to raise_error(Gitlab::PrometheusClient::Error, "Network connection error") expect(req_stub).to have_been_requested end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 01037919530a..959fb3386720 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -32,11 +32,11 @@ end end - describe '#proxy_client' do + describe '#prometheus_client' do context 'cluster is nil' do it 'returns nil' do expect(subject.cluster).to be_nil - expect(subject.proxy_client).to be_nil + expect(subject.prometheus_client).to be_nil end end @@ -45,7 +45,7 @@ subject { create(:clusters_applications_prometheus, cluster: cluster) } it 'returns nil' do - expect(subject.proxy_client).to be_nil + expect(subject.prometheus_client).to be_nil end end @@ -73,15 +73,15 @@ end it 'creates proxy prometheus rest client' do - expect(subject.proxy_client).to be_instance_of(RestClient::Resource) + expect(subject.prometheus_client).to be_instance_of(RestClient::Resource) end it 'creates proper url' do - expect(subject.proxy_client.url).to eq('http://example.com/api/v1/proxy/namespaces/gitlab-managed-apps/service/prometheus-prometheus-server:80') + expect(subject.prometheus_client.url).to eq('http://example.com/api/v1/proxy/namespaces/gitlab-managed-apps/service/prometheus-prometheus-server:80') end it 'copies options and headers from kube client to proxy client' do - expect(subject.proxy_client.options).to eq(kube_client.rest_client.options.merge(headers: kube_client.headers)) + expect(subject.prometheus_client.options).to eq(kube_client.rest_client.options.merge(headers: kube_client.headers)) end end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index ba8aa13d5adf..ac30cd27e0c9 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -64,6 +64,7 @@ describe '#metrics' do let(:deployment) { create(:deployment) } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } subject { deployment.metrics } @@ -76,17 +77,16 @@ { success: true, metrics: {}, - last_update: 42, - deployment_time: 1494408956 + last_update: 42 } end before do - allow(deployment.project).to receive_message_chain(:monitoring_service, :deployment_metrics) - .with(any_args).and_return(simple_metrics) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) + allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) end - it { is_expected.to eq(simple_metrics) } + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } end end @@ -109,11 +109,11 @@ } end - let(:prometheus_service) { double('prometheus_service') } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do - allow(project).to receive(:prometheus_service).and_return(prometheus_service) - allow(prometheus_service).to receive(:additional_deployment_metrics).and_return(simple_metrics) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) + allow(prometheus_adapter).to receive(:query).with(:additional_metrics_deployment, deployment).and_return(simple_metrics) end it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index b6a6a521f79f..6265d4907b25 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -553,8 +553,8 @@ end it 'returns the metrics from the deployment service' do - expect(project.monitoring_service) - .to receive(:environment_metrics).with(environment) + expect(environment.prometheus_adapter) + .to receive(:query).with(:environment, environment) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) @@ -609,12 +609,12 @@ context 'when the environment has additional metrics' do before do - allow(environment).to receive(:has_additional_metrics?).and_return(true) + allow(environment).to receive(:has_metrics?).and_return(true) end it 'returns the additional metrics from the deployment service' do - expect(project.prometheus_service).to receive(:additional_environment_metrics) - .with(environment) + expect(environment.prometheus_adapter).to receive(:query) + .with(:additional_metrics_environment, environment) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) @@ -623,46 +623,13 @@ context 'when the environment does not have metrics' do before do - allow(environment).to receive(:has_additional_metrics?).and_return(false) + allow(environment).to receive(:has_metrics?).and_return(false) end it { is_expected.to be_nil } end end - describe '#has_additional_metrics??' do - subject { environment.has_additional_metrics? } - - context 'when the enviroment is available' do - context 'with a deployment service' do - let(:project) { create(:prometheus_project) } - - context 'and a deployment' do - let!(:deployment) { create(:deployment, environment: environment) } - it { is_expected.to be_truthy } - end - - context 'but no deployments' do - it { is_expected.to be_falsy } - end - end - - context 'without a monitoring service' do - it { is_expected.to be_falsy } - end - end - - context 'when the environment is unavailable' do - let(:project) { create(:prometheus_project) } - - before do - environment.stop - end - - it { is_expected.to be_falsy } - end - end - describe '#slug' do it "is automatically generated" do expect(environment.slug).not_to be_nil @@ -755,4 +722,77 @@ end end end + + describe '#prometheus_adapter' do + let!(:cluster_for_all) { create(:cluster, environment_scope: '*', projects: [project]) } + let!(:cluster_for_dev) { create(:cluster, environment_scope: 'dev', projects: [project]) } + + let!(:prometheus_for_dev) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_dev) } + + context 'prometheus service can execute queries' do + let(:prometheus_service) { double(:prometheus_service, can_query?: true) } + + before do + allow(environment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it 'return prometheus service as prometheus adapter' do + expect(environment.prometheus_adapter).to eq(prometheus_service) + end + end + + context "prometheus service can't execute queries" do + let(:prometheus_service) { double(:prometheus_service, can_query?: false) } + + context 'with cluster for all environments with prometheus installed' do + let!(:prometheus_for_all) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_all) } + + context 'without environment supplied' do + it 'returns application handling all environments' do + expect(environment.prometheus_adapter).to eq(prometheus_for_all) + end + end + + context 'with dev environment supplied' do + let!(:environment) { create(:environment, project: project, name: 'dev') } + + it 'returns dev cluster prometheus application' do + expect(environment.prometheus_adapter).to eq(prometheus_for_dev) + end + end + + context 'with prod environment supplied' do + let!(:environment) { create(:environment, project: project, name: 'prod') } + + it 'returns application handling all environments' do + expect(environment.prometheus_adapter).to eq(prometheus_for_all) + end + end + end + + context 'with cluster for all environments without prometheus installed' do + context 'without environment supplied' do + it 'returns nil' do + expect(environment.prometheus_adapter).to be_nil + end + end + + context 'with dev environment supplied' do + let!(:environment) { create(:environment, project: project, name: 'dev') } + + it 'returns dev cluster prometheus application' do + expect(environment.prometheus_adapter).to eq(prometheus_for_dev) + end + end + + context 'with prod environment supplied' do + let!(:environment) { create(:environment, project: project, name: 'prod') } + + it 'returns nil' do + expect(environment.prometheus_adapter).to be_nil + end + end + end + end + end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index ed17e019d429..c5f985d6ba94 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -63,7 +63,7 @@ end context 'with valid data' do - subject { service.environment_metrics(environment) } + subject { service.query(:environment, environment) } before do stub_reactive_cache(service, prometheus_data, environment_query, environment.id) @@ -76,14 +76,14 @@ end describe '#matched_metrics' do - let(:matched_metrics_query) { Gitlab::Prometheus::Queries::MatchedMetricsQuery } - let(:client) { double(:client, label_values: nil) } + let(:matched_metrics_query) { Gitlab::Prometheus::Queries::MatchedMetricQuery } + let(:prometheus_client_wrapper) { double(:prometheus_client_wrapper, label_values: nil) } context 'with valid data' do - subject { service.matched_metrics } + subject { service.query(:matched_metrics) } before do - allow(service).to receive(:client).and_return(client) + allow(service).to receive(:prometheus_client_wrapper).and_return(prometheus_client_wrapper) synchronous_reactive_cache(service) end @@ -103,17 +103,14 @@ end context 'with valid data' do - subject { service.deployment_metrics(deployment) } - let(:fake_deployment_time) { 10 } + subject { service.query(:deployment, deployment) } before do - stub_reactive_cache(service, prometheus_data, deployment_query, deployment.environment.id, deployment.id) + stub_reactive_cache(service, prometheus_data, deployment_query, deployment.id) end it 'returns reactive data' do - expect(deployment).to receive(:created_at).and_return(fake_deployment_time) - - expect(subject).to eq(prometheus_metrics_data.merge(deployment_time: fake_deployment_time)) + expect(subject).to eq(prometheus_metrics_data) end end end @@ -161,91 +158,31 @@ end end - describe '#client' do + describe '#prometheus_client' do context 'manual configuration is enabled' do let(:api_url) { 'http://some_url' } + before do + subject.active = true subject.manual_configuration = true subject.api_url = api_url end - it 'returns simple rest client from api_url' do - expect(subject.client).to be_instance_of(Gitlab::PrometheusClient) - expect(subject.client.rest_client.url).to eq(api_url) + it 'returns rest client from api_url' do + expect(subject.prometheus_client.url).to eq(api_url) end end context 'manual configuration is disabled' do - let!(:cluster_for_all) { create(:cluster, environment_scope: '*', projects: [project]) } - let!(:cluster_for_dev) { create(:cluster, environment_scope: 'dev', projects: [project]) } - - let!(:prometheus_for_dev) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_dev) } - let(:proxy_client) { double('proxy_client') } + let(:api_url) { 'http://some_url' } before do - service.manual_configuration = false - end - - context 'with cluster for all environments with prometheus installed' do - let!(:prometheus_for_all) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_all) } - - context 'without environment supplied' do - it 'returns client handling all environments' do - expect(service).to receive(:client_from_cluster).with(cluster_for_all).and_return(proxy_client).twice - - expect(service.client).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client.rest_client).to eq(proxy_client) - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } - - it 'returns dev cluster client' do - expect(service).to receive(:client_from_cluster).with(cluster_for_dev).and_return(proxy_client).twice - - expect(service.client(environment.id)).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client(environment.id).rest_client).to eq(proxy_client) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'returns dev cluster client' do - expect(service).to receive(:client_from_cluster).with(cluster_for_all).and_return(proxy_client).twice - - expect(service.client(environment.id)).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client(environment.id).rest_client).to eq(proxy_client) - end - end + subject.manual_configuration = false + subject.api_url = api_url end - context 'with cluster for all environments without prometheus installed' do - context 'without environment supplied' do - it 'raises PrometheusError because cluster was not found' do - expect { service.client }.to raise_error(Gitlab::PrometheusError, /couldn't find cluster with Prometheus installed/) - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } - - it 'returns dev cluster client' do - expect(service).to receive(:client_from_cluster).with(cluster_for_dev).and_return(proxy_client).twice - - expect(service.client(environment.id)).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client(environment.id).rest_client).to eq(proxy_client) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'raises PrometheusError because cluster was not found' do - expect { service.client }.to raise_error(Gitlab::PrometheusError, /couldn't find cluster with Prometheus installed/) - end - end + it 'no client provided' do + expect(subject.prometheus_client).to be_nil end end end diff --git a/spec/support/prometheus/additional_metrics_shared_examples.rb b/spec/support/prometheus/additional_metrics_shared_examples.rb index dbbd4ad4d408..c7c3346d39e7 100644 --- a/spec/support/prometheus/additional_metrics_shared_examples.rb +++ b/spec/support/prometheus/additional_metrics_shared_examples.rb @@ -12,11 +12,12 @@ let(:client) { double('prometheus_client') } let(:query_result) { described_class.new(client).query(*query_params) } - let(:environment) { create(:environment, slug: 'environment-slug') } + let(:project) { create(:project) } + let(:environment) { create(:environment, slug: 'environment-slug', project: project) } before do allow(client).to receive(:label_values).and_return(metric_names) - allow(metric_group_class).to receive(:all).and_return([simple_metric_group(metrics: [simple_metric])]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group(metrics: [simple_metric])]) end context 'metrics query context' do @@ -24,13 +25,14 @@ shared_examples 'query context containing environment slug and filter' do it 'contains ci_environment_slug' do - expect(subject).to receive(:query_metrics).with(hash_including(ci_environment_slug: environment.slug)) + expect(subject).to receive(:query_metrics).with(project, hash_including(ci_environment_slug: environment.slug)) subject.query(*query_params) end it 'contains environment filter' do expect(subject).to receive(:query_metrics).with( + project, hash_including( environment_filter: "container_name!=\"POD\",environment=\"#{environment.slug}\"" ) @@ -48,7 +50,7 @@ it_behaves_like 'query context containing environment slug and filter' it 'query context contains kube_namespace' do - expect(subject).to receive(:query_metrics).with(hash_including(kube_namespace: kube_namespace)) + expect(subject).to receive(:query_metrics).with(project, hash_including(kube_namespace: kube_namespace)) subject.query(*query_params) end @@ -72,7 +74,7 @@ it_behaves_like 'query context containing environment slug and filter' it 'query context contains empty kube_namespace' do - expect(subject).to receive(:query_metrics).with(hash_including(kube_namespace: '')) + expect(subject).to receive(:query_metrics).with(project, hash_including(kube_namespace: '')) subject.query(*query_params) end @@ -81,7 +83,7 @@ context 'with one group where two metrics is found' do before do - allow(metric_group_class).to receive(:all).and_return([simple_metric_group]) + allow(metric_group_class).to receive(:common_metrics).and_return([simple_metric_group]) end context 'some queries return results' do @@ -117,7 +119,7 @@ let(:metrics) { [simple_metric(queries: [simple_query])] } before do - allow(metric_group_class).to receive(:all).and_return( + allow(metric_group_class).to receive(:common_metrics).and_return( [ simple_metric_group(name: 'group_a', metrics: [simple_metric(queries: [simple_query])]), simple_metric_group(name: 'group_b', metrics: [simple_metric(title: 'title_b', queries: [simple_query('b')])]) -- GitLab From 57de968a92f84a15736bbe37a5804fee7c47778c Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 23 Feb 2018 21:33:33 +0100 Subject: [PATCH 02/49] Squashed commit of the following: commit 22e1cb8f4b98f71d21026f69aa3e68d79946d6ae Merge: 766a42a9639 e966c6aea25 Author: Pawel Chojnacki Date: Fri Feb 23 21:30:46 2018 +0100 Merge remote-tracking branch 'upstream/master' into 38783-add-cluster-metrics.yml # Conflicts: # app/controllers/projects/prometheus/metrics_controller.rb # app/controllers/projects/prometheus_controller.rb # app/models/project_services/prometheus_service.rb # lib/gitlab/prometheus/queries/query_additional_metrics.rb # spec/controllers/projects/prometheus/metrics_controller_spec.rb # spec/models/project_services/prometheus_service_spec.rb commit 766a42a96393f502d439c1f0beb0b6cfb2c228d4 Author: Pawel Chojnacki Date: Fri Feb 23 17:58:45 2018 +0100 Move prometheus adapter to app/models/concerns commit ca84eed49811cf3064a2e5ea611af1c947c590d7 Merge: 66702099586 cb504cedc2a Author: Pawel Chojnacki Date: Fri Feb 23 15:36:03 2018 +0100 Merge remote-tracking branch 'upstream/backport_custom_metrics_ce_components' into 38783-add-cluster-metrics.yml + fix failing tests # Conflicts: # app/controllers/projects/prometheus_controller.rb # app/models/project_services/prometheus_service.rb # lib/gitlab/prometheus/queries/query_additional_metrics.rb # spec/models/project_services/prometheus_service_spec.rb commit cb504cedc2a6e353ffb56833334681e3da09fc14 Author: Pawel Chojnacki Date: Fri Feb 23 14:46:15 2018 +0100 Fix prometheus_service found by find_or_initialize_service commit 928b84c72c2c4c46e1785b9a943c1822a137de16 Author: Pawel Chojnacki Date: Thu Feb 22 23:15:11 2018 +0100 additional metrics and backported tests commit 41291383b4b6976af94eaf9e1a7b2e2a172310e7 Author: Pawel Chojnacki Date: Thu Feb 22 21:57:45 2018 +0100 Introduce Metrics controller and retire prometheus controller commit 6e7492e4c7ffa9d8621f09198071bc14be875976 Author: Pawel Chojnacki Date: Thu Feb 22 21:46:37 2018 +0100 Backport PrometheusClient::Error and all->common_metrics rename commit 66702099586c864a7f78970a0ee0dd9f3c2beeef Author: Pawel Chojnacki Date: Thu Feb 22 20:23:03 2018 +0100 fix failing tests commit ce921ea723cde61b2441ba98c9aca97606c719c8 Author: Mike Greiling Date: Wed Feb 21 23:47:30 2018 -0600 add labels to cluster metrics commit 993830c6892e4fbf53d3f72da00002d642c7e9b2 Author: Pawel Chojnacki Date: Thu Feb 22 01:20:49 2018 +0100 Fix formatting probelms and few small tests commit 66ee65d8e3f747c90d986bc3056178422156bc8e Author: Pawel Chojnacki Date: Thu Feb 22 01:08:03 2018 +0100 stop using in deployment tests environment.id commit 62c91978d15f0369988521363dae24bd7510d68d Author: Pawel Chojnacki Date: Thu Feb 22 01:06:33 2018 +0100 fix prometheus_controller and adapter tests commit 977b1d34c1d03c7233582e8328f85caf634895ed Author: Pawel Chojnacki Date: Thu Feb 22 00:19:39 2018 +0100 finish up active? -> can_query? rename commit e614f7daee58a9758d83ba3efe1649c8b80bc1e2 Author: Pawel Chojnacki Date: Wed Feb 21 23:51:33 2018 +0100 deployment prometheus adapter tests fix commit ebd726c114a6026fef0adf3eba6ee1972530148a Author: Pawel Chojnacki Date: Wed Feb 21 21:40:40 2018 +0100 Move environment dependant tests to environment commit 6d31311cd3729c29233283dded70e03a4a9a3c97 Author: Pawel Chojnacki Date: Wed Feb 21 20:40:24 2018 +0100 update monitoring service and move adding dpeloyment_time to deployment model commit 60b6bf391ab36846dce122bc6b0c5196a186267c Author: Pawel Chojnacki Date: Wed Feb 21 20:15:46 2018 +0100 adjust deployment spec and prometheus specs commit 6681662cf1c028aff2ff94aa0501732cb7119ba1 Author: Pawel Chojnacki Date: Tue Feb 20 22:30:20 2018 +0100 Revert changes to reactive caching commit e282f86c45a056889f57d3f7fd23a81c88efff6a Merge: 5751c73df59 6844a2df873 Author: Pawel Chojnacki Date: Tue Feb 20 22:08:17 2018 +0100 Merge remote-tracking branch 'upstream/master' into 38783-add-cluster-metrics.yml commit 5751c73df59d0a03840a1b4b71b0637670f971a6 Author: Pawel Chojnacki Date: Tue Feb 20 22:07:43 2018 +0100 rename active? to can_query? and cleanup environment prometheus router commit 3f3c6e1d33dcd9315979daf26a95f2aab83a7de9 Author: Pawel Chojnacki Date: Tue Feb 20 21:51:43 2018 +0100 Fix tests, and only use prometheus service if its active commit 6345838bac584c213b665d334252ccab202cb271 Author: Pawel Chojnacki Date: Tue Feb 20 16:24:27 2018 +0100 Fix typo commit 7a585d32afe8da050b5615b1d036a550e06479f5 Author: Pawel Chojnacki Date: Tue Feb 20 15:29:40 2018 +0100 Cluster id is not required commit e6af62afb11fa380f6aff1c31a81bcc9bab3b1eb Author: Pawel Chojnacki Date: Tue Feb 20 15:15:46 2018 +0100 Result transformation support commit f3b1bd7c67894f44efe33591ddb70093bd620c03 Author: Pawel Chojnacki Date: Tue Feb 20 15:07:07 2018 +0100 Fix rubocop warning and exten cluster query timeframe commit be77947cea64261a4d3dead33c3c57f413a9880c Author: Pawel Chojnacki Date: Tue Feb 20 14:42:47 2018 +0100 Fix additional metrics test commit eb3922e16221abe16f59fae1c38122f227643343 Author: Pawel Chojnacki Date: Tue Feb 20 14:34:44 2018 +0100 rename prometheus adapter methods commit 045476cd08b21593818b274ae8a44d19b705523f Author: Pawel Chojnacki Date: Tue Feb 20 13:19:36 2018 +0100 Make prometheus adapter a module commit f2daf050d8c689f72c4c61207930bc53c331f12e Author: Pawel Chojnacki Date: Tue Feb 20 12:00:44 2018 +0100 refactoring wip commit 52e4ef5587794e811dc10a0f2dca522342a865da Author: Pawel Chojnacki Date: Sun Feb 18 20:25:55 2018 +0100 cleanup prometheus adapter concept commit 3887365faab9dfcd9c00bcfc501d09ac62431a03 Author: Pawel Chojnacki Date: Sun Feb 18 19:22:11 2018 +0100 Refactor out deployment id, Rename PrometheusQuerier to PrometheusAdapter commit aa2fc2df57bd72c9a5e94f66d1f1e23990be6c3f Author: Pawel Chojnacki Date: Fri Feb 16 23:59:54 2018 +0100 Refactor prometheus client commit e43c1ca9d9874d6cf1569f40fa1aca158d9d5d91 Author: Pawel Chojnacki Date: Thu Feb 15 20:24:45 2018 +0100 Use initial version of cluster_metrics.yml commit 867821ce0b2609ebf8994220aa8e3a94d66a01e0 Author: Pawel Chojnacki Date: Thu Feb 15 05:24:14 2018 +0100 Fix querying cluster metrics commit 1601e002a064cbb10ffe110a19433c5662858f1d Author: Pawel Chojnacki Date: Thu Feb 15 04:49:03 2018 +0100 Queues for unicersal querier commit 5db198fdc925c0223be24939b76da1d544dd569c Author: Pawel Chojnacki Date: Thu Feb 15 03:35:17 2018 +0100 refactor reactive caching and prometheus querying commit b0fc00e8c9d21e961ef44b0129103e2a62928b52 Author: Pawel Chojnacki Date: Thu Feb 15 00:12:03 2018 +0100 Add generic query additional metrics commit ffe76e6a9ba196bccff22c4880e0384959ad5e48 Author: Pawel Chojnacki Date: Tue Feb 13 15:40:15 2018 +0100 Cluster Metric yml initial + Remove cluster query + remove cluster_metrics.yml + Prometheus adapter tests --- .../monitoring/utils/multiple_time_series.js | 2 +- .../projects/deployments_controller.rb | 2 +- .../projects/prometheus/metrics_controller.rb | 11 +- .../clusters/applications/prometheus.rb | 4 +- app/models/clusters/cluster.rb | 1 - app/models/clusters/platforms/kubernetes.rb | 2 +- app/models/concerns/prometheus_adapter.rb | 48 +++++ app/models/deployment.rb | 17 +- app/models/environment.rb | 33 +++- .../project_services/monitoring_service.rb | 4 +- .../project_services/prometheus_service.rb | 85 +------- .../prometheus/additional_metrics_parser.rb | 21 +- .../additional_metrics_deployment_query.rb | 2 +- lib/gitlab/prometheus/queries/base_query.rb | 4 + .../prometheus/queries/deployment_query.rb | 7 +- .../prometheus/queries/environment_query.rb | 5 + ...trics_query.rb => matched_metric_query.rb} | 2 +- .../queries/query_additional_metrics.rb | 22 ++- lib/gitlab/prometheus_client.rb | 6 +- .../projects/deployments_controller_spec.rb | 4 +- .../prometheus/metrics_controller_spec.rb | 8 +- ...dditional_metrics_deployment_query_spec.rb | 2 +- .../queries/deployment_query_spec.rb | 4 +- ...y_spec.rb => matched_metric_query_spec.rb} | 2 +- .../clusters/applications/prometheus_spec.rb | 12 +- .../concerns/prometheus_adapter_spec.rb | 121 ++++++++++++ spec/models/deployment_spec.rb | 16 +- spec/models/environment_spec.rb | 118 +++++++---- .../prometheus_service_spec.rb | 187 +----------------- 29 files changed, 388 insertions(+), 364 deletions(-) create mode 100644 app/models/concerns/prometheus_adapter.rb rename lib/gitlab/prometheus/queries/{matched_metrics_query.rb => matched_metric_query.rb} (98%) rename spec/lib/gitlab/prometheus/queries/{matched_metrics_query_spec.rb => matched_metric_query_spec.rb} (98%) create mode 100644 spec/models/concerns/prometheus_adapter_spec.rb diff --git a/app/assets/javascripts/monitoring/utils/multiple_time_series.js b/app/assets/javascripts/monitoring/utils/multiple_time_series.js index 4ce3dad440c0..b5b8e3c255df 100644 --- a/app/assets/javascripts/monitoring/utils/multiple_time_series.js +++ b/app/assets/javascripts/monitoring/utils/multiple_time_series.js @@ -76,7 +76,7 @@ function queryTimeSeries(query, graphWidth, graphHeight, graphHeightOffset, xDom metricTag = seriesCustomizationData.value || timeSeriesMetricLabel; [lineColor, areaColor] = pickColor(seriesCustomizationData.color); } else { - metricTag = timeSeriesMetricLabel || `series ${timeSeriesNumber + 1}`; + metricTag = timeSeriesMetricLabel || query.label || `series ${timeSeriesNumber + 1}`; [lineColor, areaColor] = pickColor(); } diff --git a/app/controllers/projects/deployments_controller.rb b/app/controllers/projects/deployments_controller.rb index 1a418d0f15a1..b68cdc39cb8d 100644 --- a/app/controllers/projects/deployments_controller.rb +++ b/app/controllers/projects/deployments_controller.rb @@ -24,7 +24,7 @@ def metrics end def additional_metrics - return render_404 unless deployment.has_additional_metrics? + return render_404 unless deployment.has_metrics? respond_to do |format| format.json do diff --git a/app/controllers/projects/prometheus/metrics_controller.rb b/app/controllers/projects/prometheus/metrics_controller.rb index b739d0f0f907..255fbf254fba 100644 --- a/app/controllers/projects/prometheus/metrics_controller.rb +++ b/app/controllers/projects/prometheus/metrics_controller.rb @@ -2,11 +2,12 @@ module Projects module Prometheus class MetricsController < Projects::ApplicationController before_action :authorize_admin_project! + before_action :require_prometheus_metrics! def active_common respond_to do |format| format.json do - matched_metrics = prometheus_service.matched_metrics || {} + matched_metrics = prometheus_adapter.query(:matched_metrics) || {} if matched_metrics.any? render json: matched_metrics @@ -19,8 +20,12 @@ def active_common private - def prometheus_service - @prometheus_service ||= project.find_or_initialize_service('prometheus') + def prometheus_adapter + project.prometheus_service + end + + def require_prometheus_metrics! + render_404 unless prometheus_adapter.can_query? end end end diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index aa22e9d5d581..ba63d0f3c64f 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -1,6 +1,8 @@ module Clusters module Applications class Prometheus < ActiveRecord::Base + include PrometheusAdapter + VERSION = "2.0.0".freeze self.table_name = 'clusters_applications_prometheus' @@ -38,7 +40,7 @@ def install_command Gitlab::Kubernetes::Helm::InstallCommand.new(name, chart: chart, chart_values_file: chart_values_file) end - def proxy_client + def prometheus_client return unless kube_client proxy_url = kube_client.proxy_url('service', service_name, service_port, Gitlab::Kubernetes::Helm::NAMESPACE) diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 8678f70f78ca..1fd7db301072 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -50,7 +50,6 @@ class Cluster < ActiveRecord::Base scope :disabled, -> { where(enabled: false) } scope :for_environment, -> (env) { where(environment_scope: ['*', '', env.slug]) } - scope :for_all_environments, -> { where(environment_scope: ['*', '']) } def status_name if provider diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 7ce8befeeeb4..66f629df1b41 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -141,7 +141,7 @@ def read_pods end def kubeclient_ssl_options - opts = { verify_ssl: OpenSSL::SSL::VERIFY_PEER } + opts = { verify_ssl: OpenSSL::SSL::VERIFY_NONE } if ca_pem.present? opts[:cert_store] = OpenSSL::X509::Store.new diff --git a/app/models/concerns/prometheus_adapter.rb b/app/models/concerns/prometheus_adapter.rb new file mode 100644 index 000000000000..fea99a7cd35a --- /dev/null +++ b/app/models/concerns/prometheus_adapter.rb @@ -0,0 +1,48 @@ +module PrometheusAdapter + extend ActiveSupport::Concern + + included do + include ReactiveCaching + + self.reactive_cache_key = ->(adapter) { [adapter.class.model_name.singular, adapter.id] } + self.reactive_cache_lease_timeout = 30.seconds + self.reactive_cache_refresh_interval = 30.seconds + self.reactive_cache_lifetime = 1.minute + + def prometheus_client + raise NotImplementedError + end + + def prometheus_client_wrapper + Gitlab::PrometheusClient.new(prometheus_client) + end + + def can_query? + prometheus_client.present? + end + + def query(query_name, *args) + return unless can_query? + + query_class = Gitlab::Prometheus::Queries.const_get("#{query_name.to_s.classify}Query") + + args.map! { |arg| arg.id } + + with_reactive_cache(query_class.name, *args, &query_class.method(:transform_reactive_result)) + end + + # Cache metrics for specific environment + def calculate_reactive_cache(query_class_name, *args) + return unless prometheus_client + + data = Kernel.const_get(query_class_name).new(prometheus_client_wrapper).query(*args) + { + success: true, + data: data, + last_update: Time.now.utc + } + rescue Gitlab::PrometheusClient::Error => err + { success: false, result: err.message } + end + end +end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index b6cf168d60e3..66e61c067654 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -98,28 +98,29 @@ def formatted_deployment_time end def has_metrics? - project.monitoring_service.present? + prometheus_adapter&.can_query? end def metrics return {} unless has_metrics? - project.monitoring_service.deployment_metrics(self) - end - - def has_additional_metrics? - project.prometheus_service.present? + metrics = prometheus_adapter.query(:deployment, self) + metrics&.merge(deployment_time: created_at.to_i) || {} end def additional_metrics - return {} unless project.prometheus_service.present? + return {} unless has_metrics? - metrics = project.prometheus_service.additional_deployment_metrics(self) + metrics = prometheus_adapter.query(:additional_metrics_deployment, self) metrics&.merge(deployment_time: created_at.to_i) || {} end private + def prometheus_adapter + environment.prometheus_adapter + end + def ref_path File.join(environment.ref_path, 'deployments', iid.to_s) end diff --git a/app/models/environment.rb b/app/models/environment.rb index f78c21aebe5b..2a91bd07b288 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -146,21 +146,15 @@ def terminals end def has_metrics? - project.monitoring_service.present? && available? && last_deployment.present? + prometheus_adapter&.can_query? && available? && last_deployment.present? end def metrics - project.monitoring_service.environment_metrics(self) if has_metrics? - end - - def has_additional_metrics? - project.prometheus_service.present? && available? && last_deployment.present? + prometheus_adapter.query(:environment, self) if has_metrics? end def additional_metrics - if has_additional_metrics? - project.prometheus_service.additional_environment_metrics(self) - end + prometheus_adapter.query(:additional_metrics_environment, self) if has_metrics? end def slug @@ -226,6 +220,27 @@ def folder_name self.environment_type || self.name end + def prometheus_adapter + @prometheus_adapter ||= if service_prometheus_adapter.can_query? + service_prometheus_adapter + else + cluster_prometheus_adapter + end + end + + def service_prometheus_adapter + project.find_or_initialize_service('prometheus') + end + + def cluster_prometheus_adapter + # sort results by descending order based on environment_scope being longer + # thus more closely matching environment slug + clusters = project.clusters.enabled.for_environment(self).sort_by { |c| c.environment_scope&.length }.reverse! + + cluster = clusters&.detect { |cluster| cluster.application_prometheus&.installed? } + cluster&.application_prometheus + end + private # Slugifying a name may remove the uniqueness guarantee afforded by it being diff --git a/app/models/project_services/monitoring_service.rb b/app/models/project_services/monitoring_service.rb index ee9cd78327a8..9af68b4e8210 100644 --- a/app/models/project_services/monitoring_service.rb +++ b/app/models/project_services/monitoring_service.rb @@ -9,11 +9,11 @@ def self.supported_events %w() end - def environment_metrics(environment) + def can_query? raise NotImplementedError end - def deployment_metrics(deployment) + def query(_, *_) raise NotImplementedError end end diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 58731451429d..7a0c9a33e70a 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -1,9 +1,5 @@ class PrometheusService < MonitoringService - include ReactiveService - - self.reactive_cache_lease_timeout = 30.seconds - self.reactive_cache_refresh_interval = 30.seconds - self.reactive_cache_lifetime = 1.minute + include PrometheusAdapter # Access to prometheus is directly through the API prop_accessor :api_url @@ -66,63 +62,15 @@ def fields # Check we can connect to the Prometheus API def test(*args) - client.ping + Gitlab::PrometheusClient.new(prometheus_client).ping { success: true, result: 'Checked API endpoint' } rescue Gitlab::PrometheusClient::Error => err { success: false, result: err } end - def environment_metrics(environment) - with_reactive_cache(Gitlab::Prometheus::Queries::EnvironmentQuery.name, environment.id, &rename_field(:data, :metrics)) - end - - def deployment_metrics(deployment) - metrics = with_reactive_cache(Gitlab::Prometheus::Queries::DeploymentQuery.name, deployment.environment.id, deployment.id, &rename_field(:data, :metrics)) - metrics&.merge(deployment_time: deployment.created_at.to_i) || {} - end - - def additional_environment_metrics(environment) - with_reactive_cache(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, environment.id, &:itself) - end - - def additional_deployment_metrics(deployment) - with_reactive_cache(Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery.name, deployment.environment.id, deployment.id, &:itself) - end - - def matched_metrics - with_reactive_cache(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) - end - - # Cache metrics for specific environment - def calculate_reactive_cache(query_class_name, *args) - return unless active? && project && !project.pending_delete? - - environment_id = args.first - client = client(environment_id) - - data = Kernel.const_get(query_class_name).new(client).query(*args) - { - success: true, - data: data, - last_update: Time.now.utc - } - rescue Gitlab::PrometheusClient::Error => err - { success: false, result: err.message } - end - - def client(environment_id = nil) - if manual_configuration? - Gitlab::PrometheusClient.new(RestClient::Resource.new(api_url)) - else - cluster = cluster_with_prometheus(environment_id) - raise Gitlab::PrometheusClient::Error, "couldn't find cluster with Prometheus installed" unless cluster - - rest_client = client_from_cluster(cluster) - raise Gitlab::PrometheusClient::Error, "couldn't create proxy Prometheus client" unless rest_client - - Gitlab::PrometheusClient.new(rest_client) - end + def prometheus_client + RestClient::Resource.new(api_url) if api_url && manual_configuration? && active? end def prometheus_installed? @@ -134,31 +82,6 @@ def prometheus_installed? private - def cluster_with_prometheus(environment_id = nil) - clusters = if environment_id - ::Environment.find_by(id: environment_id).try do |env| - # sort results by descending order based on environment_scope being longer - # thus more closely matching environment slug - project.clusters.enabled.for_environment(env).sort_by { |c| c.environment_scope&.length }.reverse! - end - else - project.clusters.enabled.for_all_environments - end - - clusters&.detect { |cluster| cluster.application_prometheus&.installed? } - end - - def client_from_cluster(cluster) - cluster.application_prometheus.proxy_client - end - - def rename_field(old_field, new_field) - -> (metrics) do - metrics[new_field] = metrics.delete(old_field) - metrics - end - end - def synchronize_service_state! self.active = prometheus_installed? || manual_configuration? diff --git a/lib/gitlab/prometheus/additional_metrics_parser.rb b/lib/gitlab/prometheus/additional_metrics_parser.rb index cb95daf22602..bb1172f82a19 100644 --- a/lib/gitlab/prometheus/additional_metrics_parser.rb +++ b/lib/gitlab/prometheus/additional_metrics_parser.rb @@ -1,10 +1,12 @@ module Gitlab module Prometheus module AdditionalMetricsParser + CONFIG_ROOT = 'config/prometheus'.freeze + MUTEX = Mutex.new extend self - def load_groups_from_yaml - additional_metrics_raw.map(&method(:group_from_entry)) + def load_groups_from_yaml(file_name = 'additional_metrics.yml') + yaml_metrics_raw(file_name).map(&method(:group_from_entry)) end private @@ -22,13 +24,20 @@ def group_from_entry(entry) MetricGroup.new(entry).tap(&method(:validate!)) end - def additional_metrics_raw - load_yaml_file&.map(&:deep_symbolize_keys).freeze + def yaml_metrics_raw(file_name) + load_yaml_file(file_name)&.map(&:deep_symbolize_keys).freeze end - def load_yaml_file - @loaded_yaml_file ||= YAML.load_file(Rails.root.join('config/prometheus/additional_metrics.yml')) + # rubocop:disable Gitlab/ModuleWithInstanceVariables + def load_yaml_file(file_name) + return YAML.load_file(Rails.root.join(CONFIG_ROOT, file_name)) if Rails.env.development? + + MUTEX.synchronize do + @loaded_yaml_cache ||= {} + @loaded_yaml_cache[file_name] ||= YAML.load_file(Rails.root.join(CONFIG_ROOT, file_name)) + end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end end end diff --git a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb index 972ab75d1d5c..e677ec84cd4f 100644 --- a/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb +++ b/lib/gitlab/prometheus/queries/additional_metrics_deployment_query.rb @@ -4,7 +4,7 @@ module Queries class AdditionalMetricsDeploymentQuery < BaseQuery include QueryAdditionalMetrics - def query(environment_id, deployment_id) + def query(deployment_id) Deployment.find_by(id: deployment_id).try do |deployment| query_metrics( deployment.project, diff --git a/lib/gitlab/prometheus/queries/base_query.rb b/lib/gitlab/prometheus/queries/base_query.rb index c60828165bd9..29cab6e9c15e 100644 --- a/lib/gitlab/prometheus/queries/base_query.rb +++ b/lib/gitlab/prometheus/queries/base_query.rb @@ -20,6 +20,10 @@ def initialize(client) def query(*args) raise NotImplementedError end + + def self.transform_reactive_result(result) + result + end end end end diff --git a/lib/gitlab/prometheus/queries/deployment_query.rb b/lib/gitlab/prometheus/queries/deployment_query.rb index 6e6da593178e..c2626581897a 100644 --- a/lib/gitlab/prometheus/queries/deployment_query.rb +++ b/lib/gitlab/prometheus/queries/deployment_query.rb @@ -2,7 +2,7 @@ module Gitlab module Prometheus module Queries class DeploymentQuery < BaseQuery - def query(environment_id, deployment_id) + def query(deployment_id) Deployment.find_by(id: deployment_id).try do |deployment| environment_slug = deployment.environment.slug @@ -25,6 +25,11 @@ def query(environment_id, deployment_id) } end end + + def self.transform_reactive_result(result) + result[:metrics] = result.delete :data + result + end end end end diff --git a/lib/gitlab/prometheus/queries/environment_query.rb b/lib/gitlab/prometheus/queries/environment_query.rb index 1d17d3cfd56c..b62910c8de67 100644 --- a/lib/gitlab/prometheus/queries/environment_query.rb +++ b/lib/gitlab/prometheus/queries/environment_query.rb @@ -19,6 +19,11 @@ def query(environment_id) } end end + + def self.transform_reactive_result(result) + result[:metrics] = result.delete :data + result + end end end end diff --git a/lib/gitlab/prometheus/queries/matched_metrics_query.rb b/lib/gitlab/prometheus/queries/matched_metric_query.rb similarity index 98% rename from lib/gitlab/prometheus/queries/matched_metrics_query.rb rename to lib/gitlab/prometheus/queries/matched_metric_query.rb index 5710ad47c1a3..d920e9a749fe 100644 --- a/lib/gitlab/prometheus/queries/matched_metrics_query.rb +++ b/lib/gitlab/prometheus/queries/matched_metric_query.rb @@ -1,7 +1,7 @@ module Gitlab module Prometheus module Queries - class MatchedMetricsQuery < BaseQuery + class MatchedMetricQuery < BaseQuery MAX_QUERY_ITEMS = 40.freeze def query diff --git a/lib/gitlab/prometheus/queries/query_additional_metrics.rb b/lib/gitlab/prometheus/queries/query_additional_metrics.rb index 0c280dc9a3c5..aad76e335afc 100644 --- a/lib/gitlab/prometheus/queries/query_additional_metrics.rb +++ b/lib/gitlab/prometheus/queries/query_additional_metrics.rb @@ -3,9 +3,16 @@ module Prometheus module Queries module QueryAdditionalMetrics def query_metrics(project, query_context) + matched_metrics(project).map(&query_group(query_context)) + .select(&method(:group_with_any_metrics)) + end + + protected + + def query_group(query_context) query_processor = method(:process_query).curry[query_context] - groups = matched_metrics(project).map do |group| + lambda do |group| metrics = group.metrics.map do |metric| { title: metric.title, @@ -21,8 +28,6 @@ def query_metrics(project, query_context) metrics: metrics.select(&method(:metric_with_any_queries)) } end - - groups.select(&method(:group_with_any_metrics)) end private @@ -72,12 +77,17 @@ def matched_metrics(project) end def common_query_context(environment, timeframe_start:, timeframe_end:) - { - timeframe_start: timeframe_start, - timeframe_end: timeframe_end, + base_query_context(timeframe_start, timeframe_end).merge({ ci_environment_slug: environment.slug, kube_namespace: environment.project.deployment_platform&.actual_namespace || '', environment_filter: %{container_name!="POD",environment="#{environment.slug}"} + }) + end + + def base_query_context(timeframe_start, timeframe_end) + { + timeframe_start: timeframe_start, + timeframe_end: timeframe_end } end end diff --git a/lib/gitlab/prometheus_client.rb b/lib/gitlab/prometheus_client.rb index 659021c9ac92..b66253a10e0e 100644 --- a/lib/gitlab/prometheus_client.rb +++ b/lib/gitlab/prometheus_client.rb @@ -57,7 +57,11 @@ def get(path, args) rescue OpenSSL::SSL::SSLError raise PrometheusClient::Error, "#{rest_client.url} contains invalid SSL data" rescue RestClient::ExceptionWithResponse => ex - handle_exception_response(ex.response) + if ex.response + handle_exception_response(ex.response) + else + raise PrometheusClient::Error, "Network connection error" + end rescue RestClient::Exception raise PrometheusClient::Error, "Network connection error" end diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index 73e7921fab78..6c67dfde63ac 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -129,10 +129,10 @@ end context 'when metrics are enabled' do - let(:prometheus_service) { double('prometheus_service') } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do - allow(deployment.project).to receive(:prometheus_service).and_return(prometheus_service) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) end context 'when environment has no metrics' do diff --git a/spec/controllers/projects/prometheus/metrics_controller_spec.rb b/spec/controllers/projects/prometheus/metrics_controller_spec.rb index f17f819feeef..e364e99d8570 100644 --- a/spec/controllers/projects/prometheus/metrics_controller_spec.rb +++ b/spec/controllers/projects/prometheus/metrics_controller_spec.rb @@ -4,11 +4,11 @@ let(:user) { create(:user) } let(:project) { create(:project) } - let(:prometheus_service) { double('prometheus_service') } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do allow(controller).to receive(:project).and_return(project) - allow(project).to receive(:find_or_initialize_service).with('prometheus').and_return(prometheus_service) + allow(project).to receive(:prometheus_service).and_return(prometheus_adapter) project.add_master(user) sign_in(user) @@ -18,7 +18,7 @@ context 'when prometheus metrics are enabled' do context 'when data is not present' do before do - allow(prometheus_service).to receive(:matched_metrics).and_return({}) + allow(prometheus_adapter).to receive(:query).with(:matched_metrics).and_return({}) end it 'returns no content response' do @@ -32,7 +32,7 @@ let(:sample_response) { { some_data: 1 } } before do - allow(prometheus_service).to receive(:matched_metrics).and_return(sample_response) + allow(prometheus_adapter).to receive(:query).with(:matched_metrics).and_return(sample_response) end it 'returns no content response' do diff --git a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb index 0697cb2def64..c7169717fc1f 100644 --- a/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/additional_metrics_deployment_query_spec.rb @@ -7,7 +7,7 @@ include_examples 'additional metrics query' do let(:deployment) { create(:deployment, environment: environment) } - let(:query_params) { [environment.id, deployment.id] } + let(:query_params) { [deployment.id] } it 'queries using specific time' do expect(client).to receive(:query_range).with(anything, diff --git a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb index 84dc31d97322..ffe3ad85baa3 100644 --- a/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/deployment_query_spec.rb @@ -31,7 +31,7 @@ expect(client).to receive(:query).with('avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="environment-slug"}[30m])) * 100', time: stop_time) - expect(subject.query(environment.id, deployment.id)).to eq(memory_values: nil, memory_before: nil, memory_after: nil, - cpu_values: nil, cpu_before: nil, cpu_after: nil) + expect(subject.query(deployment.id)).to eq(memory_values: nil, memory_before: nil, memory_after: nil, + cpu_values: nil, cpu_before: nil, cpu_after: nil) end end diff --git a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb b/spec/lib/gitlab/prometheus/queries/matched_metric_query_spec.rb similarity index 98% rename from spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb rename to spec/lib/gitlab/prometheus/queries/matched_metric_query_spec.rb index c95719eff1db..420218a695a5 100644 --- a/spec/lib/gitlab/prometheus/queries/matched_metrics_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/matched_metric_query_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Prometheus::Queries::MatchedMetricsQuery do +describe Gitlab::Prometheus::Queries::MatchedMetricQuery do include Prometheus::MetricBuilders let(:metric_group_class) { Gitlab::Prometheus::MetricGroup } diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 01037919530a..959fb3386720 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -32,11 +32,11 @@ end end - describe '#proxy_client' do + describe '#prometheus_client' do context 'cluster is nil' do it 'returns nil' do expect(subject.cluster).to be_nil - expect(subject.proxy_client).to be_nil + expect(subject.prometheus_client).to be_nil end end @@ -45,7 +45,7 @@ subject { create(:clusters_applications_prometheus, cluster: cluster) } it 'returns nil' do - expect(subject.proxy_client).to be_nil + expect(subject.prometheus_client).to be_nil end end @@ -73,15 +73,15 @@ end it 'creates proxy prometheus rest client' do - expect(subject.proxy_client).to be_instance_of(RestClient::Resource) + expect(subject.prometheus_client).to be_instance_of(RestClient::Resource) end it 'creates proper url' do - expect(subject.proxy_client.url).to eq('http://example.com/api/v1/proxy/namespaces/gitlab-managed-apps/service/prometheus-prometheus-server:80') + expect(subject.prometheus_client.url).to eq('http://example.com/api/v1/proxy/namespaces/gitlab-managed-apps/service/prometheus-prometheus-server:80') end it 'copies options and headers from kube client to proxy client' do - expect(subject.proxy_client.options).to eq(kube_client.rest_client.options.merge(headers: kube_client.headers)) + expect(subject.prometheus_client.options).to eq(kube_client.rest_client.options.merge(headers: kube_client.headers)) end end end diff --git a/spec/models/concerns/prometheus_adapter_spec.rb b/spec/models/concerns/prometheus_adapter_spec.rb new file mode 100644 index 000000000000..f4b9c57e71a3 --- /dev/null +++ b/spec/models/concerns/prometheus_adapter_spec.rb @@ -0,0 +1,121 @@ +require 'spec_helper' + +describe PrometheusAdapter, :use_clean_rails_memory_store_caching do + include PrometheusHelpers + include ReactiveCachingHelpers + + class TestClass + include PrometheusAdapter + end + + let(:project) { create(:prometheus_project) } + let(:service) { project.prometheus_service } + + let(:described_class) { TestClass } + let(:environment_query) { Gitlab::Prometheus::Queries::EnvironmentQuery } + + describe '#query' do + describe 'environment' do + let(:environment) { build_stubbed(:environment, slug: 'env-slug') } + + around do |example| + Timecop.freeze { example.run } + end + + context 'with valid data' do + subject { service.query(:environment, environment) } + + before do + stub_reactive_cache(service, prometheus_data, environment_query, environment.id) + end + + it 'returns reactive data' do + is_expected.to eq(prometheus_metrics_data) + end + end + end + + describe 'matched_metrics' do + let(:matched_metrics_query) { Gitlab::Prometheus::Queries::MatchedMetricQuery } + let(:prometheus_client_wrapper) { double(:prometheus_client_wrapper, label_values: nil) } + + context 'with valid data' do + subject { service.query(:matched_metrics) } + + before do + allow(service).to receive(:prometheus_client_wrapper).and_return(prometheus_client_wrapper) + synchronous_reactive_cache(service) + end + + it 'returns reactive data' do + expect(subject[:success]).to be_truthy + expect(subject[:data]).to eq([]) + end + end + end + + describe 'deployment' do + let(:deployment) { build_stubbed(:deployment) } + let(:deployment_query) { Gitlab::Prometheus::Queries::DeploymentQuery } + + around do |example| + Timecop.freeze { example.run } + end + + context 'with valid data' do + subject { service.query(:deployment, deployment) } + + before do + stub_reactive_cache(service, prometheus_data, deployment_query, deployment.id) + end + + it 'returns reactive data' do + expect(subject).to eq(prometheus_metrics_data) + end + end + end + end + + describe '#calculate_reactive_cache' do + let(:environment) { create(:environment, slug: 'env-slug') } + before do + service.manual_configuration = true + service.active = true + end + + subject do + service.calculate_reactive_cache(environment_query.name, environment.id) + end + + around do |example| + Timecop.freeze { example.run } + end + + context 'when service is inactive' do + before do + service.active = false + end + + it { is_expected.to be_nil } + end + + context 'when Prometheus responds with valid data' do + before do + stub_all_prometheus_requests(environment.slug) + end + + it { expect(subject.to_json).to eq(prometheus_data.to_json) } + it { expect(subject.to_json).to eq(prometheus_data.to_json) } + end + + [404, 500].each do |status| + context "when Prometheus responds with #{status}" do + before do + stub_all_prometheus_requests(environment.slug, status: status, body: "QUERY FAILED!") + end + + it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } + end + end + end +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index ba8aa13d5adf..ac30cd27e0c9 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -64,6 +64,7 @@ describe '#metrics' do let(:deployment) { create(:deployment) } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } subject { deployment.metrics } @@ -76,17 +77,16 @@ { success: true, metrics: {}, - last_update: 42, - deployment_time: 1494408956 + last_update: 42 } end before do - allow(deployment.project).to receive_message_chain(:monitoring_service, :deployment_metrics) - .with(any_args).and_return(simple_metrics) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) + allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) end - it { is_expected.to eq(simple_metrics) } + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } end end @@ -109,11 +109,11 @@ } end - let(:prometheus_service) { double('prometheus_service') } + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do - allow(project).to receive(:prometheus_service).and_return(prometheus_service) - allow(prometheus_service).to receive(:additional_deployment_metrics).and_return(simple_metrics) + allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) + allow(prometheus_adapter).to receive(:query).with(:additional_metrics_deployment, deployment).and_return(simple_metrics) end it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 6f24a039998c..1049e47d25d3 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -452,8 +452,8 @@ end it 'returns the metrics from the deployment service' do - expect(project.monitoring_service) - .to receive(:environment_metrics).with(environment) + expect(environment.prometheus_adapter) + .to receive(:query).with(:environment, environment) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) @@ -508,12 +508,12 @@ context 'when the environment has additional metrics' do before do - allow(environment).to receive(:has_additional_metrics?).and_return(true) + allow(environment).to receive(:has_metrics?).and_return(true) end it 'returns the additional metrics from the deployment service' do - expect(project.prometheus_service).to receive(:additional_environment_metrics) - .with(environment) + expect(environment.prometheus_adapter).to receive(:query) + .with(:additional_metrics_environment, environment) .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) @@ -522,46 +522,13 @@ context 'when the environment does not have metrics' do before do - allow(environment).to receive(:has_additional_metrics?).and_return(false) + allow(environment).to receive(:has_metrics?).and_return(false) end it { is_expected.to be_nil } end end - describe '#has_additional_metrics??' do - subject { environment.has_additional_metrics? } - - context 'when the enviroment is available' do - context 'with a deployment service' do - let(:project) { create(:prometheus_project) } - - context 'and a deployment' do - let!(:deployment) { create(:deployment, environment: environment) } - it { is_expected.to be_truthy } - end - - context 'but no deployments' do - it { is_expected.to be_falsy } - end - end - - context 'without a monitoring service' do - it { is_expected.to be_falsy } - end - end - - context 'when the environment is unavailable' do - let(:project) { create(:prometheus_project) } - - before do - environment.stop - end - - it { is_expected.to be_falsy } - end - end - describe '#slug' do it "is automatically generated" do expect(environment.slug).not_to be_nil @@ -654,4 +621,77 @@ end end end + + describe '#prometheus_adapter' do + let!(:cluster_for_all) { create(:cluster, environment_scope: '*', projects: [project]) } + let!(:cluster_for_dev) { create(:cluster, environment_scope: 'dev', projects: [project]) } + + let!(:prometheus_for_dev) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_dev) } + + context 'prometheus service can execute queries' do + let(:prometheus_service) { double(:prometheus_service, can_query?: true) } + + before do + allow(environment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it 'return prometheus service as prometheus adapter' do + expect(environment.prometheus_adapter).to eq(prometheus_service) + end + end + + context "prometheus service can't execute queries" do + let(:prometheus_service) { double(:prometheus_service, can_query?: false) } + + context 'with cluster for all environments with prometheus installed' do + let!(:prometheus_for_all) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_all) } + + context 'without environment supplied' do + it 'returns application handling all environments' do + expect(environment.prometheus_adapter).to eq(prometheus_for_all) + end + end + + context 'with dev environment supplied' do + let!(:environment) { create(:environment, project: project, name: 'dev') } + + it 'returns dev cluster prometheus application' do + expect(environment.prometheus_adapter).to eq(prometheus_for_dev) + end + end + + context 'with prod environment supplied' do + let!(:environment) { create(:environment, project: project, name: 'prod') } + + it 'returns application handling all environments' do + expect(environment.prometheus_adapter).to eq(prometheus_for_all) + end + end + end + + context 'with cluster for all environments without prometheus installed' do + context 'without environment supplied' do + it 'returns nil' do + expect(environment.prometheus_adapter).to be_nil + end + end + + context 'with dev environment supplied' do + let!(:environment) { create(:environment, project: project, name: 'dev') } + + it 'returns dev cluster prometheus application' do + expect(environment.prometheus_adapter).to eq(prometheus_for_dev) + end + end + + context 'with prod environment supplied' do + let!(:environment) { create(:environment, project: project, name: 'prod') } + + it 'returns nil' do + expect(environment.prometheus_adapter).to be_nil + end + end + end + end + end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 6693e5783a56..6a4bdb47105e 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -6,7 +6,6 @@ let(:project) { create(:prometheus_project) } let(:service) { project.prometheus_service } - let(:environment_query) { Gitlab::Prometheus::Queries::EnvironmentQuery } describe "Associations" do it { is_expected.to belong_to :project } @@ -55,197 +54,31 @@ end end - describe '#environment_metrics' do - let(:environment) { build_stubbed(:environment, slug: 'env-slug') } - - around do |example| - Timecop.freeze { example.run } - end - - context 'with valid data' do - subject { service.environment_metrics(environment) } - - before do - stub_reactive_cache(service, prometheus_data, environment_query, environment.id) - end - - it 'returns reactive data' do - is_expected.to eq(prometheus_metrics_data) - end - end - end - - describe '#matched_metrics' do - let(:matched_metrics_query) { Gitlab::Prometheus::Queries::MatchedMetricsQuery } - let(:client) { double(:client, label_values: nil) } - - context 'with valid data' do - subject { service.matched_metrics } - - before do - allow(service).to receive(:client).and_return(client) - synchronous_reactive_cache(service) - end - - it 'returns reactive data' do - expect(subject[:success]).to be_truthy - expect(subject[:data]).to eq([]) - end - end - end - - describe '#deployment_metrics' do - let(:deployment) { build_stubbed(:deployment) } - let(:deployment_query) { Gitlab::Prometheus::Queries::DeploymentQuery } - - around do |example| - Timecop.freeze { example.run } - end - - context 'with valid data' do - subject { service.deployment_metrics(deployment) } - let(:fake_deployment_time) { 10 } - - before do - stub_reactive_cache(service, prometheus_data, deployment_query, deployment.environment.id, deployment.id) - end - - it 'returns reactive data' do - expect(deployment).to receive(:created_at).and_return(fake_deployment_time) - - expect(subject).to eq(prometheus_metrics_data.merge(deployment_time: fake_deployment_time)) - end - end - end - - describe '#calculate_reactive_cache' do - let(:environment) { create(:environment, slug: 'env-slug') } - before do - service.manual_configuration = true - service.active = true - end - - subject do - service.calculate_reactive_cache(environment_query.name, environment.id) - end - - around do |example| - Timecop.freeze { example.run } - end - - context 'when service is inactive' do - before do - service.active = false - end - - it { is_expected.to be_nil } - end - - context 'when Prometheus responds with valid data' do - before do - stub_all_prometheus_requests(environment.slug) - end - - it { expect(subject.to_json).to eq(prometheus_data.to_json) } - it { expect(subject.to_json).to eq(prometheus_data.to_json) } - end - - [404, 500].each do |status| - context "when Prometheus responds with #{status}" do - before do - stub_all_prometheus_requests(environment.slug, status: status, body: "QUERY FAILED!") - end - - it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } - end - end - end - - describe '#client' do + describe '#prometheus_client' do context 'manual configuration is enabled' do let(:api_url) { 'http://some_url' } + before do + subject.active = true subject.manual_configuration = true subject.api_url = api_url end - it 'returns simple rest client from api_url' do - expect(subject.client).to be_instance_of(Gitlab::PrometheusClient) - expect(subject.client.rest_client.url).to eq(api_url) + it 'returns rest client from api_url' do + expect(subject.prometheus_client.url).to eq(api_url) end end context 'manual configuration is disabled' do - let!(:cluster_for_all) { create(:cluster, environment_scope: '*', projects: [project]) } - let!(:cluster_for_dev) { create(:cluster, environment_scope: 'dev', projects: [project]) } - - let!(:prometheus_for_dev) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_dev) } - let(:proxy_client) { double('proxy_client') } + let(:api_url) { 'http://some_url' } before do - service.manual_configuration = false - end - - context 'with cluster for all environments with prometheus installed' do - let!(:prometheus_for_all) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_all) } - - context 'without environment supplied' do - it 'returns client handling all environments' do - expect(service).to receive(:client_from_cluster).with(cluster_for_all).and_return(proxy_client).twice - - expect(service.client).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client.rest_client).to eq(proxy_client) - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } - - it 'returns dev cluster client' do - expect(service).to receive(:client_from_cluster).with(cluster_for_dev).and_return(proxy_client).twice - - expect(service.client(environment.id)).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client(environment.id).rest_client).to eq(proxy_client) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'returns dev cluster client' do - expect(service).to receive(:client_from_cluster).with(cluster_for_all).and_return(proxy_client).twice - - expect(service.client(environment.id)).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client(environment.id).rest_client).to eq(proxy_client) - end - end + subject.manual_configuration = false + subject.api_url = api_url end - context 'with cluster for all environments without prometheus installed' do - context 'without environment supplied' do - it 'raises PrometheusClient::Error because cluster was not found' do - expect { service.client }.to raise_error(Gitlab::PrometheusClient::Error, /couldn't find cluster with Prometheus installed/) - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } - - it 'returns dev cluster client' do - expect(service).to receive(:client_from_cluster).with(cluster_for_dev).and_return(proxy_client).twice - - expect(service.client(environment.id)).to be_instance_of(Gitlab::PrometheusClient) - expect(service.client(environment.id).rest_client).to eq(proxy_client) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'raises PrometheusClient::Error because cluster was not found' do - expect { service.client }.to raise_error(Gitlab::PrometheusClient::Error, /couldn't find cluster with Prometheus installed/) - end - end + it 'no client provided' do + expect(subject.prometheus_client).to be_nil end end end -- GitLab From 22e2cad910171bd3ed54ce65da55169c4c6d70b7 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Sat, 24 Feb 2018 00:56:50 +0100 Subject: [PATCH 03/49] Use deployment platform to find cluster with prometheus application --- app/models/environment.rb | 8 ++--- spec/models/environment_spec.rb | 57 ++++++--------------------------- 2 files changed, 12 insertions(+), 53 deletions(-) diff --git a/app/models/environment.rb b/app/models/environment.rb index 2a91bd07b288..966ce17a48a3 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -233,12 +233,10 @@ def service_prometheus_adapter end def cluster_prometheus_adapter - # sort results by descending order based on environment_scope being longer - # thus more closely matching environment slug - clusters = project.clusters.enabled.for_environment(self).sort_by { |c| c.environment_scope&.length }.reverse! + cluster = project.deployment_platform&.cluster + return unless cluster&.application_prometheus&.installed? - cluster = clusters&.detect { |cluster| cluster.application_prometheus&.installed? } - cluster&.application_prometheus + cluster.application_prometheus end private diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 1049e47d25d3..8c052888b046 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Environment do - set(:project) { create(:project) } + let(:project) { create(:project) } subject(:environment) { create(:environment, project: project) } it { is_expected.to belong_to(:project) } @@ -623,10 +623,7 @@ end describe '#prometheus_adapter' do - let!(:cluster_for_all) { create(:cluster, environment_scope: '*', projects: [project]) } - let!(:cluster_for_dev) { create(:cluster, environment_scope: 'dev', projects: [project]) } - - let!(:prometheus_for_dev) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_dev) } + let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [project]) } context 'prometheus service can execute queries' do let(:prometheus_service) { double(:prometheus_service, can_query?: true) } @@ -643,53 +640,17 @@ context "prometheus service can't execute queries" do let(:prometheus_service) { double(:prometheus_service, can_query?: false) } - context 'with cluster for all environments with prometheus installed' do - let!(:prometheus_for_all) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_all) } - - context 'without environment supplied' do - it 'returns application handling all environments' do - expect(environment.prometheus_adapter).to eq(prometheus_for_all) - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } + context 'with cluster with prometheus installed' do + let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } - it 'returns dev cluster prometheus application' do - expect(environment.prometheus_adapter).to eq(prometheus_for_dev) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'returns application handling all environments' do - expect(environment.prometheus_adapter).to eq(prometheus_for_all) - end + it 'returns application handling all environments' do + expect(environment.prometheus_adapter).to eq(prometheus) end end - context 'with cluster for all environments without prometheus installed' do - context 'without environment supplied' do - it 'returns nil' do - expect(environment.prometheus_adapter).to be_nil - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } - - it 'returns dev cluster prometheus application' do - expect(environment.prometheus_adapter).to eq(prometheus_for_dev) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'returns nil' do - expect(environment.prometheus_adapter).to be_nil - end + context 'with cluster without prometheus installed' do + it 'returns nil' do + expect(environment.prometheus_adapter).to be_nil end end end -- GitLab From 0feeddaa0f4831e53d8d094b0cbb84222524431a Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Sat, 24 Feb 2018 01:06:08 +0100 Subject: [PATCH 04/49] drop the ! from synchronize_service_state! + remove unused scope --- app/models/clusters/cluster.rb | 2 -- app/models/project_services/prometheus_service.rb | 4 ++-- spec/models/project_services/prometheus_service_spec.rb | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 1fd7db301072..5ecbd4cbcebe 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -49,8 +49,6 @@ class Cluster < ActiveRecord::Base scope :enabled, -> { where(enabled: true) } scope :disabled, -> { where(enabled: false) } - scope :for_environment, -> (env) { where(environment_scope: ['*', '', env.slug]) } - def status_name if provider provider.status_name diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 7a0c9a33e70a..dcaeb65dc32b 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -9,7 +9,7 @@ class PrometheusService < MonitoringService validates :api_url, url: true end - before_save :synchronize_service_state! + before_save :synchronize_service_state after_save :clear_reactive_cache! @@ -82,7 +82,7 @@ def prometheus_installed? private - def synchronize_service_state! + def synchronize_service_state self.active = prometheus_installed? || manual_configuration? true diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 6a4bdb47105e..7afb1b4a8e3e 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -117,7 +117,7 @@ end end - describe '#synchronize_service_state! before_save callback' do + describe '#synchronize_service_state before_save callback' do context 'no clusters with prometheus are installed' do context 'when service is inactive' do before do -- GitLab From 77a5c7a68e1e90e966f0a00399cf9c02a8ea1807 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Sat, 24 Feb 2018 07:05:28 +0000 Subject: [PATCH 05/49] Pull in query changes from https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17285/diffs?commit_id=43fc6dc4d3ef503748d49f54d613d905fc90397d --- config/prometheus/cluster_metrics.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config/prometheus/cluster_metrics.yml b/config/prometheus/cluster_metrics.yml index 3a6836a29dd2..cb4735f8856f 100644 --- a/config/prometheus/cluster_metrics.yml +++ b/config/prometheus/cluster_metrics.yml @@ -6,10 +6,10 @@ required_metrics: ['container_cpu_usage_seconds_total'] weight: 1 queries: - - query_range: 'sum(rate(container_cpu_usage_seconds_total{id="/"}[15m]))' + - query_range: 'avg(sum(rate(container_cpu_usage_seconds_total{id="/"}[15m])) by (job)) without (job)' label: Usage unit: "cores" - - query_range: 'sum(kube_node_status_capacity_cpu_cores)' + - query_range: 'sum(kube_node_status_capacity_cpu_cores{kubernetes_namespace="gitlab-managed-apps"})' label: Capacity unit: "cores" - title: "Memory usage" @@ -17,9 +17,9 @@ required_metrics: ['container_memory_usage_bytes'] weight: 1 queries: - - query_range: 'sum(container_memory_usage_bytes{id="/"})/2^30' + - query_range: 'avg(sum(container_memory_usage_bytes{id="/"}) by (job)) without (job) / 2^30' label: Usage unit: "GiB" - - query_range: 'sum(kube_node_status_capacity_memory_bytes)/2^30' + - query_range: 'sum(kube_node_status_capacity_memory_bytes{kubernetes_namespace="gitlab-managed-apps"})/2^30' label: Capacity - unit: "GiB" + unit: "GiB" \ No newline at end of file -- GitLab From 637c9558627655aaf08066371f0a5d50461578be Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 26 Feb 2018 12:40:40 +0100 Subject: [PATCH 06/49] Fix failing test, when deployment platform is not bound to a cluster. --- app/models/environment.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/environment.rb b/app/models/environment.rb index 966ce17a48a3..0c408257499d 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -233,8 +233,10 @@ def service_prometheus_adapter end def cluster_prometheus_adapter - cluster = project.deployment_platform&.cluster - return unless cluster&.application_prometheus&.installed? + return unless project.deployment_platform.respond_to?(:cluster) + + cluster = project.deployment_platform.cluster + return unless cluster.application_prometheus&.installed? cluster.application_prometheus end -- GitLab From 3243cdc404eac4002254b1757c696e80ccdf9056 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 26 Feb 2018 16:17:24 +0100 Subject: [PATCH 07/49] Squashed commit of the following: commit 637c9558627655aaf08066371f0a5d50461578be Author: Pawel Chojnacki Date: Mon Feb 26 12:40:40 2018 +0100 Fix failing test, when deployment platform is not bound to a cluster. commit 0feeddaa0f4831e53d8d094b0cbb84222524431a Author: Pawel Chojnacki Date: Sat Feb 24 01:06:08 2018 +0100 drop the ! from synchronize_service_state! + remove unused scope commit 22e2cad910171bd3ed54ce65da55169c4c6d70b7 Author: Pawel Chojnacki Date: Sat Feb 24 00:56:50 2018 +0100 Use deployment platform to find cluster with prometheus application --- app/models/clusters/cluster.rb | 2 - app/models/environment.rb | 10 ++-- .../project_services/prometheus_service.rb | 4 +- spec/models/environment_spec.rb | 57 +++---------------- .../prometheus_service_spec.rb | 2 +- 5 files changed, 17 insertions(+), 58 deletions(-) diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 39a72e573eeb..71e840e3baf7 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -51,8 +51,6 @@ class Cluster < ActiveRecord::Base scope :enabled, -> { where(enabled: true) } scope :disabled, -> { where(enabled: false) } - scope :for_environment, -> (env) { where(environment_scope: ['*', '', env.slug]) } - def status_name if provider provider.status_name diff --git a/app/models/environment.rb b/app/models/environment.rb index d15c3933977c..158cf04ffb8c 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -241,12 +241,12 @@ def service_prometheus_adapter end def cluster_prometheus_adapter - # sort results by descending order based on environment_scope being longer - # thus more closely matching environment slug - clusters = project.clusters.enabled.for_environment(self).sort_by { |c| c.environment_scope&.length }.reverse! + return unless project.deployment_platform.respond_to?(:cluster) - cluster = clusters&.detect { |cluster| cluster.application_prometheus&.installed? } - cluster&.application_prometheus + cluster = project.deployment_platform.cluster + return unless cluster.application_prometheus&.installed? + + cluster.application_prometheus end private diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb index 7a0c9a33e70a..dcaeb65dc32b 100644 --- a/app/models/project_services/prometheus_service.rb +++ b/app/models/project_services/prometheus_service.rb @@ -9,7 +9,7 @@ class PrometheusService < MonitoringService validates :api_url, url: true end - before_save :synchronize_service_state! + before_save :synchronize_service_state after_save :clear_reactive_cache! @@ -82,7 +82,7 @@ def prometheus_installed? private - def synchronize_service_state! + def synchronize_service_state self.active = prometheus_installed? || manual_configuration? true diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 6265d4907b25..cdd885bbbe94 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Environment do - set(:project) { create(:project) } + let(:project) { create(:project) } subject(:environment) { create(:environment, project: project) } it { is_expected.to belong_to(:project) } @@ -724,10 +724,7 @@ end describe '#prometheus_adapter' do - let!(:cluster_for_all) { create(:cluster, environment_scope: '*', projects: [project]) } - let!(:cluster_for_dev) { create(:cluster, environment_scope: 'dev', projects: [project]) } - - let!(:prometheus_for_dev) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_dev) } + let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [project]) } context 'prometheus service can execute queries' do let(:prometheus_service) { double(:prometheus_service, can_query?: true) } @@ -744,53 +741,17 @@ context "prometheus service can't execute queries" do let(:prometheus_service) { double(:prometheus_service, can_query?: false) } - context 'with cluster for all environments with prometheus installed' do - let!(:prometheus_for_all) { create(:clusters_applications_prometheus, :installed, cluster: cluster_for_all) } - - context 'without environment supplied' do - it 'returns application handling all environments' do - expect(environment.prometheus_adapter).to eq(prometheus_for_all) - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } + context 'with cluster with prometheus installed' do + let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } - it 'returns dev cluster prometheus application' do - expect(environment.prometheus_adapter).to eq(prometheus_for_dev) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'returns application handling all environments' do - expect(environment.prometheus_adapter).to eq(prometheus_for_all) - end + it 'returns application handling all environments' do + expect(environment.prometheus_adapter).to eq(prometheus) end end - context 'with cluster for all environments without prometheus installed' do - context 'without environment supplied' do - it 'returns nil' do - expect(environment.prometheus_adapter).to be_nil - end - end - - context 'with dev environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'dev') } - - it 'returns dev cluster prometheus application' do - expect(environment.prometheus_adapter).to eq(prometheus_for_dev) - end - end - - context 'with prod environment supplied' do - let!(:environment) { create(:environment, project: project, name: 'prod') } - - it 'returns nil' do - expect(environment.prometheus_adapter).to be_nil - end + context 'with cluster without prometheus installed' do + it 'returns nil' do + expect(environment.prometheus_adapter).to be_nil end end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index c5f985d6ba94..75927c2df3bb 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -221,7 +221,7 @@ end end - describe '#synchronize_service_state! before_save callback' do + describe '#synchronize_service_state before_save callback' do context 'no clusters with prometheus are installed' do context 'when service is inactive' do before do -- GitLab From 2c090539f2dbdbc11c18598c7952a5de497047ea Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 26 Feb 2018 19:57:11 +0100 Subject: [PATCH 08/49] use deployment_platform DI in environment and revert changes to k8s cert verification policy --- app/models/clusters/platforms/kubernetes.rb | 2 +- app/models/environment.rb | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 66f629df1b41..7ce8befeeeb4 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -141,7 +141,7 @@ def read_pods end def kubeclient_ssl_options - opts = { verify_ssl: OpenSSL::SSL::VERIFY_NONE } + opts = { verify_ssl: OpenSSL::SSL::VERIFY_PEER } if ca_pem.present? opts[:cert_store] = OpenSSL::X509::Store.new diff --git a/app/models/environment.rb b/app/models/environment.rb index 0c408257499d..57960a7c3b3c 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -220,6 +220,10 @@ def folder_name self.environment_type || self.name end + def deployment_platform + project.deployment_platform + end + def prometheus_adapter @prometheus_adapter ||= if service_prometheus_adapter.can_query? service_prometheus_adapter @@ -233,9 +237,9 @@ def service_prometheus_adapter end def cluster_prometheus_adapter - return unless project.deployment_platform.respond_to?(:cluster) + return unless deployment_platform.respond_to?(:cluster) - cluster = project.deployment_platform.cluster + cluster = deployment_platform.cluster return unless cluster.application_prometheus&.installed? cluster.application_prometheus -- GitLab From a3ec8457cdfc3d46eddb9ec3f43ede5acaa3498f Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 26 Feb 2018 20:09:41 +0100 Subject: [PATCH 09/49] Fix backported CE code --- app/models/environment.rb | 4 ++-- spec/models/environment_spec.rb | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/models/environment.rb b/app/models/environment.rb index 158cf04ffb8c..fddc7024807c 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -241,9 +241,9 @@ def service_prometheus_adapter end def cluster_prometheus_adapter - return unless project.deployment_platform.respond_to?(:cluster) + return unless deployment_platform.respond_to?(:cluster) - cluster = project.deployment_platform.cluster + cluster = deployment_platform.cluster return unless cluster.application_prometheus&.installed? cluster.application_prometheus diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index cdd885bbbe94..1957dfcdb136 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -493,9 +493,9 @@ end it 'returns the metrics from the deployment service' do - expect(project.monitoring_service) - .to receive(:environment_metrics).with(environment) - .and_return(:fake_metrics) + expect(environment.prometheus_adapter) + .to receive(:query).with(:environment, environment) + .and_return(:fake_metrics) is_expected.to eq(:fake_metrics) end -- GitLab From 8b88b6f2724561b8dc28bf40abf0a82847b0a565 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 27 Feb 2018 13:24:01 +0100 Subject: [PATCH 10/49] Cluster controller tests --- .../projects/clusters_controller.rb | 2 +- .../projects/clusters_controller_spec.rb | 86 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index ac2db72a38a7..4048cc62c453 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -64,7 +64,7 @@ def destroy end def metrics - render_404 unless prometheus_adapter&.can_query? + return render_404 unless prometheus_adapter&.can_query? respond_to do |format| format.json do diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 954fc79f57dc..cbd053aa11eb 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -149,6 +149,92 @@ def go end end + describe 'GET metrics' do + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + context "Can't query Prometheus" do + it 'returns not found' do + go + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'can query Prometheus' do + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true, query: nil) } + + before do + allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) + end + + it 'queries cluster metrics' do + go + + expect(prometheus_adapter).to have_received(:query).with(:cluster) + end + + context 'when response has content' do + let(:query_response) { { response: nil } } + + before do + allow(prometheus_adapter).to receive(:query).and_return(query_response) + end + + it 'returns prometheus query response' do + go + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(query_response) + end + end + + context 'when response has no content' do + let(:query_response) { {} } + + before do + allow(prometheus_adapter).to receive(:query).and_return(query_response) + end + + it 'returns prometheus query response' do + go + + expect(response).to have_gitlab_http_status(:no_content) + end + end + end + end + + def go + get :metrics, format: :json, namespace_id: project.namespace, + project_id: project, + id: cluster + end + + describe 'security' do + let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true, query: nil) } + before do + allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) + end + + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + describe 'PUT update' do context 'when cluster is provided by GCP' do let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } -- GitLab From 43a9bbce153e077c6e697f1ca4e0d49e45e634c1 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 28 Feb 2018 18:13:51 +0100 Subject: [PATCH 11/49] fix cluster controller spec --- .../projects/clusters_controller_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index cbd053aa11eb..f0d4fe8d6ea7 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -192,7 +192,7 @@ def go go expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq(query_response) + expect(response.body).to eq(query_response.to_json) end end @@ -418,8 +418,8 @@ def go_json it "destroys and redirects back to clusters list" do expect { go } .to change { Clusters::Cluster.count }.by(-1) - .and change { Clusters::Platforms::Kubernetes.count }.by(-1) - .and change { Clusters::Providers::Gcp.count }.by(-1) + .and change { Clusters::Platforms::Kubernetes.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(-1) expect(response).to redirect_to(project_clusters_path(project)) expect(flash[:notice]).to eq('Kubernetes cluster integration was successfully removed.') @@ -432,7 +432,7 @@ def go_json it "destroys and redirects back to clusters list" do expect { go } .to change { Clusters::Cluster.count }.by(-1) - .and change { Clusters::Providers::Gcp.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(-1) expect(response).to redirect_to(project_clusters_path(project)) expect(flash[:notice]).to eq('Kubernetes cluster integration was successfully removed.') @@ -446,8 +446,8 @@ def go_json it "destroys and redirects back to clusters list" do expect { go } .to change { Clusters::Cluster.count }.by(-1) - .and change { Clusters::Platforms::Kubernetes.count }.by(-1) - .and change { Clusters::Providers::Gcp.count }.by(0) + .and change { Clusters::Platforms::Kubernetes.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(0) expect(response).to redirect_to(project_clusters_path(project)) expect(flash[:notice]).to eq('Kubernetes cluster integration was successfully removed.') @@ -470,8 +470,8 @@ def go_json def go delete :destroy, namespace_id: project.namespace, - project_id: project, - id: cluster + project_id: project, + id: cluster end end end -- GitLab From 8ad9cd5581dafeb7a24da7ed119a30761a0a72fe Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 1 Mar 2018 18:14:54 +0100 Subject: [PATCH 12/49] Implement PrometheusDeploymentLocator + fix metrics controller --- .../projects/prometheus/metrics_controller.rb | 6 ++--- .../concerns/prometheus_adapter_locator.rb | 26 +++++++++++++++++++ app/models/environment.rb | 23 ++-------------- .../prometheus/metrics_controller_spec.rb | 3 +-- 4 files changed, 31 insertions(+), 27 deletions(-) create mode 100644 app/models/concerns/prometheus_adapter_locator.rb diff --git a/app/controllers/projects/prometheus/metrics_controller.rb b/app/controllers/projects/prometheus/metrics_controller.rb index 255fbf254fba..ec3c49059e15 100644 --- a/app/controllers/projects/prometheus/metrics_controller.rb +++ b/app/controllers/projects/prometheus/metrics_controller.rb @@ -1,6 +1,8 @@ module Projects module Prometheus class MetricsController < Projects::ApplicationController + include PrometheusAdapterLocator + before_action :authorize_admin_project! before_action :require_prometheus_metrics! @@ -20,10 +22,6 @@ def active_common private - def prometheus_adapter - project.prometheus_service - end - def require_prometheus_metrics! render_404 unless prometheus_adapter.can_query? end diff --git a/app/models/concerns/prometheus_adapter_locator.rb b/app/models/concerns/prometheus_adapter_locator.rb new file mode 100644 index 000000000000..f301eab5c576 --- /dev/null +++ b/app/models/concerns/prometheus_adapter_locator.rb @@ -0,0 +1,26 @@ +module PrometheusAdapterLocator + def deployment_platform + project.deployment_platform + end + + def prometheus_adapter + @prometheus_adapter ||= if service_prometheus_adapter.can_query? + service_prometheus_adapter + else + cluster_prometheus_adapter + end + end + + def service_prometheus_adapter + project.find_or_initialize_service('prometheus') + end + + def cluster_prometheus_adapter + return unless deployment_platform.respond_to?(:cluster) + + cluster = deployment_platform.cluster + return unless cluster.application_prometheus&.installed? + + cluster.application_prometheus + end +end \ No newline at end of file diff --git a/app/models/environment.rb b/app/models/environment.rb index 57960a7c3b3c..baba023c92da 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -1,4 +1,6 @@ class Environment < ActiveRecord::Base + include PrometheusAdapterLocator + # Used to generate random suffixes for the slug LETTERS = 'a'..'z' NUMBERS = '0'..'9' @@ -224,27 +226,6 @@ def deployment_platform project.deployment_platform end - def prometheus_adapter - @prometheus_adapter ||= if service_prometheus_adapter.can_query? - service_prometheus_adapter - else - cluster_prometheus_adapter - end - end - - def service_prometheus_adapter - project.find_or_initialize_service('prometheus') - end - - def cluster_prometheus_adapter - return unless deployment_platform.respond_to?(:cluster) - - cluster = deployment_platform.cluster - return unless cluster.application_prometheus&.installed? - - cluster.application_prometheus - end - private # Slugifying a name may remove the uniqueness guarantee afforded by it being diff --git a/spec/controllers/projects/prometheus/metrics_controller_spec.rb b/spec/controllers/projects/prometheus/metrics_controller_spec.rb index e364e99d8570..9d2ac42e99fb 100644 --- a/spec/controllers/projects/prometheus/metrics_controller_spec.rb +++ b/spec/controllers/projects/prometheus/metrics_controller_spec.rb @@ -7,8 +7,7 @@ let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do - allow(controller).to receive(:project).and_return(project) - allow(project).to receive(:prometheus_service).and_return(prometheus_adapter) + allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) project.add_master(user) sign_in(user) -- GitLab From e70704ff3f97627cd8260a263bc6b5d89d62c9d9 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 1 Mar 2018 20:01:15 +0100 Subject: [PATCH 13/49] Move file and fix formatting --- .../lib}/gitlab/prometheus/queries/cluster_query.rb | 0 spec/controllers/projects/clusters_controller_spec.rb | 11 ++++++----- 2 files changed, 6 insertions(+), 5 deletions(-) rename {lib => ee/lib}/gitlab/prometheus/queries/cluster_query.rb (100%) diff --git a/lib/gitlab/prometheus/queries/cluster_query.rb b/ee/lib/gitlab/prometheus/queries/cluster_query.rb similarity index 100% rename from lib/gitlab/prometheus/queries/cluster_query.rb rename to ee/lib/gitlab/prometheus/queries/cluster_query.rb diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index f0d4fe8d6ea7..8119c87af285 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -213,9 +213,10 @@ def go end def go - get :metrics, format: :json, namespace_id: project.namespace, - project_id: project, - id: cluster + get :metrics, format: :json, + namespace_id: project.namespace, + project_id: project, + id: cluster end describe 'security' do @@ -470,8 +471,8 @@ def go_json def go delete :destroy, namespace_id: project.namespace, - project_id: project, - id: cluster + project_id: project, + id: cluster end end end -- GitLab From c6a1d622534a3f6b57fdfba215d015d9139155b0 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 1 Mar 2018 20:03:38 +0100 Subject: [PATCH 14/49] add changelog --- ee/changelogs/unreleased/5029-support-cluster-metrics.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 ee/changelogs/unreleased/5029-support-cluster-metrics.yml diff --git a/ee/changelogs/unreleased/5029-support-cluster-metrics.yml b/ee/changelogs/unreleased/5029-support-cluster-metrics.yml new file mode 100644 index 000000000000..d07ead28befc --- /dev/null +++ b/ee/changelogs/unreleased/5029-support-cluster-metrics.yml @@ -0,0 +1,5 @@ +--- +title: Query cluster status +merge_request: 4701 +author: +type: added -- GitLab From 273e4142566a053e0628183fbf86ef7b94cb5a66 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 1 Mar 2018 20:20:41 +0100 Subject: [PATCH 15/49] add missing newline --- app/models/concerns/prometheus_adapter_locator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/prometheus_adapter_locator.rb b/app/models/concerns/prometheus_adapter_locator.rb index f301eab5c576..d8a3ed0de2da 100644 --- a/app/models/concerns/prometheus_adapter_locator.rb +++ b/app/models/concerns/prometheus_adapter_locator.rb @@ -23,4 +23,4 @@ def cluster_prometheus_adapter cluster.application_prometheus end -end \ No newline at end of file +end -- GitLab From 5ceec83a0154e513480a9bb08e252371bf7886c6 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Mar 2018 19:34:59 +0100 Subject: [PATCH 16/49] Implemente Prometheus:AdapterService --- .../projects/prometheus/metrics_controller.rb | 6 ++- .../concerns/prometheus_adapter_locator.rb | 26 ------------ app/models/environment.rb | 6 ++- app/services/prometheus/adapter_service.rb | 36 +++++++++++++++++ .../prometheus/adapter_service_spec.rb | 40 +++++++++++++++++++ 5 files changed, 84 insertions(+), 30 deletions(-) delete mode 100644 app/models/concerns/prometheus_adapter_locator.rb create mode 100644 app/services/prometheus/adapter_service.rb create mode 100644 spec/services/prometheus/adapter_service_spec.rb diff --git a/app/controllers/projects/prometheus/metrics_controller.rb b/app/controllers/projects/prometheus/metrics_controller.rb index ec3c49059e15..523abce35fb1 100644 --- a/app/controllers/projects/prometheus/metrics_controller.rb +++ b/app/controllers/projects/prometheus/metrics_controller.rb @@ -1,8 +1,6 @@ module Projects module Prometheus class MetricsController < Projects::ApplicationController - include PrometheusAdapterLocator - before_action :authorize_admin_project! before_action :require_prometheus_metrics! @@ -22,6 +20,10 @@ def active_common private + def prometheus_adapter + @prometheus_adapter ||= Prometheus::AdapterService.new(project).prometheus_adapter + end + def require_prometheus_metrics! render_404 unless prometheus_adapter.can_query? end diff --git a/app/models/concerns/prometheus_adapter_locator.rb b/app/models/concerns/prometheus_adapter_locator.rb deleted file mode 100644 index d8a3ed0de2da..000000000000 --- a/app/models/concerns/prometheus_adapter_locator.rb +++ /dev/null @@ -1,26 +0,0 @@ -module PrometheusAdapterLocator - def deployment_platform - project.deployment_platform - end - - def prometheus_adapter - @prometheus_adapter ||= if service_prometheus_adapter.can_query? - service_prometheus_adapter - else - cluster_prometheus_adapter - end - end - - def service_prometheus_adapter - project.find_or_initialize_service('prometheus') - end - - def cluster_prometheus_adapter - return unless deployment_platform.respond_to?(:cluster) - - cluster = deployment_platform.cluster - return unless cluster.application_prometheus&.installed? - - cluster.application_prometheus - end -end diff --git a/app/models/environment.rb b/app/models/environment.rb index baba023c92da..24d4f1d8761f 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -1,6 +1,4 @@ class Environment < ActiveRecord::Base - include PrometheusAdapterLocator - # Used to generate random suffixes for the slug LETTERS = 'a'..'z' NUMBERS = '0'..'9' @@ -159,6 +157,10 @@ def additional_metrics prometheus_adapter.query(:additional_metrics_environment, self) if has_metrics? end + def prometheus_adapter + @prometheus_adapter ||= Prometheus::AdapterService.new(project, deployment_platform).prometheus_adapter + end + def slug super.presence || generate_slug end diff --git a/app/services/prometheus/adapter_service.rb b/app/services/prometheus/adapter_service.rb new file mode 100644 index 000000000000..4504d2ccfe6c --- /dev/null +++ b/app/services/prometheus/adapter_service.rb @@ -0,0 +1,36 @@ +module Prometheus + class AdapterService + def initialize(project, deployment_platform = nil) + @project = project + + @deployment_platform = if deployment_platform + deployment_platform + else + project.deployment_platform + end + end + + attr_reader :deployment_platform, :project + + def prometheus_adapter + @prometheus_adapter ||= if service_prometheus_adapter.can_query? + service_prometheus_adapter + else + cluster_prometheus_adapter + end + end + + def service_prometheus_adapter + project.find_or_initialize_service('prometheus') + end + + def cluster_prometheus_adapter + return unless deployment_platform.respond_to?(:cluster) + + cluster = deployment_platform.cluster + return unless cluster.application_prometheus&.installed? + + cluster.application_prometheus + end + end +end diff --git a/spec/services/prometheus/adapter_service_spec.rb b/spec/services/prometheus/adapter_service_spec.rb new file mode 100644 index 000000000000..335fc5844aa4 --- /dev/null +++ b/spec/services/prometheus/adapter_service_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Prometheus::AdapterService do + let(:project) { create(:project) } + subject { described_class.new(project) } + + describe '#prometheus_adapter' do + let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [project]) } + + context 'prometheus service can execute queries' do + let(:prometheus_service) { double(:prometheus_service, can_query?: true) } + + before do + allow(project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it 'return prometheus service as prometheus adapter' do + expect(subject.prometheus_adapter).to eq(prometheus_service) + end + end + + context "prometheus service can't execute queries" do + let(:prometheus_service) { double(:prometheus_service, can_query?: false) } + + context 'with cluster with prometheus installed' do + let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } + + it 'returns application handling all environments' do + expect(subject.prometheus_adapter).to eq(prometheus) + end + end + + context 'with cluster without prometheus installed' do + it 'returns nil' do + expect(subject.prometheus_adapter).to be_nil + end + end + end + end +end -- GitLab From b53356546dc54a6674eda8fa2d4f730a70f77206 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Mar 2018 21:02:26 +0100 Subject: [PATCH 17/49] Check if prometheus_adapter is properly called --- .../projects/prometheus/metrics_controller.rb | 3 +- .../prometheus/metrics_controller_spec.rb | 18 +++++++++-- spec/models/environment_spec.rb | 32 ++----------------- 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/app/controllers/projects/prometheus/metrics_controller.rb b/app/controllers/projects/prometheus/metrics_controller.rb index 523abce35fb1..51b29855bcfc 100644 --- a/app/controllers/projects/prometheus/metrics_controller.rb +++ b/app/controllers/projects/prometheus/metrics_controller.rb @@ -21,9 +21,10 @@ def active_common private def prometheus_adapter - @prometheus_adapter ||= Prometheus::AdapterService.new(project).prometheus_adapter + @prometheus_adapter ||= ::Prometheus::AdapterService.new(project).prometheus_adapter end + def require_prometheus_metrics! render_404 unless prometheus_adapter.can_query? end diff --git a/spec/controllers/projects/prometheus/metrics_controller_spec.rb b/spec/controllers/projects/prometheus/metrics_controller_spec.rb index 9d2ac42e99fb..b2b245dba90e 100644 --- a/spec/controllers/projects/prometheus/metrics_controller_spec.rb +++ b/spec/controllers/projects/prometheus/metrics_controller_spec.rb @@ -7,13 +7,15 @@ let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } before do - allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) - project.add_master(user) sign_in(user) end describe 'GET #active_common' do + before do + allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) + end + context 'when prometheus metrics are enabled' do context 'when data is not present' do before do @@ -52,6 +54,18 @@ end end + describe '#prometheus_adapter' do + before do + allow(controller).to receive(:project).and_return(project) + end + + it 'calls prometheus adapter service' do + expect_any_instance_of(::Prometheus::AdapterService).to receive(:prometheus_adapter) + + subject.__send__(:prometheus_adapter) + end + end + def project_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project) end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 8c052888b046..ceb570ac7772 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -623,36 +623,10 @@ end describe '#prometheus_adapter' do - let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [project]) } + it 'calls prometheus adapter service' do + expect_any_instance_of(Prometheus::AdapterService).to receive(:prometheus_adapter) - context 'prometheus service can execute queries' do - let(:prometheus_service) { double(:prometheus_service, can_query?: true) } - - before do - allow(environment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service - end - - it 'return prometheus service as prometheus adapter' do - expect(environment.prometheus_adapter).to eq(prometheus_service) - end - end - - context "prometheus service can't execute queries" do - let(:prometheus_service) { double(:prometheus_service, can_query?: false) } - - context 'with cluster with prometheus installed' do - let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } - - it 'returns application handling all environments' do - expect(environment.prometheus_adapter).to eq(prometheus) - end - end - - context 'with cluster without prometheus installed' do - it 'returns nil' do - expect(environment.prometheus_adapter).to be_nil - end - end + subject.prometheus_adapter end end end -- GitLab From 1a07d83484a41d40dc7f3288f23f2ea913941aa7 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 5 Mar 2018 23:49:50 +0100 Subject: [PATCH 18/49] Add cluster query spec --- .../prometheus/queries/cluster_query_spec.rb | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 ee/spec/lib/gitlab/prometheus/queries/cluster_query_spec.rb diff --git a/ee/spec/lib/gitlab/prometheus/queries/cluster_query_spec.rb b/ee/spec/lib/gitlab/prometheus/queries/cluster_query_spec.rb new file mode 100644 index 000000000000..44ef5ac03dbf --- /dev/null +++ b/ee/spec/lib/gitlab/prometheus/queries/cluster_query_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::Prometheus::Queries::ClusterQuery do + let(:client) { double('prometheus_client', query_range: nil) } + subject { described_class.new(client) } + + around do |example| + Timecop.freeze { example.run } + end + + it 'load cluster metrics from yaml' do + expect(Gitlab::Prometheus::AdditionalMetricsParser).to receive(:load_groups_from_yaml).with('cluster_metrics.yml').and_call_original + + subject.query + end + + it 'sends queries to prometheus' do + subject.query + + expect(client).to have_received(:query_range).with(anything, start: 8.hours.ago, stop: Time.now).at_least(1) + end +end -- GitLab From f9bfeb34cb82ece9c81af211f4fefa68dd4a62f0 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Mar 2018 00:04:06 +0100 Subject: [PATCH 19/49] Remove previously moved specs --- .../prometheus_service_spec.rb | 103 ------------------ 1 file changed, 103 deletions(-) diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 08b2356fd7c1..7afb1b4a8e3e 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -54,109 +54,6 @@ end end - describe '#environment_metrics' do - let(:environment) { build_stubbed(:environment, slug: 'env-slug') } - - around do |example| - Timecop.freeze { example.run } - end - - context 'with valid data' do - subject { service.query(:environment, environment) } - - before do - stub_reactive_cache(service, prometheus_data, environment_query, environment.id) - end - - it 'returns reactive data' do - is_expected.to eq(prometheus_metrics_data) - end - end - end - - describe '#matched_metrics' do - let(:matched_metrics_query) { Gitlab::Prometheus::Queries::MatchedMetricQuery } - let(:prometheus_client_wrapper) { double(:prometheus_client_wrapper, label_values: nil) } - - context 'with valid data' do - subject { service.query(:matched_metrics) } - - before do - allow(service).to receive(:prometheus_client_wrapper).and_return(prometheus_client_wrapper) - synchronous_reactive_cache(service) - end - - it 'returns reactive data' do - expect(subject[:success]).to be_truthy - expect(subject[:data]).to eq([]) - end - end - end - - describe '#deployment_metrics' do - let(:deployment) { build_stubbed(:deployment) } - let(:deployment_query) { Gitlab::Prometheus::Queries::DeploymentQuery } - - around do |example| - Timecop.freeze { example.run } - end - - context 'with valid data' do - subject { service.query(:deployment, deployment) } - - before do - stub_reactive_cache(service, prometheus_data, deployment_query, deployment.id) - end - - it 'returns reactive data' do - expect(subject).to eq(prometheus_metrics_data) - end - end - end - - describe '#calculate_reactive_cache' do - let(:environment) { create(:environment, slug: 'env-slug') } - before do - service.manual_configuration = true - service.active = true - end - - subject do - service.calculate_reactive_cache(environment_query.name, environment.id) - end - - around do |example| - Timecop.freeze { example.run } - end - - context 'when service is inactive' do - before do - service.active = false - end - - it { is_expected.to be_nil } - end - - context 'when Prometheus responds with valid data' do - before do - stub_all_prometheus_requests(environment.slug) - end - - it { expect(subject.to_json).to eq(prometheus_data.to_json) } - it { expect(subject.to_json).to eq(prometheus_data.to_json) } - end - - [404, 500].each do |status| - context "when Prometheus responds with #{status}" do - before do - stub_all_prometheus_requests(environment.slug, status: status, body: "QUERY FAILED!") - end - - it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } - end - end - end - describe '#prometheus_client' do context 'manual configuration is enabled' do let(:api_url) { 'http://some_url' } -- GitLab From 6d51bbd7209055572c19fe4000785f4559d21e36 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Mar 2018 00:15:59 +0100 Subject: [PATCH 20/49] Fix merge artifacts --- .../projects/prometheus/metrics_controller_spec.rb | 2 +- spec/models/environment_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/controllers/projects/prometheus/metrics_controller_spec.rb b/spec/controllers/projects/prometheus/metrics_controller_spec.rb index e364e99d8570..95a11690da2a 100644 --- a/spec/controllers/projects/prometheus/metrics_controller_spec.rb +++ b/spec/controllers/projects/prometheus/metrics_controller_spec.rb @@ -8,7 +8,7 @@ before do allow(controller).to receive(:project).and_return(project) - allow(project).to receive(:prometheus_service).and_return(prometheus_adapter) + allow(controller).to receive(:prometheus_adapter).and_return(prometheus_adapter) project.add_master(user) sign_in(user) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index a8bbe66c7fea..eb86ddc257f5 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -543,9 +543,9 @@ end end - describe '#metrics' do + describe '#additional_metrics' do let(:project) { create(:prometheus_project) } - subject { environment.metrics } + subject { environment.additional_metrics } context 'when the environment has metrics' do before do -- GitLab From 6d7be79b37bd13a4f99ecf20500329fccd2e8856 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 21 Feb 2018 16:41:28 -0600 Subject: [PATCH 21/49] add prometheus cluster health monitoring empty state --- .../javascripts/clusters/components/applications.vue | 5 ++++- app/views/projects/clusters/_health.html.haml | 9 +++++++++ app/views/projects/clusters/show.html.haml | 2 ++ 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 app/views/projects/clusters/_health.html.haml diff --git a/app/assets/javascripts/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue index 1325a2682143..27136c7289fa 100644 --- a/app/assets/javascripts/clusters/components/applications.vue +++ b/app/assets/javascripts/clusters/components/applications.vue @@ -117,7 +117,10 @@ -- GitLab From fbbf103af9f0d77a50997a43515334cfdbf2c4ef Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Feb 2018 01:29:36 -0600 Subject: [PATCH 27/49] remove unnecessary wrapper class --- app/views/projects/clusters/_health.html.haml | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/app/views/projects/clusters/_health.html.haml b/app/views/projects/clusters/_health.html.haml index 2f0dfa8ac50a..83cc218f4c4b 100644 --- a/app/views/projects/clusters/_health.html.haml +++ b/app/views/projects/clusters/_health.html.haml @@ -2,21 +2,20 @@ %section.settings.no-animate.expanded#cluster-health %h4= s_('ClusterIntegration|Kubernetes cluster health') - .settings-content - - if @cluster&.application_prometheus&.installed? - #prometheus-graphs{ data: { "settings-path": edit_project_service_path(@project, 'prometheus'), - "clusters-path": project_clusters_path(@project), - "documentation-path": help_page_path('administration/monitoring/prometheus/index.md'), - "empty-getting-started-svg-path": image_path('illustrations/monitoring/getting_started.svg'), - "empty-loading-svg-path": image_path('illustrations/monitoring/loading.svg'), - "empty-unable-to-connect-svg-path": image_path('illustrations/monitoring/unable_to_connect.svg'), - "metrics-endpoint": metrics_namespace_project_cluster_path( format: :json ), - "project-path": project_path(@project), - "tags-path": project_tags_path(@project), - "has-metrics": "true" } } + - if @cluster&.application_prometheus&.installed? + #prometheus-graphs{ data: { "settings-path": edit_project_service_path(@project, 'prometheus'), + "clusters-path": project_clusters_path(@project), + "documentation-path": help_page_path('administration/monitoring/prometheus/index.md'), + "empty-getting-started-svg-path": image_path('illustrations/monitoring/getting_started.svg'), + "empty-loading-svg-path": image_path('illustrations/monitoring/loading.svg'), + "empty-unable-to-connect-svg-path": image_path('illustrations/monitoring/unable_to_connect.svg'), + "metrics-endpoint": metrics_namespace_project_cluster_path( format: :json ), + "project-path": project_path(@project), + "tags-path": project_tags_path(@project), + "has-metrics": "true" } } - - else - %p= s_("ClusterIntegration|In order to show the health of the cluster, we'll need to provision your cluster with Prometheus to collect the required data.") + - else + %p= s_("ClusterIntegration|In order to show the health of the cluster, we'll need to provision your cluster with Prometheus to collect the required data.") - %a.btn{ href: '#cluster-applications' } - = s_('ClusterIntegration|Install Prometheus') + %a.btn{ href: '#cluster-applications' } + = s_('ClusterIntegration|Install Prometheus') -- GitLab From 187f69a0ec959341cec80e63e03c47ad9f9101b2 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Feb 2018 01:57:18 -0600 Subject: [PATCH 28/49] fix spacing around axis label text in small breakpoints --- .../javascripts/monitoring/components/graph/legend.vue | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/monitoring/components/graph/legend.vue b/app/assets/javascripts/monitoring/components/graph/legend.vue index bdd00a61d851..3149397b61f0 100644 --- a/app/assets/javascripts/monitoring/components/graph/legend.vue +++ b/app/assets/javascripts/monitoring/components/graph/legend.vue @@ -62,8 +62,9 @@ }, rectTransform() { - const yCoordinate = ((this.graphHeight - this.margin.top) / 2) - + (this.yLabelWidth / 2) + 10 || 0; + const yCoordinate = (((this.graphHeight - this.margin.top) + + this.measurements.axisLabelLineOffset) / 2) + + (this.yLabelWidth / 2) || 0; return `translate(0, ${yCoordinate}) rotate(-90)`; }, -- GitLab From 7ddf687f072122839cfa98bb074a9aa63afc5af2 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Feb 2018 01:57:50 -0600 Subject: [PATCH 29/49] add ability to override graph size measurements --- app/assets/javascripts/monitoring/components/dashboard.vue | 6 ++++++ app/assets/javascripts/monitoring/components/graph.vue | 7 ++++++- .../pages/projects/clusters/show/cluster_health.js | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index 3544dd96e3e4..f1d61a1c360c 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -26,6 +26,11 @@ required: false, default: true, }, + forceSmallGraph: { + type: Boolean, + required: false, + default: false, + }, documentationPath: { type: String, required: true, @@ -165,6 +170,7 @@ :project-path="projectPath" :tags-path="tagsPath" :show-legend="showLegend" + :small-graph="forceSmallGraph" /> diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index 30236d51d308..9e67a6f21465 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -57,6 +57,11 @@ required: false, default: true, }, + smallGraph: { + type: Boolean, + required: false, + default: false, + }, }, data() { @@ -135,7 +140,7 @@ const breakpointSize = bp.getBreakpointSize(); const query = this.graphData.queries[0]; this.margin = measurements.large.margin; - if (breakpointSize === 'xs' || breakpointSize === 'sm') { + if (this.smallGraph || breakpointSize === 'xs' || breakpointSize === 'sm') { this.graphHeight = 300; this.margin = measurements.small.margin; this.measurements = measurements.small; diff --git a/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js b/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js index 513d3346715e..1ab7b38cb667 100644 --- a/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js +++ b/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js @@ -10,7 +10,7 @@ export default () => { el, render(createElement) { return createElement(Dashboard, { - props: { ...el.dataset, showLegend: false }, + props: { ...el.dataset, showLegend: false, forceSmallGraph: true }, }); }, }); -- GitLab From f1d8163d584a9482d2170d8794879ac466eef607 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Feb 2018 02:06:35 -0600 Subject: [PATCH 30/49] lighten axis tick text --- app/assets/stylesheets/pages/environments.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index 3952af434e19..a277c4038fc3 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -529,7 +529,8 @@ } > text { - font-size: 12px; + fill: $theme-gray-600; + font-size: 10px; } } -- GitLab From 236fc437d25da4186b7f87069cb63f2bca6e56e9 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Feb 2018 02:13:51 -0600 Subject: [PATCH 31/49] add settings section wrapper to empty state --- app/views/projects/clusters/_health.html.haml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/views/projects/clusters/_health.html.haml b/app/views/projects/clusters/_health.html.haml index 83cc218f4c4b..79d00764d96d 100644 --- a/app/views/projects/clusters/_health.html.haml +++ b/app/views/projects/clusters/_health.html.haml @@ -15,7 +15,8 @@ "has-metrics": "true" } } - else - %p= s_("ClusterIntegration|In order to show the health of the cluster, we'll need to provision your cluster with Prometheus to collect the required data.") + .settings-content + %p= s_("ClusterIntegration|In order to show the health of the cluster, we'll need to provision your cluster with Prometheus to collect the required data.") - %a.btn{ href: '#cluster-applications' } - = s_('ClusterIntegration|Install Prometheus') + %a.btn{ href: '#cluster-applications' } + = s_('ClusterIntegration|Install Prometheus') -- GitLab From cbf6edfb3073a34275c3fb97d88dcc5cf8baefa1 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Feb 2018 02:50:32 -0600 Subject: [PATCH 32/49] limit the size of the loading and unable-to-connect states for cluster monitoring --- app/assets/stylesheets/pages/environments.scss | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index a277c4038fc3..396c0a9c1fc5 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -383,6 +383,16 @@ } } +#cluster-health .prometheus-state { + .state-svg img { + max-height: 120px; + } + .state-description, + .state-button { + display: none; + } +} + .environments-actions { .external-url, .monitoring-url, -- GitLab From 16b55534b621504f3e4fa463aaadbf12318eb582 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Mar 2018 00:23:55 +0100 Subject: [PATCH 33/49] Fix autoformatting error --- spec/controllers/projects/clusters_controller_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 99967d4408a8..edd58bc38e12 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -425,8 +425,8 @@ def go_json it "destroys and redirects back to clusters list" do expect { go } .to change { Clusters::Cluster.count }.by(-1) - .and change { Clusters::Platforms::Kubernetes.count }.by(-1) - .and change { Clusters::Providers::Gcp.count }.by(-1) + .and change { Clusters::Platforms::Kubernetes.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(-1) expect(response).to redirect_to(project_clusters_path(project)) expect(flash[:notice]).to eq('Kubernetes cluster integration was successfully removed.') @@ -439,7 +439,7 @@ def go_json it "destroys and redirects back to clusters list" do expect { go } .to change { Clusters::Cluster.count }.by(-1) - .and change { Clusters::Providers::Gcp.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(-1) expect(response).to redirect_to(project_clusters_path(project)) expect(flash[:notice]).to eq('Kubernetes cluster integration was successfully removed.') @@ -453,8 +453,8 @@ def go_json it "destroys and redirects back to clusters list" do expect { go } .to change { Clusters::Cluster.count }.by(-1) - .and change { Clusters::Platforms::Kubernetes.count }.by(-1) - .and change { Clusters::Providers::Gcp.count }.by(0) + .and change { Clusters::Platforms::Kubernetes.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(0) expect(response).to redirect_to(project_clusters_path(project)) expect(flash[:notice]).to eq('Kubernetes cluster integration was successfully removed.') -- GitLab From 5331eb07158e9ecc47df822f1191f1a013698483 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Feb 2018 02:55:29 -0600 Subject: [PATCH 34/49] remove prometheus panel styling on cluster monitoring page --- .../monitoring/components/dashboard.vue | 6 ++++++ .../monitoring/components/graph_group.vue | 16 +++++++++++++++- .../projects/clusters/show/cluster_health.js | 7 ++++++- app/assets/stylesheets/pages/environments.scss | 17 ++++++++++------- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index f1d61a1c360c..04374d2e1dbe 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -26,6 +26,11 @@ required: false, default: true, }, + showPanels: { + type: Boolean, + required: false, + default: true, + }, forceSmallGraph: { type: Boolean, required: false, @@ -159,6 +164,7 @@ v-for="(groupData, index) in store.groups" :key="index" :name="groupData.group" + :show-panels="showPanels" > diff --git a/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js b/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js index 1ab7b38cb667..e06cd09131cf 100644 --- a/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js +++ b/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js @@ -10,7 +10,12 @@ export default () => { el, render(createElement) { return createElement(Dashboard, { - props: { ...el.dataset, showLegend: false, forceSmallGraph: true }, + props: { + ...el.dataset, + showLegend: false, + showPanels: false, + forceSmallGraph: true, + }, }); }, }); diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index 396c0a9c1fc5..dc4d9262a23f 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -383,13 +383,16 @@ } } -#cluster-health .prometheus-state { - .state-svg img { - max-height: 120px; - } - .state-description, - .state-button { - display: none; +#cluster-health { + .prometheus-state { + .state-svg img { + max-height: 120px; + } + + .state-description, + .state-button { + display: none; + } } } -- GitLab From 8e7e9d94c59e63b593b64b9568a244e78180efec Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Mon, 5 Mar 2018 15:23:10 -0600 Subject: [PATCH 35/49] move cluster health monitoring behind an EEU feature flag --- app/views/projects/clusters/show.html.haml | 4 +++- ee/app/models/license.rb | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/views/projects/clusters/show.html.haml b/app/views/projects/clusters/show.html.haml index abd76b8eca42..04acb979b49d 100644 --- a/app/views/projects/clusters/show.html.haml +++ b/app/views/projects/clusters/show.html.haml @@ -22,7 +22,9 @@ .js-cluster-application-notice .flash-container - = render 'health' + -# EE-specific + - if @cluster.project.feature_available?(:cluster_health) + = render 'health' %section.settings.no-animate.expanded#cluster-integration = render 'banner' diff --git a/ee/app/models/license.rb b/ee/app/models/license.rb index bdd24367cdfa..8011f53d410f 100644 --- a/ee/app/models/license.rb +++ b/ee/app/models/license.rb @@ -59,6 +59,7 @@ class License < ActiveRecord::Base EEU_FEATURES = EEP_FEATURES + %i[ sast sast_container + cluster_health dast epics ide -- GitLab From 75b98d051f7c34bd91568c466b3c2c66ea17d5e3 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Mon, 5 Mar 2018 17:39:56 -0600 Subject: [PATCH 36/49] fix karma tests --- spec/javascripts/monitoring/dashboard_spec.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/spec/javascripts/monitoring/dashboard_spec.js b/spec/javascripts/monitoring/dashboard_spec.js index eb8f6bbe50d2..79924a5f3a86 100644 --- a/spec/javascripts/monitoring/dashboard_spec.js +++ b/spec/javascripts/monitoring/dashboard_spec.js @@ -8,6 +8,20 @@ describe('Dashboard', () => { const fixtureName = 'environments/metrics/metrics.html.raw'; let DashboardComponent; let component; + const propsData = { + hasMetrics: 'false', + documentationPath: '/path/to/docs', + settingsPath: '/path/to/settings', + clustersPath: '/path/to/clusters', + tagsPath: '/path/to/tags', + projectPath: '/path/to/project', + metricsEndpoint: mockApiEndpoint, + deploymentEndpoint: '/endpoint/deployments', + emptyGettingStartedSvgPath: '/path/to/getting-started.svg', + emptyLoadingSvgPath: '/path/to/loading.svg', + emptyUnableToConnectSvgPath: '/path/to/unable-to-connect.svg', + }; + preloadFixtures(fixtureName); beforeEach(() => { @@ -19,6 +33,7 @@ describe('Dashboard', () => { it('shows a getting started empty state when no metrics are present', () => { component = new DashboardComponent({ el: document.querySelector('#prometheus-graphs'), + propsData, }); component.$mount(); @@ -30,7 +45,6 @@ describe('Dashboard', () => { describe('requests information to the server', () => { let mock; beforeEach(() => { - document.querySelector('#prometheus-graphs').setAttribute('data-has-metrics', 'true'); mock = new MockAdapter(axios); mock.onGet(mockApiEndpoint).reply(200, { metricsGroupsAPIResponse, @@ -44,6 +58,7 @@ describe('Dashboard', () => { it('shows up a loading state', (done) => { component = new DashboardComponent({ el: document.querySelector('#prometheus-graphs'), + propsData: { ...propsData, hasMetrics: 'true' }, }); component.$mount(); Vue.nextTick(() => { -- GitLab From 581f95a250ceb9ff8353fa67e799ce3fe6d0cae3 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Mar 2018 02:38:29 +0100 Subject: [PATCH 37/49] 1 deployment platform too many --- app/models/environment.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/environment.rb b/app/models/environment.rb index 26d36533f511..2e437c96f8e6 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -232,10 +232,6 @@ def folder_name self.environment_type || self.name end - def deployment_platform - project.deployment_platform - end - private # Slugifying a name may remove the uniqueness guarantee afforded by it being -- GitLab From 835c7813405ea1520a236c769169f118cbca8f2b Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 6 Mar 2018 01:25:39 -0600 Subject: [PATCH 38/49] prefer getElementById --- app/assets/javascripts/monitoring/monitoring_bundle.js | 2 +- .../javascripts/pages/projects/clusters/show/cluster_health.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/monitoring/monitoring_bundle.js b/app/assets/javascripts/monitoring/monitoring_bundle.js index a9b608e44332..f881a3954b98 100644 --- a/app/assets/javascripts/monitoring/monitoring_bundle.js +++ b/app/assets/javascripts/monitoring/monitoring_bundle.js @@ -2,7 +2,7 @@ import Vue from 'vue'; import Dashboard from './components/dashboard.vue'; export default () => { - const el = document.querySelector('#prometheus-graphs'); + const el = document.getElementById('prometheus-graphs'); if (el && el.dataset) { // eslint-disable-next-line no-new diff --git a/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js b/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js index e06cd09131cf..8afe1aabdcf0 100644 --- a/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js +++ b/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js @@ -2,7 +2,7 @@ import Vue from 'vue'; import Dashboard from '~/monitoring/components/dashboard.vue'; export default () => { - const el = document.querySelector('#prometheus-graphs'); + const el = document.getElementById('prometheus-graphs'); if (el && el.dataset) { // eslint-disable-next-line no-new -- GitLab From d8656e0c42637e09ab0af564477883f43b5bf67c Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 6 Mar 2018 01:28:03 -0600 Subject: [PATCH 39/49] prefer classes over IDs --- .../stylesheets/pages/environments.scss | 27 ++++++++++--------- app/views/projects/clusters/_health.html.haml | 2 +- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index dc4d9262a23f..ed3da2cd3d11 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -383,19 +383,6 @@ } } -#cluster-health { - .prometheus-state { - .state-svg img { - max-height: 120px; - } - - .state-description, - .state-button { - display: none; - } - } -} - .environments-actions { .external-url, .monitoring-url, @@ -587,3 +574,17 @@ } } } + +// EE-only +.cluster-health-graphs { + .prometheus-state { + .state-svg img { + max-height: 120px; + } + + .state-description, + .state-button { + display: none; + } + } +} diff --git a/app/views/projects/clusters/_health.html.haml b/app/views/projects/clusters/_health.html.haml index 79d00764d96d..e2637e454395 100644 --- a/app/views/projects/clusters/_health.html.haml +++ b/app/views/projects/clusters/_health.html.haml @@ -1,5 +1,5 @@ -%section.settings.no-animate.expanded#cluster-health +%section.settings.no-animate.expanded.cluster-health-graphs#cluster-health %h4= s_('ClusterIntegration|Kubernetes cluster health') - if @cluster&.application_prometheus&.installed? -- GitLab From 68bcd4feb141c402fb58385837541c2521bf6570 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 6 Mar 2018 01:43:12 -0600 Subject: [PATCH 40/49] move ee-specific files into the ee namespace --- app/assets/javascripts/pages/projects/clusters/show/index.js | 2 -- .../pages/projects/clusters/show/cluster_health.js | 0 .../assets/javascripts/pages/projects/clusters/show/index.js | 4 ++++ {app => ee/app}/views/projects/clusters/_health.html.haml | 0 4 files changed, 4 insertions(+), 2 deletions(-) rename {app => ee/app}/assets/javascripts/pages/projects/clusters/show/cluster_health.js (100%) create mode 100644 ee/app/assets/javascripts/pages/projects/clusters/show/index.js rename {app => ee/app}/views/projects/clusters/_health.html.haml (100%) diff --git a/app/assets/javascripts/pages/projects/clusters/show/index.js b/app/assets/javascripts/pages/projects/clusters/show/index.js index 0d2736d9c8e5..8001d2dd1da5 100644 --- a/app/assets/javascripts/pages/projects/clusters/show/index.js +++ b/app/assets/javascripts/pages/projects/clusters/show/index.js @@ -1,7 +1,5 @@ import ClustersBundle from '~/clusters/clusters_bundle'; -import initClusterHealth from './cluster_health'; document.addEventListener('DOMContentLoaded', () => { new ClustersBundle(); // eslint-disable-line no-new - initClusterHealth(); }); diff --git a/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js b/ee/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js similarity index 100% rename from app/assets/javascripts/pages/projects/clusters/show/cluster_health.js rename to ee/app/assets/javascripts/pages/projects/clusters/show/cluster_health.js diff --git a/ee/app/assets/javascripts/pages/projects/clusters/show/index.js b/ee/app/assets/javascripts/pages/projects/clusters/show/index.js new file mode 100644 index 000000000000..873c18c8ab0b --- /dev/null +++ b/ee/app/assets/javascripts/pages/projects/clusters/show/index.js @@ -0,0 +1,4 @@ +import '~/pages/projects/clusters/show'; +import initClusterHealth from './cluster_health'; + +document.addEventListener('DOMContentLoaded', initClusterHealth); diff --git a/app/views/projects/clusters/_health.html.haml b/ee/app/views/projects/clusters/_health.html.haml similarity index 100% rename from app/views/projects/clusters/_health.html.haml rename to ee/app/views/projects/clusters/_health.html.haml -- GitLab From 4ac53a1a4143fbc7f924ff6757a141e01d62b423 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 6 Mar 2018 03:38:42 -0600 Subject: [PATCH 41/49] coerce hasMetrics to a boolean value before instantiating the Vue component --- .../javascripts/monitoring/components/dashboard.vue | 8 ++++---- app/assets/javascripts/monitoring/monitoring_bundle.js | 6 +++++- ee/app/views/projects/clusters/_health.html.haml | 3 +-- spec/javascripts/monitoring/dashboard_spec.js | 4 ++-- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index 04374d2e1dbe..8ca94ef3e2ad 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -7,7 +7,6 @@ import EmptyState from './empty_state.vue'; import MonitoringStore from '../stores/monitoring_store'; import eventHub from '../event_hub'; - import { convertPermissionToBoolean } from '../../lib/utils/common_utils'; export default { components: { @@ -18,8 +17,9 @@ props: { hasMetrics: { - type: String, - required: true, + type: Boolean, + required: false, + default: true, }, showLegend: { type: Boolean, @@ -108,7 +108,7 @@ mounted() { this.resizeThrottled = _.throttle(this.resize, 600); - if (!convertPermissionToBoolean(this.hasMetrics)) { + if (!this.hasMetrics) { this.state = 'gettingStarted'; } else { this.getGraphsData(); diff --git a/app/assets/javascripts/monitoring/monitoring_bundle.js b/app/assets/javascripts/monitoring/monitoring_bundle.js index f881a3954b98..41270e015d49 100644 --- a/app/assets/javascripts/monitoring/monitoring_bundle.js +++ b/app/assets/javascripts/monitoring/monitoring_bundle.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import { convertPermissionToBoolean } from '~/lib/utils/common_utils'; import Dashboard from './components/dashboard.vue'; export default () => { @@ -10,7 +11,10 @@ export default () => { el, render(createElement) { return createElement(Dashboard, { - props: el.dataset, + props: { + ...el.dataset, + hasMetrics: convertPermissionToBoolean(el.dataset.hasMetrics), + }, }); }, }); diff --git a/ee/app/views/projects/clusters/_health.html.haml b/ee/app/views/projects/clusters/_health.html.haml index e2637e454395..9ddafffa2e9e 100644 --- a/ee/app/views/projects/clusters/_health.html.haml +++ b/ee/app/views/projects/clusters/_health.html.haml @@ -11,8 +11,7 @@ "empty-unable-to-connect-svg-path": image_path('illustrations/monitoring/unable_to_connect.svg'), "metrics-endpoint": metrics_namespace_project_cluster_path( format: :json ), "project-path": project_path(@project), - "tags-path": project_tags_path(@project), - "has-metrics": "true" } } + "tags-path": project_tags_path(@project) } } - else .settings-content diff --git a/spec/javascripts/monitoring/dashboard_spec.js b/spec/javascripts/monitoring/dashboard_spec.js index 79924a5f3a86..c5733ef95cf5 100644 --- a/spec/javascripts/monitoring/dashboard_spec.js +++ b/spec/javascripts/monitoring/dashboard_spec.js @@ -9,7 +9,7 @@ describe('Dashboard', () => { let DashboardComponent; let component; const propsData = { - hasMetrics: 'false', + hasMetrics: false, documentationPath: '/path/to/docs', settingsPath: '/path/to/settings', clustersPath: '/path/to/clusters', @@ -58,7 +58,7 @@ describe('Dashboard', () => { it('shows up a loading state', (done) => { component = new DashboardComponent({ el: document.querySelector('#prometheus-graphs'), - propsData: { ...propsData, hasMetrics: 'true' }, + propsData: { ...propsData, hasMetrics: true }, }); component.$mount(); Vue.nextTick(() => { -- GitLab From 94cd687b16892af81f6f871d81893c8b65f0e28e Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 6 Mar 2018 03:59:15 -0600 Subject: [PATCH 42/49] move misplaced entry point --- .../javascripts/pages/projects/environments/{ => index}/index.js | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/assets/javascripts/pages/projects/environments/{ => index}/index.js (100%) diff --git a/app/assets/javascripts/pages/projects/environments/index.js b/app/assets/javascripts/pages/projects/environments/index/index.js similarity index 100% rename from app/assets/javascripts/pages/projects/environments/index.js rename to app/assets/javascripts/pages/projects/environments/index/index.js -- GitLab From 6a8f8a4fb8f59597d90cfacf7a67af653c640b4e Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 6 Mar 2018 03:59:33 -0600 Subject: [PATCH 43/49] add missing classname to "Install Prometheus" button --- ee/app/views/projects/clusters/_health.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/views/projects/clusters/_health.html.haml b/ee/app/views/projects/clusters/_health.html.haml index 9ddafffa2e9e..3416fd9f227b 100644 --- a/ee/app/views/projects/clusters/_health.html.haml +++ b/ee/app/views/projects/clusters/_health.html.haml @@ -17,5 +17,5 @@ .settings-content %p= s_("ClusterIntegration|In order to show the health of the cluster, we'll need to provision your cluster with Prometheus to collect the required data.") - %a.btn{ href: '#cluster-applications' } + %a.btn.btn-default{ href: '#cluster-applications' } = s_('ClusterIntegration|Install Prometheus') -- GitLab From 28c544f1d541c2d4324e859aa80425b9b26c6095 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Mar 2018 16:18:28 +0100 Subject: [PATCH 44/49] Revert SSL verification config change --- app/models/clusters/platforms/kubernetes.rb | 2 +- app/models/concerns/prometheus_adapter.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index b9a362bd3081..74e30aa04d55 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -143,7 +143,7 @@ def read_pods end def kubeclient_ssl_options - opts = { verify_ssl: OpenSSL::SSL::VERIFY_NONE } + opts = { verify_ssl: OpenSSL::SSL::VERIFY_PEER } if ca_pem.present? opts[:cert_store] = OpenSSL::X509::Store.new diff --git a/app/models/concerns/prometheus_adapter.rb b/app/models/concerns/prometheus_adapter.rb index fea99a7cd35a..7fab12fa167e 100644 --- a/app/models/concerns/prometheus_adapter.rb +++ b/app/models/concerns/prometheus_adapter.rb @@ -26,7 +26,7 @@ def query(query_name, *args) query_class = Gitlab::Prometheus::Queries.const_get("#{query_name.to_s.classify}Query") - args.map! { |arg| arg.id } + args.map!(&:id) { |arg| arg.id } with_reactive_cache(query_class.name, *args, &query_class.method(:transform_reactive_result)) end -- GitLab From 0b5341c72bb094a01bfa5d28b1db87d6482aff21 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 6 Mar 2018 16:56:33 +0100 Subject: [PATCH 45/49] Fix syntax error --- app/models/concerns/prometheus_adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/prometheus_adapter.rb b/app/models/concerns/prometheus_adapter.rb index 7fab12fa167e..18cbbd871a11 100644 --- a/app/models/concerns/prometheus_adapter.rb +++ b/app/models/concerns/prometheus_adapter.rb @@ -26,7 +26,7 @@ def query(query_name, *args) query_class = Gitlab::Prometheus::Queries.const_get("#{query_name.to_s.classify}Query") - args.map!(&:id) { |arg| arg.id } + args.map!(&:id) with_reactive_cache(query_class.name, *args, &query_class.method(:transform_reactive_result)) end -- GitLab From 270478e3e115a4761d5faef573487aa33e42c2cf Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 6 Mar 2018 11:05:48 -0600 Subject: [PATCH 46/49] add new tests for metrics dashboard changes --- spec/javascripts/monitoring/dashboard_spec.js | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/spec/javascripts/monitoring/dashboard_spec.js b/spec/javascripts/monitoring/dashboard_spec.js index c5733ef95cf5..4c5a10393c67 100644 --- a/spec/javascripts/monitoring/dashboard_spec.js +++ b/spec/javascripts/monitoring/dashboard_spec.js @@ -7,7 +7,7 @@ import { metricsGroupsAPIResponse, mockApiEndpoint } from './mock_data'; describe('Dashboard', () => { const fixtureName = 'environments/metrics/metrics.html.raw'; let DashboardComponent; - let component; + const propsData = { hasMetrics: false, documentationPath: '/path/to/docs', @@ -16,7 +16,7 @@ describe('Dashboard', () => { tagsPath: '/path/to/tags', projectPath: '/path/to/project', metricsEndpoint: mockApiEndpoint, - deploymentEndpoint: '/endpoint/deployments', + deploymentEndpoint: null, emptyGettingStartedSvgPath: '/path/to/getting-started.svg', emptyLoadingSvgPath: '/path/to/loading.svg', emptyUnableToConnectSvgPath: '/path/to/unable-to-connect.svg', @@ -31,12 +31,11 @@ describe('Dashboard', () => { describe('no metrics are available yet', () => { it('shows a getting started empty state when no metrics are present', () => { - component = new DashboardComponent({ + const component = new DashboardComponent({ el: document.querySelector('#prometheus-graphs'), propsData, }); - component.$mount(); expect(component.$el.querySelector('#prometheus-graphs')).toBe(null); expect(component.state).toEqual('gettingStarted'); }); @@ -46,9 +45,7 @@ describe('Dashboard', () => { let mock; beforeEach(() => { mock = new MockAdapter(axios); - mock.onGet(mockApiEndpoint).reply(200, { - metricsGroupsAPIResponse, - }); + mock.onGet(mockApiEndpoint).reply(200, metricsGroupsAPIResponse); }); afterEach(() => { @@ -56,15 +53,43 @@ describe('Dashboard', () => { }); it('shows up a loading state', (done) => { - component = new DashboardComponent({ + const component = new DashboardComponent({ el: document.querySelector('#prometheus-graphs'), propsData: { ...propsData, hasMetrics: true }, }); - component.$mount(); + Vue.nextTick(() => { expect(component.state).toEqual('loading'); done(); }); }); + + it('hides the legend when showLegend is false', (done) => { + const component = new DashboardComponent({ + el: document.querySelector('#prometheus-graphs'), + propsData: { ...propsData, hasMetrics: true, showLegend: false }, + }); + + setTimeout(() => { + expect(component.showEmptyState).toEqual(false); + expect(component.$el.querySelector('.legend-group')).toBeFalsy(); + expect(component.$el.querySelector('.prometheus-graph-group')).toBeTruthy(); + done(); + }); + }); + + it('hides the group panels when showPanels is false', (done) => { + const component = new DashboardComponent({ + el: document.querySelector('#prometheus-graphs'), + propsData: { ...propsData, hasMetrics: true, showPanels: false }, + }); + + setTimeout(() => { + expect(component.showEmptyState).toEqual(false); + expect(component.$el.querySelector('.prometheus-panel')).toBeFalsy(); + expect(component.$el.querySelector('.prometheus-graph-group')).toBeTruthy(); + done(); + }); + }); }); }); -- GitLab From afe21c49da03397fa3a2e087f64a89914d99aed8 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 6 Mar 2018 11:09:16 -0600 Subject: [PATCH 47/49] replace fixture with simple mount point --- spec/javascripts/fixtures/environments.rb | 30 ------------------- spec/javascripts/monitoring/dashboard_spec.js | 15 ++++------ 2 files changed, 6 insertions(+), 39 deletions(-) delete mode 100644 spec/javascripts/fixtures/environments.rb diff --git a/spec/javascripts/fixtures/environments.rb b/spec/javascripts/fixtures/environments.rb deleted file mode 100644 index d2457d75419b..000000000000 --- a/spec/javascripts/fixtures/environments.rb +++ /dev/null @@ -1,30 +0,0 @@ -require 'spec_helper' - -describe Projects::EnvironmentsController, '(JavaScript fixtures)', type: :controller do - include JavaScriptFixturesHelpers - - let(:admin) { create(:admin) } - let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} - let(:project) { create(:project_empty_repo, namespace: namespace, path: 'environments-project') } - let(:environment) { create(:environment, name: 'production', project: project) } - - render_views - - before(:all) do - clean_frontend_fixtures('environments/metrics') - end - - before do - sign_in(admin) - end - - it 'environments/metrics/metrics.html.raw' do |example| - get :metrics, - namespace_id: project.namespace, - project_id: project, - id: environment.id - - expect(response).to be_success - store_frontend_fixture(response, example.description) - end -end diff --git a/spec/javascripts/monitoring/dashboard_spec.js b/spec/javascripts/monitoring/dashboard_spec.js index 4c5a10393c67..39de6f1bba60 100644 --- a/spec/javascripts/monitoring/dashboard_spec.js +++ b/spec/javascripts/monitoring/dashboard_spec.js @@ -5,7 +5,6 @@ import axios from '~/lib/utils/axios_utils'; import { metricsGroupsAPIResponse, mockApiEndpoint } from './mock_data'; describe('Dashboard', () => { - const fixtureName = 'environments/metrics/metrics.html.raw'; let DashboardComponent; const propsData = { @@ -22,21 +21,19 @@ describe('Dashboard', () => { emptyUnableToConnectSvgPath: '/path/to/unable-to-connect.svg', }; - preloadFixtures(fixtureName); - beforeEach(() => { - loadFixtures(fixtureName); + setFixtures('
'); DashboardComponent = Vue.extend(Dashboard); }); describe('no metrics are available yet', () => { it('shows a getting started empty state when no metrics are present', () => { const component = new DashboardComponent({ - el: document.querySelector('#prometheus-graphs'), + el: document.querySelector('.prometheus-graphs'), propsData, }); - expect(component.$el.querySelector('#prometheus-graphs')).toBe(null); + expect(component.$el.querySelector('.prometheus-graphs')).toBe(null); expect(component.state).toEqual('gettingStarted'); }); }); @@ -54,7 +51,7 @@ describe('Dashboard', () => { it('shows up a loading state', (done) => { const component = new DashboardComponent({ - el: document.querySelector('#prometheus-graphs'), + el: document.querySelector('.prometheus-graphs'), propsData: { ...propsData, hasMetrics: true }, }); @@ -66,7 +63,7 @@ describe('Dashboard', () => { it('hides the legend when showLegend is false', (done) => { const component = new DashboardComponent({ - el: document.querySelector('#prometheus-graphs'), + el: document.querySelector('.prometheus-graphs'), propsData: { ...propsData, hasMetrics: true, showLegend: false }, }); @@ -80,7 +77,7 @@ describe('Dashboard', () => { it('hides the group panels when showPanels is false', (done) => { const component = new DashboardComponent({ - el: document.querySelector('#prometheus-graphs'), + el: document.querySelector('.prometheus-graphs'), propsData: { ...propsData, hasMetrics: true, showPanels: false }, }); -- GitLab From 042b4720e9dcd3cc98aa839e93232acd93df17e4 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 6 Mar 2018 11:23:09 -0600 Subject: [PATCH 48/49] prefer checking explicitly for null --- spec/javascripts/monitoring/dashboard_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/javascripts/monitoring/dashboard_spec.js b/spec/javascripts/monitoring/dashboard_spec.js index 39de6f1bba60..29b355307ef0 100644 --- a/spec/javascripts/monitoring/dashboard_spec.js +++ b/spec/javascripts/monitoring/dashboard_spec.js @@ -69,7 +69,7 @@ describe('Dashboard', () => { setTimeout(() => { expect(component.showEmptyState).toEqual(false); - expect(component.$el.querySelector('.legend-group')).toBeFalsy(); + expect(component.$el.querySelector('.legend-group')).toEqual(null); expect(component.$el.querySelector('.prometheus-graph-group')).toBeTruthy(); done(); }); @@ -83,7 +83,7 @@ describe('Dashboard', () => { setTimeout(() => { expect(component.showEmptyState).toEqual(false); - expect(component.$el.querySelector('.prometheus-panel')).toBeFalsy(); + expect(component.$el.querySelector('.prometheus-panel')).toEqual(null); expect(component.$el.querySelector('.prometheus-graph-group')).toBeTruthy(); done(); }); -- GitLab From cac6a6d9d33b16f68c6edbcbe48c629d7e6a9908 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Wed, 7 Mar 2018 12:44:50 +0100 Subject: [PATCH 49/49] Use anonymous class --- spec/models/concerns/prometheus_adapter_spec.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/spec/models/concerns/prometheus_adapter_spec.rb b/spec/models/concerns/prometheus_adapter_spec.rb index f4b9c57e71a3..10c3ea634d76 100644 --- a/spec/models/concerns/prometheus_adapter_spec.rb +++ b/spec/models/concerns/prometheus_adapter_spec.rb @@ -4,14 +4,15 @@ include PrometheusHelpers include ReactiveCachingHelpers - class TestClass - include PrometheusAdapter - end - let(:project) { create(:prometheus_project) } let(:service) { project.prometheus_service } - let(:described_class) { TestClass } + let(:described_class) do + Class.new do + include PrometheusAdapter + end + end + let(:environment_query) { Gitlab::Prometheus::Queries::EnvironmentQuery } describe '#query' do -- GitLab