From 3bc76511a066626c06d3d3b76d78e928770c46e0 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Mon, 8 Jul 2019 06:03:09 +0000 Subject: [PATCH] Create CTE query for clusters hierarchy - This enables us to use a scope to query all clusters in group hierarchy order in one query, and also enables us to union to instance clusters later. - Handle case where clusters not present at level. In which case the query should go ahead and return the next level's clusters. - Swap with new CTE query behind Feature flag. This FF is default disabled. --- app/models/clusters/clusters_hierarchy.rb | 104 ++++++++++++++++++ app/models/concerns/deployment_platform.rb | 19 +++- changelogs/unreleased/clusters-group-cte.yml | 5 + .../clusters/clusters_hierarchy_spec.rb | 73 ++++++++++++ .../concerns/deployment_platform_spec.rb | 18 ++- 5 files changed, 216 insertions(+), 3 deletions(-) create mode 100644 app/models/clusters/clusters_hierarchy.rb create mode 100644 changelogs/unreleased/clusters-group-cte.yml create mode 100644 spec/models/clusters/clusters_hierarchy_spec.rb diff --git a/app/models/clusters/clusters_hierarchy.rb b/app/models/clusters/clusters_hierarchy.rb new file mode 100644 index 00000000000..dab034b7234 --- /dev/null +++ b/app/models/clusters/clusters_hierarchy.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +module Clusters + class ClustersHierarchy + DEPTH_COLUMN = :depth + + def initialize(clusterable) + @clusterable = clusterable + end + + # Returns clusters in order from deepest to highest group + def base_and_ancestors + cte = recursive_cte + cte_alias = cte.table.alias(model.table_name) + + model + .unscoped + .where('clusters.id IS NOT NULL') + .with + .recursive(cte.to_arel) + .from(cte_alias) + .order(DEPTH_COLUMN => :asc) + end + + private + + attr_reader :clusterable + + def recursive_cte + cte = Gitlab::SQL::RecursiveCTE.new(:clusters_cte) + + base_query = case clusterable + when ::Group + group_clusters_base_query + when ::Project + project_clusters_base_query + else + raise ArgumentError, "unknown type for #{clusterable}" + end + + cte << base_query + cte << parent_query(cte) + + cte + end + + def group_clusters_base_query + group_parent_id_alias = alias_as_column(groups[:parent_id], 'group_parent_id') + join_sources = ::Group.left_joins(:clusters).join_sources + + model + .unscoped + .select([clusters_star, group_parent_id_alias, "1 AS #{DEPTH_COLUMN}"]) + .where(groups[:id].eq(clusterable.id)) + .from(groups) + .joins(join_sources) + end + + def project_clusters_base_query + projects = ::Project.arel_table + project_parent_id_alias = alias_as_column(projects[:namespace_id], 'group_parent_id') + join_sources = ::Project.left_joins(:clusters).join_sources + + model + .unscoped + .select([clusters_star, project_parent_id_alias, "1 AS #{DEPTH_COLUMN}"]) + .where(projects[:id].eq(clusterable.id)) + .from(projects) + .joins(join_sources) + end + + def parent_query(cte) + group_parent_id_alias = alias_as_column(groups[:parent_id], 'group_parent_id') + + model + .unscoped + .select([clusters_star, group_parent_id_alias, cte.table[DEPTH_COLUMN] + 1]) + .from([cte.table, groups]) + .joins('LEFT OUTER JOIN cluster_groups ON cluster_groups.group_id = namespaces.id') + .joins('LEFT OUTER JOIN clusters ON cluster_groups.cluster_id = clusters.id') + .where(groups[:id].eq(cte.table[:group_parent_id])) + end + + def model + Clusters::Cluster + end + + def clusters + @clusters ||= model.arel_table + end + + def groups + @groups ||= ::Group.arel_table + end + + def clusters_star + @clusters_star ||= clusters[Arel.star] + end + + def alias_as_column(value, alias_to) + Arel::Nodes::As.new(value, Arel::Nodes::SqlLiteral.new(alias_to)) + end + end +end diff --git a/app/models/concerns/deployment_platform.rb b/app/models/concerns/deployment_platform.rb index 5a358ae2ef6..8f28c897eb6 100644 --- a/app/models/concerns/deployment_platform.rb +++ b/app/models/concerns/deployment_platform.rb @@ -12,11 +12,26 @@ module DeploymentPlatform private def find_deployment_platform(environment) - find_cluster_platform_kubernetes(environment: environment) || - find_group_cluster_platform_kubernetes(environment: environment) || + find_platform_kubernetes(environment) || find_instance_cluster_platform_kubernetes(environment: environment) end + def find_platform_kubernetes(environment) + if Feature.enabled?(:clusters_cte) + find_platform_kubernetes_with_cte(environment) + else + find_cluster_platform_kubernetes(environment: environment) || + find_group_cluster_platform_kubernetes(environment: environment) + end + end + + # EE would override this and utilize environment argument + def find_platform_kubernetes_with_cte(_environment) + Clusters::ClustersHierarchy.new(self).base_and_ancestors + .enabled.default_environment + .first&.platform_kubernetes + end + # EE would override this and utilize environment argument def find_cluster_platform_kubernetes(environment: nil) clusters.enabled.default_environment diff --git a/changelogs/unreleased/clusters-group-cte.yml b/changelogs/unreleased/clusters-group-cte.yml new file mode 100644 index 00000000000..4b51249062d --- /dev/null +++ b/changelogs/unreleased/clusters-group-cte.yml @@ -0,0 +1,5 @@ +--- +title: Use CTE to fetch clusters hierarchy in single query +merge_request: 30063 +author: +type: performance diff --git a/spec/models/clusters/clusters_hierarchy_spec.rb b/spec/models/clusters/clusters_hierarchy_spec.rb new file mode 100644 index 00000000000..0470ebe17ea --- /dev/null +++ b/spec/models/clusters/clusters_hierarchy_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Clusters::ClustersHierarchy do + describe '#base_and_ancestors' do + def base_and_ancestors(clusterable) + described_class.new(clusterable).base_and_ancestors + end + + context 'project in nested group with clusters at every level' do + let!(:cluster) { create(:cluster, :project, projects: [project]) } + let!(:child) { create(:cluster, :group, groups: [child_group]) } + let!(:parent) { create(:cluster, :group, groups: [parent_group]) } + let!(:ancestor) { create(:cluster, :group, groups: [ancestor_group]) } + + let(:ancestor_group) { create(:group) } + let(:parent_group) { create(:group, parent: ancestor_group) } + let(:child_group) { create(:group, parent: parent_group) } + let(:project) { create(:project, group: child_group) } + + it 'returns clusters for project' do + expect(base_and_ancestors(project)).to eq([cluster, child, parent, ancestor]) + end + + it 'returns clusters for child_group' do + expect(base_and_ancestors(child_group)).to eq([child, parent, ancestor]) + end + + it 'returns clusters for parent_group' do + expect(base_and_ancestors(parent_group)).to eq([parent, ancestor]) + end + + it 'returns clusters for ancestor_group' do + expect(base_and_ancestors(ancestor_group)).to eq([ancestor]) + end + end + + context 'project in a namespace' do + let!(:cluster) { create(:cluster, :project) } + + it 'returns clusters for project' do + expect(base_and_ancestors(cluster.project)).to eq([cluster]) + end + end + + context 'project in nested group with clusters at some levels' do + let!(:child) { create(:cluster, :group, groups: [child_group]) } + let!(:ancestor) { create(:cluster, :group, groups: [ancestor_group]) } + + let(:ancestor_group) { create(:group) } + let(:parent_group) { create(:group, parent: ancestor_group) } + let(:child_group) { create(:group, parent: parent_group) } + let(:project) { create(:project, group: child_group) } + + it 'returns clusters for project' do + expect(base_and_ancestors(project)).to eq([child, ancestor]) + end + + it 'returns clusters for child_group' do + expect(base_and_ancestors(child_group)).to eq([child, ancestor]) + end + + it 'returns clusters for parent_group' do + expect(base_and_ancestors(parent_group)).to eq([ancestor]) + end + + it 'returns clusters for ancestor_group' do + expect(base_and_ancestors(ancestor_group)).to eq([ancestor]) + end + end + end +end diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index 2378f400540..e2fc8a5d127 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' describe DeploymentPlatform do let(:project) { create(:project) } - describe '#deployment_platform' do + shared_examples '#deployment_platform' do subject { project.deployment_platform } context 'with no Kubernetes configuration on CI/CD, no Kubernetes Service' do @@ -84,4 +84,20 @@ describe DeploymentPlatform do end end end + + context 'legacy implementation' do + before do + stub_feature_flags(clusters_cte: false) + end + + include_examples '#deployment_platform' + end + + context 'CTE implementation' do + before do + stub_feature_flags(clusters_cte: true) + end + + include_examples '#deployment_platform' + end end