From 4a80afcba2e0404828c7c2db72e87cd96aef7be7 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 16 Jan 2025 12:58:51 +0900 Subject: [PATCH 01/48] Add clusters_managed_resources table Changelog: added --- ...16011058_create_clusters_managed_resources.rb | 16 ++++++++++++++++ db/schema_migrations/20250116011058 | 1 + 2 files changed, 17 insertions(+) create mode 100644 db/migrate/20250116011058_create_clusters_managed_resources.rb create mode 100644 db/schema_migrations/20250116011058 diff --git a/db/migrate/20250116011058_create_clusters_managed_resources.rb b/db/migrate/20250116011058_create_clusters_managed_resources.rb new file mode 100644 index 00000000000000..7b86ddc7b68708 --- /dev/null +++ b/db/migrate/20250116011058_create_clusters_managed_resources.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class CreateClustersManagedResources < Gitlab::Database::Migration[2.2] + milestone '17.9' + + def change + create_table :clusters_managed_resources do |t| + t.references :build, index: true, null: false + t.references :project, index: false + t.references :environment, index: false + t.integer :status, default: 0, null: false + + t.timestamps_with_timezone null: false + end + end +end diff --git a/db/schema_migrations/20250116011058 b/db/schema_migrations/20250116011058 new file mode 100644 index 00000000000000..cd6c6c675e5476 --- /dev/null +++ b/db/schema_migrations/20250116011058 @@ -0,0 +1 @@ +500bffacef514651056f25fe43a3073b908c94fb5d757f6aec2b710c3c29385f \ No newline at end of file -- GitLab From 584b55a3370b7dfe9fdc214689bc8f9e9d7b196a Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 16 Jan 2025 20:08:37 +0900 Subject: [PATCH 02/48] Add reference to cluster_agent and template name --- .../20250116011058_create_clusters_managed_resources.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/db/migrate/20250116011058_create_clusters_managed_resources.rb b/db/migrate/20250116011058_create_clusters_managed_resources.rb index 7b86ddc7b68708..a1cbd0125148a8 100644 --- a/db/migrate/20250116011058_create_clusters_managed_resources.rb +++ b/db/migrate/20250116011058_create_clusters_managed_resources.rb @@ -5,10 +5,12 @@ class CreateClustersManagedResources < Gitlab::Database::Migration[2.2] def change create_table :clusters_managed_resources do |t| - t.references :build, index: true, null: false - t.references :project, index: false - t.references :environment, index: false + t.references :build, null: false + t.references :project, null: false + t.references :environment, null: false + t.references :cluster_agent, null: false t.integer :status, default: 0, null: false + t.text :template_name, limit: 1024 t.timestamps_with_timezone null: false end -- GitLab From b023d5f1c18982491abfd0eb411d9c42d3c8379e Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 16 Jan 2025 20:19:01 +0900 Subject: [PATCH 03/48] Update migrations --- ...011058_create_clusters_managed_resources.rb | 18 ------------------ db/schema_migrations/20250116011058 | 1 - 2 files changed, 19 deletions(-) delete mode 100644 db/migrate/20250116011058_create_clusters_managed_resources.rb delete mode 100644 db/schema_migrations/20250116011058 diff --git a/db/migrate/20250116011058_create_clusters_managed_resources.rb b/db/migrate/20250116011058_create_clusters_managed_resources.rb deleted file mode 100644 index a1cbd0125148a8..00000000000000 --- a/db/migrate/20250116011058_create_clusters_managed_resources.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -class CreateClustersManagedResources < Gitlab::Database::Migration[2.2] - milestone '17.9' - - def change - create_table :clusters_managed_resources do |t| - t.references :build, null: false - t.references :project, null: false - t.references :environment, null: false - t.references :cluster_agent, null: false - t.integer :status, default: 0, null: false - t.text :template_name, limit: 1024 - - t.timestamps_with_timezone null: false - end - end -end diff --git a/db/schema_migrations/20250116011058 b/db/schema_migrations/20250116011058 deleted file mode 100644 index cd6c6c675e5476..00000000000000 --- a/db/schema_migrations/20250116011058 +++ /dev/null @@ -1 +0,0 @@ -500bffacef514651056f25fe43a3073b908c94fb5d757f6aec2b710c3c29385f \ No newline at end of file -- GitLab From ec8b663d657e5f93ce02f48d53ea33ddeed69da7 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 16 Jan 2025 20:34:57 +0900 Subject: [PATCH 04/48] Update structure.sql --- db/structure.sql | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/db/structure.sql b/db/structure.sql index ed1c0df8838e61..dae7ec48d36abe 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -31927,6 +31927,12 @@ CREATE INDEX index_clusters_managed_resources_on_environment_id ON clusters_mana CREATE INDEX index_clusters_managed_resources_on_project_id ON clusters_managed_resources USING btree (project_id); +CREATE INDEX index_clusters_managed_resources_on_cluster_agent_id ON clusters_managed_resources USING btree (cluster_agent_id); + +CREATE INDEX index_clusters_managed_resources_on_environment_id ON clusters_managed_resources USING btree (environment_id); + +CREATE INDEX index_clusters_managed_resources_on_project_id ON clusters_managed_resources USING btree (project_id); + CREATE INDEX index_clusters_on_enabled_and_provider_type_and_id ON clusters USING btree (enabled, provider_type, id); CREATE INDEX index_clusters_on_enabled_cluster_type_id_and_created_at ON clusters USING btree (enabled, cluster_type, id, created_at); -- GitLab From c8f50868a7a2475cc0335f56c4b976751745a0a9 Mon Sep 17 00:00:00 2001 From: Tiger Date: Tue, 21 Jan 2025 15:25:53 +1300 Subject: [PATCH 05/48] Change build_id index on clusters_managed_resources to unique --- db/structure.sql | 6 ------ 1 file changed, 6 deletions(-) diff --git a/db/structure.sql b/db/structure.sql index dae7ec48d36abe..ed1c0df8838e61 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -31927,12 +31927,6 @@ CREATE INDEX index_clusters_managed_resources_on_environment_id ON clusters_mana CREATE INDEX index_clusters_managed_resources_on_project_id ON clusters_managed_resources USING btree (project_id); -CREATE INDEX index_clusters_managed_resources_on_cluster_agent_id ON clusters_managed_resources USING btree (cluster_agent_id); - -CREATE INDEX index_clusters_managed_resources_on_environment_id ON clusters_managed_resources USING btree (environment_id); - -CREATE INDEX index_clusters_managed_resources_on_project_id ON clusters_managed_resources USING btree (project_id); - CREATE INDEX index_clusters_on_enabled_and_provider_type_and_id ON clusters USING btree (enabled, provider_type, id); CREATE INDEX index_clusters_on_enabled_cluster_type_id_and_created_at ON clusters USING btree (enabled, cluster_type, id, created_at); -- GitLab From d4da201878f2c83709f099e20a61cfbf22781a03 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 8 Jan 2025 11:32:44 +0900 Subject: [PATCH 06/48] Get environment template --- lib/gitlab/ci/build/prerequisite/factory.rb | 2 +- .../ci/build/prerequisite/managed_resource.rb | 38 +++++++++++++++++++ lib/gitlab/kas/client.rb | 16 +++++++- 3 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 lib/gitlab/ci/build/prerequisite/managed_resource.rb diff --git a/lib/gitlab/ci/build/prerequisite/factory.rb b/lib/gitlab/ci/build/prerequisite/factory.rb index 60cdf7af418b22..7b3cbd09de6076 100644 --- a/lib/gitlab/ci/build/prerequisite/factory.rb +++ b/lib/gitlab/ci/build/prerequisite/factory.rb @@ -8,7 +8,7 @@ class Factory attr_reader :build def self.prerequisites - [KubernetesNamespace] + [KubernetesNamespace, ManagedResource] end def initialize(build) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb new file mode 100644 index 00000000000000..0b905d78c2b121 --- /dev/null +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Build + module Prerequisite + class ManagedResource < Base + def unmet? + # TODO: Check "resource_management.enabled" flag defined in the agent config file + end + + def complete! + return unless unmet? + + return unless environment.cluster_agent + + kas_client.get_environment_template( + project: environment.project, + template_name: "template-1", + agent_name: environment.cluster_agent.name) + + ensure_cluster_resources + end + + private + + def kas_client + @kas_client ||= Gitlab::Kas::Client.new + end + + def environment + build.deployment.environment + end + end + end + end + end +end diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index a19a69e69f5cb8..84af64b624150a 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -10,7 +10,8 @@ class Client agent_tracker: Gitlab::Agent::AgentTracker::Rpc::AgentTracker::Stub, configuration_project: Gitlab::Agent::ConfigurationProject::Rpc::ConfigurationProject::Stub, autoflow: Gitlab::Agent::AutoFlow::Rpc::AutoFlow::Stub, - notifications: Gitlab::Agent::Notifications::Rpc::Notifications::Stub + notifications: Gitlab::Agent::Notifications::Rpc::Notifications::Stub, + managed_resources: Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub }.freeze AUTOFLOW_CI_VARIABLE_ENV_SCOPE = 'autoflow/internal-use' @@ -101,6 +102,19 @@ def send_autoflow_event(project:, type:, id:, data:) .cloud_event(request, metadata: metadata) end + def get_environment_template(project:, template_name:, agent_name:) + request = Gitlab::Agent::ManagedResources::Rpc::GetEnvironmentTemplateRequest.new( + template_name: template_name, + agent_name: agent_name, + gitaly_info: gitaly_info(project), + gitaly_repository: repository(project), + default_branch: project.default_branch_or_main + ) + + stub_for(:managed_resources) + .get_environment_template(request, metadata: metadata) + end + private def stub_for(service) -- GitLab From e571fde31cd442e3536af74f8d4a210043684560 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 8 Jan 2025 15:40:17 +0900 Subject: [PATCH 07/48] Implement render_environment_template method --- lib/gitlab/kas/client.rb | 42 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index 84af64b624150a..be21a8bdbbfcec 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -115,6 +115,48 @@ def get_environment_template(project:, template_name:, agent_name:) .get_environment_template(request, metadata: metadata) end + def get_default_environment_template + request = Gitlab::Agent::ManagedResources::Rpc::GetDefaultEnvironmentTemplateRequest.new + stub_for(:managed_resources) + .get_default_environment_template(request, metadata: metadata) + .template + end + + def render_environment_template(template:, environment:, build:) + agent = environment.cluster_agent + project = environment.project + + templating_info = Gitlab::Agent::ManagedResources::TemplatingInfo.new( + agent: Gitlab::Agent::ManagedResources::Agent.new( + id: agent.id, + name: agent.name, + url: "https://gitlab.example.com///-/cluster_agents/"), # TODO: Add real URL + environment: Gitlab::Agent::ManagedResources::Environment.new( + name: environment.name, + slug: environment.slug, + url: "https://gitlab.example.com", # TODO: This is necessary. environment.external_url, + tier: environment.tier + ), + project: Gitlab::Agent::ManagedResources::Project.new( + id: project.id, + slug: project.full_path_slug, + path: project.full_path, + url: project.web_url + ), + pipeline: Gitlab::Agent::ManagedResources::Pipeline.new(id: build.pipeline_id), + job: Gitlab::Agent::ManagedResources::Job.new(id: build.id), + user: Gitlab::Agent::ManagedResources::User.new(id: build.user_id, username: build.user.username) + ) + + request = Gitlab::Agent::ManagedResources::Rpc::RenderEnvironmentTemplateRequest.new( + template: Gitlab::Agent::ManagedResources::EnvironmentTemplate.new( + name: template.name, + data: template.data), + info: templating_info) + stub_for(:managed_resources) + .render_environment_template(request, metadata: metadata) + end + private def stub_for(service) -- GitLab From 85b65890c1b3e020c9ac5755eefd5ac2d5a627d4 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 9 Jan 2025 14:20:48 +0900 Subject: [PATCH 08/48] Implement ensure_environment method --- lib/gitlab/kas/client.rb | 49 ++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index be21a8bdbbfcec..774eccae25fb65 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -123,38 +123,49 @@ def get_default_environment_template end def render_environment_template(template:, environment:, build:) + request = Gitlab::Agent::ManagedResources::Rpc::RenderEnvironmentTemplateRequest.new( + template: Gitlab::Agent::ManagedResources::EnvironmentTemplate.new( + name: template.name, + data: template.data), + info: templating_info(environment:, build:)) + stub_for(:managed_resources) + .render_environment_template(request, metadata: metadata) + .template + end + + def ensure_environment(template:, environment:, build:) + request = Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentRequest.new( + template: Gitlab::Agent::ManagedResources::RenderedEnvironmentTemplate.new( + name: template.name, + data: template.data), + info: templating_info(environment: environment, build: build)) + stub_for(:managed_resources) + .ensure_environment(request, metadata: metadata) + end + + def templating_info(environment:, build:) agent = environment.cluster_agent project = environment.project - templating_info = Gitlab::Agent::ManagedResources::TemplatingInfo.new( + Gitlab::Agent::ManagedResources::TemplatingInfo.new( agent: Gitlab::Agent::ManagedResources::Agent.new( id: agent.id, name: agent.name, - url: "https://gitlab.example.com///-/cluster_agents/"), # TODO: Add real URL + url: agent_url(project, agent.name)), environment: Gitlab::Agent::ManagedResources::Environment.new( name: environment.name, slug: environment.slug, - url: "https://gitlab.example.com", # TODO: This is necessary. environment.external_url, - tier: environment.tier - ), + url: environment_url(project, environment), + tier: environment.tier), project: Gitlab::Agent::ManagedResources::Project.new( id: project.id, slug: project.full_path_slug, path: project.full_path, - url: project.web_url - ), + url: project.web_url), pipeline: Gitlab::Agent::ManagedResources::Pipeline.new(id: build.pipeline_id), job: Gitlab::Agent::ManagedResources::Job.new(id: build.id), user: Gitlab::Agent::ManagedResources::User.new(id: build.user_id, username: build.user.username) ) - - request = Gitlab::Agent::ManagedResources::Rpc::RenderEnvironmentTemplateRequest.new( - template: Gitlab::Agent::ManagedResources::EnvironmentTemplate.new( - name: template.name, - data: template.data), - info: templating_info) - stub_for(:managed_resources) - .render_environment_template(request, metadata: metadata) end private @@ -204,6 +215,14 @@ def token def timeout Gitlab::Kas.client_timeout_seconds.seconds end + + def agent_url(project, agent_name) + "#{Gitlab.config.gitlab.url}/#{project.full_path}/-/cluster_agents/#{agent_name}" + end + + def environment_url(project, environment) + "#{Gitlab.config.gitlab.url}/#{project.full_path}/-/environments/#{environment.id}" + end end end end -- GitLab From 6170b4d12db6a67feb18528f5e21fd9db243d3cd Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Fri, 10 Jan 2025 11:03:15 +0900 Subject: [PATCH 09/48] Ensure environment in managed resource prerequisite --- .../ci/build/prerequisite/managed_resource.rb | 38 ++++++++++--- lib/gitlab/kas/client.rb | 53 ++++++++++--------- 2 files changed, 58 insertions(+), 33 deletions(-) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index 0b905d78c2b121..e54a0994909a47 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -5,25 +5,49 @@ module Ci module Build module Prerequisite class ManagedResource < Base + attr_reader :completed + + def initialize(build) + super + @completed = false + end + def unmet? - # TODO: Check "resource_management.enabled" flag defined in the agent config file + # TODO: Check "resource_management.enabled" flag. Return false if not enabled. + + !completed end def complete! return unless unmet? - return unless environment.cluster_agent + return unless valid_for_managed_resources?(environment:, build:) - kas_client.get_environment_template( - project: environment.project, - template_name: "template-1", - agent_name: environment.cluster_agent.name) + ensure_environment - ensure_cluster_resources + @completed = true end private + def ensure_environment + template = kas_client.get_default_environment_template + + rendered_template = kas_client.render_environment_template( + template: template, + environment: environment, + build: build) + + kas_client.ensure_environment( + template: rendered_template, + environment: environment, + build: build) + end + + def valid_for_managed_resources?(environment:, build:) + environment&.cluster_agent && environment.project && build && build.user + end + def kas_client @kas_client ||= Gitlab::Kas::Client.new end diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index 774eccae25fb65..a3a30fada90daf 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -138,36 +138,11 @@ def ensure_environment(template:, environment:, build:) template: Gitlab::Agent::ManagedResources::RenderedEnvironmentTemplate.new( name: template.name, data: template.data), - info: templating_info(environment: environment, build: build)) + info: templating_info(environment:, build:)) stub_for(:managed_resources) .ensure_environment(request, metadata: metadata) end - def templating_info(environment:, build:) - agent = environment.cluster_agent - project = environment.project - - Gitlab::Agent::ManagedResources::TemplatingInfo.new( - agent: Gitlab::Agent::ManagedResources::Agent.new( - id: agent.id, - name: agent.name, - url: agent_url(project, agent.name)), - environment: Gitlab::Agent::ManagedResources::Environment.new( - name: environment.name, - slug: environment.slug, - url: environment_url(project, environment), - tier: environment.tier), - project: Gitlab::Agent::ManagedResources::Project.new( - id: project.id, - slug: project.full_path_slug, - path: project.full_path, - url: project.web_url), - pipeline: Gitlab::Agent::ManagedResources::Pipeline.new(id: build.pipeline_id), - job: Gitlab::Agent::ManagedResources::Job.new(id: build.id), - user: Gitlab::Agent::ManagedResources::User.new(id: build.user_id, username: build.user.username) - ) - end - private def stub_for(service) @@ -212,8 +187,34 @@ def token end.encoded end +<<<<<<< HEAD def timeout Gitlab::Kas.client_timeout_seconds.seconds +======= + def templating_info(environment:, build:) + agent = environment.cluster_agent + project = environment.project + + Gitlab::Agent::ManagedResources::TemplatingInfo.new( + agent: Gitlab::Agent::ManagedResources::Agent.new( + id: agent.id, + name: agent.name, + url: agent_url(project, agent.name)), + environment: Gitlab::Agent::ManagedResources::Environment.new( + name: environment.name, + slug: environment.slug, + url: environment_url(project, environment), + tier: environment.tier), + project: Gitlab::Agent::ManagedResources::Project.new( + id: project.id, + slug: project.full_path_slug, + path: project.full_path, + url: project.web_url), + pipeline: Gitlab::Agent::ManagedResources::Pipeline.new(id: build.pipeline_id), + job: Gitlab::Agent::ManagedResources::Job.new(id: build.id), + user: Gitlab::Agent::ManagedResources::User.new(id: build.user_id, username: build.user.username) + ) +>>>>>>> 6a378fa3fd81 (Ensure environment in managed resource prerequisite) end def agent_url(project, agent_name) -- GitLab From 9dc0f3ac47ca61c645c16c77a5cf559a08d09223 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Tue, 14 Jan 2025 04:17:29 +0000 Subject: [PATCH 10/48] Apply 2 suggestion(s) to 1 file(s) Co-authored-by: Tiger Watson --- lib/gitlab/kas/client.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index a3a30fada90daf..0b4bd20a064f83 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -218,11 +218,11 @@ def templating_info(environment:, build:) end def agent_url(project, agent_name) - "#{Gitlab.config.gitlab.url}/#{project.full_path}/-/cluster_agents/#{agent_name}" + Gitlab::Routing.url_helpers.project_cluster_agent_url(project, agent) end def environment_url(project, environment) - "#{Gitlab.config.gitlab.url}/#{project.full_path}/-/environments/#{environment.id}" + Gitlab::Routing.url_helpers.project_environment_url(project, environment) end end end -- GitLab From 535449bc10bf5da4d2726ef2d4c19710b5d00d8f Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 15 Jan 2025 10:48:57 +0900 Subject: [PATCH 11/48] Use cache for storing the completed status --- .../ci/build/prerequisite/managed_resource.rb | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index e54a0994909a47..28c1b21b379a30 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -15,7 +15,7 @@ def initialize(build) def unmet? # TODO: Check "resource_management.enabled" flag. Return false if not enabled. - !completed + !completed? end def complete! @@ -26,6 +26,7 @@ def complete! ensure_environment @completed = true + set_completed(true) end private @@ -53,7 +54,19 @@ def kas_client end def environment - build.deployment.environment + build.deployment&.environment + end + + def completed? + Rails.cache.fetch(cache_key) == true + end + + def set_completed(completed_status) + Rails.cache.write(cache_key, completed_status) + end + + def cache_key + @cache_key ||= "#{self.class.name.parameterize}:build-#{build.id}:is_completed)" end end end -- GitLab From 2861b9df0ebe9e55fcd1ce3f62f8645ef24c3365 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 15 Jan 2025 13:54:54 +0900 Subject: [PATCH 12/48] Update factory spec --- spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb index a38ade4bfa513f..91a1ee94954a1d 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb @@ -12,17 +12,25 @@ unmet?: unmet) end + let(:managed_resource) do + instance_double( + Gitlab::Ci::Build::Prerequisite::ManagedResource, + unmet?: unmet) + end + subject { described_class.new(build).unmet } before do expect(Gitlab::Ci::Build::Prerequisite::KubernetesNamespace) .to receive(:new).with(build).and_return(kubernetes_namespace) + expect(Gitlab::Ci::Build::Prerequisite::ManagedResource) + .to receive(:new).with(build).and_return(managed_resource) end context 'prerequisite is unmet' do let(:unmet) { true } - it { is_expected.to eq [kubernetes_namespace] } + it { is_expected.to eq [kubernetes_namespace, managed_resource] } end context 'prerequisite is met' do -- GitLab From 073f6f7d14508c9d0b792926cf1e0454f95f128b Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 16 Jan 2025 17:14:07 +0900 Subject: [PATCH 13/48] Try fetching the template named default --- .../ci/build/prerequisite/managed_resource.rb | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index 28c1b21b379a30..4aa4d7261448b0 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -7,15 +7,14 @@ module Prerequisite class ManagedResource < Base attr_reader :completed - def initialize(build) - super - @completed = false - end + DEFAULT_TEMPLATE_NAME = "default" def unmet? # TODO: Check "resource_management.enabled" flag. Return false if not enabled. - !completed? + return false unless valid_for_managed_resources?(environment:, build:) + + !status_completed? end def complete! @@ -25,14 +24,21 @@ def complete! ensure_environment - @completed = true - set_completed(true) + update_status(:completed) + rescue StandardError + # TODO: Handle the error in the follow-up + update_status(:failed) end private def ensure_environment - template = kas_client.get_default_environment_template + template = begin + kas_client.get_environment_template(environment: environment, + template_name: DEFAULT_TEMPLATE_NAME) + rescue GRPC::NotFound + kas_client.get_default_environment_template + end rendered_template = kas_client.render_environment_template( template: template, @@ -43,6 +49,8 @@ def ensure_environment template: rendered_template, environment: environment, build: build) + rescue StandardError => e + raise e end def valid_for_managed_resources?(environment:, build:) @@ -57,17 +65,13 @@ def environment build.deployment&.environment end - def completed? - Rails.cache.fetch(cache_key) == true + # TODO: Check if the process is completed + def status_completed? + true end - def set_completed(completed_status) - Rails.cache.write(cache_key, completed_status) - end - - def cache_key - @cache_key ||= "#{self.class.name.parameterize}:build-#{build.id}:is_completed)" - end + # TODO: Update the completed status + def update_status(status); end end end end -- GitLab From 7c070bb7f503c53eb4f6f80fe28270881621f0db Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Fri, 17 Jan 2025 16:30:54 +0900 Subject: [PATCH 14/48] Simplify method signature and validate arguments --- lib/gitlab/ci/build/prerequisite/managed_resource.rb | 7 +++---- lib/gitlab/kas/client.rb | 10 +++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index 4aa4d7261448b0..8d5be7f258ec56 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -34,8 +34,7 @@ def complete! def ensure_environment template = begin - kas_client.get_environment_template(environment: environment, - template_name: DEFAULT_TEMPLATE_NAME) + kas_client.get_environment_template(environment: environment, template_name: DEFAULT_TEMPLATE_NAME) rescue GRPC::NotFound kas_client.get_default_environment_template end @@ -65,12 +64,12 @@ def environment build.deployment&.environment end - # TODO: Check if the process is completed + # TODO: Fetch completed status from the database def status_completed? true end - # TODO: Update the completed status + # TODO: Update the completed status in the database def update_status(status); end end end diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index 0b4bd20a064f83..b094e0c7bf3a65 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -102,10 +102,13 @@ def send_autoflow_event(project:, type:, id:, data:) .cloud_event(request, metadata: metadata) end - def get_environment_template(project:, template_name:, agent_name:) + def get_environment_template(environment:, template_name:) + project = environment.project + return unless project && environment.cluster_agent + request = Gitlab::Agent::ManagedResources::Rpc::GetEnvironmentTemplateRequest.new( template_name: template_name, - agent_name: agent_name, + agent_name: environment.cluster_agent.name, gitaly_info: gitaly_info(project), gitaly_repository: repository(project), default_branch: project.default_branch_or_main @@ -194,6 +197,7 @@ def timeout def templating_info(environment:, build:) agent = environment.cluster_agent project = environment.project + return unless agent && project && build && build.user Gitlab::Agent::ManagedResources::TemplatingInfo.new( agent: Gitlab::Agent::ManagedResources::Agent.new( @@ -218,7 +222,7 @@ def templating_info(environment:, build:) end def agent_url(project, agent_name) - Gitlab::Routing.url_helpers.project_cluster_agent_url(project, agent) + Gitlab::Routing.url_helpers.project_cluster_agent_url(project, agent_name) end def environment_url(project, environment) -- GitLab From 743ab50ebcc87b04f2873ba0be168ad8b5727c94 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Mon, 20 Jan 2025 16:56:57 +0900 Subject: [PATCH 15/48] Add tests to kas client and prerequisite --- lib/gitlab/kas/client.rb | 1 + .../prerequisite/managed_resource_spec.rb | 123 ++++++++++++++++++ spec/lib/gitlab/kas/client_spec.rb | 120 +++++++++++++++++ 3 files changed, 244 insertions(+) create mode 100644 spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index b094e0c7bf3a65..944bcc67822484 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -116,6 +116,7 @@ def get_environment_template(environment:, template_name:) stub_for(:managed_resources) .get_environment_template(request, metadata: metadata) + .template end def get_default_environment_template diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb new file mode 100644 index 00000000000000..e37b98e9fd4a9b --- /dev/null +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Build::Prerequisite::ManagedResource, feature_category: :continuous_integration do + describe '#unmet?' do + let_it_be(:agent_management_project) { create(:project, :private, :repository) } + let_it_be(:cluster_agent) { create(:cluster_agent, project: agent_management_project) } + + let_it_be(:deployment_project) { create(:project, :private, :repository) } + let_it_be(:environment) { create(:environment, project: deployment_project, cluster_agent: cluster_agent) } + let_it_be(:user) { create(:user) } + let_it_be(:deployment) { create(:deployment, environment: environment, user: user) } + let_it_be(:build) { create(:ci_build, environment: environment, user: user, deployment: deployment) } + + let(:instance) { described_class.new(build) } + + subject(:execute_unmet) { instance.unmet? } + + context 'when the build is valid for managed resources' do + context 'when managed resources prerequisite was completed' do + it 'returns false`' do + expect(instance).to receive(:status_completed?).and_return(true) + expect(execute_unmet).to be_falsey + end + end + + context 'when managed resources prerequisite was not completed' do + it 'returns true' do + expect(instance).to receive(:status_completed?).and_return(false) + expect(execute_unmet).to be_truthy + end + end + end + + context 'when the build does not have a deployment' do + let_it_be(:build) { create(:ci_build, deployment: nil) } + + it { is_expected.to be_falsey } + end + + context 'when the build does not have an environment' do + let_it_be(:build) { create(:ci_build, environment: nil) } + + it { is_expected.to be_falsey } + end + + context 'when the build does not have a cluster agent' do + let_it_be(:environment) { create(:environment, cluster_agent: nil) } + + it { is_expected.to be_falsey } + end + end + + describe '#complete!' do + let_it_be(:agent_management_project) { create(:project, :private, :repository) } + let_it_be(:cluster_agent) { create(:cluster_agent, project: agent_management_project) } + + let_it_be(:deployment_project) { create(:project, :private, :repository) } + let_it_be(:environment) { create(:environment, project: deployment_project, cluster_agent: cluster_agent) } + let_it_be(:user) { create(:user) } + let_it_be(:deployment) { create(:deployment, environment: environment, user: user) } + let_it_be(:build) { create(:ci_build, environment: environment, user: user, deployment: deployment) } + + let(:instance) { described_class.new(build) } + + subject(:execute_complete) { instance.complete! } + + context 'when #unmet? returns false' do + before do + allow(instance).to receive(:unmet?).and_return(false) + end + + it 'does not ensure the environment and update the status' do + expect(instance).not_to receive(:ensure_environment) + expect(instance).not_to receive(:update_status) + + execute_complete + end + end + + context 'when #unmet? returns true' do + before do + allow(instance).to receive(:unmet?).and_return(true) + end + + context 'when the build is valid for managed resources' do + context 'when it successfully ensures the environment' do + before do + allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| + allow(kas_client).to receive_messages(get_environment_template: double, + render_environment_template: double) + allow(kas_client).to receive(:ensure_environment) + end + end + + it 'ensures the environment and updates the status to completed' do + expect(instance).to receive(:ensure_environment) + expect(instance).to receive(:update_status).with(:completed) + + execute_complete + end + end + + context 'when it fails to ensure the environment' do + before do + allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| + allow(kas_client).to receive_messages(get_environment_template: double, + render_environment_template: double) + allow(kas_client).to receive(:ensure_environment).and_raise(StandardError) + end + end + + it 'updates the status to failed' do + expect(instance).to receive(:update_status).with(:failed) + + execute_complete + end + end + end + end + end +end diff --git a/spec/lib/gitlab/kas/client_spec.rb b/spec/lib/gitlab/kas/client_spec.rb index a163a3d5c53a44..b7fd6ffcbd0975 100644 --- a/spec/lib/gitlab/kas/client_spec.rb +++ b/spec/lib/gitlab/kas/client_spec.rb @@ -253,5 +253,125 @@ client.list_agent_config_files(project: project) end end + + describe '#get_environment_template' do + let_it_be(:environment) { create(:environment, project: project, cluster_agent: agent) } + let(:stub) { instance_double(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub) } + let(:request) { instance_double(Gitlab::Agent::ManagedResources::Rpc::GetEnvironmentTemplateRequest) } + let(:template) { double("templates", name: "test-template", data: "{}") } + let(:response) { double(Gitlab::Agent::ManagedResources::Rpc::GetEnvironmentTemplateResponse, template: template) } + let(:template_name) { 'default' } + + subject { described_class.new.get_environment_template(environment: environment, template_name: template_name) } + + before do + expect(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub).to receive(:new) + .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) + .and_return(stub) + + expect(Gitlab::Agent::ManagedResources::Rpc::GetEnvironmentTemplateRequest).to receive(:new) + .with( + template_name: template_name, + agent_name: agent.name, + gitaly_info: instance_of(Gitlab::Agent::Entity::GitalyInfo), + gitaly_repository: instance_of(Gitlab::Agent::Entity::GitalyRepository), + default_branch: project.default_branch_or_main) + .and_return(request) + + expect(stub).to receive(:get_environment_template) + .with(request, metadata: { 'authorization' => 'bearer test-token' }) + .and_return(response) + end + + it { expect(subject).to eq(template) } + end + + describe '#get_default_environment_template' do + let(:stub) { instance_double(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub) } + let(:request) { instance_double(Gitlab::Agent::ManagedResources::Rpc::GetDefaultEnvironmentTemplateRequest) } + let(:template) { double("templates", name: "test-template", data: "{}") } + let(:response) { double(Gitlab::Agent::ManagedResources::Rpc::GetDefaultEnvironmentTemplateResponse, template: template) } + + subject { described_class.new.get_default_environment_template } + + before do + expect(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub).to receive(:new) + .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) + .and_return(stub) + + expect(Gitlab::Agent::ManagedResources::Rpc::GetDefaultEnvironmentTemplateRequest).to receive(:new) + .and_return(request) + + expect(stub).to receive(:get_default_environment_template) + .with(request, metadata: { 'authorization' => 'bearer test-token' }) + .and_return(response) + end + + it { expect(subject).to eq(template) } + end + + describe '#render_environment_template' do + let_it_be(:environment) { create(:environment, project: project, cluster_agent: agent) } + let_it_be(:user) { create(:user) } + let_it_be(:build) { create(:ci_build, user: user) } + let(:stub) { instance_double(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub) } + let(:request) { instance_double(Gitlab::Agent::ManagedResources::Rpc::RenderEnvironmentTemplateRequest) } + let(:template) { double("templates", name: "test-template", data: "{}") } + let(:response) { double(Gitlab::Agent::ManagedResources::Rpc::RenderEnvironmentTemplateResponse, template: template) } + + subject { described_class.new.render_environment_template(template: template, environment: environment, build: build) } + + before do + expect(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub).to receive(:new) + .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) + .and_return(stub) + + expect(Gitlab::Agent::ManagedResources::Rpc::RenderEnvironmentTemplateRequest).to receive(:new) + .with( + template: Gitlab::Agent::ManagedResources::EnvironmentTemplate.new( + name: template.name, + data: template.data), + info: instance_of(Gitlab::Agent::ManagedResources::TemplatingInfo)) + .and_return(request) + + expect(stub).to receive(:render_environment_template) + .with(request, metadata: { 'authorization' => 'bearer test-token' }) + .and_return(response) + end + + it { expect(subject).to eq(template) } + end + + describe '#ensure_environment' do + let_it_be(:environment) { create(:environment, project: project, cluster_agent: agent) } + let_it_be(:user) { create(:user) } + let_it_be(:build) { create(:ci_build, user: user) } + let(:stub) { instance_double(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub) } + let(:request) { instance_double(Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentRequest) } + let(:template) { double("templates", name: "test-template", data: "{}") } + let(:response) { double(Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentResponse) } + + subject { described_class.new.ensure_environment(template: template, environment: environment, build: build) } + + before do + expect(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub).to receive(:new) + .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) + .and_return(stub) + + expect(Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentRequest).to receive(:new) + .with( + template: Gitlab::Agent::ManagedResources::RenderedEnvironmentTemplate.new( + name: template.name, + data: template.data), + info: instance_of(Gitlab::Agent::ManagedResources::TemplatingInfo)) + .and_return(request) + + expect(stub).to receive(:ensure_environment) + .with(request, metadata: { 'authorization' => 'bearer test-token' }) + .and_return(response) + end + + it { expect(subject).to eq(response) } + end end end -- GitLab From f8cf8fb2f0109e08198c52435f04f4a2d5bfb696 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 22 Jan 2025 16:15:20 +0900 Subject: [PATCH 16/48] Keep track of managed resource status --- app/models/ci/build.rb | 1 + .../clusters/agents/managed_resource.rb | 6 + .../ci/build/prerequisite/managed_resource.rb | 30 ++-- lib/gitlab/kas/client.rb | 5 +- spec/factories/clusters/managed_resources.rb | 2 +- .../prerequisite/managed_resource_spec.rb | 149 +++++++++++------- 6 files changed, 124 insertions(+), 69 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index e1166c3a5508f5..19f579bd64b151 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -128,6 +128,7 @@ class Build < Ci::Processable has_one :trace_metadata, class_name: 'Ci::BuildTraceMetadata', foreign_key: :build_id, inverse_of: :build has_many :terraform_state_versions, class_name: 'Terraform::StateVersion', foreign_key: :ci_build_id, inverse_of: :build + has_many :managed_resources, class_name: 'Clusters::Agents::ManagedResource', foreign_key: :build_id, inverse_of: :build accepts_nested_attributes_for :runner_session, update_only: true accepts_nested_attributes_for :job_variables diff --git a/app/models/clusters/agents/managed_resource.rb b/app/models/clusters/agents/managed_resource.rb index 2989e526212d7b..eb75dc4924b4f0 100644 --- a/app/models/clusters/agents/managed_resource.rb +++ b/app/models/clusters/agents/managed_resource.rb @@ -11,6 +11,12 @@ class ManagedResource < ApplicationRecord belongs_to :environment validates :template_name, length: { maximum: 1024 } + + enum :status, { + processing: 0, + completed: 1, + failed: 2 + } end end end diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index 8d5be7f258ec56..be0b4661e09dfd 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -10,11 +10,11 @@ class ManagedResource < Base DEFAULT_TEMPLATE_NAME = "default" def unmet? - # TODO: Check "resource_management.enabled" flag. Return false if not enabled. + return false unless resource_management_enabled? return false unless valid_for_managed_resources?(environment:, build:) - !status_completed? + !managed_resource&.completed? end def complete! @@ -32,6 +32,11 @@ def complete! private + # TODO: Check "resource_management.enabled" flag in the follow-up MR. + def resource_management_enabled? + false + end + def ensure_environment template = begin kas_client.get_environment_template(environment: environment, template_name: DEFAULT_TEMPLATE_NAME) @@ -48,8 +53,6 @@ def ensure_environment template: rendered_template, environment: environment, build: build) - rescue StandardError => e - raise e end def valid_for_managed_resources?(environment:, build:) @@ -64,13 +67,22 @@ def environment build.deployment&.environment end - # TODO: Fetch completed status from the database - def status_completed? - true + def managed_resource + build.managed_resources.first end - # TODO: Update the completed status in the database - def update_status(status); end + def update_status(status) + if managed_resource + managed_resource.update!(status: status) + else + Clusters::Agents::ManagedResource.create!( + build: build, + project: build.project, + environment: environment, + cluster_agent: environment.cluster_agent, + status: status) + end + end end end end diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index 944bcc67822484..c978a1e8b78f07 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -191,10 +191,10 @@ def token end.encoded end -<<<<<<< HEAD def timeout Gitlab::Kas.client_timeout_seconds.seconds -======= + end + def templating_info(environment:, build:) agent = environment.cluster_agent project = environment.project @@ -219,7 +219,6 @@ def templating_info(environment:, build:) job: Gitlab::Agent::ManagedResources::Job.new(id: build.id), user: Gitlab::Agent::ManagedResources::User.new(id: build.user_id, username: build.user.username) ) ->>>>>>> 6a378fa3fd81 (Ensure environment in managed resource prerequisite) end def agent_url(project, agent_name) diff --git a/spec/factories/clusters/managed_resources.rb b/spec/factories/clusters/managed_resources.rb index 2c808a1e71bf02..f352bdccfad142 100644 --- a/spec/factories/clusters/managed_resources.rb +++ b/spec/factories/clusters/managed_resources.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - factory :clusters_managed_resource, class: 'Clusters::Agents::ManagedResource' do + factory :managed_resource, class: 'Clusters::Agents::ManagedResource' do project environment association :cluster_agent diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index e37b98e9fd4a9b..a351dbb98c70dd 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -12,43 +12,68 @@ let_it_be(:user) { create(:user) } let_it_be(:deployment) { create(:deployment, environment: environment, user: user) } let_it_be(:build) { create(:ci_build, environment: environment, user: user, deployment: deployment) } + let(:status) { :processing } + let!(:managed_resource) do + create(:managed_resource, + build: build, + project: deployment_project, + environment: environment, + cluster_agent: cluster_agent, + status: status) + end let(:instance) { described_class.new(build) } subject(:execute_unmet) { instance.unmet? } - context 'when the build is valid for managed resources' do - context 'when managed resources prerequisite was completed' do - it 'returns false`' do - expect(instance).to receive(:status_completed?).and_return(true) - expect(execute_unmet).to be_falsey - end + context 'when resource_management is not enabled' do + it 'returns false' do + expect(execute_unmet).to be_falsey end + end + + context 'when resource_management is enabled' do + before do + allow(instance).to receive(:resource_management_enabled?).and_return(true) + end + + context 'when the build is valid for managed resources' do + context 'when managed resources completed successfully' do + let!(:status) { :completed } + + it 'returns false`' do + expect(execute_unmet).to be_falsey + end + end + + context 'when managed resources failed' do + let!(:status) { :failed } - context 'when managed resources prerequisite was not completed' do - it 'returns true' do - expect(instance).to receive(:status_completed?).and_return(false) - expect(execute_unmet).to be_truthy + it 'returns true' do + expect(execute_unmet).to be_truthy + end end end - end - context 'when the build does not have a deployment' do - let_it_be(:build) { create(:ci_build, deployment: nil) } + context 'when the build does not have a deployment' do + let_it_be(:build) { create(:ci_build, deployment: nil) } - it { is_expected.to be_falsey } - end + it { is_expected.to be_falsey } + end - context 'when the build does not have an environment' do - let_it_be(:build) { create(:ci_build, environment: nil) } + context 'when the build does not have an environment' do + let_it_be(:build) { create(:ci_build, environment: nil) } - it { is_expected.to be_falsey } - end + it { is_expected.to be_falsey } + end - context 'when the build does not have a cluster agent' do - let_it_be(:environment) { create(:environment, cluster_agent: nil) } + context 'when the build does not have a cluster agent' do + before do + environment.update!(cluster_agent: nil) + end - it { is_expected.to be_falsey } + it { is_expected.to be_falsey } + end end end @@ -66,55 +91,67 @@ subject(:execute_complete) { instance.complete! } - context 'when #unmet? returns false' do - before do - allow(instance).to receive(:unmet?).and_return(false) - end - - it 'does not ensure the environment and update the status' do + context 'when resource_management is not enabled' do + it 'does nothing' do expect(instance).not_to receive(:ensure_environment) - expect(instance).not_to receive(:update_status) - - execute_complete + expect(execute_complete).to be_falsey end end - context 'when #unmet? returns true' do + context 'when resource_management is enabled' do before do - allow(instance).to receive(:unmet?).and_return(true) + allow(instance).to receive(:resource_management_enabled?).and_return(true) end - context 'when the build is valid for managed resources' do - context 'when it successfully ensures the environment' do - before do - allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| - allow(kas_client).to receive_messages(get_environment_template: double, - render_environment_template: double) - allow(kas_client).to receive(:ensure_environment) - end - end + context 'when #unmet? returns false' do + before do + allow(instance).to receive(:unmet?).and_return(false) + end - it 'ensures the environment and updates the status to completed' do - expect(instance).to receive(:ensure_environment) - expect(instance).to receive(:update_status).with(:completed) + it 'does not ensure the environment and update the status' do + expect(instance).not_to receive(:ensure_environment) + expect(instance).not_to receive(:update_status) - execute_complete - end + execute_complete + end + end + + context 'when #unmet? returns true' do + before do + allow(instance).to receive(:unmet?).and_return(true) end - context 'when it fails to ensure the environment' do - before do - allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| - allow(kas_client).to receive_messages(get_environment_template: double, - render_environment_template: double) - allow(kas_client).to receive(:ensure_environment).and_raise(StandardError) + context 'when the build is valid for managed resources' do + context 'when it successfully ensures the environment' do + before do + allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| + allow(kas_client).to receive_messages(get_environment_template: double, + render_environment_template: double) + allow(kas_client).to receive(:ensure_environment) + end + end + + it 'creates the managed resource record with the completed status' do + expect { execute_complete }.to change { Clusters::Agents::ManagedResource.count }.by(1) + managed_resource = Clusters::Agents::ManagedResource.find_by(build: build) + expect(managed_resource.status).to eq('completed') end end - it 'updates the status to failed' do - expect(instance).to receive(:update_status).with(:failed) + context 'when it fails to ensure the environment' do + before do + allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| + allow(kas_client).to receive_messages(get_environment_template: double, + render_environment_template: double) + allow(kas_client).to receive(:ensure_environment).and_raise(StandardError) + end + end - execute_complete + it 'creates the managed resource record with the failed status' do + expect { execute_complete }.to change { Clusters::Agents::ManagedResource.count }.by(1) + managed_resource = Clusters::Agents::ManagedResource.find_by(build: build) + expect(managed_resource.status).to eq('failed') + end end end end -- GitLab From bd4e2cd4c14894412eb3f2a52c4771748ffd780d Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 22 Jan 2025 07:46:07 +0000 Subject: [PATCH 17/48] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: DANGER_GITLAB_API_TOKEN --- spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb index 91a1ee94954a1d..20e1569a2f10a9 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/factory_spec.rb @@ -30,7 +30,7 @@ context 'prerequisite is unmet' do let(:unmet) { true } - it { is_expected.to eq [kubernetes_namespace, managed_resource] } + it { is_expected.to match_array [kubernetes_namespace, managed_resource] } end context 'prerequisite is met' do -- GitLab From a7bca2584b8a28084d157cd9455c6ae8f033fa37 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 22 Jan 2025 21:11:19 +0900 Subject: [PATCH 18/48] Fail the process if KAS returns the apply errors --- .../ci/build/prerequisite/managed_resource.rb | 16 ++++++--- .../prerequisite/managed_resource_spec.rb | 33 +++++++++++++++++-- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index be0b4661e09dfd..bf21ed2cc93efa 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -20,11 +20,13 @@ def unmet? def complete! return unless unmet? - return unless valid_for_managed_resources?(environment:, build:) - - ensure_environment - - update_status(:completed) + response = ensure_environment + if response.errors.any? + log_error(response.errors) + update_status(:failed) + else + update_status(:completed) + end rescue StandardError # TODO: Handle the error in the follow-up update_status(:failed) @@ -59,6 +61,10 @@ def valid_for_managed_resources?(environment:, build:) environment&.cluster_agent && environment.project && build && build.user end + def log_error(response) + # TODO: Log error + end + def kas_client @kas_client ||= Gitlab::Kas::Client.new end diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index a351dbb98c70dd..e29a427aa9f7ea 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -50,6 +50,7 @@ let!(:status) { :failed } it 'returns true' do + managed_resource.reload expect(execute_unmet).to be_truthy end end @@ -127,18 +128,19 @@ allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| allow(kas_client).to receive_messages(get_environment_template: double, render_environment_template: double) - allow(kas_client).to receive(:ensure_environment) + success_response = Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentResponse.new(errors: []) + allow(kas_client).to receive(:ensure_environment).and_return(success_response) end end it 'creates the managed resource record with the completed status' do expect { execute_complete }.to change { Clusters::Agents::ManagedResource.count }.by(1) managed_resource = Clusters::Agents::ManagedResource.find_by(build: build) - expect(managed_resource.status).to eq('completed') + expect(managed_resource.status).to eq("completed") end end - context 'when it fails to ensure the environment' do + context 'when ensure_environment raises an error' do before do allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| allow(kas_client).to receive_messages(get_environment_template: double, @@ -153,6 +155,31 @@ expect(managed_resource.status).to eq('failed') end end + + context 'when ensure_environment returns an error' do + before do + allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| + allow(kas_client).to receive_messages(get_environment_template: double, + render_environment_template: double) + error = { + group: 'group', + version: 'version', + kind: 'kind', + name: 'name', + namespace: 'namespace', + error: 'error message' + } + error_response = Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentResponse.new(errors: [error]) + allow(kas_client).to receive(:ensure_environment).and_return(error_response) + end + end + + it 'creates the managed resource record with the failed status' do + expect { execute_complete }.to change { Clusters::Agents::ManagedResource.count }.by(1) + managed_resource = Clusters::Agents::ManagedResource.find_by(build: build) + expect(managed_resource.status).to eq('failed') + end + end end end end -- GitLab From 0e29eaeaf9c6d8e98b5fd7cc6fe7549e8106fae8 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 22 Jan 2025 21:16:40 +0900 Subject: [PATCH 19/48] Update kas client specc --- spec/lib/gitlab/kas/client_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/lib/gitlab/kas/client_spec.rb b/spec/lib/gitlab/kas/client_spec.rb index b7fd6ffcbd0975..4fda7e61e770c1 100644 --- a/spec/lib/gitlab/kas/client_spec.rb +++ b/spec/lib/gitlab/kas/client_spec.rb @@ -262,11 +262,11 @@ let(:response) { double(Gitlab::Agent::ManagedResources::Rpc::GetEnvironmentTemplateResponse, template: template) } let(:template_name) { 'default' } - subject { described_class.new.get_environment_template(environment: environment, template_name: template_name) } + subject { client.get_environment_template(environment: environment, template_name: template_name) } before do expect(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub).to receive(:new) - .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) + .with('example.kas.internal', :this_channel_is_insecure, timeout: client.send(:timeout)) .and_return(stub) expect(Gitlab::Agent::ManagedResources::Rpc::GetEnvironmentTemplateRequest).to receive(:new) @@ -292,11 +292,11 @@ let(:template) { double("templates", name: "test-template", data: "{}") } let(:response) { double(Gitlab::Agent::ManagedResources::Rpc::GetDefaultEnvironmentTemplateResponse, template: template) } - subject { described_class.new.get_default_environment_template } + subject { client.get_default_environment_template } before do expect(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub).to receive(:new) - .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) + .with('example.kas.internal', :this_channel_is_insecure, timeout: client.send(:timeout)) .and_return(stub) expect(Gitlab::Agent::ManagedResources::Rpc::GetDefaultEnvironmentTemplateRequest).to receive(:new) @@ -319,11 +319,11 @@ let(:template) { double("templates", name: "test-template", data: "{}") } let(:response) { double(Gitlab::Agent::ManagedResources::Rpc::RenderEnvironmentTemplateResponse, template: template) } - subject { described_class.new.render_environment_template(template: template, environment: environment, build: build) } + subject { client.render_environment_template(template: template, environment: environment, build: build) } before do expect(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub).to receive(:new) - .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) + .with('example.kas.internal', :this_channel_is_insecure, timeout: client.send(:timeout)) .and_return(stub) expect(Gitlab::Agent::ManagedResources::Rpc::RenderEnvironmentTemplateRequest).to receive(:new) @@ -351,11 +351,11 @@ let(:template) { double("templates", name: "test-template", data: "{}") } let(:response) { double(Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentResponse) } - subject { described_class.new.ensure_environment(template: template, environment: environment, build: build) } + subject { client.ensure_environment(template: template, environment: environment, build: build) } before do expect(Gitlab::Agent::ManagedResources::Rpc::Provisioner::Stub).to receive(:new) - .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) + .with('example.kas.internal', :this_channel_is_insecure, timeout: client.send(:timeout)) .and_return(stub) expect(Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentRequest).to receive(:new) -- GitLab From 6e6e1cbb8c905c5c99ec23a49efdaa626bc0a0d8 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 23 Jan 2025 01:04:25 +0000 Subject: [PATCH 20/48] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Tiger Watson --- app/models/ci/build.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 19f579bd64b151..ff7361f587a636 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -128,7 +128,7 @@ class Build < Ci::Processable has_one :trace_metadata, class_name: 'Ci::BuildTraceMetadata', foreign_key: :build_id, inverse_of: :build has_many :terraform_state_versions, class_name: 'Terraform::StateVersion', foreign_key: :ci_build_id, inverse_of: :build - has_many :managed_resources, class_name: 'Clusters::Agents::ManagedResource', foreign_key: :build_id, inverse_of: :build + has_one :managed_resources, class_name: 'Clusters::Agents::ManagedResource', foreign_key: :build_id, inverse_of: :build accepts_nested_attributes_for :runner_session, update_only: true accepts_nested_attributes_for :job_variables -- GitLab From b4987126a2799ec99cee904efacda4e7bbbf01d8 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 23 Jan 2025 12:03:02 +0900 Subject: [PATCH 21/48] Error handling --- app/models/ci/build.rb | 2 +- .../ci/build/prerequisite/managed_resource.rb | 38 ++++++++----------- .../prerequisite/managed_resource_spec.rb | 33 ++++++++-------- 3 files changed, 34 insertions(+), 39 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ff7361f587a636..7b6cdc2438f79c 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -128,7 +128,7 @@ class Build < Ci::Processable has_one :trace_metadata, class_name: 'Ci::BuildTraceMetadata', foreign_key: :build_id, inverse_of: :build has_many :terraform_state_versions, class_name: 'Terraform::StateVersion', foreign_key: :ci_build_id, inverse_of: :build - has_one :managed_resources, class_name: 'Clusters::Agents::ManagedResource', foreign_key: :build_id, inverse_of: :build + has_one :managed_resource, class_name: 'Clusters::Agents::ManagedResource', foreign_key: :build_id, inverse_of: :build accepts_nested_attributes_for :runner_session, update_only: true accepts_nested_attributes_for :job_variables diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index bf21ed2cc93efa..ad437aae7bdddf 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -5,6 +5,8 @@ module Ci module Build module Prerequisite class ManagedResource < Base + ManagedResourceError = Class.new(StandardError) + attr_reader :completed DEFAULT_TEMPLATE_NAME = "default" @@ -20,16 +22,15 @@ def unmet? def complete! return unless unmet? + managed_resource = create_managed_resource + response = ensure_environment if response.errors.any? - log_error(response.errors) - update_status(:failed) + managed_resource.update!(status: :failed) + raise ManagedResourceError, response.errors else - update_status(:completed) + managed_resource.update!(status: :completed) end - rescue StandardError - # TODO: Handle the error in the follow-up - update_status(:failed) end private @@ -61,10 +62,6 @@ def valid_for_managed_resources?(environment:, build:) environment&.cluster_agent && environment.project && build && build.user end - def log_error(response) - # TODO: Log error - end - def kas_client @kas_client ||= Gitlab::Kas::Client.new end @@ -74,20 +71,17 @@ def environment end def managed_resource - build.managed_resources.first + build.managed_resource end - def update_status(status) - if managed_resource - managed_resource.update!(status: status) - else - Clusters::Agents::ManagedResource.create!( - build: build, - project: build.project, - environment: environment, - cluster_agent: environment.cluster_agent, - status: status) - end + def create_managed_resource + return managed_resource if managed_resource + + Clusters::Agents::ManagedResource.create!( + build: build, + project: build.project, + environment: environment, + cluster_agent: environment.cluster_agent) end end end diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index e29a427aa9f7ea..5eec71e4bc45b9 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -8,10 +8,13 @@ let_it_be(:cluster_agent) { create(:cluster_agent, project: agent_management_project) } let_it_be(:deployment_project) { create(:project, :private, :repository) } - let_it_be(:environment) { create(:environment, project: deployment_project, cluster_agent: cluster_agent) } + let_it_be_with_reload(:environment) do + create(:environment, project: deployment_project, cluster_agent: cluster_agent) + end + let_it_be(:user) { create(:user) } let_it_be(:deployment) { create(:deployment, environment: environment, user: user) } - let_it_be(:build) { create(:ci_build, environment: environment, user: user, deployment: deployment) } + let_it_be_with_reload(:build) { create(:ci_build, environment: environment, user: user, deployment: deployment) } let(:status) { :processing } let!(:managed_resource) do create(:managed_resource, @@ -86,7 +89,7 @@ let_it_be(:environment) { create(:environment, project: deployment_project, cluster_agent: cluster_agent) } let_it_be(:user) { create(:user) } let_it_be(:deployment) { create(:deployment, environment: environment, user: user) } - let_it_be(:build) { create(:ci_build, environment: environment, user: user, deployment: deployment) } + let!(:build) { create(:ci_build, environment: environment, user: user, deployment: deployment) } let(:instance) { described_class.new(build) } @@ -135,28 +138,26 @@ it 'creates the managed resource record with the completed status' do expect { execute_complete }.to change { Clusters::Agents::ManagedResource.count }.by(1) - managed_resource = Clusters::Agents::ManagedResource.find_by(build: build) - expect(managed_resource.status).to eq("completed") + expect(build.reload.managed_resource.status).to eq("completed") end end - context 'when ensure_environment raises an error' do + context 'when ensure_environment raises GRPC::InvalidArgument error' do before do allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| allow(kas_client).to receive_messages(get_environment_template: double, render_environment_template: double) - allow(kas_client).to receive(:ensure_environment).and_raise(StandardError) + allow(kas_client).to receive(:ensure_environment).and_raise(GRPC::InvalidArgument) end end - it 'creates the managed resource record with the failed status' do - expect { execute_complete }.to change { Clusters::Agents::ManagedResource.count }.by(1) - managed_resource = Clusters::Agents::ManagedResource.find_by(build: build) - expect(managed_resource.status).to eq('failed') + it 'creates the managed resource record and leaves it with the processing status' do + expect { execute_complete }.to raise_error(GRPC::InvalidArgument) + expect(build.reload.managed_resource.status).to eq('processing') end end - context 'when ensure_environment returns an error' do + context 'when ensure_environment returns the successful response but with an error information' do before do allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| allow(kas_client).to receive_messages(get_environment_template: double, @@ -174,10 +175,10 @@ end end - it 'creates the managed resource record with the failed status' do - expect { execute_complete }.to change { Clusters::Agents::ManagedResource.count }.by(1) - managed_resource = Clusters::Agents::ManagedResource.find_by(build: build) - expect(managed_resource.status).to eq('failed') + it 'tracks the error and creates the managed resource record with the failed status' do + expect { execute_complete }.to raise_error( + Gitlab::Ci::Build::Prerequisite::ManagedResource::ManagedResourceError) + expect(build.reload.managed_resource.status).to eq('failed') end end end -- GitLab From 55ebfb7e3ab3eac2844a2db11a6712085b658d91 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 23 Jan 2025 12:09:01 +0900 Subject: [PATCH 22/48] Add managed_resource to all_models.yml --- spec/lib/gitlab/import_export/all_models.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index b2bff3b96eda89..972ecbc369017d 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -479,6 +479,7 @@ builds: - job_artifacts_annotations - project_mirror - build_source +- managed_resource bridges: - user - pipeline -- GitLab From f87f6096102edfb333d809641a7e070079056cec Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 23 Jan 2025 13:30:18 +0900 Subject: [PATCH 23/48] Enable KAS for Rspec test --- .../lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index 5eec71e4bc45b9..cc6447b5d22638 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -95,6 +95,10 @@ subject(:execute_complete) { instance.complete! } + before do + allow(Gitlab::Kas).to receive(:enabled?).and_return(true) + end + context 'when resource_management is not enabled' do it 'does nothing' do expect(instance).not_to receive(:ensure_environment) -- GitLab From f3e00a8796fa5c45ba5bb9c24d3ae4c10a4dbee4 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Fri, 24 Jan 2025 01:17:38 +0000 Subject: [PATCH 24/48] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Tiger Watson --- lib/gitlab/ci/build/prerequisite/managed_resource.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index ad437aae7bdddf..94b73d91c4ec0a 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -71,8 +71,9 @@ def environment end def managed_resource - build.managed_resource + Clusters::Agents::ManagedResource.find_by_build_id(build.id) end + strong_memoize_attr :managed_resource def create_managed_resource return managed_resource if managed_resource -- GitLab From f1e8b9b27de6281a201a5019152300c18a1a1b46 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Fri, 24 Jan 2025 11:27:11 +0900 Subject: [PATCH 25/48] Add managed_resource to ignore ancestors in processable_spec --- spec/models/ci/processable_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index bf31374d4ee0c1..0b5fb54924d1ef 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -100,7 +100,7 @@ queuing_entry runtime_metadata trace_metadata dast_site_profile dast_scanner_profile stage_id dast_site_profiles_build dast_scanner_profiles_build auto_canceled_by_partition_id execution_config_id execution_config - build_source id_value].freeze + build_source id_value managed_resource].freeze end before_all do -- GitLab From b1548fe23bc652812203f2008829da2aa163847a Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Fri, 24 Jan 2025 11:46:45 +0900 Subject: [PATCH 26/48] Remove unnecessary association from Ci::Build --- app/models/ci/build.rb | 1 - .../ci/build/prerequisite/managed_resource_spec.rb | 9 ++++++--- spec/models/ci/processable_spec.rb | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 7b6cdc2438f79c..e1166c3a5508f5 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -128,7 +128,6 @@ class Build < Ci::Processable has_one :trace_metadata, class_name: 'Ci::BuildTraceMetadata', foreign_key: :build_id, inverse_of: :build has_many :terraform_state_versions, class_name: 'Terraform::StateVersion', foreign_key: :ci_build_id, inverse_of: :build - has_one :managed_resource, class_name: 'Clusters::Agents::ManagedResource', foreign_key: :build_id, inverse_of: :build accepts_nested_attributes_for :runner_session, update_only: true accepts_nested_attributes_for :job_variables diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index cc6447b5d22638..f58a95b88bc431 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -142,7 +142,8 @@ it 'creates the managed resource record with the completed status' do expect { execute_complete }.to change { Clusters::Agents::ManagedResource.count }.by(1) - expect(build.reload.managed_resource.status).to eq("completed") + managed_resource = Clusters::Agents::ManagedResource.find_by_build_id(build.id) + expect(managed_resource.status).to eq("completed") end end @@ -157,7 +158,8 @@ it 'creates the managed resource record and leaves it with the processing status' do expect { execute_complete }.to raise_error(GRPC::InvalidArgument) - expect(build.reload.managed_resource.status).to eq('processing') + managed_resource = Clusters::Agents::ManagedResource.find_by_build_id(build.id) + expect(managed_resource.status).to eq('processing') end end @@ -182,7 +184,8 @@ it 'tracks the error and creates the managed resource record with the failed status' do expect { execute_complete }.to raise_error( Gitlab::Ci::Build::Prerequisite::ManagedResource::ManagedResourceError) - expect(build.reload.managed_resource.status).to eq('failed') + managed_resource = Clusters::Agents::ManagedResource.find_by_build_id(build.id) + expect(managed_resource.status).to eq('failed') end end end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 0b5fb54924d1ef..bf31374d4ee0c1 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -100,7 +100,7 @@ queuing_entry runtime_metadata trace_metadata dast_site_profile dast_scanner_profile stage_id dast_site_profiles_build dast_scanner_profiles_build auto_canceled_by_partition_id execution_config_id execution_config - build_source id_value managed_resource].freeze + build_source id_value].freeze end before_all do -- GitLab From 9772840abfe34d6f2ec5d6df5214430a110dd643 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Mon, 27 Jan 2025 14:10:08 +0900 Subject: [PATCH 27/48] Cover mote test cases, minor fixes --- .../ci/build/prerequisite/managed_resource.rb | 4 +--- lib/gitlab/kas/client.rb | 2 +- .../prerequisite/managed_resource_spec.rb | 24 +++++++++++++++++++ spec/lib/gitlab/import_export/all_models.yml | 1 - 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index 94b73d91c4ec0a..79906eca04bf8d 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -7,8 +7,6 @@ module Prerequisite class ManagedResource < Base ManagedResourceError = Class.new(StandardError) - attr_reader :completed - DEFAULT_TEMPLATE_NAME = "default" def unmet? @@ -59,7 +57,7 @@ def ensure_environment end def valid_for_managed_resources?(environment:, build:) - environment&.cluster_agent && environment.project && build && build.user + environment&.cluster_agent && build.user end def kas_client diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index c978a1e8b78f07..8416a11482ad2e 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -212,7 +212,7 @@ def templating_info(environment:, build:) tier: environment.tier), project: Gitlab::Agent::ManagedResources::Project.new( id: project.id, - slug: project.full_path_slug, + slug: project.path, path: project.full_path, url: project.web_url), pipeline: Gitlab::Agent::ManagedResources::Pipeline.new(id: build.pipeline_id), diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index f58a95b88bc431..339d2539476e5f 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -41,6 +41,12 @@ end context 'when the build is valid for managed resources' do + context 'when the managed resource record does not exist' do + let!(:managed_resource) { nil } + + it { is_expected.to be_truthy } + end + context 'when managed resources completed successfully' do let!(:status) { :completed } @@ -147,6 +153,24 @@ end end + context 'when get_environment_template raises GRPC::NotFound error' do + before do + allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| + allow(kas_client).to receive(:get_environment_template).and_raise(GRPC::NotFound) + allow(kas_client).to receive_messages(get_environment_template: double, + render_environment_template: double) + success_response = Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentResponse.new(errors: []) + allow(kas_client).to receive(:ensure_environment).and_return(success_response) + end + end + + it 'creates the managed resource record and ensures the environment' do + expect { execute_complete }.to change { Clusters::Agents::ManagedResource.count }.by(1) + managed_resource = Clusters::Agents::ManagedResource.find_by_build_id(build.id) + expect(managed_resource.status).to eq("completed") + end + end + context 'when ensure_environment raises GRPC::InvalidArgument error' do before do allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 972ecbc369017d..b2bff3b96eda89 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -479,7 +479,6 @@ builds: - job_artifacts_annotations - project_mirror - build_source -- managed_resource bridges: - user - pipeline -- GitLab From 008a06e9948f99fee89c290f7a116925b10defe5 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Tue, 28 Jan 2025 10:36:08 +0900 Subject: [PATCH 28/48] Fix test to cover get_default_environment_template --- spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index 339d2539476e5f..0f696fc4d6092f 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -157,7 +157,7 @@ before do allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| allow(kas_client).to receive(:get_environment_template).and_raise(GRPC::NotFound) - allow(kas_client).to receive_messages(get_environment_template: double, + allow(kas_client).to receive_messages(get_default_environment_template: double, render_environment_template: double) success_response = Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentResponse.new(errors: []) allow(kas_client).to receive(:ensure_environment).and_return(success_response) -- GitLab From 9909b5fe7fa7938bc63159d48d07b90f7283f898 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 29 Jan 2025 15:05:35 +0900 Subject: [PATCH 29/48] Support new version of gitlab-kas-grpc gem --- Gemfile | 2 +- Gemfile.checksum | 2 +- Gemfile.lock | 4 ++-- Gemfile.next.checksum | 2 +- Gemfile.next.lock | 4 ++-- .../ci/build/prerequisite/managed_resource.rb | 6 +++++- lib/gitlab/kas/client.rb | 4 +++- .../build/prerequisite/managed_resource_spec.rb | 16 +++++++++++----- 8 files changed, 26 insertions(+), 14 deletions(-) diff --git a/Gemfile b/Gemfile index 041489a9a280fb..2b1f4481437dff 100644 --- a/Gemfile +++ b/Gemfile @@ -645,7 +645,7 @@ gem 'spamcheck', '~> 1.3.0', feature_category: :insider_threat gem 'gitaly', '~> 17.8.0', feature_category: :gitaly # KAS GRPC protocol definitions -gem 'gitlab-kas-grpc', '~> 17.8.0', feature_category: :deployment_management +gem 'gitlab-kas-grpc', '~> 17.9.0.pre.rc2', feature_category: :deployment_management # Lock the version before issues below are resolved: # https://gitlab.com/gitlab-org/gitlab/-/issues/473169#note_2028352939 diff --git a/Gemfile.checksum b/Gemfile.checksum index 55c2f62bafeb57..a3c6699e7cf0f9 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -233,7 +233,7 @@ {"name":"gitlab-glfm-markdown","version":"0.0.27","platform":"x86_64-darwin","checksum":"ec6775054481b3e07a97d4be945fe41d043f89dc1fa1d95cdfc6a70b439ea0e4"}, {"name":"gitlab-glfm-markdown","version":"0.0.27","platform":"x86_64-linux-gnu","checksum":"ebcccc617db4f669cd2de900e6d31fae5de67acedeb178d242063e338cb57050"}, {"name":"gitlab-glfm-markdown","version":"0.0.27","platform":"x86_64-linux-musl","checksum":"77cf356f2a2fc496ec17206d68a6a1fe4f4d680bc1aac2206c32ee5393611a15"}, -{"name":"gitlab-kas-grpc","version":"17.8.1","platform":"ruby","checksum":"fbb9cf7411d12a7dc491d688a80a1d4064bd92dfbbadce523a549b07d26226e6"}, +{"name":"gitlab-kas-grpc","version":"17.9.0.pre.rc2","platform":"ruby","checksum":"b4572e47d0807e51c7a8f7a43d44a196ebea40ed073b8cb95f72d20044ea4482"}, {"name":"gitlab-labkit","version":"0.37.0","platform":"ruby","checksum":"d2dd0a60db2149a9a8eebf2975dc23f54ac3ceb01bdba732eb1b26b86dfffa70"}, {"name":"gitlab-license","version":"2.6.0","platform":"ruby","checksum":"2c1f8ae73835640ec77bf758c1d0c9730635043c01cf77902f7976e826d7d016"}, {"name":"gitlab-mail_room","version":"0.0.25","platform":"ruby","checksum":"223ce7c3c0797b6015eaa37147884e6ddc7be9a7ee90a424358c96bc18613b1a"}, diff --git a/Gemfile.lock b/Gemfile.lock index 8b953f71cf439e..826f590e2acf1c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -745,7 +745,7 @@ GEM nokogiri (~> 1, >= 1.10.8) gitlab-glfm-markdown (0.0.27) rb_sys (~> 0.9.109) - gitlab-kas-grpc (17.8.1) + gitlab-kas-grpc (17.9.0.pre.rc2) grpc (~> 1.0) gitlab-labkit (0.37.0) actionpack (>= 5.0.0, < 8.1.0) @@ -2094,7 +2094,7 @@ DEPENDENCIES gitlab-glfm-markdown (~> 0.0.21) gitlab-housekeeper! gitlab-http! - gitlab-kas-grpc (~> 17.8.0) + gitlab-kas-grpc (~> 17.9.0.pre.rc2) gitlab-labkit (~> 0.37.0) gitlab-license (~> 2.6) gitlab-mail_room (~> 0.0.24) diff --git a/Gemfile.next.checksum b/Gemfile.next.checksum index f79428fe108488..ddc65e97d62982 100644 --- a/Gemfile.next.checksum +++ b/Gemfile.next.checksum @@ -233,7 +233,7 @@ {"name":"gitlab-glfm-markdown","version":"0.0.27","platform":"x86_64-darwin","checksum":"ec6775054481b3e07a97d4be945fe41d043f89dc1fa1d95cdfc6a70b439ea0e4"}, {"name":"gitlab-glfm-markdown","version":"0.0.27","platform":"x86_64-linux-gnu","checksum":"ebcccc617db4f669cd2de900e6d31fae5de67acedeb178d242063e338cb57050"}, {"name":"gitlab-glfm-markdown","version":"0.0.27","platform":"x86_64-linux-musl","checksum":"77cf356f2a2fc496ec17206d68a6a1fe4f4d680bc1aac2206c32ee5393611a15"}, -{"name":"gitlab-kas-grpc","version":"17.8.1","platform":"ruby","checksum":"fbb9cf7411d12a7dc491d688a80a1d4064bd92dfbbadce523a549b07d26226e6"}, +{"name":"gitlab-kas-grpc","version":"17.9.0.pre.rc2","platform":"ruby","checksum":"b4572e47d0807e51c7a8f7a43d44a196ebea40ed073b8cb95f72d20044ea4482"}, {"name":"gitlab-labkit","version":"0.37.0","platform":"ruby","checksum":"d2dd0a60db2149a9a8eebf2975dc23f54ac3ceb01bdba732eb1b26b86dfffa70"}, {"name":"gitlab-license","version":"2.6.0","platform":"ruby","checksum":"2c1f8ae73835640ec77bf758c1d0c9730635043c01cf77902f7976e826d7d016"}, {"name":"gitlab-mail_room","version":"0.0.25","platform":"ruby","checksum":"223ce7c3c0797b6015eaa37147884e6ddc7be9a7ee90a424358c96bc18613b1a"}, diff --git a/Gemfile.next.lock b/Gemfile.next.lock index e43b98d99f3514..996ca6c3264af8 100644 --- a/Gemfile.next.lock +++ b/Gemfile.next.lock @@ -757,7 +757,7 @@ GEM nokogiri (~> 1, >= 1.10.8) gitlab-glfm-markdown (0.0.27) rb_sys (~> 0.9.109) - gitlab-kas-grpc (17.8.1) + gitlab-kas-grpc (17.9.0.pre.rc2) grpc (~> 1.0) gitlab-labkit (0.37.0) actionpack (>= 5.0.0, < 8.1.0) @@ -2129,7 +2129,7 @@ DEPENDENCIES gitlab-glfm-markdown (~> 0.0.21) gitlab-housekeeper! gitlab-http! - gitlab-kas-grpc (~> 17.8.0) + gitlab-kas-grpc (~> 17.9.0.pre.rc2) gitlab-labkit (~> 0.37.0) gitlab-license (~> 2.6) gitlab-mail_room (~> 0.0.24) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index 79906eca04bf8d..6cfd05397daec8 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -25,7 +25,7 @@ def complete! response = ensure_environment if response.errors.any? managed_resource.update!(status: :failed) - raise ManagedResourceError, response.errors + raise ManagedResourceError, format_error_message(response.errors) else managed_resource.update!(status: :completed) end @@ -82,6 +82,10 @@ def create_managed_resource environment: environment, cluster_agent: environment.cluster_agent) end + + def format_error_message(object_errors) + "Failed to ensure the environment. #{object_errors.map(&:to_json).join(', ')}" + end end end end diff --git a/lib/gitlab/kas/client.rb b/lib/gitlab/kas/client.rb index 8416a11482ad2e..eea07f3fb2a89a 100644 --- a/lib/gitlab/kas/client.rb +++ b/lib/gitlab/kas/client.rb @@ -206,9 +206,11 @@ def templating_info(environment:, build:) name: agent.name, url: agent_url(project, agent.name)), environment: Gitlab::Agent::ManagedResources::Environment.new( + id: environment.id, name: environment.name, slug: environment.slug, - url: environment_url(project, environment), + page_url: environment_url(project, environment), + url: environment.external_url, tier: environment.tier), project: Gitlab::Agent::ManagedResources::Project.new( id: project.id, diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index 0f696fc4d6092f..6e7a2c4426d044 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -192,22 +192,28 @@ allow_next_instance_of(Gitlab::Kas::Client) do |kas_client| allow(kas_client).to receive_messages(get_environment_template: double, render_environment_template: double) - error = { + object = { group: 'group', version: 'version', kind: 'kind', name: 'name', - namespace: 'namespace', - error: 'error message' + namespace: 'namespace' } - error_response = Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentResponse.new(errors: [error]) + error_response = Gitlab::Agent::ManagedResources::Rpc::EnsureEnvironmentResponse.new( + errors: [Gitlab::Agent::ManagedResources::Rpc::ObjectError.new( + error: 'error message', + object: object + )] + ) allow(kas_client).to receive(:ensure_environment).and_return(error_response) end end it 'tracks the error and creates the managed resource record with the failed status' do + error_message = 'Failed to ensure the environment. {"object":{"group":"group","apiVersion":"version",' \ + '"kind":"kind","namespace":"namespace","name":"name"},"error":"error message"}' expect { execute_complete }.to raise_error( - Gitlab::Ci::Build::Prerequisite::ManagedResource::ManagedResourceError) + Gitlab::Ci::Build::Prerequisite::ManagedResource::ManagedResourceError, error_message) managed_resource = Clusters::Agents::ManagedResource.find_by_build_id(build.id) expect(managed_resource.status).to eq('failed') end -- GitLab From c487e21b1ab5f064a52a692de4c1556351dfb783 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 23 Jan 2025 14:21:05 +0900 Subject: [PATCH 30/48] Check if resource_management is enabled --- .../agents/authorizations/ci_access/finder.rb | 4 ++++ lib/gitlab/ci/build/prerequisite/managed_resource.rb | 11 +++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/finders/clusters/agents/authorizations/ci_access/finder.rb b/app/finders/clusters/agents/authorizations/ci_access/finder.rb index e55cc6ec11d196..7f24df1399ea83 100644 --- a/app/finders/clusters/agents/authorizations/ci_access/finder.rb +++ b/app/finders/clusters/agents/authorizations/ci_access/finder.rb @@ -15,6 +15,10 @@ def execute .uniq(&:agent_id) end + def project_and_group_authorizations + project_authorizations + group_authorizations + end + private attr_reader :project diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index 6cfd05397daec8..004d7df4db3f08 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -10,9 +10,8 @@ class ManagedResource < Base DEFAULT_TEMPLATE_NAME = "default" def unmet? - return false unless resource_management_enabled? - return false unless valid_for_managed_resources?(environment:, build:) + return false unless resource_management_enabled? !managed_resource&.completed? end @@ -33,9 +32,13 @@ def complete! private - # TODO: Check "resource_management.enabled" flag in the follow-up MR. def resource_management_enabled? - false + authorizations = ::Clusters::Agents::Authorizations::CiAccess::Finder.new(build.project) + .project_and_group_authorizations + + authorizations.any? do |auth| + auth.agent_id == environment.cluster_agent.id && auth.config.dig('resource_management', 'enabled') == true + end end def ensure_environment -- GitLab From 790031330330d9a9c93bf0021185d9425530c307 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Fri, 24 Jan 2025 17:15:04 +0900 Subject: [PATCH 31/48] Add feature flag --- .../beta/gitlab_managed_cluster_resources.yml | 9 +++++++++ lib/gitlab/ci/build/prerequisite/managed_resource.rb | 2 ++ 2 files changed, 11 insertions(+) create mode 100644 config/feature_flags/beta/gitlab_managed_cluster_resources.yml diff --git a/config/feature_flags/beta/gitlab_managed_cluster_resources.yml b/config/feature_flags/beta/gitlab_managed_cluster_resources.yml new file mode 100644 index 00000000000000..dc11310983bbae --- /dev/null +++ b/config/feature_flags/beta/gitlab_managed_cluster_resources.yml @@ -0,0 +1,9 @@ +--- +name: gitlab_managed_cluster_resources +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/507268 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178824 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/514901 +milestone: '17.9' +group: group::environments +type: beta +default_enabled: false diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index 004d7df4db3f08..f3338719306a20 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -33,6 +33,8 @@ def complete! private def resource_management_enabled? + return false unless Feature.enabled?(:gitlab_managed_cluster_resources, build.project) + authorizations = ::Clusters::Agents::Authorizations::CiAccess::Finder.new(build.project) .project_and_group_authorizations -- GitLab From 8fac606db19939b9adcb12df200f205caf410c51 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Fri, 24 Jan 2025 20:26:21 +0900 Subject: [PATCH 32/48] Extend ci access finder to filter by agent --- .../agents/authorizations/ci_access/finder.rb | 21 ++- .../authorizations/ci_access/finder_spec.rb | 146 ++++++++++++------ 2 files changed, 112 insertions(+), 55 deletions(-) diff --git a/app/finders/clusters/agents/authorizations/ci_access/finder.rb b/app/finders/clusters/agents/authorizations/ci_access/finder.rb index 7f24df1399ea83..2ce74d62bc13fd 100644 --- a/app/finders/clusters/agents/authorizations/ci_access/finder.rb +++ b/app/finders/clusters/agents/authorizations/ci_access/finder.rb @@ -5,8 +5,9 @@ module Agents module Authorizations module CiAccess class Finder - def initialize(project) + def initialize(project, agent: nil) @project = project + @agent = agent end def execute @@ -21,10 +22,12 @@ def project_and_group_authorizations private - attr_reader :project + attr_reader :project, :agent def implicit_authorizations - project.cluster_agents.map do |agent| + agents = agent&.project_id == project.id ? [agent] : project.cluster_agents + + agents.map do |agent| Clusters::Agents::Authorizations::CiAccess::ImplicitAuthorization.new(agent: agent) end end @@ -33,13 +36,15 @@ def implicit_authorizations def project_authorizations namespace_ids = project.group ? all_namespace_ids : project.namespace_id - Clusters::Agents::Authorizations::CiAccess::ProjectAuthorization + query = Clusters::Agents::Authorizations::CiAccess::ProjectAuthorization .where(project_id: project.id) .joins(agent: :project) .preload(agent: :project) .where(cluster_agents: { projects: { namespace_id: namespace_ids } }) .with_available_ci_access_fields(project) - .to_a + + query = query.where(agent_id: agent.id) if agent + query.to_a end def group_authorizations @@ -56,7 +61,7 @@ def group_authorizations authorizations[:group_id].eq(ordered_ancestors_cte.table[:id]) ).join_sources - Clusters::Agents::Authorizations::CiAccess::GroupAuthorization + query = Clusters::Agents::Authorizations::CiAccess::GroupAuthorization .with(ordered_ancestors_cte.to_arel) .joins(cte_join_sources) .joins(agent: :project) @@ -70,7 +75,9 @@ def group_authorizations ) .select('DISTINCT ON (agent_id) agent_group_authorizations.*') .preload(agent: :project) - .to_a + + query = query.where(agent_id: agent.id) if agent + query.to_a end # rubocop: enable CodeReuse/ActiveRecord diff --git a/spec/finders/clusters/agents/authorizations/ci_access/finder_spec.rb b/spec/finders/clusters/agents/authorizations/ci_access/finder_spec.rb index 0d010729d5c220..5db321b416139a 100644 --- a/spec/finders/clusters/agents/authorizations/ci_access/finder_spec.rb +++ b/spec/finders/clusters/agents/authorizations/ci_access/finder_spec.rb @@ -19,7 +19,9 @@ let_it_be(:staging_agent) { create(:cluster_agent, project: agent_configuration_project) } let_it_be(:production_agent) { create(:cluster_agent, project: agent_configuration_project) } - subject { described_class.new(requesting_project).execute } + let(:finder) { described_class.new(requesting_project) } + + subject { finder.execute } shared_examples_for 'access_as' do let(:config) { { access_as: { access_as => {} } } } @@ -50,34 +52,54 @@ end describe 'project authorizations' do - context 'agent configuration project does not share a root namespace with the given project' do - let(:unrelated_agent) { create(:cluster_agent) } + context 'when initialized without an agent' do + context 'agent configuration project does not share a root namespace with the given project' do + let(:unrelated_agent) { create(:cluster_agent) } + + before do + create(:agent_ci_access_project_authorization, agent: unrelated_agent, project: requesting_project) + end - before do - create(:agent_ci_access_project_authorization, agent: unrelated_agent, project: requesting_project) + it { is_expected.to be_empty } end - it { is_expected.to be_empty } - end + context 'agent configuration project shares a root namespace, but does not belong to an ancestor of the given project' do + let!(:project_authorization) { create(:agent_ci_access_project_authorization, agent: non_ancestor_agent, project: requesting_project) } - context 'agent configuration project shares a root namespace, but does not belong to an ancestor of the given project' do - let!(:project_authorization) { create(:agent_ci_access_project_authorization, agent: non_ancestor_agent, project: requesting_project) } + it { is_expected.to match_array([project_authorization]) } + end - it { is_expected.to match_array([project_authorization]) } - end + context 'with project authorizations present' do + let!(:authorization) { create(:agent_ci_access_project_authorization, agent: production_agent, project: requesting_project) } - context 'with project authorizations present' do - let!(:authorization) { create(:agent_ci_access_project_authorization, agent: production_agent, project: requesting_project) } + it { is_expected.to match_array [authorization] } + end - it { is_expected.to match_array [authorization] } + context 'with overlapping authorizations' do + let!(:agent) { create(:cluster_agent, project: requesting_project) } + let!(:project_authorization) { create(:agent_ci_access_project_authorization, agent: agent, project: requesting_project) } + let!(:group_authorization) { create(:agent_ci_access_group_authorization, agent: agent, group: bottom_level_group) } + + it { is_expected.to match_array [project_authorization] } + end + + context 'with multiple authorizations' do + let!(:authorization1) { create(:agent_ci_access_project_authorization, agent: production_agent, project: requesting_project) } + let!(:authorization2) { create(:agent_ci_access_project_authorization, agent: staging_agent, project: requesting_project) } + + it { is_expected.to contain_exactly(authorization1, authorization2) } + end end - context 'with overlapping authorizations' do - let!(:agent) { create(:cluster_agent, project: requesting_project) } - let!(:project_authorization) { create(:agent_ci_access_project_authorization, agent: agent, project: requesting_project) } - let!(:group_authorization) { create(:agent_ci_access_group_authorization, agent: agent, group: bottom_level_group) } + context 'when initialized with an agent' do + let!(:authorization1) { create(:agent_ci_access_project_authorization, agent: production_agent, project: requesting_project) } + let!(:authorization2) { create(:agent_ci_access_project_authorization, agent: staging_agent, project: requesting_project) } + + let!(:finder) { described_class.new(requesting_project, agent: production_agent) } - it { is_expected.to match_array [project_authorization] } + it 'returns authorizations for the given agent' do + expect(subject).to contain_exactly(authorization1) + end end it_behaves_like 'access_as' do @@ -86,54 +108,82 @@ end describe 'implicit authorizations' do - let!(:associated_agent) { create(:cluster_agent, project: requesting_project) } + let!(:associated_agent_1) { create(:cluster_agent, project: requesting_project) } + let!(:associated_agent_2) { create(:cluster_agent, project: requesting_project) } + + context 'when initialized without an agent' do + it 'returns all authorizations for agents directly associated with the project' do + expect(subject.count).to eq(2) + expect(subject).to all(be_a(Clusters::Agents::Authorizations::CiAccess::ImplicitAuthorization)) + expect(subject.map(&:agent)).to contain_exactly(associated_agent_1, associated_agent_2) + end + end - it 'returns authorizations for agents directly associated with the project' do - expect(subject.count).to eq(1) + context 'when initialized with an agent' do + let!(:finder) { described_class.new(requesting_project, agent: associated_agent_1) } - authorization = subject.first - expect(authorization).to be_a(Clusters::Agents::Authorizations::CiAccess::ImplicitAuthorization) - expect(authorization.agent).to eq(associated_agent) + it 'returns authorizations for the given agent' do + expect(subject.count).to eq(1) + + authorization = subject.first + expect(authorization).to be_a(Clusters::Agents::Authorizations::CiAccess::ImplicitAuthorization) + expect(authorization.agent).to eq(associated_agent_1) + end end end describe 'authorized groups' do - context 'agent configuration project is outside the requesting project hierarchy' do - let(:unrelated_agent) { create(:cluster_agent) } + context 'when initialized without an agent' do + context 'agent configuration project is outside the requesting project hierarchy' do + let(:unrelated_agent) { create(:cluster_agent) } - before do - create(:agent_ci_access_group_authorization, agent: unrelated_agent, group: top_level_group) + before do + create(:agent_ci_access_group_authorization, agent: unrelated_agent, group: top_level_group) + end + + it { is_expected.to be_empty } end - it { is_expected.to be_empty } - end + context 'multiple agents are authorized for the same group' do + let!(:staging_auth) { create(:agent_ci_access_group_authorization, agent: staging_agent, group: bottom_level_group) } + let!(:production_auth) { create(:agent_ci_access_group_authorization, agent: production_agent, group: bottom_level_group) } - context 'multiple agents are authorized for the same group' do - let!(:staging_auth) { create(:agent_ci_access_group_authorization, agent: staging_agent, group: bottom_level_group) } - let!(:production_auth) { create(:agent_ci_access_group_authorization, agent: production_agent, group: bottom_level_group) } + it 'returns authorizations for all agents' do + expect(subject).to contain_exactly(staging_auth, production_auth) + end + end - it 'returns authorizations for all agents' do - expect(subject).to contain_exactly(staging_auth, production_auth) + context 'a single agent is authorized to more than one matching group' do + let!(:bottom_level_auth) { create(:agent_ci_access_group_authorization, agent: production_agent, group: bottom_level_group) } + let!(:top_level_auth) { create(:agent_ci_access_group_authorization, agent: production_agent, group: top_level_group) } + + it 'picks the authorization for the closest group to the requesting project' do + expect(subject).to contain_exactly(bottom_level_auth) + end end - end - context 'a single agent is authorized to more than one matching group' do - let!(:bottom_level_auth) { create(:agent_ci_access_group_authorization, agent: production_agent, group: bottom_level_group) } - let!(:top_level_auth) { create(:agent_ci_access_group_authorization, agent: production_agent, group: top_level_group) } + context 'agent configuration project does not belong to an ancestor of the authorized group' do + let!(:group_authorization) { create(:agent_ci_access_group_authorization, agent: non_ancestor_agent, group: bottom_level_group) } + + it { is_expected.to match_array([group_authorization]) } + end - it 'picks the authorization for the closest group to the requesting project' do - expect(subject).to contain_exactly(bottom_level_auth) + it_behaves_like 'access_as' do + let!(:authorization) { create(:agent_ci_access_group_authorization, agent: production_agent, group: top_level_group, config: config) } end end - context 'agent configuration project does not belong to an ancestor of the authorized group' do - let!(:group_authorization) { create(:agent_ci_access_group_authorization, agent: non_ancestor_agent, group: bottom_level_group) } + context 'when initialized with an agent' do + let(:finder) { described_class.new(requesting_project, agent: production_agent) } - it { is_expected.to match_array([group_authorization]) } - end + context 'multiple agents are authorized for the same group' do + let!(:staging_auth) { create(:agent_ci_access_group_authorization, agent: staging_agent, group: bottom_level_group) } + let!(:production_auth) { create(:agent_ci_access_group_authorization, agent: production_agent, group: bottom_level_group) } - it_behaves_like 'access_as' do - let!(:authorization) { create(:agent_ci_access_group_authorization, agent: production_agent, group: top_level_group, config: config) } + it 'returns authorizations for the given agent' do + expect(subject).to contain_exactly(production_auth) + end + end end end end -- GitLab From b730d65ad74c6b1e80d170a5d3766ab1578ec2c6 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Fri, 24 Jan 2025 22:02:59 +0900 Subject: [PATCH 33/48] Use Finder#execute for finding authorizations --- .../agents/authorizations/ci_access/finder.rb | 4 - .../ci/build/prerequisite/managed_resource.rb | 6 +- .../prerequisite/managed_resource_spec.rb | 116 ++++++++++++------ 3 files changed, 79 insertions(+), 47 deletions(-) diff --git a/app/finders/clusters/agents/authorizations/ci_access/finder.rb b/app/finders/clusters/agents/authorizations/ci_access/finder.rb index 2ce74d62bc13fd..846c80bb37ca73 100644 --- a/app/finders/clusters/agents/authorizations/ci_access/finder.rb +++ b/app/finders/clusters/agents/authorizations/ci_access/finder.rb @@ -16,10 +16,6 @@ def execute .uniq(&:agent_id) end - def project_and_group_authorizations - project_authorizations + group_authorizations - end - private attr_reader :project, :agent diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index f3338719306a20..76139601f3432d 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -35,11 +35,11 @@ def complete! def resource_management_enabled? return false unless Feature.enabled?(:gitlab_managed_cluster_resources, build.project) - authorizations = ::Clusters::Agents::Authorizations::CiAccess::Finder.new(build.project) - .project_and_group_authorizations + authorizations = ::Clusters::Agents::Authorizations::CiAccess::Finder + .new(build.project, agent: environment.cluster_agent).execute authorizations.any? do |auth| - auth.agent_id == environment.cluster_agent.id && auth.config.dig('resource_management', 'enabled') == true + auth.config.dig('resource_management', 'enabled') == true end end diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index 6e7a2c4426d044..b43a771abc645b 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -4,17 +4,21 @@ RSpec.describe Gitlab::Ci::Build::Prerequisite::ManagedResource, feature_category: :continuous_integration do describe '#unmet?' do - let_it_be(:agent_management_project) { create(:project, :private, :repository) } + let_it_be(:organization) { create(:group) } + let_it_be(:agent_management_project) { create(:project, :private, :repository, group: organization) } let_it_be(:cluster_agent) { create(:cluster_agent, project: agent_management_project) } - let_it_be(:deployment_project) { create(:project, :private, :repository) } + let_it_be(:deployment_project) { create(:project, :private, :repository, group: organization) } let_it_be_with_reload(:environment) do create(:environment, project: deployment_project, cluster_agent: cluster_agent) end - let_it_be(:user) { create(:user) } + let_it_be(:user) { create(:user, developer_of: deployment_project) } let_it_be(:deployment) { create(:deployment, environment: environment, user: user) } - let_it_be_with_reload(:build) { create(:ci_build, environment: environment, user: user, deployment: deployment) } + let_it_be_with_reload(:build) do + create(:ci_build, project: deployment_project, environment: environment, user: user, deployment: deployment) + end + let(:status) { :processing } let!(:managed_resource) do create(:managed_resource, @@ -29,42 +33,7 @@ subject(:execute_unmet) { instance.unmet? } - context 'when resource_management is not enabled' do - it 'returns false' do - expect(execute_unmet).to be_falsey - end - end - - context 'when resource_management is enabled' do - before do - allow(instance).to receive(:resource_management_enabled?).and_return(true) - end - - context 'when the build is valid for managed resources' do - context 'when the managed resource record does not exist' do - let!(:managed_resource) { nil } - - it { is_expected.to be_truthy } - end - - context 'when managed resources completed successfully' do - let!(:status) { :completed } - - it 'returns false`' do - expect(execute_unmet).to be_falsey - end - end - - context 'when managed resources failed' do - let!(:status) { :failed } - - it 'returns true' do - managed_resource.reload - expect(execute_unmet).to be_truthy - end - end - end - + context 'when not valid for managed resources' do context 'when the build does not have a deployment' do let_it_be(:build) { create(:ci_build, deployment: nil) } @@ -85,6 +54,73 @@ it { is_expected.to be_falsey } end end + + context 'when valid for managed resources' do + context 'when feature flag is disabled' do + before do + stub_feature_flags(gitlab_managed_cluster_resources: false) + end + + it 'returns false' do + expect(execute_unmet).to be_falsey + end + end + + context 'when feature flag is enabled' do + before do + stub_feature_flags(gitlab_managed_cluster_resources: true) + end + + context 'when authorization exists' do + context 'when the resource_management is not enabled' do + let!(:agent_ci_access_group_authorization) do + create(:agent_ci_access_group_authorization, agent: cluster_agent, group: organization) + end + + context 'when the managed resource record has failed status' do + let!(:status) { :failed } + + it 'returns false' do + managed_resource.reload + agent_ci_access_group_authorization.reload + expect(execute_unmet).to be_falsey + end + end + end + + context 'when authorization exists with resource_management enabled' do + let!(:agent_ci_access_group_authorization) do + create(:agent_ci_access_group_authorization, agent: cluster_agent, group: organization, + config: { resource_management: { enabled: true } }) + end + + context 'when the managed resource record does not exist' do + let!(:managed_resource) { nil } + + it { is_expected.to be_truthy } + end + + context 'when the managed resource record has completed status' do + let!(:status) { :completed } + + it 'returns false`' do + expect(execute_unmet).to be_falsey + end + end + + context 'when the managed resource record has failed status' do + let!(:status) { :failed } + + it 'returns true' do + managed_resource.reload + agent_ci_access_group_authorization.reload + expect(execute_unmet).to be_truthy + end + end + end + end + end + end end describe '#complete!' do -- GitLab From 4d78aedc9723552e0274c581ced6709977eb221c Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Fri, 31 Jan 2025 16:53:12 +0900 Subject: [PATCH 34/48] Minor improvement, remove unnecessary test codes --- lib/gitlab/ci/build/prerequisite/managed_resource.rb | 8 +++----- .../gitlab/ci/build/prerequisite/managed_resource_spec.rb | 8 -------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index 76139601f3432d..c3934a17fdaac0 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -35,12 +35,10 @@ def complete! def resource_management_enabled? return false unless Feature.enabled?(:gitlab_managed_cluster_resources, build.project) - authorizations = ::Clusters::Agents::Authorizations::CiAccess::Finder - .new(build.project, agent: environment.cluster_agent).execute + authorization = ::Clusters::Agents::Authorizations::CiAccess::Finder + .new(build.project, agent: environment.cluster_agent).execute.first - authorizations.any? do |auth| - auth.config.dig('resource_management', 'enabled') == true - end + authorization.present? && authorization.config.dig('resource_management', 'enabled') == true end def ensure_environment diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index b43a771abc645b..b2180af47bd86a 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -67,10 +67,6 @@ end context 'when feature flag is enabled' do - before do - stub_feature_flags(gitlab_managed_cluster_resources: true) - end - context 'when authorization exists' do context 'when the resource_management is not enabled' do let!(:agent_ci_access_group_authorization) do @@ -81,8 +77,6 @@ let!(:status) { :failed } it 'returns false' do - managed_resource.reload - agent_ci_access_group_authorization.reload expect(execute_unmet).to be_falsey end end @@ -112,8 +106,6 @@ let!(:status) { :failed } it 'returns true' do - managed_resource.reload - agent_ci_access_group_authorization.reload expect(execute_unmet).to be_truthy end end -- GitLab From 0405d76e79fae02655a5c6a73c61eef23f3e8936 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Tue, 4 Feb 2025 15:33:59 +0900 Subject: [PATCH 35/48] Consider license feature availability --- app/models/clusters/agent.rb | 4 +++ ee/app/models/ee/clusters/agent.rb | 5 +++ .../models/gitlab_subscriptions/features.rb | 1 + ee/spec/models/ee/clusters/agent_spec.rb | 35 +++++++++++++++++++ .../ci/build/prerequisite/managed_resource.rb | 2 +- .../prerequisite/managed_resource_spec.rb | 29 +++++++++------ 6 files changed, 64 insertions(+), 12 deletions(-) diff --git a/app/models/clusters/agent.rb b/app/models/clusters/agent.rb index f0300f33cf5364..8b58f31e8ff550 100644 --- a/app/models/clusters/agent.rb +++ b/app/models/clusters/agent.rb @@ -93,6 +93,10 @@ def user_access_authorizations ).select('config').compact.first end + def resource_management_enabled? + false # Overridden in EE + end + private def all_ci_access_authorized_projects_for(user) diff --git a/ee/app/models/ee/clusters/agent.rb b/ee/app/models/ee/clusters/agent.rb index 89f57bb4dd51cf..55c9dd87b19b10 100644 --- a/ee/app/models/ee/clusters/agent.rb +++ b/ee/app/models/ee/clusters/agent.rb @@ -55,6 +55,11 @@ module Agent unversioned_latest_workspaces_agent_config: { enabled: true } ) end + + def resource_management_enabled? + ::Feature.enabled?(:gitlab_managed_cluster_resources, project) && + project.licensed_feature_available?(:agent_managed_resources) + end end end end diff --git a/ee/app/models/gitlab_subscriptions/features.rb b/ee/app/models/gitlab_subscriptions/features.rb index 3d2bdd5559b440..7f9e1087e99fab 100644 --- a/ee/app/models/gitlab_subscriptions/features.rb +++ b/ee/app/models/gitlab_subscriptions/features.rb @@ -85,6 +85,7 @@ class Features ai_chat adjourned_deletion_for_projects_and_groups admin_audit_log + agent_managed_resources auditor_user blocking_merge_requests board_assignee_lists diff --git a/ee/spec/models/ee/clusters/agent_spec.rb b/ee/spec/models/ee/clusters/agent_spec.rb index ed642ba2ea0e91..ea9053ff7a9a00 100644 --- a/ee/spec/models/ee/clusters/agent_spec.rb +++ b/ee/spec/models/ee/clusters/agent_spec.rb @@ -61,5 +61,40 @@ agent_1, agent_2, agent_3, agent_with_remote_development_config_disabled) end end + + describe '#resource_management_enabled?' do + let(:project) { create(:project) } + + subject { agent_1.resource_management_enabled? } + + context 'when feature flag is disabled' do + before do + stub_feature_flags(gitlab_managed_cluster_resources: false) + allow(agent_1).to receive(:project).and_return(project) + end + + it { is_expected.to be_falsey } + end + + context 'when feature flag is enabled' do + context 'when licensed feature is not available' do + before do + stub_licensed_features(agent_managed_resources: false) + allow(agent_1).to receive(:project).and_return(project) + end + + it { is_expected.to be_falsey } + end + + context 'when licensed feature is available' do + before do + stub_licensed_features(agent_managed_resources: true) + allow(agent_1).to receive(:project).and_return(project) + end + + it { is_expected.to be_truthy } + end + end + end end end diff --git a/lib/gitlab/ci/build/prerequisite/managed_resource.rb b/lib/gitlab/ci/build/prerequisite/managed_resource.rb index c3934a17fdaac0..777b995ce0fe07 100644 --- a/lib/gitlab/ci/build/prerequisite/managed_resource.rb +++ b/lib/gitlab/ci/build/prerequisite/managed_resource.rb @@ -33,7 +33,7 @@ def complete! private def resource_management_enabled? - return false unless Feature.enabled?(:gitlab_managed_cluster_resources, build.project) + return false unless environment.cluster_agent.resource_management_enabled? authorization = ::Clusters::Agents::Authorizations::CiAccess::Finder .new(build.project, agent: environment.cluster_agent).execute.first diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index b2180af47bd86a..ccb1a5862ba577 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -9,18 +9,18 @@ let_it_be(:cluster_agent) { create(:cluster_agent, project: agent_management_project) } let_it_be(:deployment_project) { create(:project, :private, :repository, group: organization) } - let_it_be_with_reload(:environment) do + let_it_be(:environment) do create(:environment, project: deployment_project, cluster_agent: cluster_agent) end let_it_be(:user) { create(:user, developer_of: deployment_project) } let_it_be(:deployment) { create(:deployment, environment: environment, user: user) } - let_it_be_with_reload(:build) do + let_it_be(:build) do create(:ci_build, project: deployment_project, environment: environment, user: user, deployment: deployment) end let(:status) { :processing } - let!(:managed_resource) do + let(:managed_resource) do create(:managed_resource, build: build, project: deployment_project, @@ -56,9 +56,11 @@ end context 'when valid for managed resources' do - context 'when feature flag is disabled' do + context 'when agent\'s resource management is disabled' do before do - stub_feature_flags(gitlab_managed_cluster_resources: false) + allow_next_instance_of(Clusters::Agent) do |instance| + allow(instance).to receive(:resource_management_enabled?).and_return(false) + end end it 'returns false' do @@ -66,10 +68,14 @@ end end - context 'when feature flag is enabled' do + context 'when agent\'s resource management is enabled' do + before do + allow(environment.cluster_agent).to receive(:resource_management_enabled?).and_return(true) + end + context 'when authorization exists' do context 'when the resource_management is not enabled' do - let!(:agent_ci_access_group_authorization) do + let_it_be(:agent_ci_access_group_authorization) do create(:agent_ci_access_group_authorization, agent: cluster_agent, group: organization) end @@ -83,27 +89,28 @@ end context 'when authorization exists with resource_management enabled' do - let!(:agent_ci_access_group_authorization) do + let_it_be(:agent_ci_access_group_authorization) do create(:agent_ci_access_group_authorization, agent: cluster_agent, group: organization, config: { resource_management: { enabled: true } }) end context 'when the managed resource record does not exist' do - let!(:managed_resource) { nil } + let(:managed_resource) { nil } it { is_expected.to be_truthy } end context 'when the managed resource record has completed status' do - let!(:status) { :completed } + let(:status) { :completed } it 'returns false`' do + managed_resource.reload expect(execute_unmet).to be_falsey end end context 'when the managed resource record has failed status' do - let!(:status) { :failed } + let(:status) { :failed } it 'returns true' do expect(execute_unmet).to be_truthy -- GitLab From 407fb7fc4354d7fe16d8e6a6aeba40b23b86ad49 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Tue, 4 Feb 2025 16:28:43 +0900 Subject: [PATCH 36/48] Fix test error --- spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb index ccb1a5862ba577..30bd99b1bb221a 100644 --- a/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb +++ b/spec/lib/gitlab/ci/build/prerequisite/managed_resource_spec.rb @@ -70,6 +70,7 @@ context 'when agent\'s resource management is enabled' do before do + environment.reload allow(environment.cluster_agent).to receive(:resource_management_enabled?).and_return(true) end -- GitLab From d4db823b71a02655bbf79459a09d9de7efc46504 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Tue, 4 Feb 2025 20:13:22 +0900 Subject: [PATCH 37/48] Associate cluster agent when creating a new environment --- .../environments/create_for_job_service.rb | 31 ++++++++++++++++ .../create_for_job_shared_examples.rb | 36 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/app/services/environments/create_for_job_service.rb b/app/services/environments/create_for_job_service.rb index 02545ce03e0ea6..e383378c71ca35 100644 --- a/app/services/environments/create_for_job_service.rb +++ b/app/services/environments/create_for_job_service.rb @@ -27,6 +27,8 @@ def to_resource(job) environment.auto_stop_in = expanded_auto_stop_in(job) environment.tier = job.environment_tier_from_options environment.merge_request = job.pipeline.merge_request + authorization = matching_authorization(job) + environment.cluster_agent = authorization.agent if authorization end end # rubocop: enable Performance/ActiveRecordSubtransactionMethods @@ -36,5 +38,34 @@ def expanded_auto_stop_in(job) ExpandVariables.expand(job.environment_auto_stop_in, -> { job.simple_variables.sort_and_expand_all }) end + + def cluster_agent_path(job) + environment_options(job).dig(:kubernetes, :agent) + end + + def environment_options(job) + job.options&.dig(:environment) || {} + end + + def matching_authorization(job) + return false unless cluster_agent_path(job) && job.user + + requested_project_path, requested_agent_name = expanded_cluster_agent_path(job).split(':') + + ci_access_authorizations_for_project(job).find do |authorization| + requested_project_path == authorization.config_project.full_path && + requested_agent_name == authorization.agent.name + end + end + + def ci_access_authorizations_for_project(job) + Clusters::Agents::Authorizations::CiAccess::Finder.new(job.project).execute + end + + def expanded_cluster_agent_path(job) + return unless cluster_agent_path(job) + + ExpandVariables.expand(cluster_agent_path(job), -> { variables.sort_and_expand_all }) + end end end diff --git a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb index 3acdc8c142fdc8..5162605072b6e2 100644 --- a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb +++ b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb @@ -292,6 +292,42 @@ end end + context 'when a pipeline contains matching ci_access authorization' do + let(:environment_name) { 'production' } + let(:user) { create(:user) } + let!(:job) { build(factory_type, project: project, pipeline: pipeline, user: user, **attributes) } + let_it_be(:agent) { create(:cluster_agent, project: project) } + let_it_be(:agent_path) { "#{project.full_path}:#{agent.name}" } + let_it_be(:ci_access_authorization) do + create(:agent_ci_access_project_authorization, + project: project, + config: { environment: { kubernetes: { agent: agent_path } } }) + end + + let(:attributes) do + { + environment: environment_name, + options: { + environment: { + name: environment_name, + kubernetes: { + agent: agent_path + } + } + } + } + end + + it 'creates an environment with the specified cluster agent' do + # binding.pry + expect { subject }.to change { Environment.count }.by(1) + + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to eq(agent) + end + end + def environment project.environments.find_by_name('review/master') end -- GitLab From 6a8214182fdfe97eb9c4c9120a07e8cfbd0721fc Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Tue, 4 Feb 2025 20:36:32 +0900 Subject: [PATCH 38/48] Remove comment --- .../environments/create_for_job_shared_examples.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb index 5162605072b6e2..e8cbc97b7c51e9 100644 --- a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb +++ b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb @@ -319,7 +319,6 @@ end it 'creates an environment with the specified cluster agent' do - # binding.pry expect { subject }.to change { Environment.count }.by(1) expect(subject).to be_a(Environment) -- GitLab From c53366d01b6214ca69e212b9d6009d0df970eb3a Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 5 Feb 2025 13:52:33 +0900 Subject: [PATCH 39/48] Add more checks before associating the agent during environment creation --- .../environments/create_for_job_service.rb | 18 +++- .../create_for_job_shared_examples.rb | 89 ++++++++++++++++--- 2 files changed, 92 insertions(+), 15 deletions(-) diff --git a/app/services/environments/create_for_job_service.rb b/app/services/environments/create_for_job_service.rb index e383378c71ca35..63b0ff9c9804a7 100644 --- a/app/services/environments/create_for_job_service.rb +++ b/app/services/environments/create_for_job_service.rb @@ -27,8 +27,13 @@ def to_resource(job) environment.auto_stop_in = expanded_auto_stop_in(job) environment.tier = job.environment_tier_from_options environment.merge_request = job.pipeline.merge_request - authorization = matching_authorization(job) - environment.cluster_agent = authorization.agent if authorization + + if resource_management_feature_enabled?(job) + authorization = matching_authorization(job) + if authorization && authorization.agent.resource_management_enabled? + environment.cluster_agent = authorization.agent + end + end end end # rubocop: enable Performance/ActiveRecordSubtransactionMethods @@ -48,13 +53,14 @@ def environment_options(job) end def matching_authorization(job) - return false unless cluster_agent_path(job) && job.user + return false unless cluster_agent_path(job) requested_project_path, requested_agent_name = expanded_cluster_agent_path(job).split(':') ci_access_authorizations_for_project(job).find do |authorization| requested_project_path == authorization.config_project.full_path && - requested_agent_name == authorization.agent.name + requested_agent_name == authorization.agent.name && + authorization.config.dig('resource_management', 'enabled') == true end end @@ -67,5 +73,9 @@ def expanded_cluster_agent_path(job) ExpandVariables.expand(cluster_agent_path(job), -> { variables.sort_and_expand_all }) end + + def resource_management_feature_enabled?(job) + ::Feature.enabled?(:gitlab_managed_cluster_resources, job.project) + end end end diff --git a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb index e8cbc97b7c51e9..98bc1fdec4f889 100644 --- a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb +++ b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb @@ -298,12 +298,6 @@ let!(:job) { build(factory_type, project: project, pipeline: pipeline, user: user, **attributes) } let_it_be(:agent) { create(:cluster_agent, project: project) } let_it_be(:agent_path) { "#{project.full_path}:#{agent.name}" } - let_it_be(:ci_access_authorization) do - create(:agent_ci_access_project_authorization, - project: project, - config: { environment: { kubernetes: { agent: agent_path } } }) - end - let(:attributes) do { environment: environment_name, @@ -318,12 +312,85 @@ } end - it 'creates an environment with the specified cluster agent' do - expect { subject }.to change { Environment.count }.by(1) + context 'when resource_management feature flag is enabled' do + context 'when license feature is enabled' do + before do + stub_licensed_features(agent_managed_resources: true) + end + + context 'when agent is authorized with resource_management enabled in config' do + let_it_be(:ci_access_authorization) do + create(:agent_ci_access_project_authorization, + project: project, + agent: agent, + config: { resource_management: { enabled: true } } + ) + end + + it 'creates an environment with the specified cluster agent' do + expect { subject }.to change { Environment.count }.by(1) + + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to eq(agent) + end + end + + context 'when agent is authorized without resource_management enabled in config' do + let_it_be(:ci_access_authorization) do + create(:agent_ci_access_project_authorization, + project: project, + agent: agent, + config: {} # resource_management is not enabled + ) + end + + it 'creates an environment without the specified cluster agent' do + expect { subject }.to change { Environment.count }.by(1) + + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to be_nil + end + end + end + + context 'when license feature is not enabled' do + before do + stub_licensed_features(agent_managed_resources: false) + end + + it 'creates an environment without the specified cluster agent' do + expect { subject }.to change { Environment.count }.by(1) + + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to be_nil + end + end + end + + context 'when resource_management feature flag is disabled' do + let_it_be(:ci_access_authorization) do + create(:agent_ci_access_project_authorization, + project: project, + agent: agent, + config: { resource_management: { enabled: true } } + ) + end + + before do + stub_feature_flags(gitlab_managed_cluster_resources: false) + stub_licensed_features(agent_managed_resources: true) + end + + it 'creates an environment without the specified cluster agent' do + expect { subject }.to change { Environment.count }.by(1) - expect(subject).to be_a(Environment) - expect(subject).to be_persisted - expect(subject.cluster_agent).to eq(agent) + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to be_nil + end end end -- GitLab From c4571e9247fc728663cef4afe77c63e96ffef9dc Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 5 Feb 2025 14:09:00 +0900 Subject: [PATCH 40/48] Use let_it_be instead of let --- ee/spec/models/ee/clusters/agent_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/models/ee/clusters/agent_spec.rb b/ee/spec/models/ee/clusters/agent_spec.rb index ea9053ff7a9a00..76f67f31ca5c34 100644 --- a/ee/spec/models/ee/clusters/agent_spec.rb +++ b/ee/spec/models/ee/clusters/agent_spec.rb @@ -63,7 +63,7 @@ end describe '#resource_management_enabled?' do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } subject { agent_1.resource_management_enabled? } -- GitLab From b53c9181e3b6e034766873a2dddfd9663d95edcf Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 5 Feb 2025 16:12:46 +0900 Subject: [PATCH 41/48] Add reload for ci_access_authorization --- .../environments/create_for_job_shared_examples.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb index 98bc1fdec4f889..2b741943b93e1a 100644 --- a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb +++ b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb @@ -328,6 +328,7 @@ end it 'creates an environment with the specified cluster agent' do + ci_access_authorization.reload expect { subject }.to change { Environment.count }.by(1) expect(subject).to be_a(Environment) @@ -346,6 +347,7 @@ end it 'creates an environment without the specified cluster agent' do + ci_access_authorization.reload expect { subject }.to change { Environment.count }.by(1) expect(subject).to be_a(Environment) @@ -385,6 +387,7 @@ end it 'creates an environment without the specified cluster agent' do + ci_access_authorization.reload expect { subject }.to change { Environment.count }.by(1) expect(subject).to be_a(Environment) -- GitLab From 8442c8892c25a57bfb3a6c6cbdcd6d73874e4359 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 6 Feb 2025 14:29:27 +0900 Subject: [PATCH 42/48] Move licensed feature tests under ee/spec --- .../create_for_job_service_spec.rb | 19 +++++++++++++++++++ .../create_for_job_shared_examples.rb | 18 ++++++++++++++---- 2 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 ee/spec/services/environments/create_for_job_service_spec.rb diff --git a/ee/spec/services/environments/create_for_job_service_spec.rb b/ee/spec/services/environments/create_for_job_service_spec.rb new file mode 100644 index 00000000000000..156da6ef7897f0 --- /dev/null +++ b/ee/spec/services/environments/create_for_job_service_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Environments::CreateForJobService, feature_category: :continuous_delivery do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + let(:service) { described_class.new } + + it_behaves_like 'create environment for job with cluster agent' do + let(:factory_type) { :ci_build } + end + + it_behaves_like 'create environment for job with cluster agent' do + let(:factory_type) { :ci_bridge } + end +end diff --git a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb index 2b741943b93e1a..110d0649ba8979 100644 --- a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb +++ b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb @@ -292,6 +292,20 @@ end end + def environment + project.environments.find_by_name('review/master') + end + end +end + +RSpec.shared_examples 'create environment for job with cluster agent' do + let!(:job) { build(factory_type, project: project, pipeline: pipeline, **attributes) } + let(:merge_request) {} # rubocop:disable Lint/EmptyBlock + let_it_be(:agent) { create(:cluster_agent, project: project) } + + describe '#execute' do + subject { service.execute(job) } + context 'when a pipeline contains matching ci_access authorization' do let(:environment_name) { 'production' } let(:user) { create(:user) } @@ -396,9 +410,5 @@ end end end - - def environment - project.environments.find_by_name('review/master') - end end end -- GitLab From bdc592ccbde63b0c5f5ae22bf9ee35abcf8d2b3a Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Thu, 6 Feb 2025 16:20:53 +0900 Subject: [PATCH 43/48] Remove unnecessary line from test --- .../environments/create_for_job_shared_examples.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb index 110d0649ba8979..c69923ad9b6406 100644 --- a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb +++ b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb @@ -300,7 +300,6 @@ def environment RSpec.shared_examples 'create environment for job with cluster agent' do let!(:job) { build(factory_type, project: project, pipeline: pipeline, **attributes) } - let(:merge_request) {} # rubocop:disable Lint/EmptyBlock let_it_be(:agent) { create(:cluster_agent, project: project) } describe '#execute' do -- GitLab From 29cd25b10eb0a596ffecee22f92aa72821a104f9 Mon Sep 17 00:00:00 2001 From: Tiger Date: Fri, 7 Feb 2025 15:06:51 +1300 Subject: [PATCH 44/48] Add extra test case for #resource_management_enabled? --- ee/spec/models/ee/clusters/agent_spec.rb | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/ee/spec/models/ee/clusters/agent_spec.rb b/ee/spec/models/ee/clusters/agent_spec.rb index 76f67f31ca5c34..d1c002c7bcac47 100644 --- a/ee/spec/models/ee/clusters/agent_spec.rb +++ b/ee/spec/models/ee/clusters/agent_spec.rb @@ -63,24 +63,16 @@ end describe '#resource_management_enabled?' do - let_it_be(:project) { create(:project) } - subject { agent_1.resource_management_enabled? } context 'when feature flag is disabled' do before do stub_feature_flags(gitlab_managed_cluster_resources: false) - allow(agent_1).to receive(:project).and_return(project) end - it { is_expected.to be_falsey } - end - - context 'when feature flag is enabled' do context 'when licensed feature is not available' do before do stub_licensed_features(agent_managed_resources: false) - allow(agent_1).to receive(:project).and_return(project) end it { is_expected.to be_falsey } @@ -89,11 +81,26 @@ context 'when licensed feature is available' do before do stub_licensed_features(agent_managed_resources: true) - allow(agent_1).to receive(:project).and_return(project) end - it { is_expected.to be_truthy } + it { is_expected.to be_falsey } + end + end + + context 'when licensed feature is not available' do + before do + stub_licensed_features(agent_managed_resources: false) + end + + it { is_expected.to be_falsey } + end + + context 'when licensed feature is available' do + before do + stub_licensed_features(agent_managed_resources: true) end + + it { is_expected.to be_truthy } end end end -- GitLab From a14c5d775e48e6ddcbe1c9d802b3534dcbf0e48b Mon Sep 17 00:00:00 2001 From: Tiger Date: Fri, 7 Feb 2025 16:10:47 +1300 Subject: [PATCH 45/48] Combine similar environment creation test contexts --- .../create_for_job_service_spec.rb | 19 -- .../ci_access/project_authorizations.rb | 2 + .../create_for_job_shared_examples.rb | 223 +++++++++--------- 3 files changed, 111 insertions(+), 133 deletions(-) delete mode 100644 ee/spec/services/environments/create_for_job_service_spec.rb diff --git a/ee/spec/services/environments/create_for_job_service_spec.rb b/ee/spec/services/environments/create_for_job_service_spec.rb deleted file mode 100644 index 156da6ef7897f0..00000000000000 --- a/ee/spec/services/environments/create_for_job_service_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Environments::CreateForJobService, feature_category: :continuous_delivery do - let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { create(:user) } - let_it_be(:pipeline) { create(:ci_pipeline, project: project) } - - let(:service) { described_class.new } - - it_behaves_like 'create environment for job with cluster agent' do - let(:factory_type) { :ci_build } - end - - it_behaves_like 'create environment for job with cluster agent' do - let(:factory_type) { :ci_bridge } - end -end diff --git a/spec/factories/clusters/agents/authorizations/ci_access/project_authorizations.rb b/spec/factories/clusters/agents/authorizations/ci_access/project_authorizations.rb index f23c7d8bf27927..6961881f9100b4 100644 --- a/spec/factories/clusters/agents/authorizations/ci_access/project_authorizations.rb +++ b/spec/factories/clusters/agents/authorizations/ci_access/project_authorizations.rb @@ -8,12 +8,14 @@ transient do environments { nil } protected_branches_only { false } + resource_management_enabled { false } end config do { default_namespace: 'production' }.tap do |c| c[:environments] = environments if environments c[:protected_branches_only] = protected_branches_only + c[:resource_management] = { enabled: true } if resource_management_enabled end end end diff --git a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb index c69923ad9b6406..aec4bde2673f9d 100644 --- a/spec/support/shared_examples/environments/create_for_job_shared_examples.rb +++ b/spec/support/shared_examples/environments/create_for_job_shared_examples.rb @@ -149,6 +149,115 @@ end end + context 'when job has a cluster agent attribute' do + let_it_be(:agent) { create(:cluster_agent, project: project) } + + let(:environment_name) { 'production' } + let(:agent_path) { "#{project.full_path}:#{agent.name}" } + let(:attributes) do + { + environment: environment_name, + options: { + environment: { + name: environment_name, + kubernetes: { + agent: agent_path + } + } + } + } + end + + let(:authorizations) { [ci_access_authorization] } + let(:ci_access_authorization) do + create(:agent_ci_access_project_authorization, + resource_management_enabled: true, + project: project, + agent: agent + ) + end + + before do + allow_next_instance_of(Clusters::Agents::Authorizations::CiAccess::Finder, project) do |finder| + allow(finder).to receive(:execute).and_return(authorizations) + end + end + + context 'when the agent has resource management enabled' do + before do + allow(agent).to receive(:resource_management_enabled?).and_return(true) + end + + it 'creates an environment with the specified cluster agent' do + expect { subject }.to change { Environment.count }.by(1) + + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to eq(agent) + end + + context 'when the gitlab_managed_cluster_resources feature flag is disabled' do + before do + stub_feature_flags(gitlab_managed_cluster_resources: false) + end + + it 'creates an environment without the specified cluster agent' do + expect(Clusters::Agents::Authorizations::CiAccess::Finder).not_to receive(:new) + + expect { subject }.to change { Environment.count }.by(1) + + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to be_nil + end + end + + context 'when the agent is not configured for resource_management' do + let(:ci_access_authorization) do + create(:agent_ci_access_project_authorization, + project: project, + agent: agent, + config: {} # resource_management is not enabled + ) + end + + it 'creates an environment without the specified cluster agent' do + expect { subject }.to change { Environment.count }.by(1) + + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to be_nil + end + end + + context 'when the agent is not authorized for this project' do + let(:authorizations) { [] } + + it 'creates an environment without the specified cluster agent' do + expect { subject }.to change { Environment.count }.by(1) + + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to be_nil + end + end + end + + context 'when the agent does not have resource management enabled' do + before do + allow(agent).to receive(:resource_management_enabled).and_return(false) + end + + it 'creates an environment without the specified cluster agent' do + expect { subject }.to change { Environment.count }.by(1) + + expect(subject).to be_a(Environment) + expect(subject).to be_persisted + expect(subject.cluster_agent).to be_nil + end + end + end + context 'when job starts a review app' do let(:environment_name) { 'review/$CI_COMMIT_REF_NAME' } let(:expected_environment_name) { "review/#{job.ref}" } @@ -297,117 +406,3 @@ def environment end end end - -RSpec.shared_examples 'create environment for job with cluster agent' do - let!(:job) { build(factory_type, project: project, pipeline: pipeline, **attributes) } - let_it_be(:agent) { create(:cluster_agent, project: project) } - - describe '#execute' do - subject { service.execute(job) } - - context 'when a pipeline contains matching ci_access authorization' do - let(:environment_name) { 'production' } - let(:user) { create(:user) } - let!(:job) { build(factory_type, project: project, pipeline: pipeline, user: user, **attributes) } - let_it_be(:agent) { create(:cluster_agent, project: project) } - let_it_be(:agent_path) { "#{project.full_path}:#{agent.name}" } - let(:attributes) do - { - environment: environment_name, - options: { - environment: { - name: environment_name, - kubernetes: { - agent: agent_path - } - } - } - } - end - - context 'when resource_management feature flag is enabled' do - context 'when license feature is enabled' do - before do - stub_licensed_features(agent_managed_resources: true) - end - - context 'when agent is authorized with resource_management enabled in config' do - let_it_be(:ci_access_authorization) do - create(:agent_ci_access_project_authorization, - project: project, - agent: agent, - config: { resource_management: { enabled: true } } - ) - end - - it 'creates an environment with the specified cluster agent' do - ci_access_authorization.reload - expect { subject }.to change { Environment.count }.by(1) - - expect(subject).to be_a(Environment) - expect(subject).to be_persisted - expect(subject.cluster_agent).to eq(agent) - end - end - - context 'when agent is authorized without resource_management enabled in config' do - let_it_be(:ci_access_authorization) do - create(:agent_ci_access_project_authorization, - project: project, - agent: agent, - config: {} # resource_management is not enabled - ) - end - - it 'creates an environment without the specified cluster agent' do - ci_access_authorization.reload - expect { subject }.to change { Environment.count }.by(1) - - expect(subject).to be_a(Environment) - expect(subject).to be_persisted - expect(subject.cluster_agent).to be_nil - end - end - end - - context 'when license feature is not enabled' do - before do - stub_licensed_features(agent_managed_resources: false) - end - - it 'creates an environment without the specified cluster agent' do - expect { subject }.to change { Environment.count }.by(1) - - expect(subject).to be_a(Environment) - expect(subject).to be_persisted - expect(subject.cluster_agent).to be_nil - end - end - end - - context 'when resource_management feature flag is disabled' do - let_it_be(:ci_access_authorization) do - create(:agent_ci_access_project_authorization, - project: project, - agent: agent, - config: { resource_management: { enabled: true } } - ) - end - - before do - stub_feature_flags(gitlab_managed_cluster_resources: false) - stub_licensed_features(agent_managed_resources: true) - end - - it 'creates an environment without the specified cluster agent' do - ci_access_authorization.reload - expect { subject }.to change { Environment.count }.by(1) - - expect(subject).to be_a(Environment) - expect(subject).to be_persisted - expect(subject.cluster_agent).to be_nil - end - end - end - end -end -- GitLab From 63a633255cafb185a550e36da9f86008cad29c06 Mon Sep 17 00:00:00 2001 From: Taka Nishida Date: Wed, 12 Feb 2025 21:48:05 +0900 Subject: [PATCH 46/48] Fix undefined error when accessing variables --- app/services/environments/create_for_job_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/environments/create_for_job_service.rb b/app/services/environments/create_for_job_service.rb index 63b0ff9c9804a7..74b3aedb2594d2 100644 --- a/app/services/environments/create_for_job_service.rb +++ b/app/services/environments/create_for_job_service.rb @@ -71,7 +71,7 @@ def ci_access_authorizations_for_project(job) def expanded_cluster_agent_path(job) return unless cluster_agent_path(job) - ExpandVariables.expand(cluster_agent_path(job), -> { variables.sort_and_expand_all }) + ExpandVariables.expand(cluster_agent_path(job), -> { job.simple_variables.sort_and_expand_all }) end def resource_management_feature_enabled?(job) -- GitLab From 403d0f9b0dbb5e79b7a8ea13ea7d8aba6c1d3d03 Mon Sep 17 00:00:00 2001 From: Avielle Wolfe Date: Wed, 12 Feb 2025 15:24:49 +0100 Subject: [PATCH 47/48] Add spec to fix FOSS undercoverage --- spec/models/clusters/agent_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb index 062d5062658a28..682769066fcb30 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -317,4 +317,8 @@ end end end + + describe '#resource_management_enabled?' do + it { is_expected.to be_falsey } + end end -- GitLab From 7f4e0b7efe3376ecd250f1fc915c3b39cb2d62b4 Mon Sep 17 00:00:00 2001 From: Avielle Wolfe Date: Wed, 12 Feb 2025 15:20:49 +0000 Subject: [PATCH 48/48] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Marius Bobin --- spec/models/clusters/agent_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb index 682769066fcb30..9f9d6b9544096b 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -319,6 +319,6 @@ end describe '#resource_management_enabled?' do - it { is_expected.to be_falsey } + it { expect(subject.resource_management_enabled?).to be_falsey } end end -- GitLab