From e2c73ba64e68f3c802a688e6496cb36dc6a73e2f Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Wed, 6 Dec 2023 17:04:49 +0530 Subject: [PATCH 1/5] Update previous versions of workspace resource generation - Update desired_config_generator_prev1 with contents of desired_config_generator - Update devfile_parser_prev1 with contents of devfile_parser - Update remote development shared contexts --- .../output/desired_config_generator_prev1.rb | 189 +++++-- .../reconcile/output/devfile_parser_prev1.rb | 106 ++-- .../workspaces_to_rails_infos_converter.rb | 1 + .../example.processed-devfile-prev1.yaml | 3 - .../desired_config_generator_prev1_spec.rb | 93 ++-- .../output/devfile_parser_prev1_spec.rb | 24 +- ...orkspaces_to_rails_infos_converter_spec.rb | 14 +- .../remote_development_shared_contexts.rb | 523 +++++------------- 8 files changed, 392 insertions(+), 561 deletions(-) diff --git a/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1.rb b/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1.rb index d1f239674f4335..6dcef24464b33b 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1.rb @@ -12,61 +12,125 @@ class DesiredConfigGeneratorPrev1 include States # @param [RemoteDevelopment::Workspaces::Workspace] workspace + # @param [Boolean] include_all_resources # @param [RemoteDevelopment::Logger] logger # @return [Array] - def self.generate_desired_config(workspace:, logger:) - name = workspace.name - namespace = workspace.namespace - agent = workspace.agent - desired_state = workspace.desired_state - user = workspace.user - - domain_template = "{{.port}}-#{name}.#{workspace.dns_zone}" - - workspace_inventory_config_map, owning_inventory = - create_workspace_inventory_config_map(name: name, namespace: namespace, agent_id: agent.id) - replicas = get_workspace_replicas(desired_state: desired_state) + def self.generate_desired_config(workspace:, include_all_resources:, logger:) + desired_config = [] + env_secret_name = "#{workspace.name}-env-var" + file_secret_name = "#{workspace.name}-file" + env_secret_names = [env_secret_name] + file_secret_names = [file_secret_name] + replicas = get_workspace_replicas(desired_state: workspace.desired_state) + domain_template = get_domain_template_annotation(name: workspace.name, dns_zone: workspace.dns_zone) + inventory_name = "#{workspace.name}-workspace-inventory" labels, annotations = get_labels_and_annotations( - agent_id: agent.id, - owning_inventory: owning_inventory, + agent_id: workspace.agent.id, domain_template: domain_template, + owning_inventory: inventory_name, workspace_id: workspace.id ) + k8s_inventory_for_workspace_core = get_inventory_config_map( + name: inventory_name, + namespace: workspace.namespace, + agent_id: workspace.agent.id + ) + # TODO: https://gitlab.com/groups/gitlab-org/-/epics/10461 - handle error - workspace_resources = DevfileParserPrev1.get_all( + k8s_resources_for_workspace_core = DevfileParserPrev1.get_all( processed_devfile: workspace.processed_devfile, - name: name, - namespace: namespace, + name: workspace.name, + namespace: workspace.namespace, replicas: replicas, domain_template: domain_template, labels: labels, annotations: annotations, - user: user, + env_secret_names: env_secret_names, + file_secret_names: file_secret_names, logger: logger ) # If we got no resources back from the devfile parser, this indicates some error was encountered in parsing # the processed_devfile. So we return an empty array which will result in no updates being applied by the # agent. We should not continue on and try to add anything else to the resources, as this would result # in an invalid configuration being applied to the cluster. - return [] if workspace_resources.empty? + return [] if k8s_resources_for_workspace_core.empty? - workspace_resources.insert(0, workspace_inventory_config_map) + desired_config.append(k8s_inventory_for_workspace_core, *k8s_resources_for_workspace_core) remote_development_agent_config = workspace.agent.remote_development_agent_config if remote_development_agent_config.network_policy_enabled gitlab_workspaces_proxy_namespace = remote_development_agent_config.gitlab_workspaces_proxy_namespace network_policy = get_network_policy( - name: name, - namespace: namespace, + name: workspace.name, + namespace: workspace.namespace, labels: labels, annotations: annotations, - gitlab_workspaces_proxy_namespace: gitlab_workspaces_proxy_namespace + gitlab_workspaces_proxy_namespace: gitlab_workspaces_proxy_namespace, + egress_ip_rules: remote_development_agent_config.network_policy_egress + ) + desired_config.append(network_policy) + end + + if include_all_resources + k8s_resources_for_secrets = get_k8s_resources_for_secrets( + workspace: workspace, + env_secret_name: env_secret_name, + file_secret_name: file_secret_name ) - workspace_resources.append(network_policy) + desired_config.append(*k8s_resources_for_secrets) + end + + desired_config + end + + # @param [RemoteDevelopment::Workspaces::Workspace] workspace + # @param [String] env_secret_name + # @param [String] file_secret_name + # @param [String] env_secret_name + # @param [String] file_secret_name + # @return [Array<(Hash)>] + def self.get_k8s_resources_for_secrets(workspace:, env_secret_name:, file_secret_name:) + inventory_name = "#{workspace.name}-secrets-inventory" + domain_template = get_domain_template_annotation(name: workspace.name, dns_zone: workspace.dns_zone) + labels, annotations = get_labels_and_annotations( + agent_id: workspace.agent.id, + domain_template: domain_template, + owning_inventory: inventory_name, + workspace_id: workspace.id + ) + + k8s_inventory = get_inventory_config_map( + name: inventory_name, + namespace: workspace.namespace, + agent_id: workspace.agent.id + ) + + data_for_env_var = workspace.workspace_variables.with_variable_type_env_var + data_for_env_var = data_for_env_var.each_with_object({}) do |workspace_variable, hash| + hash[workspace_variable.key] = workspace_variable.value + end + k8s_secret_for_env_var = get_secret( + name: env_secret_name, + namespace: workspace.namespace, + labels: labels, + annotations: annotations, + data: data_for_env_var + ) + + data_for_file = workspace.workspace_variables.with_variable_type_file + data_for_file = data_for_file.each_with_object({}) do |workspace_variable, hash| + hash[workspace_variable.key] = workspace_variable.value end + k8s_secret_for_file = get_secret( + name: file_secret_name, + namespace: workspace.namespace, + labels: labels, + annotations: annotations, + data: data_for_file + ) - workspace_resources + [k8s_inventory, k8s_secret_for_env_var, k8s_secret_for_file] end # @param [String] desired_state @@ -83,30 +147,34 @@ def self.get_workspace_replicas(desired_state:) # @param [String] name # @param [String] namespace # @param [Integer] agent_id - # @return [Array(Hash, String (frozen))] - def self.create_workspace_inventory_config_map(name:, namespace:, agent_id:) - owning_inventory = "#{name}-workspace-inventory" - workspace_inventory_config_map = { + # @return [Hash] + def self.get_inventory_config_map(name:, namespace:, agent_id:) + { kind: 'ConfigMap', apiVersion: 'v1', metadata: { - name: owning_inventory, + name: name, namespace: namespace, labels: { - 'cli-utils.sigs.k8s.io/inventory-id': owning_inventory, + 'cli-utils.sigs.k8s.io/inventory-id': name, 'agent.gitlab.com/id': agent_id.to_s } } }.deep_stringify_keys.to_h - [workspace_inventory_config_map, owning_inventory] end # @param [Integer] agent_id - # @param [String] owning_inventory # @param [String] domain_template + # @param [String] owning_inventory + # @param [String] object_type # @param [Integer] workspace_id - # @return [Array<(Hash, Hash)>] - def self.get_labels_and_annotations(agent_id:, owning_inventory:, domain_template:, workspace_id:) + # @return [Array] + def self.get_labels_and_annotations( + agent_id:, + domain_template:, + owning_inventory:, + workspace_id: + ) labels = { 'agent.gitlab.com/id' => agent_id.to_s } @@ -118,13 +186,48 @@ def self.get_labels_and_annotations(agent_id:, owning_inventory:, domain_templat [labels, annotations] end + # @param [String] name + # @param [String] namespace + # @param [Hash] labels + # @param [Hash] annotations + # @param [Hash] data + # @return [Hash] + def self.get_secret(name:, namespace:, labels:, annotations:, data:) + { + kind: 'Secret', + apiVersion: 'v1', + metadata: { + name: name, + namespace: namespace, + labels: labels, + annotations: annotations + }, + data: data.transform_values { |v| Base64.strict_encode64(v) } + }.deep_stringify_keys.to_h + end + + # @param [String] name + # @param [String] dns_zone + # @return [String] + def self.get_domain_template_annotation(name:, dns_zone:) + "{{.port}}-#{name}.#{dns_zone}" + end + # @param [String] name # @param [String] namespace # @param [Hash] labels # @param [Hash] annotations # @param [string] gitlab_workspaces_proxy_namespace + # @param [Array] egress_rules # @return [Hash] - def self.get_network_policy(name:, namespace:, labels:, annotations:, gitlab_workspaces_proxy_namespace:) + def self.get_network_policy( + name:, + namespace:, + labels:, + annotations:, + gitlab_workspaces_proxy_namespace:, + egress_ip_rules: + ) policy_types = [ - "Ingress", - "Egress" @@ -147,18 +250,18 @@ def self.get_network_policy(name:, namespace:, labels:, annotations:, gitlab_wor "kubernetes.io/metadata.name": "kube-system" } } - egress_except_cidr = [ - - "10.0.0.0/8", - - "172.16.0.0/12", - - "192.168.0.0/16" - ] egress = [ { ports: [{ port: 53, protocol: "TCP" }, { port: 53, protocol: "UDP" }], to: [{ namespaceSelector: kube_system_namespace_selector }] - }, - { to: [{ ipBlock: { cidr: "0.0.0.0/0", except: egress_except_cidr } }] } + } ] + egress_ip_rules.each do |egress_rule| + symbolized_egress_rule = egress_rule.deep_symbolize_keys + egress.append( + { to: [{ ipBlock: { cidr: symbolized_egress_rule[:allow], except: symbolized_egress_rule[:except] } }] } + ) + end { apiVersion: "networking.k8s.io/v1", diff --git a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1.rb b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1.rb index 07841e74aac6f6..e7a11b7f91a843 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1.rb @@ -16,7 +16,8 @@ class DevfileParserPrev1 # @param [String] domain_template # @param [Hash] labels # @param [Hash] annotations - # @param [User] user + # @param [Array] env_secret_names + # @param [Array] file_secret_names # @param [RemoteDevelopment::Logger] logger # @return [Array] def self.get_all( @@ -27,7 +28,8 @@ def self.get_all( domain_template:, labels:, annotations:, - user:, + env_secret_names:, + file_secret_names:, logger: ) begin @@ -54,13 +56,12 @@ def self.get_all( workspace_resources = YAML.load_stream(workspace_resources_yaml) workspace_resources = set_security_context(workspace_resources: workspace_resources) - workspace_resources = set_git_configuration(workspace_resources: workspace_resources, user: user) - set_workspace_environment_variables( + inject_secrets( workspace_resources: workspace_resources, - domain_template: domain_template + env_secret_names: env_secret_names, + file_secret_names: file_secret_names ) end - # rubocop:enable Metrics/ParameterLists # Devfile library allows specifying the security context of pods/containers as mentioned in @@ -78,84 +79,71 @@ def self.set_security_context(workspace_resources:) workspace_resources.each do |workspace_resource| next unless workspace_resource['kind'] == 'Deployment' - pod_spec = workspace_resource['spec']['template']['spec'] - # Explicitly set security context for the pod - pod_spec['securityContext'] = { + pod_security_context = { 'runAsNonRoot' => true, 'runAsUser' => RUN_AS_USER, 'fsGroup' => 0, 'fsGroupChangePolicy' => 'OnRootMismatch' } + container_security_context = { + 'allowPrivilegeEscalation' => false, + 'privileged' => false, + 'runAsNonRoot' => true, + 'runAsUser' => RUN_AS_USER + } + + pod_spec = workspace_resource['spec']['template']['spec'] + # Explicitly set security context for the pod + pod_spec['securityContext'] = pod_security_context # Explicitly set security context for all containers pod_spec['containers'].each do |container| - container['securityContext'] = { - 'allowPrivilegeEscalation' => false, - 'privileged' => false, - 'runAsNonRoot' => true, - 'runAsUser' => RUN_AS_USER - } + container['securityContext'] = container_security_context end # Explicitly set security context for all init containers pod_spec['initContainers'].each do |init_container| - init_container['securityContext'] = { - 'allowPrivilegeEscalation' => false, - 'privileged' => false, - 'runAsNonRoot' => true, - 'runAsUser' => RUN_AS_USER - } + init_container['securityContext'] = container_security_context end end workspace_resources end # @param [Array] workspace_resources - # @param [User] user + # @param [Array] env_secret_names + # @param [Array] file_secret_names # @return [Array] - def self.set_git_configuration(workspace_resources:, user:) + def self.inject_secrets(workspace_resources:, env_secret_names:, file_secret_names:) workspace_resources.each do |workspace_resource| next unless workspace_resource.fetch('kind') == 'Deployment' - # Set git configuration for the `gl-cloner-injector-*` - pod_spec = workspace_resource.fetch('spec').fetch('template').fetch('spec') - pod_spec.fetch('initContainers').each do |init_container| - next unless init_container.fetch('name').starts_with?('gl-cloner-injector-') - - init_container.fetch('env').concat([ - { - 'name' => 'GIT_AUTHOR_NAME', - 'value' => user.name - }, - { - 'name' => 'GIT_AUTHOR_EMAIL', - 'value' => user.email + volume_name = 'gl-workspace-variables' + volumes = [ + { + 'name' => volume_name, + 'projected' => { + 'defaultMode' => 0o774, + 'sources' => file_secret_names.map { |v| { 'secret' => { 'name' => v } } } } - ]) - end - end - workspace_resources - end - - # @param [Array] workspace_resources - # @param [String] domain_template - # @return [Array] - def self.set_workspace_environment_variables(workspace_resources:, domain_template:) - env_variables = [ - { - 'name' => 'GL_WORKSPACE_DOMAIN_TEMPLATE', - 'value' => domain_template.sub(/{{.port}}/, "${PORT}") - } - ] - workspace_resources.each do |workspace_resource| - next unless workspace_resource['kind'] == 'Deployment' + } + ] + volume_mounts = [ + { + 'name' => volume_name, + 'mountPath' => RemoteDevelopment::Workspaces::FileMounts::VARIABLES_FILE_DIR + } + ] + env_from = env_secret_names.map { |v| { 'secretRef' => { 'name' => v } } } - pod_spec = workspace_resource['spec']['template']['spec'] + pod_spec = workspace_resource.fetch('spec').fetch('template').fetch('spec') + pod_spec.fetch('volumes').concat(volumes) unless file_secret_names.empty? - pod_spec['initContainers'].each do |init_containers| - init_containers.fetch('env').concat(env_variables) + pod_spec.fetch('initContainers').each do |init_container| + init_container.fetch('volumeMounts').concat(volume_mounts) unless file_secret_names.empty? + init_container['envFrom'] = env_from unless env_secret_names.empty? end - pod_spec['containers'].each do |container| - container.fetch('env').concat(env_variables) + pod_spec.fetch('containers').each do |container| + container.fetch('volumeMounts').concat(volume_mounts) unless file_secret_names.empty? + container['envFrom'] = env_from unless env_secret_names.empty? end end workspace_resources diff --git a/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb b/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb index 363ee3992cbc7e..2bd0029c44f37f 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb @@ -56,6 +56,7 @@ def self.config_to_apply(workspace:, update_type:, logger:) else DesiredConfigGeneratorPrev1.generate_desired_config( workspace: workspace, + include_all_resources: include_all_resources, logger: logger ) end diff --git a/ee/spec/fixtures/remote_development/example.processed-devfile-prev1.yaml b/ee/spec/fixtures/remote_development/example.processed-devfile-prev1.yaml index 14dcd8241d3e72..38a466e3cc2542 100644 --- a/ee/spec/fixtures/remote_development/example.processed-devfile-prev1.yaml +++ b/ee/spec/fixtures/remote_development/example.processed-devfile-prev1.yaml @@ -62,9 +62,6 @@ components: if [ ! -d '/projects/test-project' ]; then git clone --branch master http://localhost/test-group/test-project.git /projects/test-project; - cd /projects/test-project; - git config user.name "${GIT_AUTHOR_NAME}"; - git config user.email "${GIT_AUTHOR_EMAIL}"; fi command: - "/bin/sh" diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1_spec.rb index aa084cf5d61cc5..dec2f6e62f233f 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1_spec.rb @@ -1,59 +1,42 @@ # frozen_string_literal: true -require_relative '../../../fast_spec_helper' +require 'spec_helper' RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Output::DesiredConfigGeneratorPrev1, :freeze_time, feature_category: :remote_development do include_context 'with remote development shared fixtures' describe '#generate_desired_config' do let(:logger) { instance_double(Logger) } - let(:user) { instance_double("User", name: "name", email: "name@example.com") } - let(:remote_development_agent_config) do - instance_double( - "RemoteDevelopment::RemoteDevelopmentAgentConfig", - network_policy_enabled: network_policy_enabled, - gitlab_workspaces_proxy_namespace: gitlab_workspaces_proxy_namespace - ) - end - - let(:agent) do - instance_double("Clusters::Agent", id: 1, remote_development_agent_config: remote_development_agent_config) - end - + let(:user) { create(:user) } + let(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } let(:actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } + let(:started) { true } + let(:include_all_resources) { false } let(:deployment_resource_version_from_agent) { workspace.deployment_resource_version } let(:network_policy_enabled) { true } let(:gitlab_workspaces_proxy_namespace) { 'gitlab-workspaces' } + let(:egress_ip_rules) { agent.remote_development_agent_config.network_policy_egress } let(:workspace) do - instance_double( - "RemoteDevelopment::Workspace", - id: 1, - name: "name", - namespace: "namespace", - deployment_resource_version: "1", + create( + :workspace, + agent: agent, + user: user, desired_state: desired_state, actual_state: actual_state, - dns_zone: "workspaces.localdev.me", - processed_devfile: read_devfile('example.processed-devfile-prev1.yaml'), - user: user, - agent: agent, - config_version: 1 + processed_devfile: read_devfile('example.processed-devfile-prev1.yaml') ) end let(:expected_config) do YAML.load_stream( create_config_to_apply_prev1( - workspace_id: workspace.id, - workspace_name: workspace.name, - workspace_namespace: workspace.namespace, - agent_id: workspace.agent.id, + workspace: workspace, started: started, - user_name: user.name, - user_email: user.email, - include_network_policy: network_policy_enabled + include_network_policy: network_policy_enabled, + include_all_resources: include_all_resources, + egress_ip_rules: egress_ip_rules ) ) end @@ -62,11 +45,18 @@ described_class end - context 'when desired_state results in started=true' do - let(:started) { true } + before do + allow(agent.remote_development_agent_config) + .to receive(:network_policy_enabled).and_return(network_policy_enabled) + end + context 'when desired_state results in started=true' do it 'returns expected config' do - workspace_resources = desired_config_generator.generate_desired_config(workspace: workspace, logger: logger) + workspace_resources = desired_config_generator.generate_desired_config( + workspace: workspace, + include_all_resources: include_all_resources, + logger: logger + ) expect(workspace_resources).to eq(expected_config) end @@ -77,18 +67,39 @@ let(:started) { false } it 'returns expected config' do - workspace_resources = desired_config_generator.generate_desired_config(workspace: workspace, logger: logger) + workspace_resources = desired_config_generator.generate_desired_config( + workspace: workspace, + include_all_resources: include_all_resources, + logger: logger + ) expect(workspace_resources).to eq(expected_config) end end context 'when network policy is disabled for agent' do - let(:started) { true } let(:network_policy_enabled) { false } it 'returns expected config without network policy' do - workspace_resources = desired_config_generator.generate_desired_config(workspace: workspace, logger: logger) + workspace_resources = desired_config_generator.generate_desired_config( + workspace: workspace, + include_all_resources: include_all_resources, + logger: logger + ) + + expect(workspace_resources).to eq(expected_config) + end + end + + context 'when include_all_resources is true' do + let(:include_all_resources) { true } + + it 'returns expected config' do + workspace_resources = desired_config_generator.generate_desired_config( + workspace: workspace, + include_all_resources: include_all_resources, + logger: logger + ) expect(workspace_resources).to eq(expected_config) end @@ -100,7 +111,11 @@ end it 'returns an empty array' do - workspace_resources = desired_config_generator.generate_desired_config(workspace: workspace, logger: logger) + workspace_resources = desired_config_generator.generate_desired_config( + workspace: workspace, + include_all_resources: include_all_resources, + logger: logger + ) expect(workspace_resources).to eq([]) end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1_spec.rb index 07352515b9b7eb..370c31f83e961a 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1_spec.rb @@ -26,23 +26,23 @@ ) end - let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } - let(:domain_template) { "{{.port}}-#{workspace.name}.#{workspace.dns_zone}" } + let(:env_var_secret_name) { "#{workspace.name}-env-var" } + let(:file_secret_name) { "#{workspace.name}-file" } + let(:egress_ip_rules) { RemoteDevelopment::AgentConfig::Updater::NETWORK_POLICY_EGRESS_DEFAULT } let(:expected_workspace_resources) do YAML.load_stream( create_config_to_apply_prev1( - workspace_id: workspace.id, - workspace_name: workspace.name, - workspace_namespace: workspace.namespace, - agent_id: workspace.agent.id, + workspace: workspace, + workspace_variables_env_var: {}, + workspace_variables_file: {}, started: true, include_inventory: false, include_network_policy: false, + include_all_resources: false, dns_zone: dns_zone, - user_name: user.name, - user_email: user.email + egress_ip_rules: egress_ip_rules ) ) end @@ -60,11 +60,12 @@ domain_template: domain_template, labels: { 'agent.gitlab.com/id' => workspace.agent.id }, annotations: { - 'config.k8s.io/owning-inventory' => owning_inventory, + 'config.k8s.io/owning-inventory' => "#{workspace.name}-workspace-inventory", 'workspaces.gitlab.com/host-template' => domain_template, 'workspaces.gitlab.com/id' => workspace.id }, - user: user, + env_secret_names: [env_var_secret_name], + file_secret_names: [file_secret_name], logger: logger ) @@ -93,7 +94,8 @@ domain_template: "", labels: {}, annotations: {}, - user: user, + env_secret_names: [env_var_secret_name], + file_secret_names: [file_secret_name], logger: logger ) diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb index 403ac3a075a203..067b49e688263b 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb @@ -146,20 +146,12 @@ let(:config_version) { previous_config_version } let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::FULL } let(:desired_state_updated_more_recently_than_last_response_to_agent) { false } - let(:generated_config_to_apply) do - [ - { - some_previous_version_key: 1 - } - ] - end + let(:expected_include_all_resources) { true } it "includes config_to_apply without all resources included" do allow(RemoteDevelopment::Workspaces::Reconcile::Output::DesiredConfigGeneratorPrev1) - .to(receive(:generate_desired_config)) do |**args| - expect(args).not_to have_key(:include_all_resources) - generated_config_to_apply - end + .to(receive(:generate_desired_config)) + .with(hash_including(include_all_resources: expected_include_all_resources)) { generated_config_to_apply } expect(returned_value).to eq(expected_returned_value) end diff --git a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb index 95ce81f0849b3f..1e4ac510779576 100644 --- a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb +++ b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb @@ -308,6 +308,19 @@ def create_config_to_apply( ) spec_replicas = started == true ? 1 : 0 host_template_annotation = get_workspace_host_template_annotation(workspace.name, dns_zone) + annotations = { + "config.k8s.io/owning-inventory": "#{workspace.name}-workspace-inventory", + "workspaces.gitlab.com/host-template": host_template_annotation.to_s, + "workspaces.gitlab.com/id": workspace.id.to_s + } + labels = { + "agent.gitlab.com/id": workspace.agent.id.to_s + } + secrets_annotations = { + "config.k8s.io/owning-inventory": "#{workspace.name}-secrets-inventory", + "workspaces.gitlab.com/host-template": host_template_annotation.to_s, + "workspaces.gitlab.com/id": workspace.id.to_s + } workspace_inventory = workspace_inventory( workspace_name: workspace.name, @@ -316,62 +329,56 @@ def create_config_to_apply( ) workspace_deployment = workspace_deployment( - workspace_id: workspace.id, workspace_name: workspace.name, workspace_namespace: workspace.namespace, - agent_id: workspace.agent.id, - spec_replicas: spec_replicas, - host_template_annotation: host_template_annotation + labels: labels, + annotations: annotations, + spec_replicas: spec_replicas ) workspace_service = workspace_service( - workspace_id: workspace.id, workspace_name: workspace.name, workspace_namespace: workspace.namespace, - agent_id: agent.id, - host_template_annotation: host_template_annotation + labels: labels, + annotations: annotations ) workspace_pvc = workspace_pvc( - workspace_id: workspace.id, workspace_name: workspace.name, workspace_namespace: workspace.namespace, - agent_id: workspace.agent.id, - host_template_annotation: host_template_annotation + labels: labels, + annotations: annotations ) workspace_network_policy = workspace_network_policy( - workspace_id: workspace.id, workspace_name: workspace.name, workspace_namespace: workspace.namespace, - agent_id: agent.id, - host_template_annotation: host_template_annotation, + labels: labels, + annotations: annotations, egress_ip_rules: egress_ip_rules ) workspace_secrets_inventory = workspace_secrets_inventory( workspace_name: workspace.name, workspace_namespace: workspace.namespace, - agent_id: agent.id + agent_id: workspace.agent.id ) workspace_secret_env_var = workspace_secret_env_var( - workspace_id: workspace.id, workspace_name: workspace.name, workspace_namespace: workspace.namespace, - agent_id: agent.id, - host_template_annotation: host_template_annotation, + labels: labels, + annotations: secrets_annotations, workspace_variables_env_var: workspace_variables_env_var || get_workspace_variables_env_var( workspace_variables: workspace.workspace_variables ) ) workspace_secret_file = workspace_secret_file( - workspace_id: workspace.id, workspace_name: workspace.name, workspace_namespace: workspace.namespace, - agent_id: agent.id, - host_template_annotation: host_template_annotation, + labels: labels, + annotations: secrets_annotations, workspace_variables_file: workspace_variables_file || get_workspace_variables_file( workspace_variables: workspace.workspace_variables ) @@ -391,65 +398,94 @@ def create_config_to_apply( YAML.dump(resource.deep_stringify_keys) end.join end - # rubocop:enable Metrics/AbcSize def create_config_to_apply_prev1( - workspace_id:, - workspace_name:, - workspace_namespace:, - agent_id:, + workspace:, started:, - user_name:, - user_email:, + workspace_variables_env_var: nil, + workspace_variables_file: nil, include_inventory: true, include_network_policy: true, - dns_zone: 'workspaces.localdev.me' + include_all_resources: false, + dns_zone: 'workspaces.localdev.me', + egress_ip_rules: RemoteDevelopment::AgentConfig::Updater::NETWORK_POLICY_EGRESS_DEFAULT ) spec_replicas = started == true ? 1 : 0 - host_template_annotation = get_workspace_host_template_annotation(workspace_name, dns_zone) - host_template_environment_variable = get_workspace_host_template_env_var(workspace_name, dns_zone) + host_template_annotation = get_workspace_host_template_annotation(workspace.name, dns_zone) + annotations = { + "config.k8s.io/owning-inventory": "#{workspace.name}-workspace-inventory", + "workspaces.gitlab.com/host-template": host_template_annotation.to_s, + "workspaces.gitlab.com/id": workspace.id.to_s + } + labels = { + "agent.gitlab.com/id": workspace.agent.id.to_s + } + secrets_annotations = { + "config.k8s.io/owning-inventory": "#{workspace.name}-secrets-inventory", + "workspaces.gitlab.com/host-template": host_template_annotation.to_s, + "workspaces.gitlab.com/id": workspace.id.to_s + } workspace_inventory = workspace_inventory( - workspace_name: workspace_name, - workspace_namespace: workspace_namespace, - agent_id: agent_id + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + agent_id: workspace.agent.id ) - workspace_deployment = workspace_deployment_prev1( - workspace_id: workspace_id, - workspace_name: workspace_name, - workspace_namespace: workspace_namespace, - agent_id: agent_id, - spec_replicas: spec_replicas, - host_template_annotation: host_template_annotation, - host_template_environment_variable: host_template_environment_variable, - user_name: user_name, - user_email: user_email + workspace_deployment = workspace_deployment( + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + labels: labels, + annotations: annotations, + spec_replicas: spec_replicas ) workspace_service = workspace_service( - workspace_id: workspace_id, - workspace_name: workspace_name, - workspace_namespace: workspace_namespace, - agent_id: agent_id, - host_template_annotation: host_template_annotation + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + labels: labels, + annotations: annotations ) workspace_pvc = workspace_pvc( - workspace_id: workspace_id, - workspace_name: workspace_name, - workspace_namespace: workspace_namespace, - agent_id: agent_id, - host_template_annotation: host_template_annotation + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + labels: labels, + annotations: annotations ) workspace_network_policy = workspace_network_policy( - workspace_id: workspace_id, - workspace_name: workspace_name, - workspace_namespace: workspace_namespace, - agent_id: agent_id, - host_template_annotation: host_template_annotation, - egress_ip_rules: RemoteDevelopment::AgentConfig::Updater::NETWORK_POLICY_EGRESS_DEFAULT + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + labels: labels, + annotations: annotations, + egress_ip_rules: egress_ip_rules + ) + + workspace_secrets_inventory = workspace_secrets_inventory( + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + agent_id: workspace.agent.id + ) + + workspace_secret_env_var = workspace_secret_env_var( + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + labels: labels, + annotations: secrets_annotations, + workspace_variables_env_var: workspace_variables_env_var || get_workspace_variables_env_var( + workspace_variables: workspace.workspace_variables + ) + ) + + workspace_secret_file = workspace_secret_file( + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + labels: labels, + annotations: secrets_annotations, + workspace_variables_file: workspace_variables_file || get_workspace_variables_file( + workspace_variables: workspace.workspace_variables + ) ) resources = [] @@ -458,13 +494,15 @@ def create_config_to_apply_prev1( resources << workspace_service resources << workspace_pvc resources << workspace_network_policy if include_network_policy + resources << workspace_secrets_inventory if include_all_resources && include_inventory + resources << workspace_secret_env_var if include_all_resources + resources << workspace_secret_file if include_all_resources resources.map do |resource| YAML.dump(resource.deep_stringify_keys) end.join end - - # rubocop:enable Metrics/ParameterLists + # rubocop:enable Metrics/ParameterLists, Metrics/AbcSize def workspace_inventory( workspace_name:, @@ -486,51 +524,36 @@ def workspace_inventory( end def workspace_deployment( - workspace_id:, workspace_name:, workspace_namespace:, - agent_id:, - spec_replicas:, - host_template_annotation: + labels:, + annotations:, + spec_replicas: ) variables_file_mount_path = RemoteDevelopment::Workspaces::FileMounts::VARIABLES_FILE_DIR { apiVersion: "apps/v1", kind: "Deployment", metadata: { - annotations: { - "config.k8s.io/owning-inventory": "#{workspace_name}-workspace-inventory", - "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace_id.to_s - }, + annotations: annotations, creationTimestamp: nil, - labels: { - "agent.gitlab.com/id": agent_id.to_s - }, + labels: labels, name: workspace_name.to_s, namespace: workspace_namespace.to_s }, spec: { replicas: spec_replicas, selector: { - matchLabels: { - "agent.gitlab.com/id": agent_id.to_s - } + matchLabels: labels }, strategy: { type: "Recreate" }, template: { metadata: { - annotations: { - "config.k8s.io/owning-inventory": "#{workspace_name}-workspace-inventory", - "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace_id.to_s - }, + annotations: annotations, creationTimestamp: nil, - labels: { - "agent.gitlab.com/id": agent_id.to_s - }, + labels: labels, name: workspace_name.to_s, namespace: workspace_namespace.to_s }, @@ -756,279 +779,19 @@ def workspace_deployment( } end - # rubocop:todo Metrics/ParameterLists -- refactor this to have fewer parameters - perhaps introduce a parameter object: https://refactoring.com/catalog/introduceParameterObject.html - def workspace_deployment_prev1( - workspace_id:, - workspace_name:, - workspace_namespace:, - agent_id:, - spec_replicas:, - host_template_annotation:, - host_template_environment_variable:, - user_name:, - user_email: - ) - { - apiVersion: "apps/v1", - kind: "Deployment", - metadata: { - annotations: { - "config.k8s.io/owning-inventory": "#{workspace_name}-workspace-inventory", - "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace_id.to_s - }, - creationTimestamp: nil, - labels: { - "agent.gitlab.com/id": agent_id.to_s - }, - name: workspace_name.to_s, - namespace: workspace_namespace.to_s - }, - spec: { - replicas: spec_replicas, - selector: { - matchLabels: { - "agent.gitlab.com/id": agent_id.to_s - } - }, - strategy: { - type: "Recreate" - }, - template: { - metadata: { - annotations: { - "config.k8s.io/owning-inventory": "#{workspace_name}-workspace-inventory", - "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace_id.to_s - }, - creationTimestamp: nil, - labels: { - "agent.gitlab.com/id": agent_id.to_s - }, - name: workspace_name.to_s, - namespace: workspace_namespace.to_s - }, - spec: { - containers: [ - { - command: [ - "/projects/.gl-editor/start_server.sh" - ], - env: [ - { - name: "EDITOR_VOLUME_DIR", - value: "/projects/.gl-editor" - }, - { - name: "EDITOR_PORT", - value: "60001" - }, - { - name: "SSH_PORT", - value: "60022" - }, - { - name: "PROJECTS_ROOT", - value: "/projects" - }, - { - name: "PROJECT_SOURCE", - value: "/projects" - }, - { - name: "GL_WORKSPACE_DOMAIN_TEMPLATE", - value: host_template_environment_variable.to_s - } - ], - image: "quay.io/mloriedo/universal-developer-image:ubi8-dw-demo", - imagePullPolicy: "Always", - name: "tooling-container", - ports: [ - { - containerPort: 60001, - name: "editor-server", - protocol: "TCP" - }, - { - containerPort: 60022, - name: "ssh-server", - protocol: "TCP" - } - ], - resources: {}, - volumeMounts: [ - { - mountPath: "/projects", - name: "gl-workspace-data" - } - ], - securityContext: { - allowPrivilegeEscalation: false, - privileged: false, - runAsNonRoot: true, - runAsUser: 5001 - } - } - ], - initContainers: [ - { - args: [ - <<~ARGS.chomp - if [ ! -d '/projects/test-project' ]; - then - git clone --branch master #{root_url}test-group/test-project.git /projects/test-project; - cd /projects/test-project; - git config user.name "${GIT_AUTHOR_NAME}"; - git config user.email "${GIT_AUTHOR_EMAIL}"; - fi - ARGS - ], - command: %w[/bin/sh -c], - env: [ - { - name: "PROJECTS_ROOT", - value: "/projects" - }, - { - name: "PROJECT_SOURCE", - value: "/projects" - }, - { - name: "GIT_AUTHOR_NAME", - value: user_name.to_s - }, - { - name: "GIT_AUTHOR_EMAIL", - value: user_email.to_s - }, - { - name: "GL_WORKSPACE_DOMAIN_TEMPLATE", - value: host_template_environment_variable.to_s - } - ], - image: "alpine/git:2.36.3", - imagePullPolicy: "Always", - name: "gl-cloner-injector-gl-cloner-injector-command-1", - resources: { - limits: { - cpu: "500m", - memory: "256Mi" - }, - requests: { - cpu: "100m", - memory: "128Mi" - } - }, - volumeMounts: [ - { - mountPath: "/projects", - name: "gl-workspace-data" - } - ], - securityContext: { - allowPrivilegeEscalation: false, - privileged: false, - runAsNonRoot: true, - runAsUser: 5001 - } - }, - { - env: [ - { - name: "EDITOR_VOLUME_DIR", - value: "/projects/.gl-editor" - }, - { - name: "EDITOR_PORT", - value: "60001" - }, - { - name: "SSH_PORT", - value: "60022" - }, - { - name: "PROJECTS_ROOT", - value: "/projects" - }, - { - name: "PROJECT_SOURCE", - value: "/projects" - }, - { - name: "GL_WORKSPACE_DOMAIN_TEMPLATE", - value: host_template_environment_variable.to_s - } - ], - image: "registry.gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork/web-ide-injector:2", - imagePullPolicy: "Always", - name: "gl-editor-injector-gl-editor-injector-command-2", - resources: { - limits: { - cpu: "500m", - memory: "256Mi" - }, - requests: { - cpu: "100m", - memory: "128Mi" - } - }, - volumeMounts: [ - { - mountPath: "/projects", - name: "gl-workspace-data" - } - ], - securityContext: { - allowPrivilegeEscalation: false, - privileged: false, - runAsNonRoot: true, - runAsUser: 5001 - } - } - ], - volumes: [ - { - name: "gl-workspace-data", - persistentVolumeClaim: { - claimName: "#{workspace_name}-gl-workspace-data" - } - } - ], - securityContext: { - runAsNonRoot: true, - runAsUser: 5001, - fsGroup: 0, - fsGroupChangePolicy: "OnRootMismatch" - } - } - } - }, - status: {} - } - end - - # rubocop:enable Metrics/ParameterLists - def workspace_service( - workspace_id:, workspace_name:, workspace_namespace:, - agent_id:, - host_template_annotation: + labels:, + annotations: ) { apiVersion: "v1", kind: "Service", metadata: { - annotations: { - "config.k8s.io/owning-inventory": "#{workspace_name}-workspace-inventory", - "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace_id.to_s - }, + annotations: annotations, creationTimestamp: nil, - labels: { - "agent.gitlab.com/id": agent_id.to_s - }, + labels: labels, name: workspace_name.to_s, namespace: workspace_namespace.to_s }, @@ -1045,9 +808,7 @@ def workspace_service( targetPort: 60022 } ], - selector: { - "agent.gitlab.com/id": agent_id.to_s - } + selector: labels }, status: { loadBalancer: {} @@ -1056,25 +817,18 @@ def workspace_service( end def workspace_pvc( - workspace_id:, workspace_name:, workspace_namespace:, - agent_id:, - host_template_annotation: + labels:, + annotations: ) { apiVersion: "v1", kind: "PersistentVolumeClaim", metadata: { - annotations: { - "config.k8s.io/owning-inventory": "#{workspace_name}-workspace-inventory", - "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace_id.to_s - }, + annotations: annotations, creationTimestamp: nil, - labels: { - "agent.gitlab.com/id": agent_id.to_s - }, + labels: labels, name: "#{workspace_name}-gl-workspace-data", namespace: workspace_namespace.to_s }, @@ -1093,11 +847,10 @@ def workspace_pvc( end def workspace_network_policy( - workspace_id:, workspace_name:, workspace_namespace:, - agent_id:, - host_template_annotation:, + labels:, + annotations:, egress_ip_rules: ) egress = [ @@ -1124,14 +877,8 @@ def workspace_network_policy( apiVersion: "networking.k8s.io/v1", kind: "NetworkPolicy", metadata: { - annotations: { - "config.k8s.io/owning-inventory": "#{workspace_name}-workspace-inventory", - "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace_id.to_s - }, - labels: { - "agent.gitlab.com/id": agent_id.to_s - }, + annotations: annotations, + labels: labels, name: workspace_name.to_s, namespace: workspace_namespace.to_s }, @@ -1181,11 +928,10 @@ def workspace_secrets_inventory( end def workspace_secret_env_var( - workspace_id:, workspace_name:, workspace_namespace:, - agent_id:, - host_template_annotation:, + labels:, + annotations:, workspace_variables_env_var: ) git_config_count = workspace_variables_env_var.fetch('GIT_CONFIG_COUNT', '') @@ -1205,14 +951,8 @@ def workspace_secret_env_var( metadata: { name: "#{workspace_name}-env-var", namespace: workspace_namespace.to_s, - labels: { - "agent.gitlab.com/id": agent_id.to_s - }, - annotations: { - "config.k8s.io/owning-inventory": "#{workspace_name}-secrets-inventory", - "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace_id.to_s - } + labels: labels, + annotations: annotations }, data: { GIT_CONFIG_COUNT: Base64.strict_encode64(git_config_count).to_s, @@ -1230,11 +970,10 @@ def workspace_secret_env_var( end def workspace_secret_file( - workspace_id:, workspace_name:, workspace_namespace:, - agent_id:, - host_template_annotation:, + labels:, + annotations:, workspace_variables_file: ) gl_git_credential_store = workspace_variables_file.fetch('gl_git_credential_store.sh', '') @@ -1245,14 +984,8 @@ def workspace_secret_file( metadata: { name: "#{workspace_name}-file", namespace: workspace_namespace.to_s, - labels: { - "agent.gitlab.com/id": agent_id.to_s - }, - annotations: { - "config.k8s.io/owning-inventory": "#{workspace_name}-secrets-inventory", - "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace_id.to_s - } + labels: labels, + annotations: annotations }, data: { gl_token: Base64.strict_encode64(gl_token).to_s, -- GitLab From cedfb99c536c1ebf3e96558a596973ba6c59ccba Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Thu, 7 Dec 2023 17:53:56 +0530 Subject: [PATCH 2/5] Apply container resource default and create resource quota - Generate resource quota using the agent's max_resources_per_workspace. - Add annotation for SHA256 of max_resources_per_workspace to force workspace restart when value changes. - Deep merge the default_resources_per_workspace_container into the containers and init containers of the workspace to apply the defaults. --- .../output/desired_config_generator.rb | 78 +++++++++++++++++-- .../reconcile/output/devfile_parser.rb | 30 +++++++ .../output/desired_config_generator_spec.rb | 52 ++++++++++++- .../reconcile/output/devfile_parser_spec.rb | 13 +++- .../remote_development_shared_contexts.rb | 62 +++++++++++++-- 5 files changed, 218 insertions(+), 17 deletions(-) diff --git a/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator.rb b/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator.rb index 565ea0dfa9731c..14e88730b759d8 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator.rb @@ -24,11 +24,19 @@ def self.generate_desired_config(workspace:, include_all_resources:, logger:) replicas = get_workspace_replicas(desired_state: workspace.desired_state) domain_template = get_domain_template_annotation(name: workspace.name, dns_zone: workspace.dns_zone) inventory_name = "#{workspace.name}-workspace-inventory" + + remote_development_agent_config = workspace.agent.remote_development_agent_config + max_resources_per_workspace = + remote_development_agent_config.max_resources_per_workspace.deep_symbolize_keys + default_resources_per_workspace_container = + remote_development_agent_config.default_resources_per_workspace_container.deep_symbolize_keys + labels, annotations = get_labels_and_annotations( agent_id: workspace.agent.id, domain_template: domain_template, owning_inventory: inventory_name, - workspace_id: workspace.id + workspace_id: workspace.id, + max_resources_per_workspace: max_resources_per_workspace ) k8s_inventory_for_workspace_core = get_inventory_config_map( @@ -48,6 +56,7 @@ def self.generate_desired_config(workspace:, include_all_resources:, logger:) annotations: annotations, env_secret_names: env_secret_names, file_secret_names: file_secret_names, + default_resources_per_workspace_container: default_resources_per_workspace_container, logger: logger ) # If we got no resources back from the devfile parser, this indicates some error was encountered in parsing @@ -58,7 +67,6 @@ def self.generate_desired_config(workspace:, include_all_resources:, logger:) desired_config.append(k8s_inventory_for_workspace_core, *k8s_resources_for_workspace_core) - remote_development_agent_config = workspace.agent.remote_development_agent_config if remote_development_agent_config.network_policy_enabled gitlab_workspaces_proxy_namespace = remote_development_agent_config.gitlab_workspaces_proxy_namespace network_policy = get_network_policy( @@ -73,10 +81,22 @@ def self.generate_desired_config(workspace:, include_all_resources:, logger:) end if include_all_resources + unless max_resources_per_workspace.blank? + k8s_resource_quota = get_resource_quota( + name: workspace.name, + namespace: workspace.namespace, + labels: labels, + annotations: annotations, + max_resources_per_workspace: max_resources_per_workspace + ) + desired_config.append(k8s_resource_quota) + end + k8s_resources_for_secrets = get_k8s_resources_for_secrets( workspace: workspace, env_secret_name: env_secret_name, - file_secret_name: file_secret_name + file_secret_name: file_secret_name, + max_resources_per_workspace: max_resources_per_workspace ) desired_config.append(*k8s_resources_for_secrets) end @@ -89,15 +109,22 @@ def self.generate_desired_config(workspace:, include_all_resources:, logger:) # @param [String] file_secret_name # @param [String] env_secret_name # @param [String] file_secret_name + # @param [Hash] max_resources_per_workspace # @return [Array<(Hash)>] - def self.get_k8s_resources_for_secrets(workspace:, env_secret_name:, file_secret_name:) + def self.get_k8s_resources_for_secrets( + workspace:, + env_secret_name:, + file_secret_name:, + max_resources_per_workspace: + ) inventory_name = "#{workspace.name}-secrets-inventory" domain_template = get_domain_template_annotation(name: workspace.name, dns_zone: workspace.dns_zone) labels, annotations = get_labels_and_annotations( agent_id: workspace.agent.id, domain_template: domain_template, owning_inventory: inventory_name, - workspace_id: workspace.id + workspace_id: workspace.id, + max_resources_per_workspace: max_resources_per_workspace ) k8s_inventory = get_inventory_config_map( @@ -168,12 +195,14 @@ def self.get_inventory_config_map(name:, namespace:, agent_id:) # @param [String] owning_inventory # @param [String] object_type # @param [Integer] workspace_id + # @param [Hash] max_resources_per_workspace # @return [Array] def self.get_labels_and_annotations( agent_id:, domain_template:, owning_inventory:, - workspace_id: + workspace_id:, + max_resources_per_workspace: ) labels = { 'agent.gitlab.com/id' => agent_id.to_s @@ -181,7 +210,9 @@ def self.get_labels_and_annotations( annotations = { 'config.k8s.io/owning-inventory' => owning_inventory.to_s, 'workspaces.gitlab.com/host-template' => domain_template.to_s, - 'workspaces.gitlab.com/id' => workspace_id.to_s + 'workspaces.gitlab.com/id' => workspace_id.to_s, + 'workspaces.gitlab.com/max-resources-per-workspace-sha256' => + Digest::SHA256.hexdigest(max_resources_per_workspace.sort.to_h.to_s) } [labels, annotations] end @@ -280,6 +311,39 @@ def self.get_network_policy( } }.deep_stringify_keys.to_h end + + # @param [String] name + # @param [String] namespace + # @param [Hash] labels + # @param [Hash] annotations + # @param [Hash] max_resources_per_workspace + # @return [Hash] + def self.get_resource_quota( + name:, + namespace:, + labels:, + annotations:, + max_resources_per_workspace: + ) + { + apiVersion: "v1", + kind: "ResourceQuota", + metadata: { + annotations: annotations, + labels: labels, + name: name, + namespace: namespace + }, + spec: { + hard: { + "limits.cpu": max_resources_per_workspace.dig(:limits, :cpu), + "limits.memory": max_resources_per_workspace.dig(:limits, :memory), + "requests.cpu": max_resources_per_workspace.dig(:requests, :cpu), + "requests.memory": max_resources_per_workspace.dig(:requests, :memory) + } + } + }.deep_stringify_keys.to_h + end end end end diff --git a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb index 21179d69d6cb23..661b9341870cc1 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb @@ -18,6 +18,7 @@ class DevfileParser # @param [Hash] annotations # @param [Array] env_secret_names # @param [Array] file_secret_names + # @param [Hash] default_resources_per_workspace_container # @param [RemoteDevelopment::Logger] logger # @return [Array] def self.get_all( @@ -30,6 +31,7 @@ def self.get_all( annotations:, env_secret_names:, file_secret_names:, + default_resources_per_workspace_container:, logger: ) begin @@ -56,6 +58,10 @@ def self.get_all( workspace_resources = YAML.load_stream(workspace_resources_yaml) workspace_resources = set_security_context(workspace_resources: workspace_resources) + workspace_resources = patch_default_resources( + workspace_resources: workspace_resources, + default_resources_per_workspace_container: default_resources_per_workspace_container.deep_stringify_keys + ) inject_secrets( workspace_resources: workspace_resources, env_secret_names: env_secret_names, @@ -107,6 +113,30 @@ def self.set_security_context(workspace_resources:) workspace_resources end + # @param [Array] workspace_resources + # @param [Hash] default_resources_per_workspace_container + # @return [Array] + def self.patch_default_resources(workspace_resources:, default_resources_per_workspace_container:) + workspace_resources.each do |workspace_resource| + next unless workspace_resource.fetch('kind') == 'Deployment' + + pod_spec = workspace_resource.fetch('spec').fetch('template').fetch('spec') + + pod_spec.fetch('initContainers').each do |init_container| + init_container + .fetch('resources', {}) + .deep_merge!(default_resources_per_workspace_container) { |_, val, _| val } + end + + pod_spec.fetch('containers').each do |container| + container + .fetch('resources', {}) + .deep_merge!(default_resources_per_workspace_container) { |_, val, _| val } + end + end + workspace_resources + end + # @param [Array] workspace_resources # @param [Array] env_secret_names # @param [Array] file_secret_names diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_spec.rb index bd3c14fa696bb6..fc14910a44dcdc 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_spec.rb @@ -17,6 +17,10 @@ let(:network_policy_enabled) { true } let(:gitlab_workspaces_proxy_namespace) { 'gitlab-workspaces' } let(:egress_ip_rules) { agent.remote_development_agent_config.network_policy_egress } + let(:max_resources_per_workspace) { agent.remote_development_agent_config.max_resources_per_workspace } + let(:default_resources_per_workspace_container) do + agent.remote_development_agent_config.default_resources_per_workspace_container + end let(:workspace) do create( @@ -35,7 +39,9 @@ started: started, include_network_policy: network_policy_enabled, include_all_resources: include_all_resources, - egress_ip_rules: egress_ip_rules + egress_ip_rules: egress_ip_rules, + max_resources_per_workspace: max_resources_per_workspace, + default_resources_per_workspace_container: default_resources_per_workspace_container ) ) end @@ -90,6 +96,28 @@ end end + context 'when default_resources_per_workspace_container is not empty' do + let(:default_resources_per_workspace_container) do + { limits: { cpu: "1.5", memory: "786Mi" }, requests: { cpu: "0.6", memory: "512Mi" } } + end + + before do + allow(agent.remote_development_agent_config).to receive(:default_resources_per_workspace_container) { + default_resources_per_workspace_container + } + end + + it 'returns expected config with defaults for the container resources set' do + workspace_resources = desired_config_generator.generate_desired_config( + workspace: workspace, + include_all_resources: include_all_resources, + logger: logger + ) + + expect(workspace_resources).to eq(expected_config) + end + end + context 'when include_all_resources is true' do let(:include_all_resources) { true } @@ -102,6 +130,28 @@ expect(workspace_resources).to eq(expected_config) end + + context 'when max_resources_per_workspace is not empty' do + let(:max_resources_per_workspace) do + { limits: { cpu: "1.5", memory: "786Mi" }, requests: { cpu: "0.6", memory: "512Mi" } } + end + + before do + allow(agent.remote_development_agent_config).to receive(:max_resources_per_workspace) { + max_resources_per_workspace + } + end + + it 'returns expected config with resource quota' do + workspace_resources = desired_config_generator.generate_desired_config( + workspace: workspace, + include_all_resources: include_all_resources, + logger: logger + ) + + expect(workspace_resources).to eq(expected_config) + end + end end context 'when DevfileParser returns empty array' do diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_spec.rb index b92e908625777b..d44b28ae944709 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_spec.rb @@ -29,6 +29,10 @@ let(:env_var_secret_name) { "#{workspace.name}-env-var" } let(:file_secret_name) { "#{workspace.name}-file" } let(:egress_ip_rules) { RemoteDevelopment::AgentConfig::Updater::NETWORK_POLICY_EGRESS_DEFAULT } + let(:max_resources_per_workspace) { {} } + let(:default_resources_per_workspace_container) do + { limits: { cpu: "1.5", memory: "786Mi" }, requests: { cpu: "0.6", memory: "512Mi" } } + end let(:expected_workspace_resources) do YAML.load_stream( @@ -41,7 +45,8 @@ include_network_policy: false, include_all_resources: false, dns_zone: dns_zone, - egress_ip_rules: egress_ip_rules + egress_ip_rules: egress_ip_rules, + default_resources_per_workspace_container: default_resources_per_workspace_container ) ) end @@ -61,10 +66,13 @@ annotations: { 'config.k8s.io/owning-inventory' => "#{workspace.name}-workspace-inventory", 'workspaces.gitlab.com/host-template' => domain_template, - 'workspaces.gitlab.com/id' => workspace.id + 'workspaces.gitlab.com/id' => workspace.id, + 'workspaces.gitlab.com/max-resources-per-workspace-sha256' => + Digest::SHA256.hexdigest(max_resources_per_workspace.sort.to_h.to_s) }, env_secret_names: [env_var_secret_name], file_secret_names: [file_secret_name], + default_resources_per_workspace_container: default_resources_per_workspace_container, logger: logger ) @@ -95,6 +103,7 @@ annotations: {}, env_secret_names: [env_var_secret_name], file_secret_names: [file_secret_name], + default_resources_per_workspace_container: default_resources_per_workspace_container, logger: logger ) diff --git a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb index 1e4ac510779576..70ed030910df7e 100644 --- a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb +++ b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb @@ -304,14 +304,21 @@ def create_config_to_apply( include_network_policy: true, include_all_resources: false, dns_zone: 'workspaces.localdev.me', - egress_ip_rules: RemoteDevelopment::AgentConfig::Updater::NETWORK_POLICY_EGRESS_DEFAULT + egress_ip_rules: RemoteDevelopment::AgentConfig::Updater::NETWORK_POLICY_EGRESS_DEFAULT, + max_resources_per_workspace: {}, + default_resources_per_workspace_container: {} ) spec_replicas = started == true ? 1 : 0 host_template_annotation = get_workspace_host_template_annotation(workspace.name, dns_zone) + max_resources_per_workspace_sha256 = Digest::SHA256.hexdigest( + max_resources_per_workspace.sort.to_h.to_s + ) annotations = { "config.k8s.io/owning-inventory": "#{workspace.name}-workspace-inventory", "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace.id.to_s + "workspaces.gitlab.com/id": workspace.id.to_s, + "workspaces.gitlab.com/max-resources-per-workspace-sha256": + max_resources_per_workspace_sha256 } labels = { "agent.gitlab.com/id": workspace.agent.id.to_s @@ -319,7 +326,9 @@ def create_config_to_apply( secrets_annotations = { "config.k8s.io/owning-inventory": "#{workspace.name}-secrets-inventory", "workspaces.gitlab.com/host-template": host_template_annotation.to_s, - "workspaces.gitlab.com/id": workspace.id.to_s + "workspaces.gitlab.com/id": workspace.id.to_s, + "workspaces.gitlab.com/max-resources-per-workspace-sha256": + max_resources_per_workspace_sha256 } workspace_inventory = workspace_inventory( @@ -333,7 +342,8 @@ def create_config_to_apply( workspace_namespace: workspace.namespace, labels: labels, annotations: annotations, - spec_replicas: spec_replicas + spec_replicas: spec_replicas, + default_resources_per_workspace_container: default_resources_per_workspace_container ) workspace_service = workspace_service( @@ -384,12 +394,21 @@ def create_config_to_apply( ) ) + workspace_resource_quota = workspace_resource_quota( + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + labels: labels, + annotations: annotations, + max_resources_per_workspace: max_resources_per_workspace + ) + resources = [] resources << workspace_inventory if include_inventory resources << workspace_deployment resources << workspace_service resources << workspace_pvc resources << workspace_network_policy if include_network_policy + resources << workspace_resource_quota if include_all_resources && !max_resources_per_workspace.blank? resources << workspace_secrets_inventory if include_all_resources && include_inventory resources << workspace_secret_env_var if include_all_resources resources << workspace_secret_file if include_all_resources @@ -437,7 +456,8 @@ def create_config_to_apply_prev1( workspace_namespace: workspace.namespace, labels: labels, annotations: annotations, - spec_replicas: spec_replicas + spec_replicas: spec_replicas, + default_resources_per_workspace_container: {} ) workspace_service = workspace_service( @@ -528,7 +548,8 @@ def workspace_deployment( workspace_namespace:, labels:, annotations:, - spec_replicas: + spec_replicas:, + default_resources_per_workspace_container: ) variables_file_mount_path = RemoteDevelopment::Workspaces::FileMounts::VARIABLES_FILE_DIR { @@ -600,7 +621,7 @@ def workspace_deployment( protocol: "TCP" } ], - resources: {}, + resources: default_resources_per_workspace_container, volumeMounts: [ { mountPath: "/projects", @@ -908,6 +929,33 @@ def workspace_network_policy( } end + def workspace_resource_quota( + workspace_name:, + workspace_namespace:, + labels:, + annotations:, + max_resources_per_workspace: + ) + { + apiVersion: "v1", + kind: "ResourceQuota", + metadata: { + annotations: annotations, + labels: labels, + name: workspace_name.to_s, + namespace: workspace_namespace.to_s + }, + spec: { + hard: { + "limits.cpu": max_resources_per_workspace.dig(:limits, :cpu), + "limits.memory": max_resources_per_workspace.dig(:limits, :memory), + "requests.cpu": max_resources_per_workspace.dig(:requests, :cpu), + "requests.memory": max_resources_per_workspace.dig(:requests, :memory) + } + } + } + end + def workspace_secrets_inventory( workspace_name:, workspace_namespace:, -- GitLab From 58168e6d6a8c42e937f8c6f862b780885b4dccde Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Fri, 8 Dec 2023 10:09:13 +0530 Subject: [PATCH 3/5] Update version of newly created workspaces --- .../workspaces/config_version.rb | 2 +- .../workspaces/create/workspace_creator.rb | 2 +- .../workspaces_to_rails_infos_converter.rb | 2 +- .../factories/remote_development/workspaces.rb | 2 +- .../reconcile/main_integration_spec.rb | 16 +++++++++++++--- .../workspaces_to_rails_infos_converter_spec.rb | 4 ++-- 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/ee/lib/remote_development/workspaces/config_version.rb b/ee/lib/remote_development/workspaces/config_version.rb index ad5c3641c11c7d..df8b80d448f6da 100644 --- a/ee/lib/remote_development/workspaces/config_version.rb +++ b/ee/lib/remote_development/workspaces/config_version.rb @@ -3,8 +3,8 @@ module RemoteDevelopment module Workspaces module ConfigVersion - VERSION_1 = 1 VERSION_2 = 2 + VERSION_3 = 3 end end end diff --git a/ee/lib/remote_development/workspaces/create/workspace_creator.rb b/ee/lib/remote_development/workspaces/create/workspace_creator.rb index 75558cd2ac5b35..fa5cf4fa6c41ed 100644 --- a/ee/lib/remote_development/workspaces/create/workspace_creator.rb +++ b/ee/lib/remote_development/workspaces/create/workspace_creator.rb @@ -40,7 +40,7 @@ def self.create(value) workspace.processed_devfile = YAML.dump(processed_devfile.deep_stringify_keys) # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 workspace.actual_state = CREATION_REQUESTED - workspace.config_version = RemoteDevelopment::Workspaces::ConfigVersion::VERSION_2 + workspace.config_version = RemoteDevelopment::Workspaces::ConfigVersion::VERSION_3 workspace.url = URI::HTTPS.build({ host: workspace_host(workspace: workspace), query: { diff --git a/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb b/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb index 2bd0029c44f37f..7cbe5f1de3055a 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb @@ -47,7 +47,7 @@ def self.config_to_apply(workspace:, update_type:, logger:) include_all_resources = update_type == FULL || workspace.force_include_all_resources workspace_resources = case workspace.config_version - when ConfigVersion::VERSION_2 + when ConfigVersion::VERSION_3 DesiredConfigGenerator.generate_desired_config( workspace: workspace, include_all_resources: include_all_resources, diff --git a/ee/spec/factories/remote_development/workspaces.rb b/ee/spec/factories/remote_development/workspaces.rb index 46bb4384c68d48..4b4cb25d546397 100644 --- a/ee/spec/factories/remote_development/workspaces.rb +++ b/ee/spec/factories/remote_development/workspaces.rb @@ -8,7 +8,7 @@ personal_access_token name { "workspace-#{agent.id}-#{user.id}-#{random_string}" } - config_version { RemoteDevelopment::Workspaces::ConfigVersion::VERSION_2 } + config_version { RemoteDevelopment::Workspaces::ConfigVersion::VERSION_3 } force_include_all_resources { true } add_attribute(:namespace) { "gl-rd-ns-#{agent.id}-#{user.id}-#{random_string}" } diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/main_integration_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/main_integration_spec.rb index 12fd1f70a5841d..2da75da4eb7cc9 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/main_integration_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/main_integration_spec.rb @@ -18,6 +18,10 @@ let_it_be(:user) { create(:user) } let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } let(:egress_ip_rules) { agent.remote_development_agent_config.network_policy_egress } + let(:max_resources_per_workspace) { agent.remote_development_agent_config.max_resources_per_workspace } + let(:default_resources_per_workspace_container) do + agent.remote_development_agent_config.default_resources_per_workspace_container + end let(:logger) { instance_double(::Logger) } @@ -337,7 +341,9 @@ create_config_to_apply( workspace: workspace, started: expected_value_for_started, - egress_ip_rules: egress_ip_rules + egress_ip_rules: egress_ip_rules, + max_resources_per_workspace: max_resources_per_workspace, + default_resources_per_workspace_container: default_resources_per_workspace_container ) end @@ -448,7 +454,9 @@ create_config_to_apply( workspace: workspace, started: expected_value_for_started, - egress_ip_rules: egress_ip_rules + egress_ip_rules: egress_ip_rules, + max_resources_per_workspace: max_resources_per_workspace, + default_resources_per_workspace_container: default_resources_per_workspace_container ) end @@ -520,7 +528,9 @@ workspace: unprovisioned_workspace, started: expected_value_for_started, include_all_resources: true, - egress_ip_rules: egress_ip_rules + egress_ip_rules: egress_ip_rules, + max_resources_per_workspace: max_resources_per_workspace, + default_resources_per_workspace_container: default_resources_per_workspace_container ) end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb index 067b49e688263b..d7ba95d3979697 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb @@ -10,8 +10,8 @@ let(:actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } let(:processed_devfile) { example_processed_devfile } let(:force_include_all_resources) { false } - let(:current_config_version) { RemoteDevelopment::Workspaces::ConfigVersion::VERSION_2 } - let(:previous_config_version) { RemoteDevelopment::Workspaces::ConfigVersion::VERSION_1 } + let(:current_config_version) { RemoteDevelopment::Workspaces::ConfigVersion::VERSION_3 } + let(:previous_config_version) { RemoteDevelopment::Workspaces::ConfigVersion::VERSION_2 } let(:workspace) do instance_double( "RemoteDevelopment::Workspace", -- GitLab From adc19c861c6d7e80e69ab41835964617e6db5ad5 Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Tue, 19 Dec 2023 10:57:49 +0530 Subject: [PATCH 4/5] Update naming convention for config versions --- .../workspaces/config_version.rb | 1 + .../output/desired_config_generator.rb | 2 +- ...rev1.rb => desired_config_generator_v2.rb} | 6 ++-- .../reconcile/output/devfile_parser.rb | 3 +- ...e_parser_prev1.rb => devfile_parser_v2.rb} | 2 +- .../workspaces_to_rails_infos_converter.rb | 32 +++++++++++-------- ...yaml => example.processed-devfile-v2.yaml} | 0 ...rb => desired_config_generator_v2_spec.rb} | 8 ++--- ...rev1_spec.rb => devfile_parser_v2_spec.rb} | 6 ++-- ...orkspaces_to_rails_infos_converter_spec.rb | 2 +- .../remote_development_shared_contexts.rb | 13 ++++++-- 11 files changed, 45 insertions(+), 30 deletions(-) rename ee/lib/remote_development/workspaces/reconcile/output/{desired_config_generator_prev1.rb => desired_config_generator_v2.rb} (98%) rename ee/lib/remote_development/workspaces/reconcile/output/{devfile_parser_prev1.rb => devfile_parser_v2.rb} (99%) rename ee/spec/fixtures/remote_development/{example.processed-devfile-prev1.yaml => example.processed-devfile-v2.yaml} (100%) rename ee/spec/lib/remote_development/workspaces/reconcile/output/{desired_config_generator_prev1_spec.rb => desired_config_generator_v2_spec.rb} (95%) rename ee/spec/lib/remote_development/workspaces/reconcile/output/{devfile_parser_prev1_spec.rb => devfile_parser_v2_spec.rb} (96%) diff --git a/ee/lib/remote_development/workspaces/config_version.rb b/ee/lib/remote_development/workspaces/config_version.rb index df8b80d448f6da..b04f2cda3f44f6 100644 --- a/ee/lib/remote_development/workspaces/config_version.rb +++ b/ee/lib/remote_development/workspaces/config_version.rb @@ -5,6 +5,7 @@ module Workspaces module ConfigVersion VERSION_2 = 2 VERSION_3 = 3 + LATEST_VERSION = VERSION_3 end end end diff --git a/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator.rb b/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator.rb index 14e88730b759d8..8c32a5e2f1b43f 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator.rb @@ -249,7 +249,7 @@ def self.get_domain_template_annotation(name:, dns_zone:) # @param [Hash] labels # @param [Hash] annotations # @param [string] gitlab_workspaces_proxy_namespace - # @param [Array] egress_rules + # @param [Array] egress_ip_rules # @return [Hash] def self.get_network_policy( name:, diff --git a/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1.rb b/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_v2.rb similarity index 98% rename from ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1.rb rename to ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_v2.rb index 6dcef24464b33b..59dadd7e7da63a 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator_v2.rb @@ -8,7 +8,7 @@ module Output # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 # noinspection RubyClassMethodNamingConvention,RubyLocalVariableNamingConvention,RubyParameterNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ # rubocop:enable Layout/LineLength - class DesiredConfigGeneratorPrev1 + class DesiredConfigGeneratorV2 include States # @param [RemoteDevelopment::Workspaces::Workspace] workspace @@ -38,7 +38,7 @@ def self.generate_desired_config(workspace:, include_all_resources:, logger:) ) # TODO: https://gitlab.com/groups/gitlab-org/-/epics/10461 - handle error - k8s_resources_for_workspace_core = DevfileParserPrev1.get_all( + k8s_resources_for_workspace_core = DevfileParserV2.get_all( processed_devfile: workspace.processed_devfile, name: workspace.name, namespace: workspace.namespace, @@ -218,7 +218,7 @@ def self.get_domain_template_annotation(name:, dns_zone:) # @param [Hash] labels # @param [Hash] annotations # @param [string] gitlab_workspaces_proxy_namespace - # @param [Array] egress_rules + # @param [Array] egress_ip_rules # @return [Hash] def self.get_network_policy( name:, diff --git a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb index 661b9341870cc1..a44026fd333d97 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb @@ -60,7 +60,8 @@ def self.get_all( workspace_resources = set_security_context(workspace_resources: workspace_resources) workspace_resources = patch_default_resources( workspace_resources: workspace_resources, - default_resources_per_workspace_container: default_resources_per_workspace_container.deep_stringify_keys + default_resources_per_workspace_container: + default_resources_per_workspace_container.deep_stringify_keys.to_h ) inject_secrets( workspace_resources: workspace_resources, diff --git a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1.rb b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_v2.rb similarity index 99% rename from ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1.rb rename to ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_v2.rb index e7a11b7f91a843..399f92856bac3c 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser_v2.rb @@ -7,7 +7,7 @@ module Workspaces module Reconcile module Output # noinspection RubyClassMethodNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ - class DevfileParserPrev1 + class DevfileParserV2 # rubocop:todo Metrics/ParameterLists -- refactor this to have fewer parameters - perhaps introduce a parameter object: https://refactoring.com/catalog/introduceParameterObject.html # @param [String] processed_devfile # @param [String] name diff --git a/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb b/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb index 7cbe5f1de3055a..4469c9d131a2a8 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb @@ -46,20 +46,24 @@ def self.config_to_apply(workspace:, update_type:, logger:) include_all_resources = update_type == FULL || workspace.force_include_all_resources - workspace_resources = case workspace.config_version - when ConfigVersion::VERSION_3 - DesiredConfigGenerator.generate_desired_config( - workspace: workspace, - include_all_resources: include_all_resources, - logger: logger - ) - else - DesiredConfigGeneratorPrev1.generate_desired_config( - workspace: workspace, - include_all_resources: include_all_resources, - logger: logger - ) - end + workspace_resources = + case workspace.config_version + when ConfigVersion::LATEST_VERSION + DesiredConfigGenerator.generate_desired_config( + workspace: workspace, + include_all_resources: include_all_resources, + logger: logger + ) + else + namespace = "RemoteDevelopment::Workspaces::Reconcile::Output" + generator_class_name = "#{namespace}::DesiredConfigGeneratorV#{workspace.config_version}" + generator_class = Object.const_get(generator_class_name, false) + generator_class.generate_desired_config( + workspace: workspace, + include_all_resources: include_all_resources, + logger: logger + ) + end desired_config_to_apply_array = workspace_resources.map do |resource| YAML.dump(resource.deep_stringify_keys) diff --git a/ee/spec/fixtures/remote_development/example.processed-devfile-prev1.yaml b/ee/spec/fixtures/remote_development/example.processed-devfile-v2.yaml similarity index 100% rename from ee/spec/fixtures/remote_development/example.processed-devfile-prev1.yaml rename to ee/spec/fixtures/remote_development/example.processed-devfile-v2.yaml diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_v2_spec.rb similarity index 95% rename from ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1_spec.rb rename to ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_v2_spec.rb index dec2f6e62f233f..322d47f77bced5 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_prev1_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_v2_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Output::DesiredConfigGeneratorPrev1, :freeze_time, feature_category: :remote_development do +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Output::DesiredConfigGeneratorV2, :freeze_time, feature_category: :remote_development do include_context 'with remote development shared fixtures' describe '#generate_desired_config' do @@ -25,13 +25,13 @@ user: user, desired_state: desired_state, actual_state: actual_state, - processed_devfile: read_devfile('example.processed-devfile-prev1.yaml') + processed_devfile: read_devfile('example.processed-devfile-v2.yaml') ) end let(:expected_config) do YAML.load_stream( - create_config_to_apply_prev1( + create_config_to_apply_v2( workspace: workspace, started: started, include_network_policy: network_policy_enabled, @@ -107,7 +107,7 @@ context 'when DevfileParser returns empty array' do before do - allow(RemoteDevelopment::Workspaces::Reconcile::Output::DevfileParserPrev1).to receive(:get_all).and_return([]) + allow(RemoteDevelopment::Workspaces::Reconcile::Output::DevfileParserV2).to receive(:get_all).and_return([]) end it 'returns an empty array' do diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_v2_spec.rb similarity index 96% rename from ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1_spec.rb rename to ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_v2_spec.rb index 370c31f83e961a..cec1b6b966ff1c 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_prev1_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_v2_spec.rb @@ -2,14 +2,14 @@ require_relative '../../../fast_spec_helper' -RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Output::DevfileParserPrev1, feature_category: :remote_development do +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Output::DevfileParserV2, feature_category: :remote_development do include_context 'with remote development shared fixtures' let(:dns_zone) { "workspaces.localdev.me" } let(:logger) { instance_double(Logger) } let(:user) { instance_double("User", name: "name", email: "name@example.com") } let(:agent) { instance_double("Clusters::Agent", id: 1) } - let(:processed_devfile) { read_devfile('example.processed-devfile-prev1.yaml') } + let(:processed_devfile) { read_devfile('example.processed-devfile-v2.yaml') } let(:workspace) do instance_double( "RemoteDevelopment::Workspace", @@ -33,7 +33,7 @@ let(:expected_workspace_resources) do YAML.load_stream( - create_config_to_apply_prev1( + create_config_to_apply_v2( workspace: workspace, workspace_variables_env_var: {}, workspace_variables_file: {}, diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb index d7ba95d3979697..851ecd166c866e 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb @@ -149,7 +149,7 @@ let(:expected_include_all_resources) { true } it "includes config_to_apply without all resources included" do - allow(RemoteDevelopment::Workspaces::Reconcile::Output::DesiredConfigGeneratorPrev1) + allow(RemoteDevelopment::Workspaces::Reconcile::Output::DesiredConfigGeneratorV2) .to(receive(:generate_desired_config)) .with(hash_including(include_all_resources: expected_include_all_resources)) { generated_config_to_apply } diff --git a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb index 70ed030910df7e..5e2f40d883c631 100644 --- a/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb +++ b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb @@ -294,8 +294,16 @@ def create_workspace_rails_info( }.compact end + def create_config_to_apply(workspace:, **args) + latest_config_version = ::RemoteDevelopment::Workspaces::ConfigVersion::LATEST_VERSION + config_version = + workspace.respond_to?(:config_version) ? workspace.config_version : latest_config_version + method_name = "create_config_to_apply_v#{config_version}" + send(method_name, workspace: workspace, **args) + end + # rubocop:disable Metrics/ParameterLists, Metrics/AbcSize -- Cleanup as part of https://gitlab.com/gitlab-org/gitlab/-/issues/421687 - def create_config_to_apply( + def create_config_to_apply_v3( workspace:, started:, workspace_variables_env_var: nil, @@ -418,7 +426,7 @@ def create_config_to_apply( end.join end - def create_config_to_apply_prev1( + def create_config_to_apply_v2( workspace:, started:, workspace_variables_env_var: nil, @@ -522,6 +530,7 @@ def create_config_to_apply_prev1( YAML.dump(resource.deep_stringify_keys) end.join end + # rubocop:enable Metrics/ParameterLists, Metrics/AbcSize def workspace_inventory( -- GitLab From 4912d4f23625e1dea1aeee0cfa2b8024c66dd141 Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Sun, 24 Dec 2023 08:57:11 +0530 Subject: [PATCH 5/5] Use latest config version while creating workspace Re-add config version 1 as it is used in backgorund migration --- ee/lib/remote_development/workspaces/config_version.rb | 1 + .../remote_development/workspaces/create/workspace_creator.rb | 2 +- .../workspaces/reconcile/output/devfile_parser.rb | 2 ++ ee/spec/factories/remote_development/workspaces.rb | 2 +- 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ee/lib/remote_development/workspaces/config_version.rb b/ee/lib/remote_development/workspaces/config_version.rb index b04f2cda3f44f6..967bbe9d740061 100644 --- a/ee/lib/remote_development/workspaces/config_version.rb +++ b/ee/lib/remote_development/workspaces/config_version.rb @@ -3,6 +3,7 @@ module RemoteDevelopment module Workspaces module ConfigVersion + VERSION_1 = 1 VERSION_2 = 2 VERSION_3 = 3 LATEST_VERSION = VERSION_3 diff --git a/ee/lib/remote_development/workspaces/create/workspace_creator.rb b/ee/lib/remote_development/workspaces/create/workspace_creator.rb index fa5cf4fa6c41ed..1ee86962b3ce6a 100644 --- a/ee/lib/remote_development/workspaces/create/workspace_creator.rb +++ b/ee/lib/remote_development/workspaces/create/workspace_creator.rb @@ -40,7 +40,7 @@ def self.create(value) workspace.processed_devfile = YAML.dump(processed_devfile.deep_stringify_keys) # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 workspace.actual_state = CREATION_REQUESTED - workspace.config_version = RemoteDevelopment::Workspaces::ConfigVersion::VERSION_3 + workspace.config_version = RemoteDevelopment::Workspaces::ConfigVersion::LATEST_VERSION workspace.url = URI::HTTPS.build({ host: workspace_host(workspace: workspace), query: { diff --git a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb index a44026fd333d97..4473ffdc41c543 100644 --- a/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb +++ b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb @@ -123,6 +123,8 @@ def self.patch_default_resources(workspace_resources:, default_resources_per_wor pod_spec = workspace_resource.fetch('spec').fetch('template').fetch('spec') + # the purpose of this deep_merge (and the one below) is to ensure + # the values from the devfile override any defaults defined at the agent pod_spec.fetch('initContainers').each do |init_container| init_container .fetch('resources', {}) diff --git a/ee/spec/factories/remote_development/workspaces.rb b/ee/spec/factories/remote_development/workspaces.rb index 4b4cb25d546397..a5a84ff453b1cb 100644 --- a/ee/spec/factories/remote_development/workspaces.rb +++ b/ee/spec/factories/remote_development/workspaces.rb @@ -8,7 +8,7 @@ personal_access_token name { "workspace-#{agent.id}-#{user.id}-#{random_string}" } - config_version { RemoteDevelopment::Workspaces::ConfigVersion::VERSION_3 } + config_version { RemoteDevelopment::Workspaces::ConfigVersion::LATEST_VERSION } force_include_all_resources { true } add_attribute(:namespace) { "gl-rd-ns-#{agent.id}-#{user.id}-#{random_string}" } -- GitLab