diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index 9cea9ca10803cd1bd6d6c966f7f2ddef33687707..b3a5aefe6a8a613bb9b6d70d5807d91d3c4c32c3 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -45,6 +45,8 @@ class Workspace < ApplicationRecord scope :by_project_ids, ->(ids) { where(project_id: ids) } scope :with_actual_states, ->(actual_states) { where(actual_state: actual_states) } + scope :ordered_by_id, -> { order(:id) } + before_save :touch_desired_state_updated_at, if: ->(workspace) do workspace.new_record? || workspace.desired_state_changed? end diff --git a/ee/lib/remote_development/logger.rb b/ee/app/services/remote_development/logger.rb similarity index 100% rename from ee/lib/remote_development/logger.rb rename to ee/app/services/remote_development/logger.rb diff --git a/ee/app/services/remote_development/workspaces/reconcile_service.rb b/ee/app/services/remote_development/workspaces/reconcile_service.rb index 95855fb101bbbae3920bd8d4ab3cfa35976aa362..8e775b28dcb61d1e5ebceb5f3ef957e1b716d759 100644 --- a/ee/app/services/remote_development/workspaces/reconcile_service.rb +++ b/ee/app/services/remote_development/workspaces/reconcile_service.rb @@ -3,6 +3,8 @@ module RemoteDevelopment module Workspaces class ReconcileService + include ServiceResponseFactory + # NOTE: This constructor intentionally does not follow all of the conventions from # https://docs.gitlab.com/ee/development/reusing_abstractions.html#service-classes # suggesting that the dependencies be passed via the constructor. @@ -17,48 +19,25 @@ def execute(agent:, params:) # additional authorization checks here. # See https://gitlab.com/gitlab-org/gitlab/-/issues/409038 - # TODO: https://gitlab.com/groups/gitlab-org/-/epics/10461 - # We need to perform all processing in an explicit transaction, so that any unexpected exceptions will - # cause the transaction to be rolled back. This might not be necessary if we weren't having to rescue - # all exceptions below due to another problem. See the to do comment below in rescue clause for more info - ApplicationRecord.transaction do - process(agent, params) - end - rescue => e # rubocop:disable Style:RescueStandardError - # TODO: https://gitlab.com/groups/gitlab-org/-/epics/10461 - # If anything in the service class throws an exception, it ends up calling - # #handle_api_exception, in lib/api/helpers.rb, which tries to get current_user, - # which ends up calling API::Helpers#unauthorized! in lib/api/helpers.rb, - # when ends up setting @current_user to a Rack::Response, which blows up later - # in API::Helpers#current_user (lib/api/helpers.rb#79), when we try to get - # #preferred_language off of it. - # So we have to catch all exceptions and handle as a ServiceResponse.error - # in order to avoid this. - # How do the other ga4k requests like starboard_vulnerability handle this? - # UPDATE: See more context in https://gitlab.com/gitlab-org/gitlab/-/issues/402718#note_1343933650 - - Gitlab::ErrorTracking.track_exception(e, error_type: 'reconcile', agent_id: agent.id) - ServiceResponse.error( - message: "Unexpected reconcile error. Exception class: #{e.class}.", - reason: :internal_server_error + # NOTE: We inject these dependencies which depend upon the main Rails monolith, so that the domain layer + # does not directly depend on them, and also so that we can use fast_spec_helper in more places. + logger = RemoteDevelopment::Logger.build + + response_hash = Reconcile::Main.main( + # NOTE: We pass the original params in a separate key, so they can be separately and independently validated + # against a JSON schema, then flattened and converted to have symbol keys instead of string keys. + # We do not want to do any direct processing or manipulation of them here in the service layer, + # because that would be introducing domain logic into the service layer and coupling it to the + # shape and contents of the params payload. + original_params: params, + agent: agent, + logger: logger ) - end - - private - - # @param [Clusters::Agent] agent - # @param [Hash] params - # @return [ServiceResponse] - def process(agent, params) - parsed_params, error = RemoteDevelopment::Workspaces::Reconcile::ParamsParser.new.parse(params: params) - return ServiceResponse.error(message: error.message, reason: error.reason) if error - - reconcile_processor = Reconcile::ReconcileProcessor.new - payload, error = reconcile_processor.process(agent: agent, **parsed_params) - return ServiceResponse.error(message: error.message, reason: error.reason) if error + # Type-check payload using rightward assignment + response_hash[:payload] => { workspace_rails_infos: Array } if response_hash[:payload] - ServiceResponse.success(payload: payload) + create_service_response(response_hash) end end end diff --git a/ee/lib/remote_development/README.md b/ee/lib/remote_development/README.md index c407a6ad0ed500ce5c84703466cbc45641359b23..50eacc4a8f896e59539c267f9e30ce38097d3652 100644 --- a/ee/lib/remote_development/README.md +++ b/ee/lib/remote_development/README.md @@ -66,6 +66,7 @@ This layered approach also implies that we avoid directly referring to classes w If possible, we prefer to inject instances of these classes, or the classes themselves, into the Domain Logic layer from the Service layer. This also means that we can use `ee/spec/lib/remote_development/fast_spec_helper.rb` in most places in the Domain Logic layer. However, we may use the normal `spec_helper` for Domain Logic classes which make direct use of Rails. For example, classes which directly use ActiveRecord models and/or associations, where the usage of `fast_spec_helper` would require significant mocking, and not provide as much coverage of the ActiveRecord interactions. +See [this thread](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126785#note_1494395384) for a more detailed discussion of when/why to use `fast_spec_helper` or not. ## Type safety @@ -112,6 +113,10 @@ x => {y: Integer => i} # {:y=>"Not an Integer"}: Integer === "Not an integer" d Also note that `#deconstruct_keys` must be implemented in order to use these pattern matching features. +However, note that sometimes we will avoid using this type of hardcoded type checking, if it means that +we will be able to use `fast_spec_helper`, and there is otherwise sufficient test coverage to ensure +that the types are correct. See [this comment thread](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126785#note_1494395384) for a more detailed discussion. + #### Pattern matching and destructuring without types Also note that the destructuring a hash or array, even without the type checks (e.g. `x => {y: i}`), is still a form of type safety, because it will raise a `NoMatchingPatternKeyError` exception if the hash or array does not have the expected structure. @@ -252,7 +257,10 @@ class UpdateService end ``` -Next, you see the `ee/lib/remote_development/workspaces/update/main.rb` class, which implements an ROP chain with two steps, `authorize` and `update`: +Next, you see the `ee/lib/remote_development/workspaces/update/main.rb` class, which implements an ROP chain with two steps, `authorize` and `update`. + +Note that the `Main` class also has no domain logic in it itself other than invoking the steps and matching the the domain messages and transforming them into a response hash. We want to avoid that coupling, because all domain logic should live in the cohesive classes that are called by `Main` via the ROP pattern: + ```ruby class Main @@ -277,7 +285,9 @@ class Main end ``` -...and here is an example of the `ee/lib/remote_development/workspaces/update/updater.rb` class implementing the business logic in the "chain": +...and here is an example of the `ee/lib/remote_development/workspaces/update/updater.rb` class implementing the business logic in the "chain". +In this case, it contains the cohesive logic to update a workspace, and no other +unrelated domain logic: ```ruby class Updater diff --git a/ee/lib/remote_development/messages.rb b/ee/lib/remote_development/messages.rb index 879a16236798765632976e316b95ac6693433e22..94e5679622a1910576d043decfe9bfeb185e933e 100644 --- a/ee/lib/remote_development/messages.rb +++ b/ee/lib/remote_development/messages.rb @@ -30,6 +30,9 @@ module Messages # Workspace update errors WorkspaceUpdateFailed = Class.new(Message) + # Workspace reconcile errors + WorkspaceReconcileParamsValidationFailed = Class.new(Message) + #--------------------------------------------------------- # Domain Events - message name should describe the outcome #--------------------------------------------------------- @@ -41,5 +44,6 @@ module Messages # Workspace domain events WorkspaceCreateSuccessful = Class.new(Message) WorkspaceUpdateSuccessful = Class.new(Message) + WorkspaceReconcileSuccessful = Class.new(Message) end end diff --git a/ee/lib/remote_development/workspaces/reconcile/actual_state_calculator.rb b/ee/lib/remote_development/workspaces/reconcile/actual_state_calculator.rb deleted file mode 100644 index d593d9ac5934df136dfca4209e638f7a6dfd289b..0000000000000000000000000000000000000000 --- a/ee/lib/remote_development/workspaces/reconcile/actual_state_calculator.rb +++ /dev/null @@ -1,186 +0,0 @@ -# frozen_string_literal: true - -module RemoteDevelopment - module Workspaces - module Reconcile - # noinspection RubyConstantNamingConvention,RubyParameterNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ - class ActualStateCalculator - include States - - CONDITION_TYPE_PROGRESSING = 'Progressing' - CONDITION_TYPE_AVAILABLE = 'Available' - PROGRESSING_CONDITION_REASON_NEW_REPLICA_SET_CREATED = 'NewReplicaSetCreated' - PROGRESSING_CONDITION_REASON_FOUND_NEW_REPLICA_SET = 'FoundNewReplicaSet' - PROGRESSING_CONDITION_REASON_REPLICA_SET_UPDATED = 'ReplicaSetUpdated' - PROGRESSING_CONDITION_REASON_NEW_REPLICA_SET_AVAILABLE = 'NewReplicaSetAvailable' - PROGRESSING_CONDITION_REASON_PROGRESS_DEADLINE_EXCEEDED = 'ProgressDeadlineExceeded' - AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_UNAVAILABLE = 'MinimumReplicasUnavailable' - AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE = 'MinimumReplicasAvailable' - - DEPLOYMENT_PROGRESSING_STATUS_PROGRESSING = [ - PROGRESSING_CONDITION_REASON_NEW_REPLICA_SET_CREATED, - PROGRESSING_CONDITION_REASON_FOUND_NEW_REPLICA_SET, - PROGRESSING_CONDITION_REASON_REPLICA_SET_UPDATED - ].freeze - - DEPLOYMENT_PROGRESSING_STATUS_COMPLETE = [ - PROGRESSING_CONDITION_REASON_NEW_REPLICA_SET_AVAILABLE - ].freeze - - DEPLOYMENT_PROGRESSING_STATUS_FAILED = [ - PROGRESSING_CONDITION_REASON_PROGRESS_DEADLINE_EXCEEDED - ].freeze - - # rubocop: disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize - # @param [Hash] latest_k8s_deployment_info - # @param [String (frozen)] termination_progress - # @param [Hash] latest_error_details - # @return [String (frozen)] - def calculate_actual_state(latest_k8s_deployment_info:, termination_progress: nil, latest_error_details: nil) - # The workspace can be marked as TERMINATED even if latest_error_details are present for the workspace as - # nothing further can be done for the workspace as it no longer exists in the cluster - # In every other case, the existence of latest_error_details should transition the workspace to ERROR state - return TERMINATED if termination_progress == TerminationProgress::TERMINATED - return ERROR if latest_error_details - return TERMINATING if termination_progress == TerminationProgress::TERMINATING - - # if latest_k8s_deployment_info is missing, but workspace isn't Terminated or Terminating, this is an Unknown - # state and should likely be accompanied by a value in the Error field, as this should be detectable by - # agentk. At that point, this may not be necessary, and we can detect the error state earlier and return in a - # guard clause before this point. - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/396882#note_1377670883 - # Error field is not yet implemented, double check the above comment once it is implemented - return UNKNOWN unless latest_k8s_deployment_info - - spec = latest_k8s_deployment_info['spec'] - status = latest_k8s_deployment_info['status'] - conditions = status&.[]('conditions') - return UNKNOWN unless spec && status && conditions - - progressing_condition = conditions.detect do |condition| - condition['type'] == CONDITION_TYPE_PROGRESSING - end - return UNKNOWN if progressing_condition.nil? - - progressing_reason = progressing_condition['reason'] - spec_replicas = spec['replicas'] - return UNKNOWN if progressing_reason.nil? || spec_replicas.nil? - - # https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#deployment-status - - # If the deployment has been marked failed, we know that the workspace has failed - # A deployment is failed if - # - Insufficient quota - # - Readiness probe failures - # - Image pull errors - # - Insufficient permissions - # - Limit ranges - # - Application runtime misconfiguration - return FAILED if DEPLOYMENT_PROGRESSING_STATUS_FAILED.include?(progressing_reason) - - # If the deployment is still in progress, the workspace can only be either starting or stopping - # A deployment is in progress if - # - The Deployment creates a new ReplicaSet. - # - The Deployment is scaling up its newest ReplicaSet. - # - The Deployment is scaling down its older ReplicaSet(s). - # - New Pods become ready or available (ready for at least MinReadySeconds). - if DEPLOYMENT_PROGRESSING_STATUS_PROGRESSING.include?(progressing_reason) - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409777 - # This does not appear to be the normal STOPPING or STARTING scenario, because the progressing_reason - # always remains 'NewReplicaSetAvailable' even when transitioning between Running and Stopped. - return STOPPING if spec_replicas == 0 - return STARTING if spec_replicas == 1 - end - - # https://github.com/kubernetes/kubernetes/blob/3615291/pkg/controller/deployment/sync.go#L513-L516 - status_available_replicas = status.fetch('availableReplicas', 0) - status_unavailable_replicas = status.fetch('unavailableReplicas', 0) - - available_condition = conditions.detect do |condition| - condition['type'] == CONDITION_TYPE_AVAILABLE - end - return UNKNOWN if available_condition.nil? - - available_reason = available_condition['reason'] - return UNKNOWN if available_reason.nil? - - # If a deployment has been marked complete, the workspace state needs to be further calculated - # A deployment is complete if - # - All of the replicas associated with the Deployment have been updated to the latest version - # you've specified, meaning any updates you've requested have been completed. - # - All of the replicas associated with the Deployment are available. - # - No old replicas for the Deployment are running. - if DEPLOYMENT_PROGRESSING_STATUS_COMPLETE.include?(progressing_reason) - # rubocop:disable Layout/MultilineOperationIndentation - Currently can't override default RubyMine formatting - - # If a deployment is complete and the desired and available replicas are 0, the workspace is stopped - if available_reason == AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE && - spec_replicas == 0 && status_available_replicas == 0 - return STOPPED - end - - # If a deployment is complete and the Available condition has reason MinimumReplicasAvailable - # and the desired and available replicas are equal - # and there are no unavailable replicas - # then the workspace is running - if available_reason == AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE && - spec_replicas == status_available_replicas && - status_unavailable_replicas == 0 - return RUNNING - end - - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409777 - # This appears to be the normal STOPPING scenario, because the progressing_reason always remains - # 'NewReplicaSetAvailable' when transitioning between Running and Stopped. Confirm if different - # handling of STOPPING status above is also necessary. - # In normal usage (at least in local dev), this transition always happens so fast that this - # state is never sent in a reconciliation request, even with a 1-second polling interval. - # It always stopped immediately in under a second, and thus the next poll after a Stopped - # request always ends up with spec_replicas == 0 && status_available_replicas == 0 and - # matches the STOPPED state above. - if available_reason == AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE && - spec_replicas == 0 && status_available_replicas == 1 - return STOPPING - end - - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409777 - # This appears to be the normal STARTING scenario, because the progressing_reason always remains - # 'NewReplicaSetAvailable' and available_reason is either 'MinimumReplicasAvailable' or - # 'MinimumReplicasUnavailable' when transitioning between Stopped and Running. Confirm if different - # handling of STARTING status above is also necessary. - if [ - AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE, - AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_UNAVAILABLE - ].include?(available_reason) && - spec_replicas == 1 && status_available_replicas == 0 - return STARTING - end - - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409777 - # This is unreachable by any of the currently implemented fixture scenarios, because it matches the - # normal behavior when transioning between Stopped and Running. We need to determine what - # a failure scenario actually looks like and how it differs, if at all, from a normal STARTING - # scenario. Logic is commented out to avoid undercoverage failure. See related TODOs above. - # If a deployment is complete and the Available condition has reason MinimumReplicasUnavailable - # and the desired and available replicas are not equal - # and there are unavailable replicas - # then the workspace is failed - # Example: Deployment is completed and the ReplicaSet is available and up-to-date. - # But the Pods of the ReplicaSet are not available as they are in CrashLoopBackOff - # if available_reason == AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_UNAVAILABLE && - # spec_replicas != status_available_replicas && - # status_unavailable_replicas != 0 - # return FAILED - # end - - # rubocop:enable Layout/MultilineOperationIndentation - Currently can't override default RubyMine formatting - end - - UNKNOWN - end - - # rubocop: enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize - end - end - end -end diff --git a/ee/lib/remote_development/workspaces/reconcile/agent_info.rb b/ee/lib/remote_development/workspaces/reconcile/agent_info.rb deleted file mode 100644 index e05188c3fea9eda90edb2ad0b3416bd35b6ae74f..0000000000000000000000000000000000000000 --- a/ee/lib/remote_development/workspaces/reconcile/agent_info.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -module RemoteDevelopment - module Workspaces - module Reconcile - # noinspection RubyParameterNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ - class AgentInfo - attr_reader :name, :namespace, :actual_state, :deployment_resource_version - - # @param [String] name - # @param [String] namespace - # @param [String] actual_state - # @param [String] deployment_resource_version - # @return [RemoteDevelopment::Workspaces::Reconcile::AgentInfo] - def initialize(name:, namespace:, actual_state:, deployment_resource_version:) - @name = name - @namespace = namespace - @actual_state = actual_state - @deployment_resource_version = deployment_resource_version - end - - # @param [RemoteDevelopment::Workspaces::AgentInfo] other - # @return [TrueClass, FalseClass] - def ==(other) - return false unless other && self.class == other.class - - other.name == name && - other.namespace == namespace && - other.actual_state == actual_state && - other.deployment_resource_version == deployment_resource_version - end - end - end - end -end diff --git a/ee/lib/remote_development/workspaces/reconcile/agent_info_parser.rb b/ee/lib/remote_development/workspaces/reconcile/agent_info_parser.rb deleted file mode 100644 index 9d430a677b763d30ef7783df6b609c91a5a0d3a1..0000000000000000000000000000000000000000 --- a/ee/lib/remote_development/workspaces/reconcile/agent_info_parser.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -module RemoteDevelopment - module Workspaces - module Reconcile - class AgentInfoParser - # @param [Hash] workspace_agent_info - # @return [RemoteDevelopment::Workspaces::Reconcile::AgentInfo] - def parse(workspace_agent_info:) - # workspace_agent_info.fetch('latest_k8s_deployment_info') is not used since the field may not be present - latest_k8s_deployment_info = workspace_agent_info['latest_k8s_deployment_info'] - - actual_state = ActualStateCalculator.new.calculate_actual_state( - latest_k8s_deployment_info: latest_k8s_deployment_info, - # workspace_agent_info.fetch('termination_progress') is not used since the field may not be present - termination_progress: workspace_agent_info['termination_progress'], - latest_error_details: workspace_agent_info['error_details'] - ) - - deployment_resource_version = latest_k8s_deployment_info&.dig('metadata', 'resourceVersion') - - # If the actual state of the workspace is Terminated, the only keys which will be put into the - # AgentInfo object are name, namespace and actual_state - info = { - name: workspace_agent_info.fetch('name'), - actual_state: actual_state, - namespace: workspace_agent_info.fetch('namespace'), - deployment_resource_version: deployment_resource_version - } - - AgentInfo.new(**info) - end - end - end - end -end diff --git a/ee/lib/remote_development/workspaces/reconcile/desired_config_generator.rb b/ee/lib/remote_development/workspaces/reconcile/desired_config_generator.rb deleted file mode 100644 index 1db8815d328f4711e53d847135f2ea10c3422faa..0000000000000000000000000000000000000000 --- a/ee/lib/remote_development/workspaces/reconcile/desired_config_generator.rb +++ /dev/null @@ -1,182 +0,0 @@ -# frozen_string_literal: true - -module RemoteDevelopment - module Workspaces - module Reconcile - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - # rubocop:disable Layout/LineLength - # noinspection RubyInstanceMethodNamingConvention,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 DesiredConfigGenerator - include States - - # @param [RemoteDevelopment::Workspaces::Workspace] workspace - # @return [Array] - def generate_desired_config(workspace:) - 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) - labels, annotations = get_labels_and_annotations( - agent_id: agent.id, - owning_inventory: owning_inventory, - domain_template: domain_template, - workspace_id: workspace.id - ) - - # TODO: https://gitlab.com/groups/gitlab-org/-/epics/10461 - handle error - workspace_resources = DevfileParser.new.get_all( - processed_devfile: workspace.processed_devfile, - name: name, - namespace: namespace, - replicas: replicas, - domain_template: domain_template, - labels: labels, - annotations: annotations, - user: user - ) - # 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? - - workspace_resources.insert(0, workspace_inventory_config_map) - - 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, - labels: labels, - annotations: annotations, - gitlab_workspaces_proxy_namespace: gitlab_workspaces_proxy_namespace - ) - workspace_resources.append(network_policy) - end - - workspace_resources - end - - private - - # @param [String] desired_state - # @return [Integer] - def get_workspace_replicas(desired_state:) - return 1 if [ - CREATION_REQUESTED, - RUNNING - ].include?(desired_state) - - 0 - end - - # @param [String] name - # @param [String] namespace - # @param [Integer] agent_id - # @return [Array(Hash, String (frozen))] - def create_workspace_inventory_config_map(name:, namespace:, agent_id:) - owning_inventory = "#{name}-workspace-inventory" - workspace_inventory_config_map = { - kind: 'ConfigMap', - apiVersion: 'v1', - metadata: { - name: owning_inventory, - namespace: namespace, - labels: { - 'cli-utils.sigs.k8s.io/inventory-id': owning_inventory, - '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 [Integer] workspace_id - # @return [Array<(Hash, Hash)>] - def get_labels_and_annotations(agent_id:, owning_inventory:, domain_template:, workspace_id:) - labels = { - 'agent.gitlab.com/id' => agent_id.to_s - } - 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 - } - [labels, annotations] - end - - # @param [String] name - # @param [String] namespace - # @param [Hash] labels - # @param [Hash] annotations - # @param [string] gitlab_workspaces_proxy_namespace - # @return [Hash] - def get_network_policy(name:, namespace:, labels:, annotations:, gitlab_workspaces_proxy_namespace:) - policy_types = [ - - "Ingress", - - "Egress" - ] - - proxy_namespace_selector = { - matchLabels: { - "kubernetes.io/metadata.name": gitlab_workspaces_proxy_namespace - } - } - proxy_pod_selector = { - matchLabels: { - "app.kubernetes.io/name": "gitlab-workspaces-proxy" - } - } - ingress = [{ from: [{ namespaceSelector: proxy_namespace_selector, podSelector: proxy_pod_selector }] }] - - kube_system_namespace_selector = { - matchLabels: { - "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 = [ - { to: [{ ipBlock: { cidr: "0.0.0.0/0", except: egress_except_cidr } }] }, - { - ports: [{ port: 53, protocol: "TCP" }, { port: 53, protocol: "UDP" }], - to: [{ namespaceSelector: kube_system_namespace_selector }] - } - ] - - { - apiVersion: "networking.k8s.io/v1", - kind: "NetworkPolicy", - metadata: { - annotations: annotations, - labels: labels, - name: name, - namespace: namespace - }, - spec: { - egress: egress, - ingress: ingress, - podSelector: {}, - policyTypes: policy_types - } - }.deep_stringify_keys.to_h - end - end - end - end -end diff --git a/ee/lib/remote_development/workspaces/reconcile/devfile_parser.rb b/ee/lib/remote_development/workspaces/reconcile/devfile_parser.rb deleted file mode 100644 index 7c30bcd3f2e8d33d0d055c638aa7a40cfb58282f..0000000000000000000000000000000000000000 --- a/ee/lib/remote_development/workspaces/reconcile/devfile_parser.rb +++ /dev/null @@ -1,158 +0,0 @@ -# frozen_string_literal: true - -require 'devfile' - -module RemoteDevelopment - module Workspaces - module Reconcile - # noinspection RubyInstanceMethodNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ - class DevfileParser - # @param [String] processed_devfile - # @param [String] name - # @param [String] namespace - # @param [Integer] replicas - # @param [String] domain_template - # @param [Hash] labels - # @param [Hash] annotations - # @param [User] user - # @return [Array] - def get_all(processed_devfile:, name:, namespace:, replicas:, domain_template:, labels:, annotations:, user:) - begin - workspace_resources_yaml = Devfile::Parser.get_all( - processed_devfile, - name, - namespace, - YAML.dump(labels.deep_stringify_keys), - YAML.dump(annotations.deep_stringify_keys), - replicas, - domain_template, - 'none' - ) - rescue Devfile::CliError => e - logger.warn( - message: 'Error parsing devfile with Devfile::Parser.get_all', - error_type: 'reconcile_devfile_parser_error', - workspace_name: name, - workspace_namespace: namespace, - devfile_parser_error: e.message - ) - return [] - end - - 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( - workspace_resources: workspace_resources, - domain_template: domain_template - ) - end - - private - - # @return [RemoteDevelopment::Logger] - def logger - @logger ||= RemoteDevelopment::Logger.build - end - - # Devfile library allows specifying the security context of pods/containers as mentioned in - # https://github.com/devfile/api/issues/920 through `pod-overrides` and `container-overrides` attributes. - # However, https://github.com/devfile/library/pull/158 which is implementing this feature, - # is not part of v2.2.0 which is the latest release of the devfile which is being used in the devfile-gem. - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409189 - # Once devfile library releases a new version, update the devfile-gem and - # move the logic of setting the security context in the `devfile_processor` as part of workspace creation. - RUN_AS_USER = 5001 - - # @param [Array] workspace_resources - # @return [Array] - def 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'] = { - 'runAsNonRoot' => true, - 'runAsUser' => RUN_AS_USER, - 'fsGroup' => 0, - 'fsGroupChangePolicy' => 'OnRootMismatch' - } - # 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 - } - 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 - } - end - end - workspace_resources - end - - # @param [Array] workspace_resources - # @param [User] user - # @return [Array] - def set_git_configuration(workspace_resources:, user:) - 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 - } - ]) - end - end - workspace_resources - end - - # @param [Array] workspace_resources - # @param [String] domain_template - # @return [Array] - def 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' - - pod_spec = workspace_resource['spec']['template']['spec'] - - pod_spec['initContainers'].each do |init_containers| - init_containers.fetch('env').concat(env_variables) - end - - pod_spec['containers'].each do |container| - container.fetch('env').concat(env_variables) - end - end - workspace_resources - end - end - end - end -end diff --git a/ee/lib/remote_development/workspaces/reconcile/input/actual_state_calculator.rb b/ee/lib/remote_development/workspaces/reconcile/input/actual_state_calculator.rb new file mode 100644 index 0000000000000000000000000000000000000000..b60bcde76319af9a4baf421ba8368daa7139f01e --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/input/actual_state_calculator.rb @@ -0,0 +1,190 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Input + # noinspection RubyConstantNamingConvention,RubyParameterNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + class ActualStateCalculator + include States + + CONDITION_TYPE_PROGRESSING = "Progressing" + CONDITION_TYPE_AVAILABLE = "Available" + PROGRESSING_CONDITION_REASON_NEW_REPLICA_SET_CREATED = "NewReplicaSetCreated" + PROGRESSING_CONDITION_REASON_FOUND_NEW_REPLICA_SET = "FoundNewReplicaSet" + PROGRESSING_CONDITION_REASON_REPLICA_SET_UPDATED = "ReplicaSetUpdated" + PROGRESSING_CONDITION_REASON_NEW_REPLICA_SET_AVAILABLE = "NewReplicaSetAvailable" + PROGRESSING_CONDITION_REASON_PROGRESS_DEADLINE_EXCEEDED = "ProgressDeadlineExceeded" + AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_UNAVAILABLE = "MinimumReplicasUnavailable" + AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE = "MinimumReplicasAvailable" + + DEPLOYMENT_PROGRESSING_STATUS_PROGRESSING = [ + PROGRESSING_CONDITION_REASON_NEW_REPLICA_SET_CREATED, + PROGRESSING_CONDITION_REASON_FOUND_NEW_REPLICA_SET, + PROGRESSING_CONDITION_REASON_REPLICA_SET_UPDATED + ].freeze + + DEPLOYMENT_PROGRESSING_STATUS_COMPLETE = [ + PROGRESSING_CONDITION_REASON_NEW_REPLICA_SET_AVAILABLE + ].freeze + + DEPLOYMENT_PROGRESSING_STATUS_FAILED = [ + PROGRESSING_CONDITION_REASON_PROGRESS_DEADLINE_EXCEEDED + ].freeze + + # rubocop: disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize + # @param [Hash] latest_k8s_deployment_info + # @param [String (frozen)] termination_progress + # @param [Hash] latest_error_details + # @return [String (frozen)] + def self.calculate_actual_state( + latest_k8s_deployment_info:, termination_progress: nil, latest_error_details: nil) + # The workspace can be marked as TERMINATED even if latest_error_details are present for the workspace as + # nothing further can be done for the workspace as it no longer exists in the cluster + # In every other case, the existence of latest_error_details should transition the workspace to ERROR state + return TERMINATED if termination_progress == TerminationProgress::TERMINATED + return ERROR if latest_error_details + return TERMINATING if termination_progress == TerminationProgress::TERMINATING + + # if latest_k8s_deployment_info is missing, but workspace isn't Terminated or Terminating, this is an + # Unknown state and should likely be accompanied by a value in the Error field, as this should be detectable + # by agentk. At that point, this may not be necessary, and we can detect the error state earlier and return + # in a guard clause before this point. + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/396882#note_1377670883 + # Error field is not yet implemented, double check the above comment once it is implemented + return UNKNOWN unless latest_k8s_deployment_info + + spec = latest_k8s_deployment_info[:spec] + status = latest_k8s_deployment_info[:status] + conditions = status&.[](:conditions) + return UNKNOWN unless spec && status && conditions + + progressing_condition = conditions.detect do |condition| + condition[:type] == CONDITION_TYPE_PROGRESSING + end + return UNKNOWN if progressing_condition.nil? + + progressing_reason = progressing_condition[:reason] + spec_replicas = spec[:replicas] + return UNKNOWN if progressing_reason.nil? || spec_replicas.nil? + + # https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#deployment-status + + # If the deployment has been marked failed, we know that the workspace has failed + # A deployment is failed if + # - Insufficient quota + # - Readiness probe failures + # - Image pull errors + # - Insufficient permissions + # - Limit ranges + # - Application runtime misconfiguration + return FAILED if DEPLOYMENT_PROGRESSING_STATUS_FAILED.include?(progressing_reason) + + # If the deployment is still in progress, the workspace can only be either starting or stopping + # A deployment is in progress if + # - The Deployment creates a new ReplicaSet. + # - The Deployment is scaling up its newest ReplicaSet. + # - The Deployment is scaling down its older ReplicaSet(s). + # - New Pods become ready or available (ready for at least MinReadySeconds). + if DEPLOYMENT_PROGRESSING_STATUS_PROGRESSING.include?(progressing_reason) + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409777 + # This does not appear to be the normal STOPPING or STARTING scenario, because the + # progressing_reason always remains "NewReplicaSetAvailable" even when transitioning between Running + # and Stopped. + return STOPPING if spec_replicas == 0 + return STARTING if spec_replicas == 1 + end + + # https://github.com/kubernetes/kubernetes/blob/3615291/pkg/controller/deployment/sync.go#L513-L516 + status_available_replicas = status.fetch(:availableReplicas, 0) + status_unavailable_replicas = status.fetch(:unavailableReplicas, 0) + + available_condition = conditions.detect do |condition| + condition[:type] == CONDITION_TYPE_AVAILABLE + end + return UNKNOWN if available_condition.nil? + + available_reason = available_condition[:reason] + return UNKNOWN if available_reason.nil? + + # If a deployment has been marked complete, the workspace state needs to be further calculated + # A deployment is complete if + # - All of the replicas associated with the Deployment have been updated to the latest version + # you've specified, meaning any updates you've requested have been completed. + # - All of the replicas associated with the Deployment are available. + # - No old replicas for the Deployment are running. + if DEPLOYMENT_PROGRESSING_STATUS_COMPLETE.include?(progressing_reason) + # rubocop:disable Layout/MultilineOperationIndentation - Currently can't override default RubyMine formatting + + # If a deployment is complete and the desired and available replicas are 0, the workspace is stopped + if available_reason == AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE && + spec_replicas == 0 && status_available_replicas == 0 + return STOPPED + end + + # If a deployment is complete and the Available condition has reason MinimumReplicasAvailable + # and the desired and available replicas are equal + # and there are no unavailable replicas + # then the workspace is running + if available_reason == AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE && + spec_replicas == status_available_replicas && + status_unavailable_replicas == 0 + return RUNNING + end + + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409777 + # This appears to be the normal STOPPING scenario, because the progressing_reason always remains + # "NewReplicaSetAvailable" when transitioning between Running and Stopped. Confirm if different + # handling of STOPPING status above is also necessary. + # In normal usage (at least in local dev), this transition always happens so fast that this + # state is never sent in a reconciliation request, even with a 1-second polling interval. + # It always stopped immediately in under a second, and thus the next poll after a Stopped + # request always ends up with spec_replicas == 0 && status_available_replicas == 0 and + # matches the STOPPED state above. + if available_reason == AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE && + spec_replicas == 0 && status_available_replicas == 1 + return STOPPING + end + + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409777 + # This appears to be the normal STARTING scenario, because the progressing_reason always remains + # "NewReplicaSetAvailable" and available_reason is either 'MinimumReplicasAvailable' or + # "MinimumReplicasUnavailable" when transitioning between Stopped and Running. Confirm if different + # handling of STARTING status above is also necessary. + if [ + AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_AVAILABLE, + AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_UNAVAILABLE + ].include?(available_reason) && + spec_replicas == 1 && status_available_replicas == 0 + return STARTING + end + + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409777 + # This is unreachable by any of the currently implemented fixture scenarios, because it matches the + # normal behavior when transioning between Stopped and Running. We need to determine what + # a failure scenario actually looks like and how it differs, if at all, from a normal STARTING + # scenario. Logic is commented out to avoid undercoverage failure. See related TODOs above. + # If a deployment is complete and the Available condition has reason MinimumReplicasUnavailable + # and the desired and available replicas are not equal + # and there are unavailable replicas + # then the workspace is failed + # Example: Deployment is completed and the ReplicaSet is available and up-to-date. + # But the Pods of the ReplicaSet are not available as they are in CrashLoopBackOff + # if available_reason == AVAILABLE_CONDITION_REASON_MINIMUM_REPLICAS_UNAVAILABLE && + # spec_replicas != status_available_replicas && + # status_unavailable_replicas != 0 + # return FAILED + # end + + # rubocop:enable Layout/MultilineOperationIndentation - Currently can't override default RubyMine formatting + end + + UNKNOWN + end + + # rubocop: enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/input/agent_info.rb b/ee/lib/remote_development/workspaces/reconcile/input/agent_info.rb new file mode 100644 index 0000000000000000000000000000000000000000..e48093b7c7ec5f64ce6e73362bd18d5b0367f5fd --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/input/agent_info.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Input + # noinspection RubyParameterNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + class AgentInfo + attr_reader :name, :namespace, :actual_state, :deployment_resource_version + + # @param [String] name + # @param [String] namespace + # @param [String] actual_state + # @param [String] deployment_resource_version + # @return [RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo] + def initialize(name:, namespace:, actual_state:, deployment_resource_version:) + @name = name + @namespace = namespace + @actual_state = actual_state + @deployment_resource_version = deployment_resource_version + end + + # @param [RemoteDevelopment::Workspaces::Input::AgentInfo] other + # @return [TrueClass, FalseClass] + def ==(other) + return false unless other && self.class == other.class + + other.name == name && + other.namespace == namespace && + other.actual_state == actual_state && + other.deployment_resource_version == deployment_resource_version + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/input/agent_infos_observer.rb b/ee/lib/remote_development/workspaces/reconcile/input/agent_infos_observer.rb new file mode 100644 index 0000000000000000000000000000000000000000..fb52d2792ee10c4e8c11d962d73be5c7824a6a5b --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/input/agent_infos_observer.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Input + # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + class AgentInfosObserver + NORMAL = "normal" + ABNORMAL = "abnormal" + + # @param [Hash] value + # @return [Hash] + def self.observe(value) + value => { + agent: agent, # Skip type checking so we can use fast_spec_helper + update_type: String => update_type, + workspace_agent_infos_by_name: Hash => workspace_agent_infos_by_name, + logger: logger, # Skip type checking to avoid coupling to Rails logger + } + + abnormal_agent_infos, normal_agent_infos = + workspace_agent_infos_by_name.values.partition do |agent_info| + [States::UNKNOWN, States::ERROR].include? agent_info.actual_state + end + + normal_count = normal_agent_infos.length + abnormal_count = abnormal_agent_infos.length + total_count = normal_count + abnormal_count + + # Log normal agent infos at debug level + logger.debug( + message: "Parsed #{total_count} total workspace agent infos from params, with " \ + "#{normal_count} in a NORMAL actual_state and #{abnormal_count} in an ABNORMAL actual_state", + agent_id: agent.id, + update_type: update_type, + actual_state_type: NORMAL, + total_count: total_count, + normal_count: normal_count, + abnormal_count: abnormal_count, + normal_agent_infos: normal_agent_infos.map do |agent_info| + { + name: agent_info.name, + namespace: agent_info.namespace, + actual_state: agent_info.actual_state, + deployment_resource_version: agent_info.deployment_resource_version + } + end, + abnormal_agent_infos: abnormal_agent_infos.map do |agent_info| + { + name: agent_info.name, + namespace: agent_info.namespace, + actual_state: agent_info.actual_state, + deployment_resource_version: agent_info.deployment_resource_version + } + end + ) + + # Log abnormal agent infos at warn level + if abnormal_agent_infos.present? + logger.warn( + message: "Parsed #{abnormal_count} workspace agent infos with an " \ + "ABNORMAL actual_state from params (total: #{total_count})", + error_type: "abnormal_actual_state", + agent_id: agent.id, + update_type: update_type, + actual_state_type: ABNORMAL, + total_count: total_count, + normal_count: normal_count, + abnormal_count: abnormal_count, + abnormal_agent_infos: abnormal_agent_infos.map do |agent_info| + { + name: agent_info.name, + namespace: agent_info.namespace, + actual_state: agent_info.actual_state, + deployment_resource_version: agent_info.deployment_resource_version + } + end + ) + end + + value + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/input/factory.rb b/ee/lib/remote_development/workspaces/reconcile/input/factory.rb new file mode 100644 index 0000000000000000000000000000000000000000..1c3359cb17cbcdc7034631ba37407b9a2c5a2ee7 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/input/factory.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Input + class Factory + # @param [Hash] agent_info_hash_from_params + # @return [RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo] + # noinspection RubyParameterNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + def self.build(agent_info_hash_from_params:) + # Hash#[] instead of Hash#fetch or destructuring is used, since the field may not be present + latest_k8s_deployment_info = agent_info_hash_from_params[:latest_k8s_deployment_info] + + actual_state = ActualStateCalculator.calculate_actual_state( + latest_k8s_deployment_info: latest_k8s_deployment_info, + # workspace_agent_info.fetch('termination_progress') is not used since the field may not be present + termination_progress: agent_info_hash_from_params[:termination_progress], + latest_error_details: agent_info_hash_from_params[:error_details] + ) + + deployment_resource_version = latest_k8s_deployment_info&.dig(:metadata, :resourceVersion) + + # If the actual state of the workspace is Terminated, the only keys which will be put into the + # AgentInfo object are name and actual_state + info = { + name: agent_info_hash_from_params.fetch(:name), + actual_state: actual_state, + namespace: agent_info_hash_from_params.fetch(:namespace), + deployment_resource_version: deployment_resource_version + } + + AgentInfo.new(**info) + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/input/params_extractor.rb b/ee/lib/remote_development/workspaces/reconcile/input/params_extractor.rb new file mode 100644 index 0000000000000000000000000000000000000000..e6df500ad7cf42848964cae652b5c81df3148501 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/input/params_extractor.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Input + # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + class ParamsExtractor + include Messages + + # @param [Hash] value + # @return [Hash] + def self.extract(value) + value => { original_params: Hash => original_params } + + original_params.symbolize_keys => { + update_type: String => update_type, + workspace_agent_infos: Array => workspace_agent_info_hashes_from_params, + } + + # We extract the original string-keyed params, move them to the top level of the value hash with descriptive + # names, and deep-symbolize keys. The original_params will still remain in the value hash as well for + # debugging purposes. + + value.merge( + { + update_type: update_type, + workspace_agent_info_hashes_from_params: workspace_agent_info_hashes_from_params + }.deep_symbolize_keys.to_h + ) + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/input/params_to_infos_converter.rb b/ee/lib/remote_development/workspaces/reconcile/input/params_to_infos_converter.rb new file mode 100644 index 0000000000000000000000000000000000000000..5d9a23d7203591bb96b62115e28b6a349395386a --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/input/params_to_infos_converter.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Input + # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + class ParamsToInfosConverter + include Messages + + # @param [Hash] value + # @return [Hash] + def self.convert(value) + value => { workspace_agent_info_hashes_from_params: Array => workspace_agent_info_hashes_from_params } + + # Convert the workspace_agent_info_hashes_from_params array into an array of AgentInfo objects + workspace_agent_infos_by_name = + workspace_agent_info_hashes_from_params.each_with_object({}) do |agent_info_hash_from_params, hash| + agent_info = Factory.build(agent_info_hash_from_params: agent_info_hash_from_params) + hash[agent_info.name.to_sym] = agent_info + end + + value.merge(workspace_agent_infos_by_name: workspace_agent_infos_by_name) + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/input/params_validator.rb b/ee/lib/remote_development/workspaces/reconcile/input/params_validator.rb new file mode 100644 index 0000000000000000000000000000000000000000..920b289d72d905d8605103ea49556f4a0a4a9513 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/input/params_validator.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Input + # noinspection RubyClassMethodNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + class ParamsValidator + include Messages + include UpdateTypes + + # @param [Hash] value + # @return [Result] + def self.validate(value) + value => { original_params: Hash => original_params } + + # NOTE: We deep_stringify_keys here, because even though they will be strings in the a real request, + # so we can still pass keys as symbols during tests, and not have to worry about passing string + # keys in tests. This is the only place where keys need to be strings, because of the JSON schema + # validation, all other places we convert and work with the keys as symbols. + errors = validate_original_params_against_schema(original_params.deep_stringify_keys) + + if errors.none? + Result.ok(value) + else + Result.err(WorkspaceReconcileParamsValidationFailed.new(details: errors.join(". "))) + end + end + + # @param [Hash] original_params + # @return [Array] + def self.validate_original_params_against_schema(original_params) + workspace_error_details_schema = { + "required" => %w[error_type], + "properties" => { + "error_type" => { + "type" => "string", + "enum" => [ErrorType::APPLIER] + }, + "error_message" => { + "type" => "string" + } + } + } + workspace_agent_info_schema = { + "properties" => { + "termination_progress" => { + "type" => "string", + "enum" => [TerminationProgress::TERMINATING, TerminationProgress::TERMINATED] + }, + "error_details" => workspace_error_details_schema + } + } + + schema = { + "type" => "object", + "required" => %w[update_type workspace_agent_infos], + "properties" => { + "update_type" => { + "type" => "string", + "enum" => [PARTIAL, FULL] + }, + "workspace_agent_infos" => { + "type" => "array", + "items" => workspace_agent_info_schema + } + } + } + + schemer = JSONSchemer.schema(schema) + errors = schemer.validate(original_params) + errors.map { |error| JSONSchemer::Errors.pretty(error) } + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/main.rb b/ee/lib/remote_development/workspaces/reconcile/main.rb new file mode 100644 index 0000000000000000000000000000000000000000..d4f82235389ec01c922bbf80ed70af24fc8081e8 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/main.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + class Main + include Messages + + extend MessageSupport + private_class_method :generate_error_response_from_message + + # @param [Hash] value + # @return [Hash] + # @raise [UnmatchedResultError] + def self.main(value) + initial_result = Result.ok(value) + + result = + initial_result + .and_then(Input::ParamsValidator.method(:validate)) + .map(Input::ParamsExtractor.method(:extract)) + .map(Input::ParamsToInfosConverter.method(:convert)) + .map(Input::AgentInfosObserver.method(:observe)) + .map(Persistence::WorkspacesFromAgentInfosUpdater.method(:update)) + .map(Persistence::OrphanedWorkspacesObserver.method(:observe)) + .map(Persistence::WorkspacesToBeReturnedFinder.method(:find)) + .map(Output::WorkspacesToRailsInfosConverter.method(:convert)) + .map(Persistence::WorkspacesToBeReturnedUpdater.method(:update)) + .map(Output::RailsInfosObserver.method(:observe)) + .map( + # As the final step, return the workspace_rails_infos in a WorkspaceReconcileSuccessful message + ->(value) do + RemoteDevelopment::Messages::WorkspaceReconcileSuccessful.new( + workspace_rails_infos: value.fetch(:workspace_rails_infos) + ) + end + ) + + case result + in { err: WorkspaceReconcileParamsValidationFailed => message } + generate_error_response_from_message(message: message, reason: :bad_request) + in { ok: WorkspaceReconcileSuccessful => message } + message.context => { workspace_rails_infos: Array } # Type-check the payload before returning it + { status: :success, payload: message.context } + else + raise UnmatchedResultError.new(result: result) + end + end + end + 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 new file mode 100644 index 0000000000000000000000000000000000000000..0d12d379e0f9fdb8d5bb77e13442b354281f3316 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/output/desired_config_generator.rb @@ -0,0 +1,184 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Output + # rubocop:disable Layout/LineLength + # 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 DesiredConfigGenerator + include States + + # @param [RemoteDevelopment::Workspaces::Workspace] workspace + # @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) + labels, annotations = get_labels_and_annotations( + agent_id: agent.id, + owning_inventory: owning_inventory, + domain_template: domain_template, + workspace_id: workspace.id + ) + + # TODO: https://gitlab.com/groups/gitlab-org/-/epics/10461 - handle error + workspace_resources = DevfileParser.get_all( + processed_devfile: workspace.processed_devfile, + name: name, + namespace: namespace, + replicas: replicas, + domain_template: domain_template, + labels: labels, + annotations: annotations, + user: user, + 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? + + workspace_resources.insert(0, workspace_inventory_config_map) + + 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, + labels: labels, + annotations: annotations, + gitlab_workspaces_proxy_namespace: gitlab_workspaces_proxy_namespace + ) + workspace_resources.append(network_policy) + end + + workspace_resources + end + + # @param [String] desired_state + # @return [Integer] + def self.get_workspace_replicas(desired_state:) + return 1 if [ + CREATION_REQUESTED, + RUNNING + ].include?(desired_state) + + 0 + end + + # @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 = { + kind: 'ConfigMap', + apiVersion: 'v1', + metadata: { + name: owning_inventory, + namespace: namespace, + labels: { + 'cli-utils.sigs.k8s.io/inventory-id': owning_inventory, + '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 [Integer] workspace_id + # @return [Array<(Hash, Hash)>] + def self.get_labels_and_annotations(agent_id:, owning_inventory:, domain_template:, workspace_id:) + labels = { + 'agent.gitlab.com/id' => agent_id.to_s + } + 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 + } + [labels, annotations] + end + + # @param [String] name + # @param [String] namespace + # @param [Hash] labels + # @param [Hash] annotations + # @param [string] gitlab_workspaces_proxy_namespace + # @return [Hash] + def self.get_network_policy(name:, namespace:, labels:, annotations:, gitlab_workspaces_proxy_namespace:) + policy_types = [ + - "Ingress", + - "Egress" + ] + + proxy_namespace_selector = { + matchLabels: { + "kubernetes.io/metadata.name": gitlab_workspaces_proxy_namespace + } + } + proxy_pod_selector = { + matchLabels: { + "app.kubernetes.io/name": "gitlab-workspaces-proxy" + } + } + ingress = [{ from: [{ namespaceSelector: proxy_namespace_selector, podSelector: proxy_pod_selector }] }] + + kube_system_namespace_selector = { + matchLabels: { + "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 = [ + { to: [{ ipBlock: { cidr: "0.0.0.0/0", except: egress_except_cidr } }] }, + { + ports: [{ port: 53, protocol: "TCP" }, { port: 53, protocol: "UDP" }], + to: [{ namespaceSelector: kube_system_namespace_selector }] + } + ] + + { + apiVersion: "networking.k8s.io/v1", + kind: "NetworkPolicy", + metadata: { + annotations: annotations, + labels: labels, + name: name, + namespace: namespace + }, + spec: { + egress: egress, + ingress: ingress, + podSelector: {}, + policyTypes: policy_types + } + }.deep_stringify_keys.to_h + end + end + 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 new file mode 100644 index 0000000000000000000000000000000000000000..c239895a8e07407bbbf9729876358e8ec0840f02 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/output/devfile_parser.rb @@ -0,0 +1,154 @@ +# frozen_string_literal: true + +require 'devfile' + +module RemoteDevelopment + 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 DevfileParser + # @param [String] processed_devfile + # @param [String] name + # @param [String] namespace + # @param [Integer] replicas + # @param [String] domain_template + # @param [Hash] labels + # @param [Hash] annotations + # @param [User] user + # @param [RemoteDevelopment::Logger] logger + # @return [Array] + def self.get_all(processed_devfile:, name:, namespace:, replicas:, domain_template:, labels:, annotations:, user:, logger:) # rubocop:disable Metrics/ParameterLists + begin + workspace_resources_yaml = Devfile::Parser.get_all( + processed_devfile, + name, + namespace, + YAML.dump(labels.deep_stringify_keys), + YAML.dump(annotations.deep_stringify_keys), + replicas, + domain_template, + 'none' + ) + rescue Devfile::CliError => e + logger.warn( + message: 'Error parsing devfile with Devfile::Parser.get_all', + error_type: 'reconcile_devfile_parser_error', + workspace_name: name, + workspace_namespace: namespace, + devfile_parser_error: e.message + ) + return [] + end + + 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( + workspace_resources: workspace_resources, + domain_template: domain_template + ) + end + + # Devfile library allows specifying the security context of pods/containers as mentioned in + # https://github.com/devfile/api/issues/920 through `pod-overrides` and `container-overrides` attributes. + # However, https://github.com/devfile/library/pull/158 which is implementing this feature, + # is not part of v2.2.0 which is the latest release of the devfile which is being used in the devfile-gem. + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409189 + # Once devfile library releases a new version, update the devfile-gem and move + # the logic of setting the security context as part of workspace creation. + RUN_AS_USER = 5001 + + # @param [Array] workspace_resources + # @return [Array] + 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'] = { + 'runAsNonRoot' => true, + 'runAsUser' => RUN_AS_USER, + 'fsGroup' => 0, + 'fsGroupChangePolicy' => 'OnRootMismatch' + } + # 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 + } + 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 + } + end + end + workspace_resources + end + + # @param [Array] workspace_resources + # @param [User] user + # @return [Array] + def self.set_git_configuration(workspace_resources:, user:) + 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 + } + ]) + 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' + + pod_spec = workspace_resource['spec']['template']['spec'] + + pod_spec['initContainers'].each do |init_containers| + init_containers.fetch('env').concat(env_variables) + end + + pod_spec['containers'].each do |container| + container.fetch('env').concat(env_variables) + end + end + workspace_resources + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/output/rails_infos_observer.rb b/ee/lib/remote_development/workspaces/reconcile/output/rails_infos_observer.rb new file mode 100644 index 0000000000000000000000000000000000000000..ef06efdbd2d85a0b61374428d464117e8f0e73f6 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/output/rails_infos_observer.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Output + class RailsInfosObserver + # @param [Hash] value + # @return [Hash] + def self.observe(value) + value => { + agent: agent, # Skip type checking so we can use fast_spec_helper + update_type: String => update_type, + workspace_rails_infos: Array => workspace_rails_infos, + logger: logger, # Skip type checking to avoid coupling to Rails logger + } + + logger.debug( + message: 'Returning workspace_rails_infos', + agent_id: agent.id, + update_type: update_type, + count: workspace_rails_infos.length, + workspace_rails_infos: workspace_rails_infos.map do |rails_info| + rails_info.reject { |k, _| k == :config_to_apply } + end + ) + + value + end + end + end + end + end +end 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 new file mode 100644 index 0000000000000000000000000000000000000000..8b6eb9e4f3346df8433df7053b182d0234197763 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Output + # noinspection RubyClassModuleNamingConvention,RubyClassMethodNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + class WorkspacesToRailsInfosConverter + include Messages + include UpdateTypes + + # @param [Hash] value + # @return [Hash] + def self.convert(value) + value => { + update_type: String => update_type, + workspaces_to_be_returned: Array => workspaces_to_be_returned, + logger: logger + } + + # Create an array of workspace_rails_info hashes based on the workspaces. These indicate the desired updates + # to the workspace, which will be returned in the payload to the agent to be applied to kubernetes + workspace_rails_infos = workspaces_to_be_returned.map do |workspace| + workspace_rails_info = { + name: workspace.name, + namespace: workspace.namespace, + desired_state: workspace.desired_state, + actual_state: workspace.actual_state, + deployment_resource_version: workspace.deployment_resource_version, + # NOTE: config_to_apply should be returned as null if config_to_apply returned nil + config_to_apply: config_to_apply(workspace: workspace, update_type: update_type, logger: logger) + } + + workspace_rails_info + end + + value.merge(workspace_rails_infos: workspace_rails_infos) + end + + # @param [RemoteDevelopment::Workspace] workspace + # @param [String (frozen)] update_type + # @param [RemoteDevelopment::Logger] logger + # @return [nil, String] + def self.config_to_apply(workspace:, update_type:, logger:) + return unless should_include_config_to_apply?(update_type: update_type, workspace: workspace) + + workspace_resources = DesiredConfigGenerator.generate_desired_config(workspace: workspace, logger: logger) + + desired_config_to_apply_array = workspace_resources.map do |resource| + YAML.dump(resource.deep_stringify_keys) + end + + return unless desired_config_to_apply_array.present? + + desired_config_to_apply_array.join + end + + # @param [String (frozen)] update_type + # @param [RemoteDevelopment::Workspace] workspace + # @return [Boolean] + def self.should_include_config_to_apply?(update_type:, workspace:) + return true if update_type == FULL + + return true if workspace.desired_state_updated_more_recently_than_last_response_to_agent? + + false + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/params_parser.rb b/ee/lib/remote_development/workspaces/reconcile/params_parser.rb deleted file mode 100644 index bf27744fa135c4c48775d5769db6b1946da50296..0000000000000000000000000000000000000000 --- a/ee/lib/remote_development/workspaces/reconcile/params_parser.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require 'json_schemer' - -module RemoteDevelopment - module Workspaces - module Reconcile - class ParamsParser - include UpdateType - - # @param [Hash] params - # @return [Array<(Hash | nil, RemoteDevelopment::Error | nil)>] - def parse(params:) - error_message = validate_params(params) - - if error_message - error = Error.new( - message: error_message, - reason: :unprocessable_entity - ) - return [nil, error] - end - - parsed_params = { - workspace_agent_infos: params.fetch('workspace_agent_infos'), - update_type: params.fetch('update_type') - } - [ - parsed_params, - nil - ] - end - - private - - # @param [Hash] params - # @return [void, String] - def validate_params(params) - workspace_error_details_schema = { - "required" => %w[error_type], - "properties" => { - "error_type" => { - "type" => "string", - "enum" => [ErrorType::APPLIER] - }, - "error_message" => { - "type" => "string" - } - } - } - workspace_agent_info_schema = { - "properties" => { - "termination_progress" => { - "type" => "string", - "enum" => [TerminationProgress::TERMINATING, TerminationProgress::TERMINATED] - }, - "error_details" => workspace_error_details_schema - } - } - - schema = { - "type" => "object", - "required" => %w[update_type workspace_agent_infos], - "properties" => { - "update_type" => { - "type" => "string", - "enum" => [PARTIAL, FULL] - }, - "workspace_agent_infos" => { - "type" => "array", - "items" => workspace_agent_info_schema - } - } - } - schemer = JSONSchemer.schema(schema) - - errs = schemer.validate(params) - return if errs.none? - - errs.map { |err| JSONSchemer::Errors.pretty(err) }.join(". ") - end - end - end - end -end diff --git a/ee/lib/remote_development/workspaces/reconcile/persistence/orphaned_workspaces_observer.rb b/ee/lib/remote_development/workspaces/reconcile/persistence/orphaned_workspaces_observer.rb new file mode 100644 index 0000000000000000000000000000000000000000..05e8e75db5d559fd66d4e06a758f616dcd8087f8 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/persistence/orphaned_workspaces_observer.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Persistence + # noinspection RubyLocalVariableNamingConvention,RubyParameterNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + class OrphanedWorkspacesObserver + # @param [Hash] value + # @return [Hash] + def self.observe(value) + value => { + agent: agent, # Skip type checking so we can use fast_spec_helper + update_type: String => update_type, + workspace_agent_infos_by_name: Hash => workspace_agent_infos_by_name, + workspaces_from_agent_infos: Array => workspaces_from_agent_infos, + logger: logger, # Skip type checking to avoid coupling to Rails logger + } + + orphaned_workspace_agent_infos = detect_orphaned_workspaces( + workspace_agent_infos_by_name: workspace_agent_infos_by_name, + persisted_workspace_names: workspaces_from_agent_infos.map(&:name) + ) + + if orphaned_workspace_agent_infos.present? + logger.warn( + message: + "Received orphaned workspace agent info for workspace(s) where no persisted workspace record exists", + error_type: "orphaned_workspace", + agent_id: agent.id, + update_type: update_type, + count: orphaned_workspace_agent_infos.length, + orphaned_workspaces: orphaned_workspace_agent_infos.map do |agent_info| + { + name: agent_info.name, + namespace: agent_info.namespace, + actual_state: agent_info.actual_state + } + end + ) + end + + value + end + + # @param [Hash] workspace_agent_infos_by_name + # @param [Array] persisted_workspace_names + # @return [Array] + def self.detect_orphaned_workspaces(workspace_agent_infos_by_name:, persisted_workspace_names:) + workspace_agent_infos_by_name.reject do |name, _| + persisted_workspace_names.include?(name.to_s) + end.values + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/persistence/workspaces_from_agent_infos_updater.rb b/ee/lib/remote_development/workspaces/reconcile/persistence/workspaces_from_agent_infos_updater.rb new file mode 100644 index 0000000000000000000000000000000000000000..9120707cc353b82a18c093a093505fe9dd3c4cbb --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/persistence/workspaces_from_agent_infos_updater.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Persistence + # rubocop:disable Layout/LineLength + # noinspection RubyLocalVariableNamingConvention,RubyClassModuleNamingConvention,RubyClassMethodNamingConvention,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 + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + class WorkspacesFromAgentInfosUpdater + # @param [Hash] value + # @return [Hash] + def self.update(value) + value => { + agent: agent, # Skip type checking to avoid coupling to Rails monolith + workspace_agent_infos_by_name: Hash => workspace_agent_infos_by_name, + } + + workspaces_from_agent_infos = agent.workspaces.where(name: workspace_agent_infos_by_name.keys).to_a # rubocop:disable CodeReuse/ActiveRecord + + # Update persisted workspaces which match the names of the workspaces in the AgentInfo objects array + workspaces_from_agent_infos.each do |persisted_workspace| + workspace_agent_info = workspace_agent_infos_by_name.fetch(persisted_workspace.name.to_sym) + # Update the persisted workspaces with the latest info from the AgentInfo objects we received + update_persisted_workspace_with_latest_info( + persisted_workspace: persisted_workspace, + deployment_resource_version: workspace_agent_info.deployment_resource_version, + actual_state: workspace_agent_info.actual_state + ) + end + + value.merge( + workspaces_from_agent_infos: workspaces_from_agent_infos + ) + end + + # @param [RemoteDevelopment::Workspace] persisted_workspace + # @param [String] deployment_resource_version + # @param [String] actual_state + # @return [void] + def self.update_persisted_workspace_with_latest_info( + persisted_workspace:, + deployment_resource_version:, + actual_state: + ) + # Handle the special case of RESTART_REQUESTED. desired_state is only set to 'RESTART_REQUESTED' until the + # actual_state is detected as 'STOPPED', then we switch the desired_state to 'RUNNING' so it will restart. + # See: https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/blob/main/doc/architecture.md?plain=0#possible-desired_state-values + if persisted_workspace.desired_state == States::RESTART_REQUESTED && actual_state == States::STOPPED + persisted_workspace.desired_state = States::RUNNING + end + + # Ensure workspaces are terminated after max time-to-live. This is a temporary approach, we eventually want + # to replace this with some mechanism to detect workspace activity and only shut down inactive workspaces. + # Until then, this is the workaround to ensure workspaces don't live indefinitely. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/390597 + if persisted_workspace.created_at + persisted_workspace.max_hours_before_termination.hours < Time.current + persisted_workspace.desired_state = States::TERMINATED + end + + persisted_workspace.actual_state = actual_state + + # In some cases a deployment resource version may not be present, e.g. if the initial creation request for + # workspace creation resulted in an Error. + persisted_workspace.deployment_resource_version = deployment_resource_version if deployment_resource_version + + persisted_workspace.save! + + nil + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_finder.rb b/ee/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..baa94c3873eb427ae6eee0d9bdc978eff5e81aa3 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_finder.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Persistence + # rubocop:disable Layout/LineLength + # noinspection RubyLocalVariableNamingConvention,RubyClassMethodNamingConvention,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 WorkspacesToBeReturnedFinder + include UpdateTypes + + # @param [Hash] value + # @return [Hash] + def self.find(value) + value => { + agent: agent, # Skip type checking to avoid coupling to Rails monolith + update_type: String => update_type, + workspaces_from_agent_infos: Array => workspaces_from_agent_infos, + } + + workspaces_to_be_returned_query = + generate_workspaces_to_be_returned_query( + agent: agent, + update_type: update_type, + workspaces_from_agent_infos: workspaces_from_agent_infos + ) + + workspaces_to_be_returned = workspaces_to_be_returned_query.to_a + + value.merge( + workspaces_to_be_returned: workspaces_to_be_returned + ) + end + + # @param [Clusters::Agent] agent + # @param [String] update_type + # @param [Array] workspaces_from_agent_infos + # @return [ActiveRecord::Relation] + def self.generate_workspaces_to_be_returned_query(agent:, update_type:, workspaces_from_agent_infos:) + # For a FULL update, return all workspaces for the agent which exist in the database + return agent.workspaces.all if update_type == FULL + + # For a PARTIAL update, return: + # 1. Workspaces with_desired_state_updated_more_recently_than_last_response_to_agent + # 2. Workspaces which we received from the agent in the agent_infos array + workspaces_from_agent_infos_ids = workspaces_from_agent_infos.map(&:id) + agent + .workspaces + .with_desired_state_updated_more_recently_than_last_response_to_agent + .or(agent.workspaces.id_in(workspaces_from_agent_infos_ids)) + .ordered_by_id + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_updater.rb b/ee/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_updater.rb new file mode 100644 index 0000000000000000000000000000000000000000..dfccf2c20278f6a384de60c0e58fee15d99b11b7 --- /dev/null +++ b/ee/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_updater.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Reconcile + module Persistence + class WorkspacesToBeReturnedUpdater + # @param [Hash] value + # @return [Hash] + def self.update(value) + value => { + agent: agent, # Skip type checking to avoid coupling to Rails monolith + workspaces_to_be_returned: Array => workspaces_to_be_returned, + } + + # Update the responded_to_agent_at at this point, after we have already done all the calculations + # related to state. Do it as a single query, so that they will all have the same timestamp. + + workspaces_to_be_returned_ids = workspaces_to_be_returned.map(&:id) + + agent.workspaces.where(id: workspaces_to_be_returned_ids).touch_all(:responded_to_agent_at) # rubocop:disable CodeReuse/ActiveRecord + + value + end + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/reconcile/reconcile_processor.rb b/ee/lib/remote_development/workspaces/reconcile/reconcile_processor.rb deleted file mode 100644 index 05c34fdabde2cb270274e77bd6c62192e3a248df..0000000000000000000000000000000000000000 --- a/ee/lib/remote_development/workspaces/reconcile/reconcile_processor.rb +++ /dev/null @@ -1,230 +0,0 @@ -# frozen_string_literal: true - -module RemoteDevelopment - module Workspaces - module Reconcile - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - # rubocop:disable Layout/LineLength - # noinspection RubyInstanceMethodNamingConvention,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 ReconcileProcessor - include UpdateType - - # rubocop:disable Metrics/AbcSize - # @param [Clusters::Agent] agent - # @param [Array] workspace_agent_infos - # @param [String] update_type - # @return [Array<(Hash | nil, RemoteDevelopment::Error | nil)>] - def process(agent:, workspace_agent_infos:, update_type:) - logger.debug( - message: 'Beginning ReconcileProcessor', - agent_id: agent.id, - update_type: update_type - ) - # parse an array of AgentInfo objects from the workspace_agent_infos array - workspace_agent_infos_by_name = workspace_agent_infos.each_with_object({}) do |workspace_agent_info, hash| - info = AgentInfoParser.new.parse(workspace_agent_info: workspace_agent_info) - hash[info.name] = info - - next unless [States::UNKNOWN, States::ERROR].include? info.actual_state - - logger.warn( - message: 'Abnormal workspace actual_state', - error_type: 'abnormal_workspace_state', - actual_state: info.actual_state, - workspace_deployment_status: workspace_agent_info['latest_k8s_deployment_info']&.fetch('status', {}).to_s - ) - end - names_from_agent_infos = workspace_agent_infos_by_name.keys - - logger.debug( - message: 'Parsed workspaces from workspace_agent_infos', - agent_id: agent.id, - update_type: update_type, - count: names_from_agent_infos.length, - workspace_agent_infos: workspace_agent_infos_by_name.values.map do |agent_info| - { - name: agent_info.name, - namespace: agent_info.namespace, - actual_state: agent_info.actual_state, - deployment_resource_version: agent_info.deployment_resource_version - } - end - ) - - persisted_workspaces_from_agent_infos = agent.workspaces.where(name: names_from_agent_infos) # rubocop:disable CodeReuse/ActiveRecord - - check_for_orphaned_workspaces( - workspace_agent_infos_by_name: workspace_agent_infos_by_name, - persisted_workspace_names: persisted_workspaces_from_agent_infos.map(&:name), - agent_id: agent.id, - update_type: update_type - ) - - # Update persisted workspaces which match the names of the workspaces in the AgentInfo objects array - persisted_workspaces_from_agent_infos.each do |persisted_workspace| - workspace_agent_info = workspace_agent_infos_by_name[persisted_workspace.name] - # Update the persisted workspaces with the latest info from the AgentInfo objects we received - update_persisted_workspace_with_latest_info( - persisted_workspace: persisted_workspace, - deployment_resource_version: workspace_agent_info.deployment_resource_version, - actual_state: workspace_agent_info.actual_state - ) - end - - if update_type == FULL - # For a FULL update, return all workspaces for the agent which exist in the database - workspaces_to_return_in_rails_infos_query = agent.workspaces.all - else - # For a PARTIAL update, return: - # 1. Workspaces with_desired_state_updated_more_recently_than_last_response_to_agent - # 2. Workspaces which we received from the agent in the agent_infos array - workspaces_from_agent_infos_ids = persisted_workspaces_from_agent_infos.map(&:id) - workspaces_to_return_in_rails_infos_query = - agent - .workspaces - .with_desired_state_updated_more_recently_than_last_response_to_agent - .or(agent.workspaces.id_in(workspaces_from_agent_infos_ids)) - end - - workspaces_to_return_in_rails_infos = workspaces_to_return_in_rails_infos_query.to_a - - # Create an array workspace_rails_info hashes based on the workspaces. These indicate the desired updates - # to the workspace, which will be returned in the payload to the agent to be applied to kubernetes - workspace_rails_infos = workspaces_to_return_in_rails_infos.map do |workspace| - workspace_rails_info = { - name: workspace.name, - namespace: workspace.namespace, - desired_state: workspace.desired_state, - actual_state: workspace.actual_state, - deployment_resource_version: workspace.deployment_resource_version, - # NOTE: config_to_apply will be null if there is no config to apply, i.e. if a guard clause returned false - config_to_apply: config_to_apply(workspace: workspace, update_type: update_type) - } - - workspace_rails_info - end - - # Update the responded_to_agent_at at this point, after we have already done all the calculations - # related to state. Do it outside of the loop so it will be a single query, and also so that they - # will all have the same timestamp. - workspaces_to_return_in_rails_infos_query.touch_all(:responded_to_agent_at) - - payload = { workspace_rails_infos: workspace_rails_infos } - - logger.debug( - message: 'Returning workspace_rails_infos', - agent_id: agent.id, - update_type: update_type, - count: workspace_rails_infos.length, - workspace_rails_infos: workspace_rails_infos.map do |rails_info| - { - name: rails_info.fetch(:name), - namespace: rails_info.fetch(:namespace), - desired_state: rails_info.fetch(:desired_state), - actual_state: rails_info.fetch(:actual_state), - deployment_resource_version: rails_info.fetch(:deployment_resource_version) - } - end - ) - - [payload, nil] - end - # rubocop:enable Metrics/AbcSize - - private - - # @param [RemoteDevelopment::Workspace] workspace - # @param [String (frozen)] update_type - # @return [void, String] - def config_to_apply(workspace:, update_type:) - # NOTE: If update_type==FULL, we always return the config. - return if update_type == PARTIAL && - !workspace.desired_state_updated_more_recently_than_last_response_to_agent? - - workspace_resources = DesiredConfigGenerator.new.generate_desired_config(workspace: workspace) - - desired_config_to_apply_array = workspace_resources.map do |resource| - YAML.dump(resource.deep_stringify_keys) - end - - return unless desired_config_to_apply_array.present? - - desired_config_to_apply_array.join - end - - # @param [Hash] workspace_agent_infos_by_name - # @param [Array] persisted_workspace_names - # @param [Integer] agent_id - # @param [String] update_type - # @return [void] - def check_for_orphaned_workspaces( - workspace_agent_infos_by_name:, - persisted_workspace_names:, - agent_id:, - update_type: - ) - orphaned_workspace_agent_infos = workspace_agent_infos_by_name.reject do |name, _| - persisted_workspace_names.include?(name) - end.values - - return unless orphaned_workspace_agent_infos.present? - - logger.warn( - message: - 'Received orphaned workspace agent info for workspace(s) where no persisted workspace record exists', - error_type: 'orphaned_workspace', - agent_id: agent_id, - update_type: update_type, - count: orphaned_workspace_agent_infos.length, - orphaned_workspace_names: orphaned_workspace_agent_infos.map(&:name), - orphaned_workspace_namespaces: orphaned_workspace_agent_infos.map(&:namespace) - ) - nil - end - - # @param [RemoteDevelopment::Workspace] persisted_workspace - # @param [String] deployment_resource_version - # @param [String] actual_state - # @return [void] - def update_persisted_workspace_with_latest_info( - persisted_workspace:, - deployment_resource_version:, - actual_state: - ) - # Handle the special case of RESTART_REQUESTED. desired_state is only set to 'RESTART_REQUESTED' until the - # actual_state is detected as 'STOPPED', then we switch the desired_state to 'RUNNING' so it will restart. - # See: https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/blob/main/doc/architecture.md?plain=0#possible-desired_state-values - if persisted_workspace.desired_state == States::RESTART_REQUESTED && actual_state == States::STOPPED - persisted_workspace.desired_state = States::RUNNING - end - - # Ensure workspaces are terminated after a max time-to-live. This is a temporary approach, we eventually want - # to replace this with some mechanism to detect workspace activity and only shut down inactive workspaces. - # Until then, this is the workaround to ensure workspaces don't live indefinitely. - # See https://gitlab.com/gitlab-org/gitlab/-/issues/390597 - if persisted_workspace.created_at + persisted_workspace.max_hours_before_termination.hours < Time.current - persisted_workspace.desired_state = States::TERMINATED - end - - persisted_workspace.actual_state = actual_state - - # In some cases a deployment resource version may not be present, e.g. if the initial creation request for the - # workspace creation resulted in an Error. - persisted_workspace.deployment_resource_version = deployment_resource_version if deployment_resource_version - - persisted_workspace.save! - - nil - end - - # TODO: https://gitlab.com/groups/gitlab-org/-/epics/10461 - # Dry up memoized logger factory to a shared concern - # @return [RemoteDevelopment::Logger] - def logger - @logger ||= RemoteDevelopment::Logger.build - end - end - end - end -end diff --git a/ee/lib/remote_development/workspaces/reconcile/update_type.rb b/ee/lib/remote_development/workspaces/reconcile/update_types.rb similarity index 87% rename from ee/lib/remote_development/workspaces/reconcile/update_type.rb rename to ee/lib/remote_development/workspaces/reconcile/update_types.rb index 073fd0a4c84c67641b10c5cb1f012eecfc28ec19..d4103febf67ae5bf20650cf4998892f67eb55eca 100644 --- a/ee/lib/remote_development/workspaces/reconcile/update_type.rb +++ b/ee/lib/remote_development/workspaces/reconcile/update_types.rb @@ -3,7 +3,7 @@ module RemoteDevelopment module Workspaces module Reconcile - module UpdateType + module UpdateTypes PARTIAL = 'partial' FULL = 'full' end diff --git a/ee/spec/factories/remote_development/workspaces.rb b/ee/spec/factories/remote_development/workspaces.rb index 405e2160dbdd11c6106056da0cce7318b157bdc5..498a7555c3de2340a53297332ac89995832cdfb3 100644 --- a/ee/spec/factories/remote_development/workspaces.rb +++ b/ee/spec/factories/remote_development/workspaces.rb @@ -68,11 +68,12 @@ end after(:create) do |workspace, evaluator| - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-25400 if evaluator.skip_realistic_after_create_timestamp_updates # Set responded_to_agent_at to a non-nil value unless it has already been set + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 workspace.update!(responded_to_agent_at: workspace.updated_at) unless workspace.responded_to_agent_at - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-25400 elsif evaluator.is_after_reconciliation_finish # The most recent activity was reconciliation where info for the workspace was reported to the agent # This DOES NOT necessarily mean that the actual and desired states for the workspace are now the same diff --git a/ee/spec/features/remote_development/workspaces_spec.rb b/ee/spec/features/remote_development/workspaces_spec.rb index dce636085a2bce6d7f792ed0c28eb17b39ff0676..2bca4e6cef639ce9f8520d29e8a4c85ba21fb74c 100644 --- a/ee/spec/features/remote_development/workspaces_spec.rb +++ b/ee/spec/features/remote_development/workspaces_spec.rb @@ -160,7 +160,7 @@ def simulate_second_poll(id:, name:, namespace:) # SIMULATE SECOND POLL REQUEST FROM AGENTK TO UPDATE WORKSPACE TO RUNNING STATE resource_version = '1' - workspace_agent_info = create_workspace_agent_info( + workspace_agent_info = create_workspace_agent_info_hash( workspace_id: id, workspace_name: name, workspace_namespace: namespace, @@ -195,7 +195,7 @@ def simulate_third_poll(id:, name:, namespace:) # SIMULATE THIRD POLL REQUEST FROM AGENTK TO UPDATE WORKSPACE TO STOPPING STATE resource_version = '1' - workspace_agent_info = create_workspace_agent_info( + workspace_agent_info = create_workspace_agent_info_hash( workspace_id: id, workspace_name: name, workspace_namespace: namespace, @@ -243,7 +243,7 @@ def simulate_fourth_poll(id:, name:, namespace:) # SIMULATE FOURTH POLL REQUEST FROM AGENTK TO UPDATE WORKSPACE TO STOPPED STATE resource_version = '2' - workspace_agent_info = create_workspace_agent_info( + workspace_agent_info = create_workspace_agent_info_hash( workspace_id: id, workspace_name: name, workspace_namespace: namespace, diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/devfile_parser_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/devfile_parser_spec.rb deleted file mode 100644 index bae91592013125d1d298cc98db6a14d9b815b833..0000000000000000000000000000000000000000 --- a/ee/spec/lib/remote_development/workspaces/reconcile/devfile_parser_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe RemoteDevelopment::Workspaces::Reconcile::DevfileParser, feature_category: :remote_development do - include_context 'with remote development shared fixtures' - - let_it_be(:user) { create(:user) } - let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } - let_it_be(:workspace) { create(:workspace, agent: agent, user: user) } - let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } - - let(:domain_template) { "{{.port}}-#{workspace.name}.#{workspace.dns_zone}" } - - describe '#get_all' do - let(:expected_workspace_resources) do - YAML.load_stream( - create_config_to_apply( - workspace_id: workspace.id, - workspace_name: workspace.name, - workspace_namespace: workspace.namespace, - agent_id: workspace.agent.id, - owning_inventory: owning_inventory, - started: true, - include_inventory: false, - include_network_policy: false, - user_name: user.name, - user_email: user.email - ) - ) - end - - subject do - described_class.new - end - - it 'returns workspace_resources' do - workspace_resources = subject.get_all( - processed_devfile: example_processed_devfile, - name: workspace.name, - namespace: workspace.namespace, - replicas: 1, - domain_template: domain_template, - labels: { 'agent.gitlab.com/id' => workspace.agent.id }, - annotations: { - 'config.k8s.io/owning-inventory' => owning_inventory, - 'workspaces.gitlab.com/host-template' => domain_template, - 'workspaces.gitlab.com/id' => workspace.id - }, - user: user - ) - - expect(workspace_resources).to eq(expected_workspace_resources) - end - end - - context "when Devfile::CliError is raised" do - let(:logger) { instance_double(RemoteDevelopment::Logger) } - - before do - allow(Devfile::Parser).to receive(:get_all).and_raise(Devfile::CliError.new("some error")) - allow(RemoteDevelopment::Logger).to receive(:build) { logger } - end - - it "logs the error" do - expect(logger).to receive(:warn).with( - message: 'Error parsing devfile with Devfile::Parser.get_all', - error_type: 'reconcile_devfile_parser_error', - workspace_name: workspace.name, - workspace_namespace: workspace.namespace, - devfile_parser_error: "some error" - ) - - workspace_resources = subject.get_all( - processed_devfile: "", - name: workspace.name, - namespace: workspace.namespace, - replicas: 1, - domain_template: "", - labels: {}, - annotations: {}, - user: user - ) - - expect(workspace_resources).to eq([]) - end - end -end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/actual_state_calculator_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/input/actual_state_calculator_spec.rb similarity index 89% rename from ee/spec/lib/remote_development/workspaces/reconcile/actual_state_calculator_spec.rb rename to ee/spec/lib/remote_development/workspaces/reconcile/input/actual_state_calculator_spec.rb index 3bed53c7555a0e50e327ec7edb45f16b99819d41..d78852b3de1b8b9e8c57d82469ac620211aae4c2 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/actual_state_calculator_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/input/actual_state_calculator_spec.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true -require 'spec_helper' +require_relative '../../../fast_spec_helper' -RSpec.describe RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator, feature_category: :remote_development do +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator, feature_category: :remote_development do include_context 'with remote development shared fixtures' describe '.calculate_actual_state' do subject do - described_class.new + described_class end context 'with cases parameterized from shared fixtures' do @@ -15,7 +15,7 @@ [ # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409783 # These are currently taken from only the currently supported cases in - # remote_development_shared_contexts.rb#create_workspace_agent_info, + # remote_development_shared_contexts.rb#create_workspace_agent_info_hash, # but we should ensure they are providing full and # realistic coverage of all possible relevant states. # Note that `nil` is passed when the argument will not be used by @@ -43,7 +43,7 @@ with_them do let(:latest_k8s_deployment_info) do - workspace_agent_info = create_workspace_agent_info( + workspace_agent_info_hash = create_workspace_agent_info_hash( workspace_id: 1, workspace_name: 'name', workspace_namespace: 'namespace', @@ -56,7 +56,7 @@ user_name: "does not matter", user_email: "does@not.matter" ) - workspace_agent_info.fetch('latest_k8s_deployment_info') + workspace_agent_info_hash.fetch(:latest_k8s_deployment_info) end it 'calculates correct actual state' do @@ -77,7 +77,7 @@ end # NOTE: The remaining examples below in this file existed before we added the RSpec parameterized - # section above with tests based on create_workspace_agent_info. Some of them may be + # section above with tests based on create_workspace_agent_info_hash. Some of them may be # redundant now. context 'when the deployment is completed successfully' do @@ -95,8 +95,8 @@ type: Available - reason: NewReplicaSetAvailable type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -118,8 +118,8 @@ type: Available - reason: NewReplicaSetAvailable type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -141,8 +141,8 @@ type: Available - reason: NewReplicaSetAvailable type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -164,8 +164,8 @@ conditions: - reason: NewReplicaSetCreated type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -185,8 +185,8 @@ conditions: - reason: FoundNewReplicaSet type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -206,8 +206,8 @@ conditions: - reason: ReplicaSetUpdated type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -227,8 +227,8 @@ conditions: - reason: ReplicaSetUpdated type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -248,8 +248,8 @@ conditions: - reason: ReplicaSetUpdated type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -269,8 +269,8 @@ conditions: - reason: test type: test - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -295,8 +295,8 @@ - reason: ProgressDeadlineExceeded type: Progressing unavailableReplicas: 1 - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -319,8 +319,8 @@ - reason: NewReplicaSetAvailable type: Progressing unavailableReplicas: 1 - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -344,8 +344,8 @@ conditions: - reason: ReplicaSetUpdated type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -364,8 +364,8 @@ conditions: - reason: ReplicaSetUpdated type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -380,8 +380,8 @@ <<~WORKSPACE_STATUS_YAML spec: replicas: 0 - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -400,8 +400,8 @@ test: - reason: ReplicaSetUpdated type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -419,8 +419,8 @@ status: conditions: - type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -441,8 +441,8 @@ type: Available - reason: unrecognized type: Progressing - WORKSPACE_STATUS_YAML - ) + WORKSPACE_STATUS_YAML + ).deep_symbolize_keys end it 'returns the expected actual state' do @@ -454,7 +454,9 @@ context 'when termination_progress is Terminating' do let(:expected_actual_state) { RemoteDevelopment::Workspaces::States::TERMINATING } - let(:termination_progress) { RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator::TERMINATING } + let(:termination_progress) do + RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator::TERMINATING + end it 'returns the expected actual state' do expect( @@ -468,7 +470,9 @@ context 'when termination_progress is Terminated' do let(:expected_actual_state) { RemoteDevelopment::Workspaces::States::TERMINATED } - let(:termination_progress) { RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator::TERMINATED } + let(:termination_progress) do + RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator::TERMINATED + end it 'returns the expected actual state' do expect( @@ -483,9 +487,9 @@ context 'when latest_error_details is present' do let(:latest_error_details) do { - "error_details" => { - "error_type" => RemoteDevelopment::Workspaces::Reconcile::ErrorType::APPLIER, - "error_details" => "error encountered while applying k8s configs" + error_details: { + error_type: RemoteDevelopment::Workspaces::Reconcile::ErrorType::APPLIER, + error_details: "error encountered while applying k8s configs" } } end @@ -511,7 +515,8 @@ expect( subject.calculate_actual_state( latest_k8s_deployment_info: nil, - termination_progress: RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator::TERMINATED, + termination_progress: + RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator::TERMINATED, latest_error_details: latest_error_details ) ).to be(expected_actual_state) @@ -525,7 +530,8 @@ expect( subject.calculate_actual_state( latest_k8s_deployment_info: nil, - termination_progress: RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator::TERMINATING, + termination_progress: + RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator::TERMINATING, latest_error_details: latest_error_details ) ).to be(expected_actual_state) diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/agent_info_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/input/agent_info_spec.rb similarity index 82% rename from ee/spec/lib/remote_development/workspaces/reconcile/agent_info_spec.rb rename to ee/spec/lib/remote_development/workspaces/reconcile/input/agent_info_spec.rb index e98fb9816d09a6f80958a28918dca5bf8833d6bb..bd2a6811aab2d5769863d14e2b37da468d83d749 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/agent_info_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/input/agent_info_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require_relative '../../../fast_spec_helper' -RSpec.describe RemoteDevelopment::Workspaces::Reconcile::AgentInfo, feature_category: :remote_development do +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo, feature_category: :remote_development do let(:agent_info_constructor_args) do { name: 'name', diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/input/agent_infos_observer_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/input/agent_infos_observer_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e50ba1477a5caee0768fe898353592aaa64ccd3b --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/input/agent_infos_observer_spec.rb @@ -0,0 +1,257 @@ +# frozen_string_literal: true + +require_relative '../../../fast_spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfosObserver, feature_category: :remote_development do + let(:agent) { instance_double("Clusters::Agent", id: 1) } + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::PARTIAL } + let(:logger) { instance_double(::Logger) } + let(:normal_agent_info) do + RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo.new( + name: "normal_workspace", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::STARTING, + deployment_resource_version: "1" + ) + end + + let(:abnormal_agent_info1) do + RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo.new( + name: "abnormal_workspace1", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::ERROR, + deployment_resource_version: "1" + ) + end + + let(:abnormal_agent_info2) do + RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo.new( + name: "abnormal_workspace2", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::UNKNOWN, + deployment_resource_version: "1" + ) + end + + let(:value) do + { + agent: agent, + update_type: update_type, + workspace_agent_infos_by_name: workspace_agent_infos_by_name, + logger: logger + } + end + + subject do + described_class.observe(value) + end + + context "when normal and abnormal workspaces exist" do + let(:workspace_agent_infos_by_name) do + { + normal_workspace: normal_agent_info, + abnormal_workspace1: abnormal_agent_info1, + abnormal_workspace2: abnormal_agent_info2 + } + end + + before do + allow(logger).to receive(:debug) + allow(logger).to receive(:warn) + end + + it "logs normal workspaces at debug level", :unlimited_max_formatted_output_length do + expect(logger).to receive(:debug).with( + message: "Parsed 3 total workspace agent infos from params, " \ + "with 1 in a NORMAL actual_state and 2 in an ABNORMAL actual_state", + agent_id: agent.id, + update_type: update_type, + actual_state_type: RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfosObserver::NORMAL, + total_count: 3, + normal_count: 1, + abnormal_count: 2, + normal_agent_infos: [ + { + name: "normal_workspace", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::STARTING, + deployment_resource_version: "1" + } + ], + abnormal_agent_infos: [ + { + name: "abnormal_workspace1", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::ERROR, + deployment_resource_version: "1" + }, + { + name: "abnormal_workspace2", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::UNKNOWN, + deployment_resource_version: "1" + } + ] + ) + + expect(subject).to eq(value) + end + + it "logs abnormal workspaces at warn level", :unlimited_max_formatted_output_length do + expect(logger).to receive(:warn).with( + message: "Parsed 2 workspace agent infos with an ABNORMAL actual_state from params (total: 3)", + error_type: "abnormal_actual_state", + agent_id: agent.id, + update_type: update_type, + actual_state_type: RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfosObserver::ABNORMAL, + total_count: 3, + normal_count: 1, + abnormal_count: 2, + abnormal_agent_infos: [ + { + name: "abnormal_workspace1", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::ERROR, + deployment_resource_version: "1" + }, + { + name: "abnormal_workspace2", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::UNKNOWN, + deployment_resource_version: "1" + } + ] + ) + + expect(subject).to eq(value) + end + end + + context "when only normal workspaces exist" do + let(:workspace_agent_infos_by_name) do + { + normal_workspace: normal_agent_info + } + end + + it "logs normal workspaces at debug level", :unlimited_max_formatted_output_length do + expect(logger).to receive(:debug).with( + message: "Parsed 1 total workspace agent infos from params, " \ + "with 1 in a NORMAL actual_state and 0 in an ABNORMAL actual_state", + agent_id: agent.id, + update_type: update_type, + actual_state_type: RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfosObserver::NORMAL, + total_count: 1, + normal_count: 1, + abnormal_count: 0, + normal_agent_infos: [ + { + name: "normal_workspace", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::STARTING, + deployment_resource_version: "1" + } + ], + abnormal_agent_infos: [] + ) + + expect(subject).to eq(value) + end + end + + context "when only abnormal workspaces exist" do + let(:workspace_agent_infos_by_name) do + { + abnormal_workspace1: abnormal_agent_info1, + abnormal_workspace2: abnormal_agent_info2 + } + end + + before do + allow(logger).to receive(:debug) + allow(logger).to receive(:warn) + end + + it "logs zero normal workspaces at debug level", :unlimited_max_formatted_output_length do + expect(logger).to receive(:debug).with( + message: "Parsed 2 total workspace agent infos from params, " \ + "with 0 in a NORMAL actual_state and 2 in an ABNORMAL actual_state", + agent_id: agent.id, + update_type: update_type, + actual_state_type: RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfosObserver::NORMAL, + total_count: 2, + normal_count: 0, + abnormal_count: 2, + normal_agent_infos: [], + abnormal_agent_infos: [ + { + name: "abnormal_workspace1", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::ERROR, + deployment_resource_version: "1" + }, + { + name: "abnormal_workspace2", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::UNKNOWN, + deployment_resource_version: "1" + } + ] + ) + + expect(subject).to eq(value) + end + + it "logs abnormal workspaces at warn level", :unlimited_max_formatted_output_length do + expect(logger).to receive(:warn).with( + message: "Parsed 2 workspace agent infos with an ABNORMAL actual_state from params (total: 2)", + error_type: "abnormal_actual_state", + agent_id: agent.id, + update_type: update_type, + actual_state_type: RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfosObserver::ABNORMAL, + total_count: 2, + normal_count: 0, + abnormal_count: 2, + abnormal_agent_infos: [ + { + name: "abnormal_workspace1", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::ERROR, + deployment_resource_version: "1" + }, + { + name: "abnormal_workspace2", + namespace: "namespace", + actual_state: RemoteDevelopment::Workspaces::States::UNKNOWN, + deployment_resource_version: "1" + } + ] + ) + + expect(subject).to eq(value) + end + end + + context "when there are no workspaces" do + let(:workspace_agent_infos_by_name) { {} } + + it "still works" do + expect(logger).not_to receive(:warn) + + expect(logger).to receive(:debug).with( + message: "Parsed 0 total workspace agent infos from params, " \ + "with 0 in a NORMAL actual_state and 0 in an ABNORMAL actual_state", + agent_id: agent.id, + update_type: update_type, + actual_state_type: RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfosObserver::NORMAL, + total_count: 0, + normal_count: 0, + abnormal_count: 0, + normal_agent_infos: [], + abnormal_agent_infos: [] + ) + + expect(subject).to eq(value) + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/agent_info_parser_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/input/factory_spec.rb similarity index 68% rename from ee/spec/lib/remote_development/workspaces/reconcile/agent_info_parser_spec.rb rename to ee/spec/lib/remote_development/workspaces/reconcile/input/factory_spec.rb index f90284565be71590fda9110e9b398f4edf464fdc..cc4311ffcd042f5429eb7619af5bec25599124da 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/agent_info_parser_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/input/factory_spec.rb @@ -1,22 +1,23 @@ # frozen_string_literal: true -require 'spec_helper' +require_relative '../../../fast_spec_helper' -RSpec.describe RemoteDevelopment::Workspaces::Reconcile::AgentInfoParser, feature_category: :remote_development do +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Input::Factory, feature_category: :remote_development do include_context 'with remote development shared fixtures' - let(:user) { create(:user) } - let(:workspace) { create(:workspace) } - let(:namespace) { workspace.namespace } + let(:namespace) { "namespace" } + let(:agent) { instance_double("Clusters::Agent", id: 1) } + let(:user) { instance_double("User", name: "name", email: "name@example.com") } + let(:workspace) { instance_double("RemoteDevelopment::Workspace", id: 1, name: "name", namespace: namespace) } - let(:workspace_agent_info) do - create_workspace_agent_info( + let(:workspace_agent_info_hash) do + create_workspace_agent_info_hash( workspace_id: workspace.id, workspace_name: workspace.name, workspace_namespace: namespace, - agent_id: workspace.agent.id, + agent_id: agent.id, owning_inventory: "#{workspace.name}-workspace-inventory", - resource_version: '1', + resource_version: "1", previous_actual_state: previous_actual_state, current_actual_state: current_actual_state, workspace_exists: false, @@ -26,10 +27,10 @@ end let(:expected_namespace) { workspace.namespace } - let(:expected_deployment_resource_version) { '1' } + let(:expected_deployment_resource_version) { "1" } let(:expected_agent_info) do - ::RemoteDevelopment::Workspaces::Reconcile::AgentInfo.new( + ::RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo.new( name: workspace.name, namespace: expected_namespace, actual_state: current_actual_state, @@ -38,14 +39,14 @@ end subject do - described_class.new.parse(workspace_agent_info: workspace_agent_info) + described_class.build(agent_info_hash_from_params: workspace_agent_info_hash) end before do - allow_next_instance_of(::RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator) do |instance| + allow_next_instance_of(::RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator) do |instance| # rubocop:disable RSpec/ExpectInHook expect(instance).to receive(:calculate_actual_state).with( - latest_k8s_deployment_info: workspace_agent_info['latest_k8s_deployment_info'], + latest_k8s_deployment_info: workspace_agent_info_hash[:latest_k8s_deployment_info], termination_progress: termination_progress, latest_error_details: nil ) { current_actual_state } @@ -53,7 +54,7 @@ end end - describe '#parse' do + describe '#build' do context 'when current actual state is not Terminated or Unknown' do let(:previous_actual_state) { ::RemoteDevelopment::Workspaces::States::STARTING } let(:current_actual_state) { ::RemoteDevelopment::Workspaces::States::RUNNING } @@ -67,8 +68,10 @@ context 'when current actual state is Terminating' do let(:previous_actual_state) { ::RemoteDevelopment::Workspaces::States::RUNNING } let(:current_actual_state) { ::RemoteDevelopment::Workspaces::States::TERMINATING } - let(:termination_progress) { RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator::TERMINATING } let(:expected_deployment_resource_version) { nil } + let(:termination_progress) do + RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator::TERMINATING + end it 'returns an AgentInfo object without deployment_resource_version populated' do expect(subject).to eq(expected_agent_info) @@ -78,14 +81,17 @@ context 'when current actual state is Terminated' do let(:previous_actual_state) { ::RemoteDevelopment::Workspaces::States::TERMINATING } let(:current_actual_state) { ::RemoteDevelopment::Workspaces::States::TERMINATED } - let(:termination_progress) { RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator::TERMINATED } let(:expected_deployment_resource_version) { nil } + let(:termination_progress) do + RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator::TERMINATED + end it 'returns an AgentInfo object without deployment_resource_version populated' do expect(subject).to eq(expected_agent_info) end end + # TODO: Should this case even be possible? See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126127#note_1492911475 context "when namespace is missing in the payload" do let(:previous_actual_state) { ::RemoteDevelopment::Workspaces::States::STARTING } let(:current_actual_state) { ::RemoteDevelopment::Workspaces::States::RUNNING } diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/input/params_extractor_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/input/params_extractor_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ef95041e0ff09df7ff3ff5924f1cab192b2195b7 --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/input/params_extractor_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require_relative '../../../fast_spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Input::ParamsExtractor, feature_category: :remote_development do + let(:agent) { instance_double("Clusters::Agent") } + let(:original_params) do + { + "update_type" => "full", + "workspace_agent_infos" => [ + { + "name" => "my-workspace", + "actual_state" => "unknown" + } + ] + } + end + + let(:value) do + { + agent: agent, + original_params: original_params, + existing_symbol_key_entry: "entry1", + existing_string_key_entry: "entry2" + } + end + + subject do + described_class.extract(value) + end + + it "extracts and flattens agent and params contents to top level and deep symbolizes keys" do + expect(subject).to eq( + { + agent: agent, + update_type: "full", + original_params: original_params, + workspace_agent_info_hashes_from_params: [ + { + name: "my-workspace", + actual_state: "unknown" + } + ], + existing_symbol_key_entry: "entry1", + existing_string_key_entry: "entry2" + } + ) + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/input/params_to_infos_converter_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/input/params_to_infos_converter_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2b67995484aca75c7bce9dc1868c106f20e954c4 --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/input/params_to_infos_converter_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require_relative '../../../fast_spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Input::ParamsToInfosConverter, feature_category: :remote_development do + let(:workspace_agent_info_hashes_from_params) do + [ + { + name: "workspace1" + }, + { + name: "workspace2" + } + ] + end + + let(:expected_agent_info_1) do + instance_double("RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo", name: "workspace1") + end + + let(:expected_agent_info_2) do + instance_double("RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo", name: "workspace2") + end + + let(:value) { { workspace_agent_info_hashes_from_params: workspace_agent_info_hashes_from_params } } + + subject do + described_class.convert(value) + end + + before do + allow(RemoteDevelopment::Workspaces::Reconcile::Input::Factory) + .to receive(:build) + .with(agent_info_hash_from_params: workspace_agent_info_hashes_from_params[0]) { expected_agent_info_1 } + allow(RemoteDevelopment::Workspaces::Reconcile::Input::Factory) + .to receive(:build) + .with(agent_info_hash_from_params: workspace_agent_info_hashes_from_params[1]) { expected_agent_info_2 } + end + + it "converts array of workspace agent info hashes from params into array of AgentInfo value objects" do + expect(subject).to eq( + value.merge( + workspace_agent_infos_by_name: { + workspace1: expected_agent_info_1, + workspace2: expected_agent_info_2 + } + ) + ) + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/input/params_validator_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/input/params_validator_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..af7f4c2facb079c5247da95f7554e0f499a862e5 --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/input/params_validator_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require_relative '../../../fast_spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Input::ParamsValidator, feature_category: :remote_development do + include ResultMatchers + + let(:update_type) { "full" } + let(:workspace_error_details) do + { + error_type: "applier", + error_message: "something has gone wrong" + } + end + + let(:workspace_agent_infos) do + [{ + termination_progress: "Terminated", + error_details: workspace_error_details + }] + end + + let(:original_params) do + { + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + } + end + + let(:value) { { original_params: original_params } } + + subject(:result) do + described_class.validate(value) + end + + context 'when original_params are valid' do + let(:update_type) { "full" } + + it 'returns an ok Result containing the original value which was passed' do + expect(result).to eq(Result.ok(value)) + end + + context "when error_details nil" do + let(:workspace_error_details) { nil } + + it 'returns an ok Result containing the original value which was passed' do + expect(result).to eq(Result.ok(value)) + end + end + end + + context 'when original_params are invalid' do + shared_examples 'err result' do |expected_error_details:| + it 'returns an err Result containing error details nil original_params and an error' do + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceReconcileParamsValidationFailed) + message.context => { details: String => error_details } + expect(error_details).to eq(expected_error_details) + end + end + end + + context 'when missing required entries' do + let(:original_params) { {} } + + it_behaves_like 'err result', expected_error_details: + %(root is missing required keys: update_type, workspace_agent_infos) + end + + context 'for workspace_agent_infos' do + context 'when not an array' do + let(:workspace_agent_infos) { "NOT AN ARRAY" } + + it_behaves_like 'err result', expected_error_details: + %(property '/workspace_agent_infos' is not of type: array) + end + end + + context 'for update_type' do + context 'when not "partial" or "full"' do + let(:update_type) { "INVALID UPDATE TYPE" } + + it_behaves_like 'err result', expected_error_details: + %(property '/update_type' is not one of: ["partial", "full"]) + end + end + + context 'for error_details' do + context 'when error_type is missing' do + let(:workspace_error_details) do + { + error_message: "something has gone wrong" + } + end + + it_behaves_like 'err result', expected_error_details: + "property '/workspace_agent_infos/0/error_details' is missing required keys: error_type" + end + + context 'when error_type has an invalid value' do + let(:workspace_error_details) do + { + error_type: "unknown", + error_message: "something has gone wrong" + } + end + + it_behaves_like 'err result', expected_error_details: + %(property '/workspace_agent_infos/0/error_details/error_type' is not one of: ["applier"]) + end + end + + context 'for termination_progress' do + context 'when termination_progress is invalid' do + let(:workspace_agent_infos) do + [{ + termination_progress: "invalidValue" + }] + end + + it_behaves_like 'err result', expected_error_details: + %(property '/workspace_agent_infos/0/termination_progress' is not one of: ["Terminating", "Terminated"]) + end + end + end +end 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 new file mode 100644 index 0000000000000000000000000000000000000000..5956387a083959835f3cc66826247d2d6d63d16f --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/main_integration_spec.rb @@ -0,0 +1,604 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Main, "Integration", :freeze_time, feature_category: :remote_development do + include_context 'with remote development shared fixtures' + + shared_examples 'max_hours_before_termination handling' do + it 'sets desired_state to Terminated' do + response = subject + expect(response[:message]).to be_nil + expect(response.dig(:payload, :workspace_rails_infos)).not_to be_nil + + expect(workspace.reload.desired_state).to eq(RemoteDevelopment::Workspaces::States::TERMINATED) + end + end + + let_it_be(:user) { create(:user) } + let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } + let(:logger) { instance_double(::Logger) } + + let(:expected_value_for_started) { true } + + subject do + described_class.main( + agent: agent, + logger: logger, + original_params: { + workspace_agent_infos: workspace_agent_infos, + update_type: update_type + } + ) + end + + before do + allow(logger).to receive(:debug) + end + + context 'when update_type is full' do + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::FULL } + let(:workspace_agent_infos) { [] } + + it 'updates workspace record and returns proper workspace_rails_info entry' do + create(:workspace, agent: agent, user: user) + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos.length).to eq(1) + workspace_rails_info = workspace_rails_infos.first + + # NOTE: We don't care about any specific expectations, just that the existing workspace + # still has a config returned in the rails_info response even though it was not sent by the agent. + expect(workspace_rails_info[:config_to_apply]).not_to be_nil + end + end + + context 'when update_type is partial' do + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::PARTIAL } + + context 'when receiving agent updates for a workspace which exists in the db' do + let(:desired_state) { RemoteDevelopment::Workspaces::States::STOPPED } + let(:actual_state) { current_actual_state } + let(:previous_actual_state) { RemoteDevelopment::Workspaces::States::STOPPING } + let(:current_actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } + let(:workspace_exists) { false } + let(:deployment_resource_version_from_agent) { '2' } + let(:expected_desired_state) { desired_state } + let(:expected_actual_state) { actual_state } + let(:expected_deployment_resource_version) { deployment_resource_version_from_agent } + let(:expected_config_to_apply) { nil } + let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } + let(:error_from_agent) { nil } + + let(:workspace_agent_info) do + create_workspace_agent_info_hash( + workspace_id: workspace.id, + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + agent_id: workspace.agent.id, + owning_inventory: owning_inventory, + resource_version: deployment_resource_version_from_agent, + previous_actual_state: previous_actual_state, + current_actual_state: current_actual_state, + workspace_exists: workspace_exists, + user_name: user.name, + user_email: user.email, + error_details: error_from_agent + ) + end + + let(:workspace_agent_infos) { [workspace_agent_info] } + + let(:expected_workspace_rails_info) do + { + name: workspace.name, + namespace: workspace.namespace, + desired_state: expected_desired_state, + actual_state: expected_actual_state, + deployment_resource_version: expected_deployment_resource_version, + config_to_apply: expected_config_to_apply + } + end + + let(:expected_workspace_rails_infos) { [expected_workspace_rails_info] } + + let(:workspace) do + create( + :workspace, + agent: agent, + user: user, + desired_state: desired_state, + actual_state: actual_state + ) + end + + context 'with max_hours_before_termination expired' do + let(:workspace) do + create( + :workspace, + :without_realistic_after_create_timestamp_updates, + agent: agent, + user: user, + desired_state: desired_state, + actual_state: actual_state, + max_hours_before_termination: 24, + created_at: 25.hours.ago + ) + end + + context 'when state would otherwise be sent' do + let(:desired_state) { RemoteDevelopment::Workspaces::States::STOPPED } + let(:actual_state) { RemoteDevelopment::Workspaces::States::RUNNING } + + it_behaves_like 'max_hours_before_termination handling' + end + + context 'when desired_state is RestartRequested and actual_state is Stopped' do + let(:desired_state) { RemoteDevelopment::Workspaces::States::RESTART_REQUESTED } + let(:actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } + + it_behaves_like 'max_hours_before_termination handling' + end + end + + context "when the agent encounters an error while starting the workspace" do + let(:actual_state) { RemoteDevelopment::Workspaces::States::STARTING } + let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } + let(:expected_actual_state) { RemoteDevelopment::Workspaces::States::ERROR } + let(:error_from_agent) do + { + error_type: RemoteDevelopment::Workspaces::Reconcile::ErrorType::APPLIER, + error_message: "some applier error" + } + end + + let(:workspace) do + create( + :workspace, + :after_initial_reconciliation, + agent: agent, + user: user, + desired_state: desired_state, + actual_state: actual_state + ) + end + + it 'returns proper workspace_rails_info entry with no config to apply' do + # verify initial states in db (sanity check of match between factory and fixtures) + expect(workspace.desired_state).to eq(desired_state) + expect(workspace.actual_state).to eq(actual_state) + + # expect abnormal agent info to be logged at warn level + expect(logger).to receive(:warn).with(hash_including(error_type: "abnormal_actual_state")) + + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos.length).to eq(1) + + workspace.reload + + expect(workspace.deployment_resource_version) + .to eq(expected_deployment_resource_version) + + # test the config to apply first to get a more specific diff if it fails + # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + provisioned_workspace_rails_info = + workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } + # Since the workspace is now in Error state, the config should not be returned to the agent + expect(provisioned_workspace_rails_info.fetch(:config_to_apply)).to be_nil + + # then test everything in the infos + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end + end + + # rubocop:disable RSpec/MultipleMemoizedHelpers + context 'when only some workspaces fail in devfile flattener' do + let(:workspace) { create(:workspace, name: "workspace1", agent: agent, user: user) } + let(:invalid_devfile_yaml) { read_devfile('example.invalid-extra-field-devfile.yaml') } + let(:workspace2) do + create(:workspace, devfile: invalid_devfile_yaml, name: "workspace-failing-flatten", + agent: agent, user: user) + end + + let(:workspace2_agent_info) do + create_workspace_agent_info_hash( + workspace_id: workspace2.id, + workspace_name: workspace2.name, + workspace_namespace: workspace2.namespace, + agent_id: workspace2.agent.id, + owning_inventory: owning_inventory, + resource_version: deployment_resource_version_from_agent, + previous_actual_state: previous_actual_state, + current_actual_state: current_actual_state, + workspace_exists: workspace_exists, + user_name: user.name, + user_email: user.email + ) + end + + # NOTE: Reverse the order so that the failing one is processed first and ensures that the second valid + # one is still processed successfully. + let(:workspace_agent_infos) { [workspace2_agent_info, workspace_agent_info] } + + let(:expected_workspace2_rails_info) do + { + name: workspace2.name, + namespace: workspace2.namespace, + desired_state: expected_desired_state, + actual_state: expected_actual_state, + deployment_resource_version: expected_deployment_resource_version, + config_to_apply: nil + } + end + + let(:expected_workspace_rails_infos) { [expected_workspace2_rails_info, expected_workspace_rails_info] } + + it 'returns proper workspace_rails_info entries' do + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos.length).to eq(2) + + workspace.reload + workspace2.reload + + expect(workspace.deployment_resource_version) + .to eq(expected_deployment_resource_version) + + expect(workspace2.deployment_resource_version) + .to eq(expected_deployment_resource_version) + + workspace2_rails_info = + workspace_rails_infos.detect { |info| info.fetch(:name) == workspace2.name } + expect(workspace2_rails_info.fetch(:config_to_apply)).to be_nil + + # then test everything in the infos + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end + end + # rubocop:enable RSpec/MultipleMemoizedHelpers + + context 'with timestamp precondition checks' do + # NOTE: rubocop:disable RSpec/ExpectInHook could be avoided with a helper method or custom expectation, + # but this works for now. + # rubocop:disable RSpec/ExpectInHook + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + before do + # Ensure that both desired_state_updated_at and responded_to_agent_at are before Time.current, + # so that we can test for any necessary differences after processing updates them + expect(workspace.desired_state_updated_at).to be_before(Time.current) + expect(workspace.responded_to_agent_at).to be_before(Time.current) + end + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + after do + # After processing, the responded_to_agent_at should always have been updated + workspace.reload + expect(workspace.responded_to_agent_at) + .not_to be_before(workspace.desired_state_updated_at) + end + # rubocop:enable RSpec/ExpectInHook + + context 'when desired_state matches actual_state' do + # rubocop:disable RSpec/ExpectInHook + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + before do + expect(workspace.responded_to_agent_at) + .to be_after(workspace.desired_state_updated_at) + end + # rubocop:enable RSpec/ExpectInHook + + context 'when state is Stopped' do + let(:desired_state) { RemoteDevelopment::Workspaces::States::STOPPED } + + it 'updates workspace record and returns proper workspace_rails_info entry' do + # verify initial states in db (sanity check of match between factory and fixtures) + expect(workspace.desired_state).to eq(desired_state) + expect(workspace.actual_state).to eq(actual_state) + + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos.length).to eq(1) + + workspace.reload + + expect(workspace.desired_state).to eq(workspace.actual_state) + expect(workspace.deployment_resource_version) + .to eq(expected_deployment_resource_version) + + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end + end + + context 'when state is Terminated' do + let(:desired_state) { RemoteDevelopment::Workspaces::States::TERMINATED } + let(:previous_actual_state) { RemoteDevelopment::Workspaces::States::TERMINATED } + let(:current_actual_state) { RemoteDevelopment::Workspaces::States::TERMINATED } + let(:expected_deployment_resource_version) { workspace.deployment_resource_version } + + it 'updates workspace record and returns proper workspace_rails_info entry' do + # verify initial states in db (sanity check of match between factory and fixtures) + expect(workspace.desired_state).to eq(desired_state) + expect(workspace.actual_state).to eq(actual_state) + + # We could do this with a should_not_change block but this reads cleaner IMO + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos.length).to eq(1) + + workspace.reload + + expect(workspace.desired_state).to eq(workspace.actual_state) + expect(workspace.deployment_resource_version) + .to eq(expected_deployment_resource_version) + + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end + end + end + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + context 'when desired_state does not match actual_state' do + let(:deployment_resource_version_from_agent) { workspace.deployment_resource_version } + let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } + + let(:expected_config_to_apply) do + create_config_to_apply( + workspace_id: workspace.id, + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + agent_id: workspace.agent.id, + owning_inventory: owning_inventory, + started: expected_value_for_started, + user_name: user.name, + user_email: user.email + ) + end + + let(:expected_workspace_rails_infos) { [expected_workspace_rails_info] } + + # rubocop:disable RSpec/ExpectInHook + before do + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + expect(workspace.responded_to_agent_at) + .to be_before(workspace.desired_state_updated_at) + end + # rubocop:enable RSpec/ExpectInHook + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + context 'when desired_state is Running' do + let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } + + # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + it 'returns proper workspace_rails_info entry with config_to_apply' do + # verify initial states in db (sanity check of match between factory and fixtures) + expect(workspace.desired_state).to eq(desired_state) + expect(workspace.actual_state).to eq(actual_state) + + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos.length).to eq(1) + + workspace.reload + + expect(workspace.deployment_resource_version) + .to eq(expected_deployment_resource_version) + + # test the config to apply first to get a more specific diff if it fails + provisioned_workspace_rails_info = + workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } + expect(provisioned_workspace_rails_info.fetch(:config_to_apply)) + .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) + + # then test everything in the infos + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end + end + + context 'when desired_state is Terminated' do + let(:desired_state) { RemoteDevelopment::Workspaces::States::TERMINATED } + let(:expected_value_for_started) { false } + + # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + it 'returns proper workspace_rails_info entry with config_to_apply' do + # verify initial states in db (sanity check of match between factory and fixtures) + expect(workspace.desired_state).to eq(desired_state) + expect(workspace.actual_state).to eq(actual_state) + + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos.length).to eq(1) + + workspace.reload + + expect(workspace.deployment_resource_version) + .to eq(expected_deployment_resource_version) + + # test the config to apply first to get a more specific diff if it fails + provisioned_workspace_rails_info = + workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } + expect(provisioned_workspace_rails_info.fetch(:config_to_apply)) + .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) + + # then test everything in the infos + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end + end + + context 'when desired_state is RestartRequested and actual_state is Stopped' do + let(:desired_state) { RemoteDevelopment::Workspaces::States::RESTART_REQUESTED } + let(:expected_desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } + + # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + it 'changes desired_state to Running' do + # verify initial states in db (sanity check of match between factory and fixtures) + expect(workspace.desired_state).to eq(desired_state) + expect(workspace.actual_state).to eq(actual_state) + + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos.length).to eq(1) + + workspace.reload + expect(workspace.desired_state).to eq(expected_desired_state) + + # test the config to apply first to get a more specific diff if it fails + provisioned_workspace_rails_info = + workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } + expect(provisioned_workspace_rails_info[:config_to_apply]) + .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) + + # then test everything in the infos + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end + end + + context 'when actual_state is Unknown' do + let(:current_actual_state) { RemoteDevelopment::Workspaces::States::UNKNOWN } + let(:expected_actual_state) { RemoteDevelopment::Workspaces::States::UNKNOWN } + let(:expected_value_for_started) { false } + + let(:expected_config_to_apply) do + create_config_to_apply( + workspace_id: workspace.id, + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + agent_id: workspace.agent.id, + owning_inventory: owning_inventory, + started: expected_value_for_started, + user_name: user.name, + user_email: user.email + ) + end + + it 'returns the proper response' do + # expect abnormal agent info to be logged at warn level + expect(logger).to receive(:warn).with(hash_including(error_type: "abnormal_actual_state")) + + response = subject + expect(response[:message]).to be_nil + + # Do redundant but progressively higher level checks on the response, so we can have better diffs + # for debugging if any of the lower-level checks fail. + config_to_apply_hash = YAML.safe_load( + response[:payload].fetch(:workspace_rails_infos)[0][:config_to_apply] + ) + expected_config_to_apply_hash = YAML.safe_load(expected_config_to_apply) + expect(config_to_apply_hash).to eq(expected_config_to_apply_hash) + + expect(response[:payload][:workspace_rails_infos][0][:config_to_apply]).to eq(expected_config_to_apply) + + expect(response[:payload][:workspace_rails_infos]).to eq(expected_workspace_rails_infos) + end + end + end + end + end + + context 'when receiving agent updates for a workspace which does not exist in the db' do + let(:workspace_name) { 'non-existent-workspace' } + let(:workspace_namespace) { 'does-not-matter' } + + let(:workspace_agent_info) do + create_workspace_agent_info_hash( + workspace_id: 1, + workspace_name: workspace_name, + workspace_namespace: workspace_namespace, + agent_id: '1', + owning_inventory: 'does-not-matter', + resource_version: '42', + previous_actual_state: RemoteDevelopment::Workspaces::States::STOPPING, + current_actual_state: RemoteDevelopment::Workspaces::States::STOPPED, + workspace_exists: false, + user_name: user.name, + user_email: user.email + ) + end + + let(:workspace_agent_infos) { [workspace_agent_info] } + + let(:expected_workspace_rails_infos) { [] } + + it 'logs orphaned workspace and does not attempt to update the workspace in the db' do + expect(logger).to receive(:warn).with(hash_including(error_type: "orphaned_workspace")) + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos).to be_empty + end + end + + context 'when new unprovisioned workspace exists in database"' do + let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } + let(:actual_state) { RemoteDevelopment::Workspaces::States::CREATION_REQUESTED } + + let_it_be(:unprovisioned_workspace) do + create(:workspace, :unprovisioned, agent: agent, user: user) + end + + let(:workspace_agent_infos) { [] } + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + let(:owning_inventory) { "#{unprovisioned_workspace.name}-workspace-inventory" } + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + let(:expected_config_to_apply) do + create_config_to_apply( + workspace_id: unprovisioned_workspace.id, + workspace_name: unprovisioned_workspace.name, + workspace_namespace: unprovisioned_workspace.namespace, + agent_id: unprovisioned_workspace.agent.id, + owning_inventory: owning_inventory, + started: expected_value_for_started, + user_name: user.name, + user_email: user.email + ) + end + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + let(:expected_unprovisioned_workspace_rails_info) do + { + name: unprovisioned_workspace.name, + namespace: unprovisioned_workspace.namespace, + desired_state: desired_state, + actual_state: actual_state, + deployment_resource_version: nil, + config_to_apply: expected_config_to_apply + } + end + + let(:expected_workspace_rails_infos) { [expected_unprovisioned_workspace_rails_info] } + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + it 'returns proper workspace_rails_info entry' do + # verify initial states in db (sanity check of match between factory and fixtures) + expect(unprovisioned_workspace.desired_state).to eq(desired_state) + expect(unprovisioned_workspace.actual_state).to eq(actual_state) + + response = subject + expect(response[:message]).to be_nil + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) + expect(workspace_rails_infos.length).to eq(1) + + # test the config to apply first to get a more specific diff if it fails + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + unprovisioned_workspace_rails_info = + workspace_rails_infos.detect { |info| info.fetch(:name) == unprovisioned_workspace.name } + expect(unprovisioned_workspace_rails_info.fetch(:config_to_apply)) + .to eq(expected_unprovisioned_workspace_rails_info.fetch(:config_to_apply)) + + # then test everything in the infos + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/reconcile_processor_scenarios_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/main_reconcile_scenarios_spec.rb similarity index 94% rename from ee/spec/lib/remote_development/workspaces/reconcile/reconcile_processor_scenarios_spec.rb rename to ee/spec/lib/remote_development/workspaces/reconcile/main_reconcile_scenarios_spec.rb index bec1bda0c85d64177cffeb3510975b3b2de23e14..90fa677241cbadc66fe3dbcbaa7423b786e8565a 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/reconcile_processor_scenarios_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/main_reconcile_scenarios_spec.rb @@ -7,11 +7,16 @@ # https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/-/blob/main/doc/workspace-updates.md # are not yet implemented - most or all are related to ERROR or FAILURE states, because the fixtures are not yet # implemented. -RSpec.describe ::RemoteDevelopment::Workspaces::Reconcile::ReconcileProcessor, 'Partial Update Scenarios', feature_category: :remote_development do +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Main, 'Partial Update Scenarios', feature_category: :remote_development do include_context 'with remote development shared fixtures' + let(:logger) { instance_double(::Logger) } let_it_be(:user) { create(:user) } + before do + allow(logger).to receive(:debug) + end + # See following documentation for details on all scenarios: # # https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/-/blob/main/doc/workspace-updates.md @@ -26,7 +31,7 @@ # # agent_actual_state_updates: Array of actual state updates from agent. nil if agent reports no info for workspace, # otherwise an array of [previous_actual_state, current_actual_state, workspace_exists] - # to be used as args when calling #create_workspace_agent_info to generate the workspace agent info fixture. + # to be used as args when calling #create_workspace_agent_info_hash to generate the workspace agent info fixture. # # response_expectations: Array corresponding to entries in agent_actual_state_updates, representing # expected rails_info hash response to agent for the workspace. Array is a 2-tuple of booleans for @@ -142,6 +147,9 @@ # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31544 # noinspection RubyInstanceMethodNamingConvention,RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ it 'behaves as expected' do + expect(logger).not_to receive(:warn) + expect(logger).not_to receive(:error) + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31544 expected_db_expectations_length = (initial_db_state ? 1 : 0) + (user_desired_state_update ? 1 : 0) + agent_actual_state_updates.length @@ -189,7 +197,7 @@ # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31544 # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ agent_actual_state_updates.each_with_index do |actual_state_update_fixture_args, response_expectations_index| - update_type = RemoteDevelopment::Workspaces::Reconcile::UpdateType::PARTIAL + update_type = RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::PARTIAL deployment_resource_version_from_agent ||= initial_resource_version workspace_agent_infos = @@ -199,7 +207,7 @@ workspace_exists = actual_state_update_fixture_args[2] deployment_resource_version_from_agent = (deployment_resource_version_from_agent.to_i + 1).to_s [ - create_workspace_agent_info( + create_workspace_agent_info_hash( workspace_id: workspace.id, workspace_name: workspace.name, workspace_namespace: workspace.namespace, @@ -217,12 +225,17 @@ [] end - result = described_class.new.process( + response = described_class.main( agent: workspace.agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type + original_params: { + "workspace_agent_infos" => workspace_agent_infos, + "update_type" => update_type + }, + logger: logger ) - workspace_rails_infos = result[0].fetch(:workspace_rails_infos) + + expect(response[:message]).to be_nil # assert there is not an error, if there is this will display the message + workspace_rails_infos = response.fetch(:payload).fetch(:workspace_rails_infos) # assert on the rails_info response to the agent expect(workspace_rails_infos.size).to eq(1) diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/main_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/main_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ea049754d3a4369395a5b6949630da5a4dda9775 --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/main_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require_relative '../../fast_spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Main, feature_category: :remote_development do + include RemoteDevelopment::RailwayOrientedProgrammingHelpers + + let(:rails_infos) { [double] } + let(:value) { { workspace_rails_infos: rails_infos } } + let(:error_details) { 'some error details' } + let(:err_message_context) { { details: error_details } } + + # rubocop:disable Layout/LineLength - keep all the class and method fixtures as single-liners easier scanning/editing + # Classes + + let(:params_validator_class) { RemoteDevelopment::Workspaces::Reconcile::Input::ParamsValidator } + let(:params_extractor_class) { RemoteDevelopment::Workspaces::Reconcile::Input::ParamsExtractor } + let(:params_to_infos_converter_class) { RemoteDevelopment::Workspaces::Reconcile::Input::ParamsToInfosConverter } + let(:agent_infos_observer_class) { RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfosObserver } + let(:workspaces_from_agent_infos_updater_class) { RemoteDevelopment::Workspaces::Reconcile::Persistence::WorkspacesFromAgentInfosUpdater } + let(:orphaned_workspaces_observer_class) { RemoteDevelopment::Workspaces::Reconcile::Persistence::OrphanedWorkspacesObserver } + let(:workspaces_to_be_returned_finder_class) { RemoteDevelopment::Workspaces::Reconcile::Persistence::WorkspacesToBeReturnedFinder } + let(:workspaces_to_rails_infos_converter_class) { RemoteDevelopment::Workspaces::Reconcile::Output::WorkspacesToRailsInfosConverter } + let(:workspaces_to_be_returned_updater_class) { RemoteDevelopment::Workspaces::Reconcile::Persistence::WorkspacesToBeReturnedUpdater } + let(:rails_infos_observer_class) { RemoteDevelopment::Workspaces::Reconcile::Output::RailsInfosObserver } + + # Methods + + let(:params_validator_method) { params_validator_class.singleton_method(:validate) } + let(:params_extractor_method) { params_extractor_class.singleton_method(:extract) } + let(:params_to_infos_converter_method) { params_to_infos_converter_class.singleton_method(:convert) } + let(:agent_infos_observer_method) { agent_infos_observer_class.singleton_method(:observe) } + let(:workspaces_from_agent_infos_updater_method) { workspaces_from_agent_infos_updater_class.singleton_method(:update) } + let(:orphaned_workspaces_observer_method) { orphaned_workspaces_observer_class.singleton_method(:observe) } + let(:workspaces_to_be_returned_finder_method) { workspaces_to_be_returned_finder_class.singleton_method(:find) } + let(:workspaces_to_rails_infos_converter_method) { workspaces_to_rails_infos_converter_class.singleton_method(:convert) } + let(:workspaces_to_be_returned_updater_method) { workspaces_to_be_returned_updater_class.singleton_method(:update) } + let(:rails_infos_observer_method) { rails_infos_observer_class.singleton_method(:observe) } + # rubocop:enable Layout/LineLength + + # Subject + + subject(:response) { described_class.main(value) } + + before do + allow(params_validator_class).to receive(:method) { params_validator_method } + allow(params_extractor_class).to receive(:method) { params_extractor_method } + allow(params_to_infos_converter_class).to receive(:method) { params_to_infos_converter_method } + allow(agent_infos_observer_class).to receive(:method) { agent_infos_observer_method } + allow(workspaces_from_agent_infos_updater_class).to receive(:method) { workspaces_from_agent_infos_updater_method } + allow(orphaned_workspaces_observer_class).to receive(:method) { orphaned_workspaces_observer_method } + allow(workspaces_to_be_returned_finder_class).to receive(:method) { workspaces_to_be_returned_finder_method } + allow(workspaces_to_rails_infos_converter_class).to receive(:method) { workspaces_to_rails_infos_converter_method } + allow(workspaces_to_be_returned_updater_class).to receive(:method) { workspaces_to_be_returned_updater_method } + allow(rails_infos_observer_class).to receive(:method) { rails_infos_observer_method } + end + + context 'when the ParamsValidator returns an err Result' do + it 'returns an error response' do + expect(params_validator_method).to receive(:call).with(value) do + Result.err(RemoteDevelopment::Messages::WorkspaceReconcileParamsValidationFailed.new) + end + expect(response) + .to eq({ status: :error, message: 'Workspace reconcile params validation failed', reason: :bad_request }) + end + end + + context 'when the ParamsValidator returns an ok Result' do + before do + stub_methods_to_return_ok_result( + params_validator_method + ) + + stub_methods_to_return_value( + params_extractor_method, + params_to_infos_converter_method, + agent_infos_observer_method, + workspaces_from_agent_infos_updater_method, + orphaned_workspaces_observer_method, + workspaces_to_be_returned_finder_method, + workspaces_to_rails_infos_converter_method, + workspaces_to_be_returned_updater_method, + rails_infos_observer_method + ) + end + + it 'returns a workspace reconcile success response with the workspace as the payload' do + expect(response).to eq({ + status: :success, + payload: value + }) + end + end + + context 'when an invalid Result is returned' do + it 'raises an UnmatchedResultError' do + expect(params_validator_method).to receive(:call).with(value) do + Result.err(RemoteDevelopment::Messages::WorkspaceReconcileSuccessful.new) + end + + expect { response }.to raise_error(RemoteDevelopment::UnmatchedResultError) + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/desired_config_generator_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_spec.rb similarity index 60% rename from ee/spec/lib/remote_development/workspaces/reconcile/desired_config_generator_spec.rb rename to ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_spec.rb index a5aaadf998b406b597b477a73af28da3a2f9d71b..1c2167931efd876b2a7ebd5b1ab52e8bfcf50757 100644 --- a/ee/spec/lib/remote_development/workspaces/reconcile/desired_config_generator_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/desired_config_generator_spec.rb @@ -1,23 +1,45 @@ # frozen_string_literal: true -require 'spec_helper' +require_relative '../../../fast_spec_helper' -RSpec.describe RemoteDevelopment::Workspaces::Reconcile::DesiredConfigGenerator, :freeze_time, feature_category: :remote_development do +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Output::DesiredConfigGenerator, :freeze_time, feature_category: :remote_development do include_context 'with remote development shared fixtures' describe '#generate_desired_config' do - let_it_be(:user) { create(:user) } - let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } + 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(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } let(:actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } let(:deployment_resource_version_from_agent) { workspace.deployment_resource_version } let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } let(:network_policy_enabled) { true } + let(:gitlab_workspaces_proxy_namespace) { 'gitlab-workspaces' } let(:workspace) do - create( - :workspace, agent: agent, user: user, - desired_state: desired_state, actual_state: actual_state + instance_double( + "RemoteDevelopment::Workspace", + id: 1, + name: "name", + namespace: "namespace", + deployment_resource_version: "1", + desired_state: desired_state, + actual_state: actual_state, + dns_zone: "workspaces.localdev.me", + processed_devfile: example_processed_devfile, + user: user, + agent: agent ) end @@ -38,20 +60,14 @@ end subject do - described_class.new - end - - before do - allow(agent.remote_development_agent_config).to receive(:network_policy_enabled) do - network_policy_enabled - end + described_class end context 'when desired_state results in started=true' do let(:started) { true } it 'returns expected config' do - workspace_resources = subject.generate_desired_config(workspace: workspace) + workspace_resources = subject.generate_desired_config(workspace: workspace, logger: logger) expect(workspace_resources).to eq(expected_config) end @@ -62,7 +78,7 @@ let(:started) { false } it 'returns expected config' do - workspace_resources = subject.generate_desired_config(workspace: workspace) + workspace_resources = subject.generate_desired_config(workspace: workspace, logger: logger) expect(workspace_resources).to eq(expected_config) end @@ -73,7 +89,7 @@ let(:network_policy_enabled) { false } it 'returns expected config without network policy' do - workspace_resources = subject.generate_desired_config(workspace: workspace) + workspace_resources = subject.generate_desired_config(workspace: workspace, logger: logger) expect(workspace_resources).to eq(expected_config) end @@ -81,13 +97,11 @@ context 'when DevfileParser returns empty array' do before do - allow_next_instance_of(RemoteDevelopment::Workspaces::Reconcile::DevfileParser) do |instance| - allow(instance).to receive(:get_all).and_return([]) - end + allow(RemoteDevelopment::Workspaces::Reconcile::Output::DevfileParser).to receive(:get_all).and_return([]) end it 'returns an empty array' do - workspace_resources = subject.generate_desired_config(workspace: workspace) + workspace_resources = subject.generate_desired_config(workspace: workspace, logger: logger) expect(workspace_resources).to eq([]) end 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 new file mode 100644 index 0000000000000000000000000000000000000000..2ee8f3a1a13e955ee29051fe4b01e0305c94f5ea --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/devfile_parser_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require_relative '../../../fast_spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Output::DevfileParser, feature_category: :remote_development do + include_context 'with remote development shared fixtures' + + 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(:workspace) do + instance_double( + "RemoteDevelopment::Workspace", + id: 1, + name: "name", + namespace: "namespace", + deployment_resource_version: "1", + desired_state: RemoteDevelopment::Workspaces::States::RUNNING, + actual_state: RemoteDevelopment::Workspaces::States::STOPPED, + dns_zone: "workspaces.localdev.me", + processed_devfile: example_processed_devfile, + user: user, + agent: agent + ) + end + + let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } + + let(:domain_template) { "{{.port}}-#{workspace.name}.#{workspace.dns_zone}" } + + let(:expected_workspace_resources) do + YAML.load_stream( + create_config_to_apply( + workspace_id: workspace.id, + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + agent_id: workspace.agent.id, + owning_inventory: owning_inventory, + started: true, + include_inventory: false, + include_network_policy: false, + user_name: user.name, + user_email: user.email + ) + ) + end + + subject do + described_class + end + + it 'returns workspace_resources' do + workspace_resources = subject.get_all( + processed_devfile: example_processed_devfile, + name: workspace.name, + namespace: workspace.namespace, + replicas: 1, + domain_template: domain_template, + labels: { 'agent.gitlab.com/id' => workspace.agent.id }, + annotations: { + 'config.k8s.io/owning-inventory' => owning_inventory, + 'workspaces.gitlab.com/host-template' => domain_template, + 'workspaces.gitlab.com/id' => workspace.id + }, + user: user, + logger: logger + ) + + expect(workspace_resources).to eq(expected_workspace_resources) + end + + context "when Devfile::CliError is raised" do + before do + allow(Devfile::Parser).to receive(:get_all).and_raise(Devfile::CliError.new("some error")) + end + + it "logs the error" do + expect(logger).to receive(:warn).with( + message: 'Error parsing devfile with Devfile::Parser.get_all', + error_type: 'reconcile_devfile_parser_error', + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + devfile_parser_error: "some error" + ) + + workspace_resources = subject.get_all( + processed_devfile: "", + name: workspace.name, + namespace: workspace.namespace, + replicas: 1, + domain_template: "", + labels: {}, + annotations: {}, + user: user, + logger: logger + ) + + expect(workspace_resources).to eq([]) + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/output/rails_infos_observer_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/output/rails_infos_observer_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..690f75aff3574e4a38d082cda23da321f163cf6f --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/rails_infos_observer_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require_relative '../../../fast_spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Output::RailsInfosObserver, feature_category: :remote_development do + let(:agent) { instance_double("Clusters::Agent", id: 1) } + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::PARTIAL } + let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } + let(:actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } + let(:logger) { instance_double(::Logger) } + + let(:workspace_rails_infos) do + [ + { + name: "workspace1", + namespace: "namespace1", + deployment_resource_version: "1", + desired_state: desired_state, + actual_state: actual_state, + config_to_apply: :does_not_matter_should_not_be_logged + }, + { + name: "workspace2", + namespace: "namespace2", + deployment_resource_version: "2", + desired_state: desired_state, + actual_state: actual_state, + config_to_apply: :does_not_matter_should_not_be_logged + } + ] + end + + let(:expected_logged_workspace_rails_infos) do + [ + { + name: "workspace1", + namespace: "namespace1", + deployment_resource_version: "1", + desired_state: desired_state, + actual_state: actual_state + }, + { + name: "workspace2", + namespace: "namespace2", + deployment_resource_version: "2", + desired_state: desired_state, + actual_state: actual_state + } + ] + end + + let(:value) do + { + agent: agent, + update_type: update_type, + workspace_rails_infos: workspace_rails_infos, + logger: logger + } + end + + subject do + described_class.observe(value) + end + + it "logs workspace_rails_infos", :unlimited_max_formatted_output_length do + expect(logger).to receive(:debug).with( + message: 'Returning workspace_rails_infos', + agent_id: agent.id, + update_type: update_type, + count: workspace_rails_infos.length, + workspace_rails_infos: expected_logged_workspace_rails_infos + ) + + expect(subject).to eq(value) + end +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 new file mode 100644 index 0000000000000000000000000000000000000000..8c09961812ab42c3c9265b8000fab4d3e7d4d279 --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/output/workspaces_to_rails_infos_converter_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require_relative '../../../fast_spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Output::WorkspacesToRailsInfosConverter, feature_category: :remote_development do + let(:logger) { instance_double(Logger) } + let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } + let(:actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } + let(:config_to_apply) { { foo: "bar" } } + let(:config_to_apply_yaml) { "---\nfoo: bar\n" } + let(:workspace1) do + instance_double( + "RemoteDevelopment::Workspace", + id: 1, + name: "workspace1", + namespace: "namespace1", + deployment_resource_version: "1", + desired_state: desired_state, + actual_state: actual_state + ) + end + + let(:workspace2) do + instance_double( + "RemoteDevelopment::Workspace", + id: 1, + name: "workspace2", + namespace: "namespace2", + deployment_resource_version: "2", + desired_state: desired_state, + actual_state: actual_state + ) + end + + let(:value) { { update_type: update_type, workspaces_to_be_returned: [workspace1, workspace2], logger: logger } } + + subject do + described_class.convert(value) + end + + before do + allow(RemoteDevelopment::Workspaces::Reconcile::Output::DesiredConfigGenerator) + .to receive(:generate_desired_config) { [config_to_apply] } + end + + context "when update_type is FULL" do + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::FULL } + + it "merges workspace_rails_infos into value and includes config_to_apply" do + expect(subject).to eq( + value.merge( + workspace_rails_infos: [ + { + name: "workspace1", + namespace: "namespace1", + deployment_resource_version: "1", + desired_state: desired_state, + actual_state: actual_state, + config_to_apply: config_to_apply_yaml + }, + { + name: "workspace2", + namespace: "namespace2", + deployment_resource_version: "2", + desired_state: desired_state, + actual_state: actual_state, + config_to_apply: config_to_apply_yaml + } + ] + ) + ) + end + end + + context "when update_type is PARTIAL" do + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::PARTIAL } + + before do + allow(workspace1).to receive(:desired_state_updated_more_recently_than_last_response_to_agent?).and_return(true) + allow(workspace2) + .to receive(:desired_state_updated_more_recently_than_last_response_to_agent?).and_return(false) + end + + context "when workspace.desired_state_updated_more_recently_than_last_response_to_agent == true" do + it "includes config_to_apply" do + expect(subject[:workspace_rails_infos][0][:config_to_apply]).to eq(config_to_apply_yaml) + end + end + + context "when workspace.desired_state_updated_more_recently_than_last_response_to_agent == false" do + it "sets config_to_apply to nil" do + expect(subject[:workspace_rails_infos][1][:config_to_apply]).to eq(nil) + end + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/params_parser_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/params_parser_spec.rb deleted file mode 100644 index 9c52e8e05af196d58153ebd498263fdf6ef684f1..0000000000000000000000000000000000000000 --- a/ee/spec/lib/remote_development/workspaces/reconcile/params_parser_spec.rb +++ /dev/null @@ -1,146 +0,0 @@ -# frozen_string_literal: true - -require 'fast_spec_helper' - -RSpec.describe ::RemoteDevelopment::Workspaces::Reconcile::ParamsParser, :freeze_time, feature_category: :remote_development do - let(:update_type) { 'full' } - let(:workspace_error_details) do - { - "error_type" => "applier", - "error_message" => "something has gone wrong" - } - end - - let(:workspace_agent_infos) do - [{ - "termination_progress" => "Terminated", - "error_details" => workspace_error_details - }] - end - - let(:params) do - { - 'update_type' => update_type, - 'workspace_agent_infos' => workspace_agent_infos - } - end - - subject { described_class.new.parse(params: params) } - - context 'when the params are valid' do - shared_examples "returns parsed params without error" do - it 'returns the parsed params' do - expect(subject).to eq([{ - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - }, nil]) - end - end - - context "when all fields are populated with valid values" do - it_behaves_like "returns parsed params without error" - end - - context "when error_details is missing" do - let(:workspace_error_details) { nil } - - it_behaves_like "returns parsed params without error" - end - end - - context 'when the params are invalid' do - shared_examples 'returns nil params and an error' do - it 'returns nil params and an error' do - params, error = subject - expect(params).to be_nil - expect(error.message).to eq(expected_error_message) - expect(error.reason).to eq(expected_error_reason) - end - end - - let(:expected_error_reason) { :unprocessable_entity } - - context 'when workspace_agent_info is missing' do - let(:expected_error_message) { 'root is missing required keys: workspace_agent_infos' } - let(:params) do - { - 'update_type' => update_type - } - end - - it_behaves_like 'returns nil params and an error' - end - - context 'for update_type' do - context 'when missing' do - let(:expected_error_message) { 'root is missing required keys: update_type' } - let(:params) do - { - 'workspace_agent_infos' => workspace_agent_infos - } - end - - it_behaves_like 'returns nil params and an error' - end - - context 'when invalid' do - let(:expected_error_message) { %(property '/update_type' is not one of: ["partial", "full"]) } - let(:params) do - { - 'update_type' => 'invalid_update_type', - 'workspace_agent_infos' => workspace_agent_infos - } - end - - it_behaves_like 'returns nil params and an error' - end - end - - context 'for error_details' do - context 'when error_type is missing' do - let(:expected_error_message) do - "property '/workspace_agent_infos/0/error_details' is missing required keys: error_type" - end - - let(:workspace_error_details) do - { - "error_message" => "something has gone wrong" - } - end - - it_behaves_like 'returns nil params and an error' - end - - context 'when error_type has an invalid value' do - let(:expected_error_message) do - %(property '/workspace_agent_infos/0/error_details/error_type' is not one of: ["applier"]) - end - - let(:workspace_error_details) do - { - "error_type" => "unknown", - "error_message" => "something has gone wrong" - } - end - - it_behaves_like 'returns nil params and an error' - end - end - - context 'for termination_progress' do - context 'when termination_progress is invalid' do - let(:workspace_agent_infos) do - [{ - "termination_progress" => "invalidValue" - }] - end - - let(:expected_error_message) do - %(property '/workspace_agent_infos/0/termination_progress' is not one of: ["Terminating", "Terminated"]) - end - - it_behaves_like 'returns nil params and an error' - end - end - end -end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/persistence/orphaned_workspaces_observer_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/persistence/orphaned_workspaces_observer_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..fd4f61a8c103de83aef83e80d5ef86822da15c0b --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/persistence/orphaned_workspaces_observer_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require_relative '../../../fast_spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Persistence::OrphanedWorkspacesObserver, feature_category: :remote_development do + let(:agent) { instance_double("Clusters::Agent", id: 1) } + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::PARTIAL } + let(:logger) { instance_double(::Logger) } + + let(:workspace) { instance_double("RemoteDevelopment::Workspace", name: "name", namespace: "namespace") } + + let(:persisted_workspace_agent_info) do + RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo.new( + name: workspace.name, + namespace: workspace.namespace, + actual_state: RemoteDevelopment::Workspaces::States::STOPPED, + deployment_resource_version: "1" + ) + end + + let(:orphaned_workspace_agent_info) do + RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo.new( + name: "orphaned_workspace", + namespace: "orphaned_workspace_namespace", + actual_state: RemoteDevelopment::Workspaces::States::RUNNING, + deployment_resource_version: "1" + ) + end + + let(:workspaces_from_agent_infos) { [workspace] } + + let(:value) do + { + agent: agent, + update_type: update_type, + workspace_agent_infos_by_name: workspace_agent_infos_by_name, + workspaces_from_agent_infos: workspaces_from_agent_infos, + logger: logger + } + end + + subject do + described_class.observe(value) + end + + context "when orphaned workspaces exist" do + let(:workspace_agent_infos_by_name) do + { + persisted_workspace_agent_info.name => persisted_workspace_agent_info, + orphaned_workspace_agent_info.name => orphaned_workspace_agent_info + }.symbolize_keys + end + + it "logs orphaned workspaces at warn level", :unlimited_max_formatted_output_length do + expect(logger).to receive(:warn).with( + message: "Received orphaned workspace agent info for workspace(s) where no persisted workspace record exists", + error_type: "orphaned_workspace", + agent_id: agent.id, + update_type: update_type, + count: 1, + orphaned_workspaces: [ + { + name: orphaned_workspace_agent_info.name, + namespace: orphaned_workspace_agent_info.namespace, + actual_state: orphaned_workspace_agent_info.actual_state + } + ] + ) + + expect(subject).to eq(value) + end + end + + context "when no orphaned workspaces exist" do + let(:workspace_agent_infos_by_name) do + { + persisted_workspace_agent_info.name => persisted_workspace_agent_info + }.symbolize_keys + end + + it "does not log" do + expect(logger).not_to receive(:warn) + + expect(subject).to eq(value) + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/persistence/workspaces_from_agent_infos_updater_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/persistence/workspaces_from_agent_infos_updater_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7d5a1ad2cd30434a86db0e6791f6a70a11f2a069 --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/persistence/workspaces_from_agent_infos_updater_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Persistence::WorkspacesFromAgentInfosUpdater, feature_category: :remote_development do + include_context 'with remote development shared fixtures' + + let_it_be(:user) { create(:user) } + let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } + + let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } + let(:actual_state) { RemoteDevelopment::Workspaces::States::STARTING } + + let(:workspace) do + create( + :workspace, + agent: agent, + user: user, + desired_state: desired_state, + actual_state: actual_state + ) + end + + let(:workspace_agent_info) do + RemoteDevelopment::Workspaces::Reconcile::Input::AgentInfo.new( + name: workspace.name, + namespace: workspace.namespace, + actual_state: actual_state, + deployment_resource_version: "1" + ) + end + + let(:workspace_agent_infos_by_name) do + { + workspace_agent_info.name => workspace_agent_info + }.symbolize_keys + end + + let(:value) do + { + agent: agent, + workspace_agent_infos_by_name: workspace_agent_infos_by_name + } + end + + subject do + described_class.update(value) # rubocop:disable Rails/SaveBang + end + + it "returns persisted workspaces" do + expect(subject).to eq(value.merge(workspaces_from_agent_infos: [workspace])) + end + + context "when persisted workspace desired_state is RESTART_REQUESTED and actual_state is STOPPED" do + let(:desired_state) { RemoteDevelopment::Workspaces::States::RESTART_REQUESTED } + let(:actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } + + it "sets persisted workspace desired state to RUNNING" do + expect(subject).to eq(value.merge(workspaces_from_agent_infos: [workspace])) + expect(workspace.reload.desired_state).to eq(RemoteDevelopment::Workspaces::States::RUNNING) + end + end + + context "when persisted workspace created_at + max_hours_before_termination.hours < Time.current" do + before do + workspace.update!(created_at: 2.days.ago, max_hours_before_termination: 1) + end + + it "sets persisted workspace desired state to TERMINATED" do + expect(subject).to eq(value.merge(workspaces_from_agent_infos: [workspace])) + expect(workspace.reload.desired_state).to eq(RemoteDevelopment::Workspaces::States::TERMINATED) + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_finder_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..db6b0cf7c6bca47a022a30cc2c2ef2646a0c8bab --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_finder_spec.rb @@ -0,0 +1,143 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Persistence::WorkspacesToBeReturnedFinder, feature_category: :remote_development do + let_it_be(:user) { create(:user) } + let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } + + let_it_be(:workspace_only_returned_by_full_update, reload: true) do + create( + :workspace, + :without_realistic_after_create_timestamp_updates, + name: "workspace_only_returned_by_full_update", + agent: agent, + user: user, + responded_to_agent_at: 2.hours.ago + ) + end + + let_it_be(:workspace_from_agent_info, reload: true) do + create( + :workspace, + :without_realistic_after_create_timestamp_updates, + name: "workspace_from_agent_info", + agent: agent, + user: user, + responded_to_agent_at: 2.hours.ago + ) + end + + let_it_be(:workspace_with_new_update_to_desired_state, reload: true) do + create( + :workspace, + :without_realistic_after_create_timestamp_updates, + name: "workspace_with_new_update_to_desired_state", + agent: agent, + user: user, + responded_to_agent_at: 2.hours.ago + ) + end + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + let(:workspaces_from_agent_infos) { [workspace_from_agent_info] } + + let(:value) do + { + agent: agent, + update_type: update_type, + workspaces_from_agent_infos: workspaces_from_agent_infos + } + end + + subject do + described_class.find(value) + end + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + before do + agent.reload + + # desired_state_updated_at IS NOT more recent than responded_to_agent_at + workspace_only_returned_by_full_update.update_attribute( + :desired_state_updated_at, + workspace_only_returned_by_full_update.responded_to_agent_at - 1.hour + ) + + # desired_state_updated_at IS NOT more recent than responded_to_agent_at + workspace_from_agent_info.update_attribute( + :desired_state_updated_at, + workspace_from_agent_info.responded_to_agent_at - 1.hour + ) + + # desired_state_updated_at IS more recent than responded_to_agent_at + workspace_with_new_update_to_desired_state.update_attribute( + :desired_state_updated_at, + workspace_with_new_update_to_desired_state.responded_to_agent_at + 1.hour + ) + + subject + end + + context "with fixture sanity checks" do + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::FULL } + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + it "has the expected fixtures" do + expect(workspace_only_returned_by_full_update.desired_state_updated_at) + .to be < workspace_only_returned_by_full_update.responded_to_agent_at + expect(workspace_with_new_update_to_desired_state.desired_state_updated_at) + .to be > workspace_with_new_update_to_desired_state.responded_to_agent_at + expect(workspace_from_agent_info.desired_state_updated_at) + .to be < workspace_from_agent_info.responded_to_agent_at + end + end + + context "for update_type FULL" do + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::FULL } + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + let(:expected_workspaces_to_be_returned) do + [ + # Includes ALL workspaces including workspace_only_returned_by_full_update + workspace_only_returned_by_full_update, + workspace_from_agent_info, + workspace_with_new_update_to_desired_state + ] + end + + it "returns all workspaces" do + expect(subject.fetch(:workspaces_to_be_returned).map(&:name)) + .to match_array(expected_workspaces_to_be_returned.map(&:name)) + expect(subject).to match(hash_including(value)) + end + + it "preserves existing value entries", + :unlimited_max_formatted_output_length do + expect(subject).to eq(value.merge(workspaces_to_be_returned: expected_workspaces_to_be_returned)) + end + end + + context "for update_type PARTIAL" do + let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateTypes::PARTIAL } + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + let(:expected_workspaces_to_be_returned) do + [ + # Does NOT include workspace_only_returned_by_full_update + workspace_from_agent_info, + workspace_with_new_update_to_desired_state + ] + end + + it "returns only workspaces with new updates to desired state or in workspaces_from_agent_infos" do + expect(subject.fetch(:workspaces_to_be_returned).map(&:name)) + .to eq(expected_workspaces_to_be_returned.map(&:name)) + end + + it "preserves existing value entries", + :unlimited_max_formatted_output_length do + expect(subject).to eq(value.merge(workspaces_to_be_returned: expected_workspaces_to_be_returned)) + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_updater_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_updater_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2997818c83058281d0bf9d2db542cf193ccc69b5 --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/reconcile/persistence/workspaces_to_be_returned_updater_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::Workspaces::Reconcile::Persistence::WorkspacesToBeReturnedUpdater, feature_category: :remote_development do + let_it_be(:user) { create(:user) } + let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } + + let_it_be(:workspace1) do + create( + :workspace, + :without_realistic_after_create_timestamp_updates, + name: "workspace1", + agent: agent, + user: user + ) + end + + let_it_be(:workspace2) do + create( + :workspace, + :without_realistic_after_create_timestamp_updates, + name: "workspace2", + agent: agent, + user: user + ) + end + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + let(:workspaces_to_be_returned) { [workspace1, workspace2] } + + let(:value) do + { + agent: agent, + workspaces_to_be_returned: workspaces_to_be_returned + } + end + + subject do + described_class.update(value) # rubocop:disable Rails/SaveBang - This is not an ActiveRecord model + end + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + before do + workspace1.update_attribute(:responded_to_agent_at, 2.hours.ago) + workspace2.update_attribute(:responded_to_agent_at, 2.hours.ago) + agent.reload + end + + context "with fixture sanity checks" do + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + it "has the expected fixtures" do + expect(workspace1.responded_to_agent_at).to be < 1.hour.ago + expect(workspace2.responded_to_agent_at).to be < 1.hour.ago + end + end + + context "for update_type FULL" do + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + it "updates all workspaces", :unlimited_max_formatted_output_length do + subject + expect(workspace1.reload.responded_to_agent_at).to be > 1.minute.ago + expect(workspace2.reload.responded_to_agent_at).to be > 1.minute.ago + end + + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31543 + it "preserves existing value entries" do + subject + expect(subject).to eq(value.merge(workspaces_to_be_returned: [workspace1.reload, workspace2.reload])) + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/reconcile/reconcile_processor_spec.rb b/ee/spec/lib/remote_development/workspaces/reconcile/reconcile_processor_spec.rb deleted file mode 100644 index 0d794ee567c020c0058077487360dba0c68870f2..0000000000000000000000000000000000000000 --- a/ee/spec/lib/remote_development/workspaces/reconcile/reconcile_processor_spec.rb +++ /dev/null @@ -1,589 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ::RemoteDevelopment::Workspaces::Reconcile::ReconcileProcessor, :freeze_time, feature_category: :remote_development do - include_context 'with remote development shared fixtures' - - describe '#process' do - shared_examples 'max_hours_before_termination handling' do - it 'sets desired_state to Terminated' do - _, error = subject.process(agent: agent, workspace_agent_infos: workspace_agent_infos, update_type: update_type) - expect(error).to be_nil - - expect(workspace.reload.desired_state).to eq(RemoteDevelopment::Workspaces::States::TERMINATED) - end - end - - let_it_be(:user) { create(:user) } - let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } - let(:expected_value_for_started) { true } - - subject do - described_class.new - end - - context 'when update_type is full' do - let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateType::FULL } - - it 'updates workspace record and returns proper workspace_rails_info entry' do - create(:workspace, agent: agent, user: user) - payload, error = subject.process(agent: agent, workspace_agent_infos: [], update_type: update_type) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos.length).to eq(1) - workspace_rails_info = workspace_rails_infos.first - - # NOTE: We don't care about any specific expectations, just that the existing workspace - # still has a config returned in the rails_info response even though it was not sent by the agent. - expect(workspace_rails_info[:config_to_apply]).not_to be_nil - end - end - - context 'when update_type is partial' do - let(:update_type) { RemoteDevelopment::Workspaces::Reconcile::UpdateType::PARTIAL } - - context 'when receiving agent updates for a workspace which exists in the db' do - let(:desired_state) { RemoteDevelopment::Workspaces::States::STOPPED } - let(:actual_state) { current_actual_state } - let(:previous_actual_state) { RemoteDevelopment::Workspaces::States::STOPPING } - let(:current_actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } - let(:workspace_exists) { false } # todo: why is this false - let(:deployment_resource_version_from_agent) { '2' } - let(:expected_desired_state) { desired_state } - let(:expected_actual_state) { actual_state } - let(:expected_deployment_resource_version) { deployment_resource_version_from_agent } - let(:expected_config_to_apply) { nil } - let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } - let(:error_from_agent) { nil } - - let(:workspace_agent_info) do - create_workspace_agent_info( - workspace_id: workspace.id, - workspace_name: workspace.name, - workspace_namespace: workspace.namespace, - agent_id: workspace.agent.id, - owning_inventory: owning_inventory, - resource_version: deployment_resource_version_from_agent, - previous_actual_state: previous_actual_state, - current_actual_state: current_actual_state, - workspace_exists: workspace_exists, - user_name: user.name, - user_email: user.email, - error_details: error_from_agent - ) - end - - let(:workspace_agent_infos) { [workspace_agent_info] } - - let(:expected_workspace_rails_info) do - { - name: workspace.name, - namespace: workspace.namespace, - desired_state: expected_desired_state, - actual_state: expected_actual_state, - deployment_resource_version: expected_deployment_resource_version, - config_to_apply: expected_config_to_apply - } - end - - let(:expected_workspace_rails_infos) { [expected_workspace_rails_info] } - - let(:workspace) do - create( - :workspace, - agent: agent, - user: user, - desired_state: desired_state, - actual_state: actual_state - ) - end - - context 'with max_hours_before_termination expired' do - let(:workspace) do - create( - :workspace, - :without_realistic_after_create_timestamp_updates, - agent: agent, - user: user, - desired_state: desired_state, - actual_state: actual_state, - max_hours_before_termination: 24, - created_at: 25.hours.ago - ) - end - - context 'when state would otherwise be sent' do - let(:desired_state) { RemoteDevelopment::Workspaces::States::STOPPED } - let(:actual_state) { RemoteDevelopment::Workspaces::States::RUNNING } - - it_behaves_like 'max_hours_before_termination handling' - end - - context 'when desired_state is RestartRequested and actual_state is Stopped' do - let(:desired_state) { RemoteDevelopment::Workspaces::States::RESTART_REQUESTED } - let(:actual_state) { RemoteDevelopment::Workspaces::States::STOPPED } - - it_behaves_like 'max_hours_before_termination handling' - end - end - - context "when the agent encounters an error while starting the workspace" do - let(:actual_state) { RemoteDevelopment::Workspaces::States::STARTING } - let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } - let(:expected_actual_state) { RemoteDevelopment::Workspaces::States::ERROR } - let(:error_from_agent) do - { - "error_type" => RemoteDevelopment::Workspaces::Reconcile::ErrorType::APPLIER, - "error_message" => "some applier error" - } - end - - let(:workspace) do - create( - :workspace, - :after_initial_reconciliation, - agent: agent, - user: user, - desired_state: desired_state, - actual_state: actual_state - ) - end - - it 'returns proper workspace_rails_info entry with no config to apply' do - # verify initial states in db (sanity check of match between factory and fixtures) - expect(workspace.desired_state).to eq(desired_state) - expect(workspace.actual_state).to eq(actual_state) - - payload, error = subject.process( - agent: agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos.length).to eq(1) - - workspace.reload - - expect(workspace.deployment_resource_version) - .to eq(expected_deployment_resource_version) - - # test the config to apply first to get a more specific diff if it fails - # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ - provisioned_workspace_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } - # Since the workspace is now in Error state, the config should not be returned to the agent - expect(provisioned_workspace_rails_info.fetch(:config_to_apply)).to be_nil - - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end - - context 'when only some workspaces fail in devfile flattener' do - let(:workspace) { create(:workspace, name: "workspace1", agent: agent, user: user) } - let(:invalid_devfile_yaml) { read_devfile('example.invalid-extra-field-devfile.yaml') } - let(:workspace2) do - create(:workspace, devfile: invalid_devfile_yaml, name: "workspace-failing-flatten", - agent: agent, user: user) - end - - let(:workspace2_agent_info) do - create_workspace_agent_info( - workspace_id: workspace2.id, - workspace_name: workspace2.name, - workspace_namespace: workspace2.namespace, - agent_id: workspace2.agent.id, - owning_inventory: owning_inventory, - resource_version: deployment_resource_version_from_agent, - previous_actual_state: previous_actual_state, - current_actual_state: current_actual_state, - workspace_exists: workspace_exists, - user_name: user.name, - user_email: user.email - ) - end - - # NOTE: Reverse the order so that the failing one is processed first and ensures that the second valid - # one is still processed successfully. - let(:workspace_agent_infos) { [workspace2_agent_info, workspace_agent_info] } - - let(:expected_workspace2_rails_info) do - { - name: workspace2.name, - namespace: workspace2.namespace, - desired_state: expected_desired_state, - actual_state: expected_actual_state, - deployment_resource_version: expected_deployment_resource_version, - config_to_apply: nil - } - end - - let(:expected_workspace_rails_infos) { [expected_workspace2_rails_info, expected_workspace_rails_info] } - - it 'returns proper workspace_rails_info entries' do - payload, error = subject.process( - agent: agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos.length).to eq(2) - - workspace.reload - workspace2.reload - - expect(workspace.deployment_resource_version) - .to eq(expected_deployment_resource_version) - - expect(workspace2.deployment_resource_version) - .to eq(expected_deployment_resource_version) - - workspace2_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == workspace2.name } - expect(workspace2_rails_info.fetch(:config_to_apply)).to be_nil - - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end - - context 'with timestamp precondition checks' do - # NOTE: rubocop:disable RSpec/ExpectInHook could be avoided with a helper method or custom expectation, - # but this works for now. - # rubocop:disable RSpec/ExpectInHook - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - before do - # Ensure that both desired_state_updated_at and responded_to_agent_at are before Time.current, - # so that we can test for any necessary differences after processing updates them - expect(workspace.desired_state_updated_at).to be_before(Time.current) - expect(workspace.responded_to_agent_at).to be_before(Time.current) - end - - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - after do - # After processing, the responded_to_agent_at should always have been updated - workspace.reload - expect(workspace.responded_to_agent_at) - .not_to be_before(workspace.desired_state_updated_at) - end - # rubocop:enable RSpec/ExpectInHook - - context 'when desired_state matches actual_state' do - # rubocop:disable RSpec/ExpectInHook - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - before do - expect(workspace.responded_to_agent_at) - .to be_after(workspace.desired_state_updated_at) - end - # rubocop:enable RSpec/ExpectInHook - - context 'when state is Stopped' do - let(:desired_state) { RemoteDevelopment::Workspaces::States::STOPPED } - - it 'updates workspace record and returns proper workspace_rails_info entry' do - # verify initial states in db (sanity check of match between factory and fixtures) - expect(workspace.desired_state).to eq(desired_state) - expect(workspace.actual_state).to eq(actual_state) - - payload, error = subject.process( - agent: agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos.length).to eq(1) - - workspace.reload - - expect(workspace.desired_state).to eq(workspace.actual_state) - expect(workspace.deployment_resource_version) - .to eq(expected_deployment_resource_version) - - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end - - context 'when state is Terminated' do - let(:desired_state) { RemoteDevelopment::Workspaces::States::TERMINATED } - let(:previous_actual_state) { RemoteDevelopment::Workspaces::States::TERMINATED } - let(:current_actual_state) { RemoteDevelopment::Workspaces::States::TERMINATED } - let(:expected_deployment_resource_version) { workspace.deployment_resource_version } - - it 'updates workspace record and returns proper workspace_rails_info entry' do - # verify initial states in db (sanity check of match between factory and fixtures) - expect(workspace.desired_state).to eq(desired_state) - expect(workspace.actual_state).to eq(actual_state) - - # We could do this with a should_not_change block but this reads cleaner IMO - payload, error = subject.process( - agent: agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos.length).to eq(1) - - workspace.reload - - expect(workspace.desired_state).to eq(workspace.actual_state) - expect(workspace.deployment_resource_version) - .to eq(expected_deployment_resource_version) - - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end - end - - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - context 'when desired_state does not match actual_state' do - let(:deployment_resource_version_from_agent) { workspace.deployment_resource_version } - let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } - - let(:expected_config_to_apply) do - create_config_to_apply( - workspace_id: workspace.id, - workspace_name: workspace.name, - workspace_namespace: workspace.namespace, - agent_id: workspace.agent.id, - owning_inventory: owning_inventory, - started: expected_value_for_started, - user_name: user.name, - user_email: user.email - ) - end - - let(:expected_workspace_rails_infos) { [expected_workspace_rails_info] } - - # rubocop:disable RSpec/ExpectInHook - before do - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - expect(workspace.responded_to_agent_at) - .to be_before(workspace.desired_state_updated_at) - end - # rubocop:enable RSpec/ExpectInHook - - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - context 'when desired_state is Running' do - let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } - - # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ - it 'returns proper workspace_rails_info entry with config_to_apply' do - # verify initial states in db (sanity check of match between factory and fixtures) - expect(workspace.desired_state).to eq(desired_state) - expect(workspace.actual_state).to eq(actual_state) - - payload, error = subject.process( - agent: agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos.length).to eq(1) - - workspace.reload - - expect(workspace.deployment_resource_version) - .to eq(expected_deployment_resource_version) - - # test the config to apply first to get a more specific diff if it fails - provisioned_workspace_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } - expect(provisioned_workspace_rails_info.fetch(:config_to_apply)) - .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) - - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end - - context 'when desired_state is Terminated' do - let(:desired_state) { RemoteDevelopment::Workspaces::States::TERMINATED } - let(:expected_value_for_started) { false } - - # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ - it 'returns proper workspace_rails_info entry with config_to_apply' do - # verify initial states in db (sanity check of match between factory and fixtures) - expect(workspace.desired_state).to eq(desired_state) - expect(workspace.actual_state).to eq(actual_state) - - payload, error = subject.process( - agent: agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos.length).to eq(1) - - workspace.reload - - expect(workspace.deployment_resource_version) - .to eq(expected_deployment_resource_version) - - # test the config to apply first to get a more specific diff if it fails - provisioned_workspace_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } - expect(provisioned_workspace_rails_info.fetch(:config_to_apply)) - .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) - - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end - - context 'when desired_state is RestartRequested and actual_state is Stopped' do - let(:desired_state) { RemoteDevelopment::Workspaces::States::RESTART_REQUESTED } - let(:expected_desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } - - # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ - it 'changes desired_state to Running' do - # verify initial states in db (sanity check of match between factory and fixtures) - expect(workspace.desired_state).to eq(desired_state) - expect(workspace.actual_state).to eq(actual_state) - - payload, error = subject.process(agent: agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos.length).to eq(1) - - workspace.reload - expect(workspace.desired_state).to eq(expected_desired_state) - - # test the config to apply first to get a more specific diff if it fails - provisioned_workspace_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == workspace.name } - expect(provisioned_workspace_rails_info[:config_to_apply]) - .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) - - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end - - context 'when actual_state is Unknown' do - let(:current_actual_state) { RemoteDevelopment::Workspaces::States::UNKNOWN } - - it 'has test coverage for logging in conditional' do - subject.process(agent: agent, workspace_agent_infos: workspace_agent_infos, update_type: update_type) - end - end - end - end - end - - context 'when receiving agent updates for a workspace which does not exist in the db' do - let(:workspace_name) { 'non-existent-workspace' } - let(:workspace_namespace) { 'does-not-matter' } - - let(:workspace_agent_info) do - create_workspace_agent_info( - workspace_id: 1, - workspace_name: workspace_name, - workspace_namespace: workspace_namespace, - agent_id: '1', - owning_inventory: 'does-not-matter', - resource_version: '42', - previous_actual_state: RemoteDevelopment::Workspaces::States::STOPPING, - current_actual_state: RemoteDevelopment::Workspaces::States::STOPPED, - workspace_exists: false, - user_name: user.name, - user_email: user.email - ) - end - - let(:workspace_agent_infos) { [workspace_agent_info] } - - let(:expected_workspace_rails_infos) { [] } - - it 'prints an error and does not attempt to update the workspace in the db' do - payload, error = subject.process( - agent: agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos).to be_empty - end - end - - context 'when new unprovisioned workspace exists in database"' do - let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } - let(:actual_state) { RemoteDevelopment::Workspaces::States::CREATION_REQUESTED } - - let_it_be(:unprovisioned_workspace) do - create(:workspace, :unprovisioned, agent: agent, user: user) - end - - let(:workspace_agent_infos) { [] } - - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - let(:owning_inventory) { "#{unprovisioned_workspace.name}-workspace-inventory" } - - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - let(:expected_config_to_apply) do - create_config_to_apply( - workspace_id: unprovisioned_workspace.id, - workspace_name: unprovisioned_workspace.name, - workspace_namespace: unprovisioned_workspace.namespace, - agent_id: unprovisioned_workspace.agent.id, - owning_inventory: owning_inventory, - started: expected_value_for_started, - user_name: user.name, - user_email: user.email - ) - end - - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - let(:expected_unprovisioned_workspace_rails_info) do - { - name: unprovisioned_workspace.name, - namespace: unprovisioned_workspace.namespace, - desired_state: desired_state, - actual_state: actual_state, - deployment_resource_version: nil, - config_to_apply: expected_config_to_apply - } - end - - let(:expected_workspace_rails_infos) { [expected_unprovisioned_workspace_rails_info] } - - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ - it 'returns proper workspace_rails_info entry' do - # verify initial states in db (sanity check of match between factory and fixtures) - expect(unprovisioned_workspace.desired_state).to eq(desired_state) - expect(unprovisioned_workspace.actual_state).to eq(actual_state) - - payload, error = subject.process( - agent: agent, - workspace_agent_infos: workspace_agent_infos, - update_type: update_type - ) - expect(error).to be_nil - workspace_rails_infos = payload.fetch(:workspace_rails_infos) - expect(workspace_rails_infos.length).to eq(1) - - # test the config to apply first to get a more specific diff if it fails - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 - unprovisioned_workspace_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == unprovisioned_workspace.name } - expect(unprovisioned_workspace_rails_info.fetch(:config_to_apply)) - .to eq(expected_unprovisioned_workspace_rails_info.fetch(:config_to_apply)) - - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end - end - end -end diff --git a/ee/spec/services/remote_development/workspaces/reconcile_service_spec.rb b/ee/spec/services/remote_development/workspaces/reconcile_service_spec.rb index 692189bb80f1708ea71d10afe854e52ff9938495..8fc07f04b5d84d6852f3d42992d710b9b735fc8a 100644 --- a/ee/spec/services/remote_development/workspaces/reconcile_service_spec.rb +++ b/ee/spec/services/remote_development/workspaces/reconcile_service_spec.rb @@ -3,90 +3,41 @@ require 'spec_helper' RSpec.describe ::RemoteDevelopment::Workspaces::ReconcileService, feature_category: :remote_development do - describe '#execute' do - let(:agent) { instance_double(Clusters::Agent, id: 1) } - let(:params) { instance_double(Hash) } + let(:agent) { instance_double(Clusters::Agent, id: 1) } + let(:params) { instance_double(Hash) } + let(:workspace_rails_infos) { [] } - subject do + describe '#execute' do + subject(:service_response) do described_class.new.execute(agent: agent, params: params) end - context 'when params parse successfully' do - let(:parsed_params) { { some_param: 'some value' } } - - before do - allow_next_instance_of(RemoteDevelopment::Workspaces::Reconcile::ParamsParser) do |parser| - allow(parser).to receive(:parse).with(params: params).and_return([parsed_params, nil]) - end - - allow_next_instance_of(RemoteDevelopment::Workspaces::Reconcile::ReconcileProcessor) do |processor| - allow(processor).to receive(:process) do |**args| - # rubocop:disable RSpec/ExpectInHook - not sure how to avoid this expectation without duplicating the block - expect(args).to eq(agent: agent, **parsed_params) - # rubocop:enable RSpec/ExpectInHook - end.and_return(processor_result) - end - end - - context 'when reconciliation is successful' do - let(:payload) { instance_double(Hash) } - let(:processor_result) { [payload, nil] } - - it 'returns a success ServiceResponse' do - expect(subject).to be_a(ServiceResponse) - expect(subject.payload).to eq(payload) - expect(subject.message).to be_nil - end - end - - context 'when reconciliation processing fails' do - let(:message) { 'error message' } - let(:reason) { :bad_request } - let(:error) { RemoteDevelopment::Error.new(message: message, reason: reason) } - let(:processor_result) { [nil, error] } - - it 'returns an error ServiceResponse' do - expect(subject).to be_a(ServiceResponse) - expect(subject.payload).to eq({}) # NOTE: A nil payload gets turned into an empty hash - expect(subject.message).to eq(message) - expect(subject.reason).to eq(reason) - end - end + before do + allow(RemoteDevelopment::Workspaces::Reconcile::Main) + .to receive(:main).with( + agent: agent, + original_params: params, + logger: instance_of(RemoteDevelopment::Logger) + ).and_return(response_hash) end - context 'when params parsing fails' do - let(:message) { 'error message' } - let(:reason) { :unprocessable_entity } - let(:error) { RemoteDevelopment::Error.new(message: message, reason: reason) } + context 'when success' do + let(:response_hash) { { status: :success, payload: { workspace_rails_infos: workspace_rails_infos } } } - it 'returns an error ServiceResponse' do - allow_next_instance_of(RemoteDevelopment::Workspaces::Reconcile::ParamsParser) do |parser| - expect(parser).to receive(:parse).with(params: params).and_return([nil, error]) - end - expect(subject).to be_a(ServiceResponse) - expect(subject.payload).to eq({}) # NOTE: A nil payload gets turned into an empty hash - expect(subject.message).to eq(message) - expect(subject.reason).to eq(reason) + it 'returns a success ServiceResponse' do + expect(service_response).to be_success + expect(service_response.payload.fetch(:workspace_rails_infos)).to eq(workspace_rails_infos) end end - context 'when there is an unexpected exception' do - let(:reason) { :internal_server_error } - let(:exception) { RuntimeError.new('unexpected error') } - - it 'returns an error ServiceResponse' do - allow_next_instance_of(RemoteDevelopment::Workspaces::Reconcile::ParamsParser) do |parser| - expect(parser).to receive(:parse).and_raise(exception) - end + context 'when error' do + let(:response_hash) { { status: :error, message: 'error', reason: :bad_request } } - expect(Gitlab::ErrorTracking).to receive(:track_exception).with(exception, - error_type: 'reconcile', - agent_id: agent.id - ) - expect(subject).to be_a(ServiceResponse) - expect(subject.payload).to eq({}) # NOTE: A nil payload gets turned into an empty hash - expect(subject.message).to match(/Unexpected reconcile error/) - expect(subject.reason).to eq(reason) + it 'returns an error success ServiceResponse' do + expect(service_response).to be_error + service_response => { message:, reason: } + expect(message).to eq('error') + expect(reason).to eq(:bad_request) end end end diff --git a/ee/spec/support/shared_contexts/remote_development/agent_info_status_fixture_not_implemented_error.rb b/ee/spec/support/shared_contexts/remote_development/agent_info_status_fixture_not_implemented_error.rb index d712eb9b880929eab7f8d93c33a26c79a0cf8849..9536bdf6d76a66fc44909f0a8072d64ac024868d 100644 --- a/ee/spec/support/shared_contexts/remote_development/agent_info_status_fixture_not_implemented_error.rb +++ b/ee/spec/support/shared_contexts/remote_development/agent_info_status_fixture_not_implemented_error.rb @@ -2,7 +2,7 @@ module RemoteDevelopment # This exception indicates that the agent_info fixture STATUS_YAML - # generated by remote_development_shared_contexts.rb#create_workspace_agent_info for the given + # generated by remote_development_shared_contexts.rb#create_workspace_agent_info_hash for the given # state transition has not yet been correctly implemented. AgentInfoStatusFixtureNotImplementedError = Class.new(RuntimeError) 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 6250c9e73cc3e38036c88800c40be4f2f645043a..7b4637c5a57f79f68a02ca53916866f97d0b7002 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 @@ -7,7 +7,7 @@ # rubocop:disable Layout/LineLength # noinspection RubyInstanceMethodNamingConvention, 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 - def create_workspace_agent_info( + def create_workspace_agent_info_hash( workspace_id:, workspace_name:, workspace_namespace:, @@ -29,16 +29,18 @@ def create_workspace_agent_info( # Default some of the parameters which can be derived from others: e.g. owning_inventory, workspace_namespace info = { - 'name' => workspace_name, - 'namespace' => workspace_namespace + name: workspace_name, + namespace: workspace_namespace } if current_actual_state == RemoteDevelopment::Workspaces::States::TERMINATED - info['termination_progress'] = RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator::TERMINATED + info[:termination_progress] = + RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator::TERMINATED end if current_actual_state == RemoteDevelopment::Workspaces::States::TERMINATING - info['termination_progress'] = RemoteDevelopment::Workspaces::Reconcile::ActualStateCalculator::TERMINATING + info[:termination_progress] = + RemoteDevelopment::Workspaces::Reconcile::Input::ActualStateCalculator::TERMINATING end if [ @@ -54,7 +56,6 @@ def create_workspace_agent_info( ].include?(current_actual_state) ? 0 : 1 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) - root_url = Gitlab::Routing.url_helpers.root_url # rubocop:disable Lint/DuplicateBranch status = @@ -248,7 +249,7 @@ def create_workspace_agent_info( # updatedReplicas: 1 # STATUS_YAML else - msg = 'Unsupported state transition passed for create_workspace_agent_info fixture creation: ' \ + msg = 'Unsupported state transition passed for create_workspace_agent_info_hash fixture creation: ' \ "actual_state: #{previous_actual_state} -> #{current_actual_state}, " \ "existing_workspace: #{workspace_exists}" raise RemoteDevelopment::AgentInfoStatusFixtureNotImplementedError, msg @@ -407,10 +408,11 @@ def create_workspace_agent_info( #{status.indent(2)} RESOURCES_YAML - info['latest_k8s_deployment_info'] = YAML.safe_load(latest_k8s_deployment_info) - info['error_details'] = error_details - info + info[:latest_k8s_deployment_info] = YAML.safe_load(latest_k8s_deployment_info) + info[:error_details] = error_details + info.deep_symbolize_keys.to_h end + # rubocop:enable Metrics/ParameterLists # rubocop:enable Metrics/CyclomaticComplexity # rubocop:enable Metrics/PerceivedComplexity @@ -435,7 +437,7 @@ def create_workspace_rails_info( end # rubocop:disable Metrics/ParameterLists - # noinspection RubyLocalVariableNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + # noinspection RubyLocalVariableNamingConvention,RubyParameterNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ def create_config_to_apply( workspace_id:, workspace_name:, @@ -452,7 +454,6 @@ def create_config_to_apply( 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) - root_url = Gitlab::Routing.url_helpers.root_url inventory_config = <<~RESOURCES_YAML --- kind: ConfigMap @@ -718,6 +719,7 @@ def create_config_to_apply( YAML.dump(resource) end.join end + # rubocop:enable Metrics/ParameterLists # noinspection RubyInstanceMethodNamingConvention - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ @@ -740,7 +742,7 @@ def example_flattened_devfile def example_processed_devfile devfile_contents = read_devfile('example.processed-devfile.yaml') - devfile_contents.gsub!('http://localhost/', Gitlab::Routing.url_helpers.root_url) + devfile_contents.gsub!('http://localhost/', root_url) devfile_contents end @@ -749,4 +751,10 @@ def example_processed_devfile def read_devfile(filename) File.read(Rails.root.join('ee/spec/fixtures/remote_development', filename).to_s) end + + def root_url + # NOTE: Default to http://example.com/ if GitLab::Application is not defined. This allows this helper to be used + # from ee/spec/remote_development/fast_spec_helper.rb + defined?(Gitlab::Application) ? Gitlab::Routing.url_helpers.root_url : 'https://example.com/' + end end