Merge branch 'disable_alerts_for_prometheus_cluster_integrations' into 'master'

Disable automatic setup of alerts for Prometheus cluster integration

See merge request gitlab-org/gitlab!58853

app/models/clusters/applications/prometheus.rb

@@ -44,6 +44,10 @@ module Clusters
         end
       end
 
+      def managed_prometheus?
+        !externally_installed? && !uninstalled?
+      end
+
       def updated_since?(timestamp)
         last_update_started_at &&
           last_update_started_at > timestamp &&
@@ -70,6 +74,7 @@ module Clusters
         )
       end
 
+      # Deprecated, to be removed in %14.0 as part of https://gitlab.com/groups/gitlab-org/-/epics/4280
       def patch_command(values)
         helm_command_module::PatchCommand.new(
           name: name,

app/models/concerns/prometheus_adapter.rb

@@ -26,6 +26,11 @@ module PrometheusAdapter
       }
     end
 
+    # Overridden in app/models/clusters/applications/prometheus.rb
+    def managed_prometheus?
+      false
+    end
+
     # This is a light-weight check if a prometheus client is properly configured.
     def configured?
       raise NotImplemented

app/services/clusters/applications/prometheus_update_service.rb

@@ -2,6 +2,7 @@
 
 module Clusters
   module Applications
+    # Deprecated, to be removed in %14.0 as part of https://gitlab.com/groups/gitlab-org/-/epics/4280
     class PrometheusUpdateService < BaseHelmService
       attr_accessor :project
 
@@ -11,6 +12,8 @@ module Clusters
       end
 
       def execute
+        raise NotImplementedError, 'Externally installed prometheus should not be modified!' unless app.managed_prometheus?
+
         app.make_updating!
 
         helm_api.update(patch_command(values))

app/services/clusters/applications/schedule_update_service.rb

@@ -14,6 +14,7 @@ module Clusters
 
       def execute
         return unless application
+        return unless application.managed_prometheus?
 
         if recently_scheduled?
           worker_class.perform_in(BACKOFF_DELAY, application.name, application.id, project.id, Time.current)

app/workers/cluster_update_app_worker.rb

 # frozen_string_literal: true
 
+# Deprecated, to be removed in %14.0 as part of https://gitlab.com/groups/gitlab-org/-/epics/4280
 class ClusterUpdateAppWorker # rubocop:disable Scalability/IdempotentWorker
   UpdateAlreadyInProgressError = Class.new(StandardError)
 
@@ -35,6 +36,7 @@ class ClusterUpdateAppWorker # rubocop:disable Scalability/IdempotentWorker
   # rubocop: enable CodeReuse/ActiveRecord
 
   def update_prometheus(app, scheduled_time, project)
+    return unless app.managed_prometheus?
     return if app.updated_since?(scheduled_time)
     return if app.update_in_progress?
 

changelogs/unreleased/disable_alerts_for_prometheus_cluster_integrations.yml

+---
+title: Disable automatic setup of alerts for Prometheus cluster integration
+merge_request: 58853
+author:
+type: changed

doc/operations/metrics/alerts.md

@@ -49,6 +49,10 @@ as soon as the alert fires:
 
 ![Linked Runbook in charts](img/linked_runbooks_on_charts.png)
 
+## Prometheus cluster integrations
+
+Alerts are not currently supported for [Prometheus cluster integrations](../../user/clusters/integrations.md).
+
 ## External Prometheus instances
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9258) in GitLab Ultimate 11.8.

doc/user/clusters/integrations.md

@@ -20,6 +20,9 @@ You can integrate your Kubernetes cluster with
 [Prometheus](https://prometheus.io/) for monitoring key metrics of your
 apps directly from the GitLab UI.
 
+[Alerts](../../operations/metrics/alerts.md) are not currently
+supported.
+
 Once enabled, you will see metrics from services available in the
 [metrics library](../project/integrations/prometheus_library/index.md).
 

spec/models/clusters/applications/prometheus_spec.rb

@@ -65,6 +65,26 @@ RSpec.describe Clusters::Applications::Prometheus do
     end
   end
 
+  describe '#managed_prometheus?' do
+    subject { prometheus.managed_prometheus? }
+
+    let(:prometheus) { build(:clusters_applications_prometheus) }
+
+    it { is_expected.to be_truthy }
+
+    context 'externally installed' do
+      let(:prometheus) { build(:clusters_applications_prometheus, :externally_installed) }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'uninstalled' do
+      let(:prometheus) { build(:clusters_applications_prometheus, :uninstalled) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#can_uninstall?' do
     let(:prometheus) { create(:clusters_applications_prometheus) }
 

spec/services/clusters/applications/prometheus_update_service_spec.rb

@@ -9,83 +9,102 @@ RSpec.describe Clusters::Applications::PrometheusUpdateService do
     let(:cluster) { create(:cluster, :provided_by_user, :with_installed_helm, projects: [project]) }
     let(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
     let(:empty_alerts_values_update_yaml) { "---\nalertmanager:\n  enabled: false\nserverFiles:\n  alerts: {}\n" }
-    let!(:patch_command) { application.patch_command(empty_alerts_values_update_yaml) }
     let(:helm_client) { instance_double(::Gitlab::Kubernetes::Helm::API) }
 
     subject(:service) { described_class.new(application, project) }
 
-    before do
-      allow(service).to receive(:patch_command).with(empty_alerts_values_update_yaml).and_return(patch_command)
-      allow(service).to receive(:helm_api).and_return(helm_client)
+    context 'when prometheus is a Clusters::Integrations::Prometheus' do
+      let(:application) { create(:clusters_integrations_prometheus, cluster: cluster) }
+
+      it 'raises NotImplementedError' do
+        expect { service.execute }.to raise_error(NotImplementedError)
+      end
     end
 
-    context 'when there are no errors' do
-      before do
-        expect(helm_client).to receive(:update).with(patch_command)
+    context 'when prometheus is externally installed' do
+      let(:application) { create(:clusters_applications_prometheus, :externally_installed, cluster: cluster) }
 
-        allow(::ClusterWaitForAppUpdateWorker)
-          .to receive(:perform_in)
-          .and_return(nil)
+      it 'raises NotImplementedError' do
+        expect { service.execute }.to raise_error(NotImplementedError)
       end
+    end
 
-      it 'make the application updating' do
-        expect(application.cluster).not_to be_nil
-
-        service.execute
+    context 'when prometheus is a Clusters::Applications::Prometheus' do
+      let!(:patch_command) { application.patch_command(empty_alerts_values_update_yaml) }
 
-        expect(application).to be_updating
+      before do
+        allow(service).to receive(:patch_command).with(empty_alerts_values_update_yaml).and_return(patch_command)
+        allow(service).to receive(:helm_api).and_return(helm_client)
       end
 
-      it 'updates current config' do
-        prometheus_config_service = spy(:prometheus_config_service)
+      context 'when there are no errors' do
+        before do
+          expect(helm_client).to receive(:update).with(patch_command)
 
-        expect(Clusters::Applications::PrometheusConfigService)
-          .to receive(:new)
-          .with(project, cluster, application)
-          .and_return(prometheus_config_service)
+          allow(::ClusterWaitForAppUpdateWorker)
+            .to receive(:perform_in)
+            .and_return(nil)
+        end
 
-        expect(prometheus_config_service)
-          .to receive(:execute)
-          .and_return(YAML.safe_load(empty_alerts_values_update_yaml))
+        it 'make the application updating' do
+          expect(application.cluster).not_to be_nil
 
-        service.execute
-      end
+          service.execute
 
-      it 'schedules async update status check' do
-        expect(::ClusterWaitForAppUpdateWorker).to receive(:perform_in).once
+          expect(application).to be_updating
+        end
 
-        service.execute
-      end
-    end
+        it 'updates current config' do
+          prometheus_config_service = spy(:prometheus_config_service)
 
-    context 'when k8s cluster communication fails' do
-      before do
-        error = ::Kubeclient::HttpError.new(500, 'system failure', nil)
-        allow(helm_client).to receive(:update).and_raise(error)
-      end
+          expect(Clusters::Applications::PrometheusConfigService)
+            .to receive(:new)
+            .with(project, cluster, application)
+            .and_return(prometheus_config_service)
+
+          expect(prometheus_config_service)
+            .to receive(:execute)
+            .and_return(YAML.safe_load(empty_alerts_values_update_yaml))
 
-      it 'make the application update errored' do
-        service.execute
+          service.execute
+        end
 
-        expect(application).to be_update_errored
-        expect(application.status_reason).to match(/kubernetes error:/i)
+        it 'schedules async update status check' do
+          expect(::ClusterWaitForAppUpdateWorker).to receive(:perform_in).once
+
+          service.execute
+        end
       end
-    end
 
-    context 'when application cannot be persisted' do
-      let(:application) { build(:clusters_applications_prometheus, :installed) }
+      context 'when k8s cluster communication fails' do
+        before do
+          error = ::Kubeclient::HttpError.new(500, 'system failure', nil)
+          allow(helm_client).to receive(:update).and_raise(error)
+        end
 
-      before do
-        allow(application).to receive(:make_updating!).once
-          .and_raise(ActiveRecord::RecordInvalid.new(application))
+        it 'make the application update errored' do
+          service.execute
+
+          expect(application).to be_update_errored
+          expect(application.status_reason).to match(/kubernetes error:/i)
+        end
       end
 
-      it 'make the application update errored' do
-        expect(helm_client).not_to receive(:update)
+      context 'when application cannot be persisted' do
+        let(:application) { build(:clusters_applications_prometheus, :installed) }
+
+        before do
+          allow(application).to receive(:make_updating!).once
+            .and_raise(ActiveRecord::RecordInvalid.new(application))
+        end
+
+        it 'make the application update errored' do
+          expect(helm_client).not_to receive(:update)
 
-        service.execute
+          service.execute
 
-        expect(application).to be_update_errored
+          expect(application).to be_update_errored
+        end
       end
     end
   end

spec/services/clusters/applications/schedule_update_service_spec.rb

@@ -10,6 +10,32 @@ RSpec.describe Clusters::Applications::ScheduleUpdateService do
       freeze_time { example.run }
     end
 
+    context 'when the application is a Clusters::Integrations::Prometheus' do
+      let(:application) { create(:clusters_integrations_prometheus) }
+
+      it 'does nothing' do
+        service = described_class.new(application, project)
+
+        expect(::ClusterUpdateAppWorker).not_to receive(:perform_in)
+        expect(::ClusterUpdateAppWorker).not_to receive(:perform_async)
+
+        service.execute
+      end
+    end
+
+    context 'when the application is externally installed' do
+      let(:application) { create(:clusters_applications_prometheus, :externally_installed) }
+
+      it 'does nothing' do
+        service = described_class.new(application, project)
+
+        expect(::ClusterUpdateAppWorker).not_to receive(:perform_in)
+        expect(::ClusterUpdateAppWorker).not_to receive(:perform_async)
+
+        service.execute
+      end
+    end
+
     context 'when application is able to be updated' do
       context 'when the application was recently scheduled' do
         it 'schedules worker with a backoff delay' do

spec/workers/cluster_update_app_worker_spec.rb

@@ -46,6 +46,16 @@ RSpec.describe ClusterUpdateAppWorker do
       subject.perform(application.name, application.id, project.id, Time.current)
     end
 
+    context 'application is externally installed' do
+      it 'does not execute PrometheusUpdateService' do
+        application = create(:clusters_applications_prometheus, :externally_installed)
+
+        expect(prometheus_update_service).not_to receive(:execute)
+
+        subject.perform(application.name, application.id, project.id, Time.current)
+      end
+    end
+
     context 'with exclusive lease' do
       let_it_be(:user) { create(:user) }