Merge branch...

Merge branch '14857-new-metric-button-not-visible-when-prometheus-app-installed-on-group-instance-clusters' into 'master'

Fix Prometheus integration with group/instance clusters

See merge request gitlab-org/gitlab!19692

app/models/clusters/applications/prometheus.rb

@@ -13,15 +13,21 @@ module Clusters
       include ::Clusters::Concerns::ApplicationStatus
       include ::Clusters::Concerns::ApplicationVersion
       include ::Clusters::Concerns::ApplicationData
+      include AfterCommitQueue
       default_value_for :version, VERSION
-      after_destroy :disable_prometheus_integration
+      after_destroy do
+        run_after_commit do
+          disable_prometheus_integration
+        end
+      end
       state_machine :status do
         after_transition any => [:installed] do |application|
-          application.cluster.projects.each do |project|
-            project.find_or_initialize_service('prometheus').update!(active: true)
+          application.run_after_commit do
+            Clusters::Applications::ActivateServiceWorker
+              .perform_async(application.cluster_id, ::PrometheusService.to_param) # rubocop:disable CodeReuse/ServiceClass
           end
         end
       end
@@ -98,9 +104,8 @@ module Clusters
       private
       def disable_prometheus_integration
-        cluster.projects.each do |project|
-          project.prometheus_service&.update!(active: false)
-        end
+        ::Clusters::Applications::DeactivateServiceWorker
+          .perform_async(cluster_id, ::PrometheusService.to_param) # rubocop:disable CodeReuse/ServiceClass
       end
       def kube_client

app/models/clusters/cluster.rb

@@ -34,6 +34,7 @@ module Clusters
     has_many :cluster_groups, class_name: 'Clusters::Group'
     has_many :groups, through: :cluster_groups, class_name: '::Group'
+    has_many :groups_projects, through: :groups, source: :projects, class_name: '::Project'
     # we force autosave to happen when we save `Cluster` model
     has_one :provider_gcp, class_name: 'Clusters::Providers::Gcp', autosave: true
@@ -177,6 +178,13 @@ module Clusters
       end
     end
+    def all_projects
+      return projects if project_type?
+      return groups_projects if group_type?
+
+      ::Project.all
+    end
+
     def status_name
       return cleanup_status_name if cleanup_errored?
       return :cleanup_ongoing unless cleanup_not_started?

app/models/project.rb

@@ -404,6 +404,7 @@ class Project < ApplicationRecord
   scope :with_push, -> { joins(:events).where('events.action = ?', Event::PUSHED) }
   scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') }
   scope :with_statistics, -> { includes(:statistics) }
+  scope :with_service, ->(service) { joins(service).eager_load(service) }
   scope :with_shared_runners, -> { where(shared_runners_enabled: true) }
   scope :with_container_registry, -> { where(container_registry_enabled: true) }
   scope :inside_path, ->(path) do
@@ -1256,8 +1257,9 @@ class Project < ApplicationRecord
   def all_clusters
     group_clusters = Clusters::Cluster.joins(:groups).where(cluster_groups: { group_id: ancestors_upto } )
-    Clusters::Cluster.from_union([clusters, group_clusters])
+    instance_clusters = Clusters::Cluster.instance_type
+    Clusters::Cluster.from_union([clusters, group_clusters, instance_clusters])
   end
   def items_for(entity)

app/models/project_services/prometheus_service.rb

@@ -88,7 +88,7 @@ class PrometheusService < MonitoringService
     return false if template?
     return false unless project
-    project.clusters.enabled.any? { |cluster| cluster.application_prometheus_available? }
+    project.all_clusters.enabled.any? { |cluster| cluster.application_prometheus_available? }
   end
   def allow_local_api_url?

app/workers/all_queues.yml

@@ -51,6 +51,8 @@
 - gcp_cluster:clusters_cleanup_app
 - gcp_cluster:clusters_cleanup_project_namespace
 - gcp_cluster:clusters_cleanup_service_account
+- gcp_cluster:clusters_applications_activate_service
+- gcp_cluster:clusters_applications_deactivate_service
 - github_import_advance_stage
 - github_importer:github_import_import_diff_note

app/workers/clusters/applications/activate_service_worker.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class ActivateServiceWorker
+      include ApplicationWorker
+      include ClusterQueue
+
+      def perform(cluster_id, service_name)
+        cluster = Clusters::Cluster.find_by_id(cluster_id)
+        return unless cluster
+
+        cluster.all_projects.find_each do |project|
+          project.find_or_initialize_service(service_name).update!(active: true)
+        end
+      end
+    end
+  end
+end

app/workers/clusters/applications/deactivate_service_worker.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Applications
+    class DeactivateServiceWorker
+      include ApplicationWorker
+      include ClusterQueue
+
+      def perform(cluster_id, service_name)
+        cluster = Clusters::Cluster.find_by_id(cluster_id)
+        raise cluster_missing_error(service_name) unless cluster
+
+        service = "#{service_name}_service".to_sym
+        cluster.all_projects.with_service(service).find_each do |project|
+          project.public_send(service).update!(active: false) # rubocop:disable GitlabSecurity/PublicSend
+        end
+      end
+
+      def cluster_missing_error(service)
+        ActiveRecord::RecordNotFound.new("Can't deactivate #{service} services, host cluster not found! Some inconsistent records may be left in database.")
+      end
+    end
+  end
+end

changelogs/unreleased/14857-new-metric-button-not-visible-when-prometheus-app-installed-on-grou.yml

+---
+title: Activate projects Prometheus service integration when Prometheus managed application is installed on shared cluster
+merge_request:
+author:
+type: fixed

spec/models/clusters/applications/prometheus_spec.rb

@@ -12,35 +12,29 @@ describe Clusters::Applications::Prometheus do
   include_examples 'cluster application initial status specs'
   describe 'after_destroy' do
-    let(:project) { create(:project) }
-    let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) }
-    let!(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
-    let!(:prometheus_service) { project.create_prometheus_service(active: true) }
-    it 'deactivates prometheus_service after destroy' do
-      expect do
-        application.destroy!
-        prometheus_service.reload
-      end.to change(prometheus_service, :active).from(true).to(false)
+    context 'cluster type is project' do
+      let(:cluster) { create(:cluster, :with_installed_helm) }
+      let(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
+      it 'deactivates prometheus_service after destroy' do
+        expect(Clusters::Applications::DeactivateServiceWorker)
+          .to receive(:perform_async).with(cluster.id, 'prometheus')
+        application.destroy!
+      end
     end
   end
   describe 'transition to installed' do
     let(:project) { create(:project) }
-    let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) }
-    let(:prometheus_service) { double('prometheus_service') }
-
-    subject { create(:clusters_applications_prometheus, :installing, cluster: cluster) }
-
-    before do
-      allow(project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service
-    end
-    it 'ensures Prometheus service is activated' do
-      expect(prometheus_service).to receive(:update!).with(active: true)
-      subject.make_installed
+    let(:cluster) { create(:cluster, :with_installed_helm) }
+    let(:application) { create(:clusters_applications_prometheus, :installing, cluster: cluster) }
+    it 'schedules post installation job' do
+      expect(Clusters::Applications::ActivateServiceWorker)
+        .to receive(:perform_async).with(cluster.id, 'prometheus')
+      application.make_installed
     end
   end

spec/models/clusters/cluster_spec.rb

@@ -16,6 +16,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
   it { is_expected.to have_many(:projects) }
   it { is_expected.to have_many(:cluster_groups) }
   it { is_expected.to have_many(:groups) }
+  it { is_expected.to have_many(:groups_projects) }
   it { is_expected.to have_one(:provider_gcp) }
   it { is_expected.to have_one(:provider_aws) }
   it { is_expected.to have_one(:platform_kubernetes) }
@@ -616,6 +617,36 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
   end
+  describe '#all_projects' do
+    context 'cluster_type is project_type' do
+      let(:project) { create(:project) }
+      let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) }
+
+      it 'returns projects' do
+        expect(cluster.all_projects).to match_array [project]
+      end
+    end
+
+    context 'cluster_type is group_type' do
+      let(:group) { create(:group) }
+      let!(:project) { create(:project, group: group) }
+      let(:cluster) { create(:cluster_for_group, :with_installed_helm, groups: [group]) }
+
+      it 'returns group projects' do
+        expect(cluster.all_projects.ids).to match_array [project.id]
+      end
+    end
+
+    context 'cluster_type is instance_type' do
+      let!(:project) { create(:project) }
+      let(:cluster) { create(:cluster, :instance) }
+
+      it "returns all instance's projects" do
+        expect(cluster.all_projects.ids).to match_array [project.id]
+      end
+    end
+  end
+
   describe '#kube_ingress_domain' do
     let(:cluster) { create(:cluster, :provided_by_gcp) }

spec/models/project_services/prometheus_service_spec.rb

@@ -156,11 +156,34 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
   describe '#prometheus_available?' do
     context 'clusters with installed prometheus' do
-      let!(:cluster) { create(:cluster, projects: [project]) }
-      let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
-      it 'returns true' do
-        expect(service.prometheus_available?).to be(true)
+      before do
+        create(:clusters_applications_prometheus, :installed, cluster: cluster)
+      end
+      context 'cluster belongs to project' do
+        let(:cluster) { create(:cluster, projects: [project]) }
+
+        it 'returns true' do
+          expect(service.prometheus_available?).to be(true)
+        end
+      end
+
+      context 'cluster belongs to projects group' do
+        set(:group) { create(:group) }
+        let(:project) { create(:prometheus_project, group: group) }
+        let(:cluster) { create(:cluster_for_group, :with_installed_helm, groups: [group]) }
+
+        it 'returns true' do
+          expect(service.prometheus_available?).to be(true)
+        end
+      end
+
+      context 'cluster belongs to gitlab instance' do
+        let(:cluster) { create(:cluster, :instance) }
+
+        it 'returns true' do
+          expect(service.prometheus_available?).to be(true)
+        end
       end
     end

spec/models/project_spec.rb

@@ -1376,6 +1376,16 @@ describe Project do
     end
   end
+  describe '.with_service' do
+    before do
+      create_list(:prometheus_project, 2)
+    end
+
+    it 'avoid n + 1' do
+      expect { described_class.with_service(:prometheus_service).map(&:prometheus_service) }.not_to exceed_query_limit(1)
+    end
+  end
+
   context 'repository storage by default' do
     let(:project) { build(:project) }
@@ -5099,6 +5109,17 @@ describe Project do
         expect(subject).to contain_exactly(cluster, group_cluster)
       end
     end
+
+    context 'project is hosted on instance with integrated cluster' do
+      let(:group_cluster) { create(:cluster, :group) }
+      let(:instance_cluster) { create(:cluster, :instance) }
+      let(:group) { group_cluster.group }
+      let(:project) { create(:project, group: group) }
+
+      it 'returns all available clusters for this project' do
+        expect(subject).to contain_exactly(cluster, group_cluster, instance_cluster)
+      end
+    end
   end
   describe '#object_pool_params' do

spec/workers/clusters/applications/activate_service_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Applications::ActivateServiceWorker, '#perform' do
+  context 'cluster exists' do
+    describe 'prometheus service' do
+      let(:service_name) { 'prometheus' }
+
+      before do
+        create(:clusters_applications_prometheus, :installed, cluster: cluster)
+      end
+
+      context 'cluster type: group' do
+        let(:group) { create(:group) }
+        let(:project) { create(:project, group: group) }
+        let(:cluster) { create(:cluster_for_group, :with_installed_helm, groups: [group]) }
+
+        it 'ensures Prometheus service is activated' do
+          expect { described_class.new.perform(cluster.id, service_name) }
+            .to change { project.reload.prometheus_service&.active }.from(nil).to(true)
+        end
+      end
+
+      context 'cluster type: project' do
+        let(:project) { create(:project) }
+        let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) }
+
+        it 'ensures Prometheus service is activated' do
+          expect { described_class.new.perform(cluster.id, service_name) }
+            .to change { project.reload.prometheus_service&.active }.from(nil).to(true)
+        end
+      end
+
+      context 'cluster type: instance' do
+        let(:project) { create(:project) }
+        let(:cluster) { create(:cluster, :instance) }
+
+        it 'ensures Prometheus service is activated' do
+          expect { described_class.new.perform(cluster.id, service_name) }
+            .to change { project.reload.prometheus_service&.active }.from(nil).to(true)
+        end
+      end
+    end
+  end
+
+  context 'cluster does not exist' do
+    it 'does not raise Record Not Found error' do
+      expect { described_class.new.perform(0, 'ignored in this context') }.not_to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+end

spec/workers/clusters/applications/deactivate_service_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Clusters::Applications::DeactivateServiceWorker, '#perform' do
+  context 'cluster exists' do
+    describe 'prometheus service' do
+      let(:service_name) { 'prometheus' }
+      let!(:application) { create(:clusters_applications_prometheus, :installed, cluster: cluster) }
+
+      context 'prometheus service exists' do
+        let!(:prometheus_service) { create(:prometheus_service, project: project, manual_configuration: false, active: true) }
+
+        before do
+          application.delete # prometheus service before save synchronises active stated with application existance.
+        end
+
+        context 'cluster type: group' do
+          let(:group) { create(:group) }
+          let(:project) { create(:project, group: group) }
+          let(:cluster) { create(:cluster_for_group, :with_installed_helm, groups: [group]) }
+
+          it 'ensures Prometheus service is deactivated' do
+            expect { described_class.new.perform(cluster.id, service_name) }
+              .to change { prometheus_service.reload.active }.from(true).to(false)
+          end
+        end
+
+        context 'cluster type: project' do
+          let(:project) { create(:project) }
+          let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) }
+
+          it 'ensures Prometheus service is deactivated' do
+            expect { described_class.new.perform(cluster.id, service_name) }
+              .to change { prometheus_service.reload.active }.from(true).to(false)
+          end
+        end
+
+        context 'cluster type: instance' do
+          let(:project) { create(:project) }
+          let(:cluster) { create(:cluster, :with_installed_helm, :instance) }
+
+          it 'ensures Prometheus service is deactivated' do
+            expect { described_class.new.perform(cluster.id, service_name) }
+              .to change { prometheus_service.reload.active }.from(true).to(false)
+          end
+        end
+      end
+
+      context 'prometheus service does not exist' do
+        context 'cluster type: project' do
+          let(:project) { create(:project) }
+          let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) }
+
+          it 'does not raise errors' do
+            expect { described_class.new.perform(cluster.id, service_name) }.not_to raise_error
+          end
+        end
+      end
+    end
+  end
+
+  context 'cluster does not exist' do
+    it 'raises Record Not Found error' do
+      expect { described_class.new.perform(0, 'ignored in this context') }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
+end