Commit 681f2ee5 authored by Tiago Botelho's avatar Tiago Botelho

Fixes broken backend specs

parent e4ba4ccf
...@@ -7,7 +7,7 @@ class PrometheusAlert < ActiveRecord::Base ...@@ -7,7 +7,7 @@ class PrometheusAlert < ActiveRecord::Base
belongs_to :environment, required: true, validate: true, inverse_of: :prometheus_alerts belongs_to :environment, required: true, validate: true, inverse_of: :prometheus_alerts
belongs_to :project, required: true, validate: true, inverse_of: :prometheus_alerts belongs_to :project, required: true, validate: true, inverse_of: :prometheus_alerts
belongs_to :prometheus_metric, required: true, validate: true belongs_to :prometheus_metric, required: true, validate: true, inverse_of: :prometheus_alert
after_save :clear_prometheus_adapter_cache! after_save :clear_prometheus_adapter_cache!
after_destroy :clear_prometheus_adapter_cache! after_destroy :clear_prometheus_adapter_cache!
......
class PrometheusMetric < ActiveRecord::Base class PrometheusMetric < ActiveRecord::Base
belongs_to :project, required: true, validate: true, inverse_of: :prometheus_metrics belongs_to :project, required: true, validate: true, inverse_of: :prometheus_metrics
has_one :prometheus_alert has_one :prometheus_alert, inverse_of: :prometheus_metric
enum group: [:business, :response, :system] enum group: [:business, :response, :system]
......
...@@ -49,7 +49,6 @@ module EE ...@@ -49,7 +49,6 @@ module EE
@subject.feature_available?(:pod_logs, @user) @subject.feature_available?(:pod_logs, @user)
end end
with_scope :subject
condition(:prometheus_alerts_enabled) do condition(:prometheus_alerts_enabled) do
@subject.feature_available?(:prometheus_alerts, @user) @subject.feature_available?(:prometheus_alerts, @user)
end end
......
...@@ -29,7 +29,7 @@ module Clusters ...@@ -29,7 +29,7 @@ module Clusters
def recently_scheduled? def recently_scheduled?
return false unless application.last_update_started_at return false unless application.last_update_started_at
application.last_update_started_at >= Time.now - BACKOFF_DELAY application.last_update_started_at.utc >= Time.now.utc - BACKOFF_DELAY
end end
end end
end end
......
module Projects module Projects
module Prometheus module Prometheus
module Metrics module Metrics
class DestroyService < BaseService class DestroyService < Metrics::BaseService
def execute def execute
schedule_alert_update if has_alert? schedule_alert_update if has_alert?
metric.destroy metric.destroy
......
module Projects module Projects
module Prometheus module Prometheus
module Metrics module Metrics
class UpdateService < BaseService class UpdateService < Metrics::BaseService
def execute def execute
metric.update(params) metric.update!(params)
schedule_alert_update if requires_alert_update? schedule_alert_update if requires_alert_update?
metric metric
end end
......
...@@ -13,7 +13,7 @@ class ClusterUpdateAppWorker ...@@ -13,7 +13,7 @@ class ClusterUpdateAppWorker
find_application(app_name, app_id) do |app| find_application(app_name, app_id) do |app|
break if app.updated_since?(scheduled_time) break if app.updated_since?(scheduled_time)
raise UpdateAlreadyInProgressError if app.update_in_progress? break if app.update_in_progress?
Clusters::Applications::PrometheusUpdateService.new(app, project).execute Clusters::Applications::PrometheusUpdateService.new(app, project).execute
end end
......
...@@ -16,11 +16,11 @@ module EE ...@@ -16,11 +16,11 @@ module EE
end end
proc do |group| proc do |group|
group[:metrics]&.map! do |metric| group[:metrics] = group[:metrics]&.map do |metric|
key = metric[:id] key = metric[:id]
if key && alerts_map[key] if key && alerts_map[key]
metric[:queries]&.map! do |item| metric[:queries] = metric[:queries]&.map do |item|
item[:alert_path] = alert_path(alerts_map, key, project, environment) item[:alert_path] = alert_path(alerts_map, key, project, environment)
item item
......
...@@ -14,6 +14,14 @@ describe Projects::Prometheus::AlertsController do ...@@ -14,6 +14,14 @@ describe Projects::Prometheus::AlertsController do
describe 'GET #index' do describe 'GET #index' do
context 'when project has no prometheus alert' do context 'when project has no prometheus alert' do
it 'renders forbidden when unlicensed' do
stub_licensed_features(prometheus_alerts: false)
get :index, project_params
expect(response).to have_gitlab_http_status(:not_found)
end
it 'returns an empty response' do it 'returns an empty response' do
get :index, project_params get :index, project_params
...@@ -27,14 +35,6 @@ describe Projects::Prometheus::AlertsController do ...@@ -27,14 +35,6 @@ describe Projects::Prometheus::AlertsController do
create_list(:prometheus_alert, 3, project: project, environment: environment) create_list(:prometheus_alert, 3, project: project, environment: environment)
end end
it 'renders forbidden when unlicensed' do
stub_licensed_features(prometheus_alerts: false)
get :index, project_params
expect(response).to have_gitlab_http_status(:not_found)
end
it 'contains prometheus alerts' do it 'contains prometheus alerts' do
get :index, project_params get :index, project_params
...@@ -47,7 +47,7 @@ describe Projects::Prometheus::AlertsController do ...@@ -47,7 +47,7 @@ describe Projects::Prometheus::AlertsController do
describe 'GET #show' do describe 'GET #show' do
context 'when alert does not exist' do context 'when alert does not exist' do
it 'renders 404' do it 'renders 404' do
get :show, project_params(id: PrometheusAlert.all.maximum(:prometheus_metric_id).to_i) get :show, project_params(id: PrometheusAlert.all.maximum(:prometheus_metric_id).to_i + 1)
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
...@@ -67,7 +67,7 @@ describe Projects::Prometheus::AlertsController do ...@@ -67,7 +67,7 @@ describe Projects::Prometheus::AlertsController do
it 'renders the alert' do it 'renders the alert' do
alert_params = { alert_params = {
"id" => alert.id, "id" => alert.id,
"name" => alert.name, "title" => alert.title,
"query" => alert.query, "query" => alert.query,
"operator" => alert.computed_operator, "operator" => alert.computed_operator,
"threshold" => alert.threshold, "threshold" => alert.threshold,
...@@ -88,7 +88,7 @@ describe Projects::Prometheus::AlertsController do ...@@ -88,7 +88,7 @@ describe Projects::Prometheus::AlertsController do
notification_service = spy notification_service = spy
alert_params = { alert_params = {
"alert" => alert.name, "alert" => alert.title,
"expr" => "#{alert.query} #{alert.computed_operator} #{alert.threshold}", "expr" => "#{alert.query} #{alert.computed_operator} #{alert.threshold}",
"for" => "5m", "for" => "5m",
"labels" => { "labels" => {
...@@ -98,10 +98,10 @@ describe Projects::Prometheus::AlertsController do ...@@ -98,10 +98,10 @@ describe Projects::Prometheus::AlertsController do
} }
allow(NotificationService).to receive(:new).and_return(notification_service) allow(NotificationService).to receive(:new).and_return(notification_service)
expect(notification_service).to receive_message_chain(:async, :prometheus_alerts_fired).with(project, [alert_params])
post :notify, project_params(alerts: [alert]) post :notify, project_params(alerts: [alert])
expect(notification_service).to have_received(:prometheus_alerts_fired).with(project, [alert_params])
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
end end
end end
...@@ -123,7 +123,7 @@ describe Projects::Prometheus::AlertsController do ...@@ -123,7 +123,7 @@ describe Projects::Prometheus::AlertsController do
it 'creates a new prometheus alert' do it 'creates a new prometheus alert' do
schedule_update_service = spy schedule_update_service = spy
alert_params = { alert_params = {
"name" => metric.title, "title" => metric.title,
"query" => metric.query, "query" => metric.query,
"operator" => ">", "operator" => ">",
"threshold" => 1.0 "threshold" => 1.0
...@@ -155,7 +155,7 @@ describe Projects::Prometheus::AlertsController do ...@@ -155,7 +155,7 @@ describe Projects::Prometheus::AlertsController do
it 'renders forbidden when unlicensed' do it 'renders forbidden when unlicensed' do
stub_licensed_features(prometheus_alerts: false) stub_licensed_features(prometheus_alerts: false)
put :update, project_params(id: alert.prometheus_metric_id, name: "bar") put :update, project_params(id: alert.prometheus_metric_id, operator: "<")
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
...@@ -163,9 +163,9 @@ describe Projects::Prometheus::AlertsController do ...@@ -163,9 +163,9 @@ describe Projects::Prometheus::AlertsController do
it 'updates an already existing prometheus alert' do it 'updates an already existing prometheus alert' do
alert_params = { alert_params = {
"id" => alert.id, "id" => alert.id,
"name" => "bar", "title" => alert.title,
"query" => alert.query, "query" => alert.query,
"operator" => alert.computed_operator, "operator" => "<",
"threshold" => alert.threshold, "threshold" => alert.threshold,
"alert_path" => Gitlab::Routing.url_helpers.project_prometheus_alert_path(project, alert.prometheus_metric_id, environment_id: alert.environment.id, format: :json) "alert_path" => Gitlab::Routing.url_helpers.project_prometheus_alert_path(project, alert.prometheus_metric_id, environment_id: alert.environment.id, format: :json)
} }
......
...@@ -3,8 +3,6 @@ FactoryBot.define do ...@@ -3,8 +3,6 @@ FactoryBot.define do
project project
environment environment
prometheus_metric prometheus_metric
name { generate(:title) }
query "foo"
operator :gt operator :gt
threshold 1 threshold 1
end end
......
require 'spec_helper' require 'spec_helper'
describe PrometheusAlert do describe PrometheusAlert do
let(:metric) { create(:prometheus_metric) }
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:environment) } it { is_expected.to belong_to(:environment) }
end end
describe 'validation' do
it { is_expected.to validate_presence_of(:name) }
end
describe '#full_query' do describe '#full_query' do
it 'returns the concatenated query' do it 'returns the concatenated query' do
subject.name = "bar"
subject.query = "foo"
subject.operator = "gt" subject.operator = "gt"
subject.threshold = 1 subject.threshold = 1
subject.prometheus_metric_id = 1 subject.prometheus_metric_id = metric.id
expect(subject.full_query).to eq("foo > 1.0") expect(subject.full_query).to eq("#{metric.query} > 1.0")
end end
end end
describe '#to_param' do describe '#to_param' do
it 'returns the params of the prometheus alert' do it 'returns the params of the prometheus alert' do
subject.name = "bar"
subject.query = "foo"
subject.operator = "gt" subject.operator = "gt"
subject.threshold = 1 subject.threshold = 1
subject.prometheus_metric_id = 1 subject.prometheus_metric_id = metric.id
alert_params = { alert_params = {
"alert" => "bar", "alert" => metric.title,
"expr" => "foo > 1.0", "expr" => "#{metric.query} > 1.0",
"for" => "5m", "for" => "5m",
"labels" => { "labels" => {
"gitlab" => "hook", "gitlab" => "hook",
"gitlab_alert_id" => 1 "gitlab_alert_id" => metric.id
} }
} }
......
...@@ -15,7 +15,7 @@ describe PrometheusAlertEntity do ...@@ -15,7 +15,7 @@ describe PrometheusAlertEntity do
end end
it 'exposes prometheus_alert attributes' do it 'exposes prometheus_alert attributes' do
expect(subject).to include(:id, :name, :query, :operator, :threshold) expect(subject).to include(:id, :title, :query, :operator, :threshold)
end end
it 'exposes alert_path' do it 'exposes alert_path' do
......
require 'spec_helper'
describe Projects::Prometheus::Metrics::DestroyService do
let(:metric) { create(:prometheus_metric) }
subject { described_class.new(metric) }
it 'destroys metric' do
subject.execute
expect(PrometheusMetric.find_by(id: metric.id)).to be_nil
end
context 'when metric has a prometheus alert associated' do
it 'schedules a prometheus alert update' do
create(:prometheus_alert, prometheus_metric: metric)
schedule_update_service = spy
allow(::Clusters::Applications::ScheduleUpdateService).to receive(:new).and_return(schedule_update_service)
subject.execute
expect(schedule_update_service).to have_received(:execute)
end
end
end
require 'spec_helper'
describe Projects::Prometheus::Metrics::UpdateService do
let(:metric) { create(:prometheus_metric) }
it 'updates the prometheus metric' do
expect do
described_class.new(metric, { title: "bar" }).execute
end.to change { metric.reload.title }.to("bar")
end
context 'when metric has a prometheus alert associated' do
let(:schedule_update_service) { spy }
before do
create(:prometheus_alert, prometheus_metric: metric)
allow(::Clusters::Applications::ScheduleUpdateService).to receive(:new).and_return(schedule_update_service)
end
context 'when updating title' do
it 'schedules a prometheus alert update' do
described_class.new(metric, { title: "bar" }).execute
expect(schedule_update_service).to have_received(:execute)
end
end
context 'when updating query' do
it 'schedules a prometheus alert update' do
described_class.new(metric, { query: "sum(bar)" }).execute
expect(schedule_update_service).to have_received(:execute)
end
end
it 'does not schedule a prometheus alert update without title nor query being changed' do
described_class.new(metric, { y_label: "bar" }).execute
expect(schedule_update_service).not_to have_received(:execute)
end
end
end
...@@ -26,12 +26,10 @@ describe ClusterUpdateAppWorker do ...@@ -26,12 +26,10 @@ describe ClusterUpdateAppWorker do
end end
context 'when another worker is already running' do context 'when another worker is already running' do
it 'raises UpdateAlreadyInProgressError' do it 'returns nil' do
application = create(:clusters_applications_prometheus, :updating) application = create(:clusters_applications_prometheus, :updating)
expect do expect(subject.perform(application.name, application.id, project.id, Time.now)).to be_nil
subject.perform(application.name, application.id, project.id, Time.now)
end.to raise_error(described_class::UpdateAlreadyInProgressError)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment