From 0c6b5b623bbb72045d512462acab342218799df7 Mon Sep 17 00:00:00 2001 From: Chad Woolley Date: Wed, 5 Apr 2023 20:00:49 -0700 Subject: [PATCH 1/2] Properly handle full and partial sync - part 1 --- .../20221225010101_create_workspaces_table.rb | 2 + db/structure.sql | 2 + doc/api/graphql/reference/index.md | 4 + .../remote_development/workspace_type.rb | 12 ++ ee/app/models/remote_development/workspace.rb | 23 ++- .../state_reconciliation_service.rb | 37 +++-- .../desired_config_generator.rb | 6 - .../workspace_agent_info_parser.rb | 5 +- .../workspace_updates_processor.rb | 144 ++++++++++-------- .../remote_development/workspaces.rb | 28 +++- .../remote_development/workspace_type_spec.rb | 5 +- .../desired_config_generator_spec.rb | 70 ++++----- .../workspace_create_processor_spec.rb | 4 +- .../workspace_updates_processor_spec.rb | 40 ++++- .../remote_development/workspace_spec.rb | 33 ++++ 15 files changed, 275 insertions(+), 140 deletions(-) diff --git a/db/migrate/20221225010101_create_workspaces_table.rb b/db/migrate/20221225010101_create_workspaces_table.rb index f4b8edd845ded6..ebb3eddc9e0017 100644 --- a/db/migrate/20221225010101_create_workspaces_table.rb +++ b/db/migrate/20221225010101_create_workspaces_table.rb @@ -14,7 +14,9 @@ def up t.text :name, limit: 64, null: false, index: { unique: true } t.text :namespace, limit: 64, null: false t.text :desired_state, limit: 32, null: false + t.datetime_with_timezone :desired_state_updated_at, null: false t.text :actual_state, limit: 32, null: false + t.datetime_with_timezone :agent_info_reported_at t.text :editor, limit: 256, null: false t.text :devfile_ref, limit: 256, null: false t.text :devfile_path, limit: 2048, null: false diff --git a/db/structure.sql b/db/structure.sql index a4f6b3a4727394..0b7009793f8988 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -24301,7 +24301,9 @@ CREATE TABLE workspaces ( name text NOT NULL, namespace text NOT NULL, desired_state text NOT NULL, + desired_state_updated_at timestamp with time zone NOT NULL, actual_state text NOT NULL, + agent_info_reported_at timestamp with time zone, editor text NOT NULL, devfile_ref text NOT NULL, devfile_path text NOT NULL, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index e7a96684e39a98..6a0805360fdb92 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -22608,9 +22608,12 @@ Represents a remote development workspace. | Name | Type | Description | | ---- | ---- | ----------- | | `actualState` | [`String!`](#string) | Actual state of the workspace. | +| `agentInfoReportedAt` | [`Time`](#time) | Timestamp of last report of desired state from GA4K. | | `clusterAgent` | [`ClusterAgent!`](#clusteragent) | Kubernetes Agent associated with the workspace. | +| `createdAt` | [`Time!`](#time) | Timestamp of workspace creation. | | `deploymentResourceVersion` | [`Int`](#int) | ResourceVersion of the Deployment resource for the workspace. | | `desiredState` | [`String!`](#string) | Desired state of the workspace. | +| `desiredStateUpdatedAt` | [`Time!`](#time) | Timestamp of last update to desired state. | | `devfile` | [`String!`](#string) | Source YAML of the devfile used to configure the workspace. | | `devfilePath` | [`String!`](#string) | Project repo git path containing the devfile used to configure the workspace. | | `devfileRef` | [`String!`](#string) | Project repo git ref containing the devfile used to configure the workspace. | @@ -22622,6 +22625,7 @@ Represents a remote development workspace. | `namespace` | [`String!`](#string) | Namespace of the workspace in Kubernetes. | | `processedDevfile` | [`String!`](#string) | Processed YAML of the devfile used to configure the workspace. | | `project` | [`Project!`](#project) | Project providing the Devfile for the workspace. | +| `updatedAt` | [`Time!`](#time) | Timestamp of last update to any mutable workspace property. | | `url` | [`String!`](#string) | URL of the workspace. | | `user` | [`UserCore!`](#usercore) | Owner of the workspace. | diff --git a/ee/app/graphql/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index f3d6c849ec556d..51159f5c3fc8b9 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -36,9 +36,15 @@ class WorkspaceType < ::Types::BaseObject field :desired_state, GraphQL::Types::String, null: false, description: 'Desired state of the workspace.' + field :desired_state_updated_at, Types::TimeType, + null: false, description: 'Timestamp of last update to desired state.' + field :actual_state, GraphQL::Types::String, null: false, description: 'Actual state of the workspace.' + field :agent_info_reported_at, Types::TimeType, + null: true, description: 'Timestamp of last report of desired state from GA4K.' + field :displayed_state, GraphQL::Types::String, null: false, description: 'Displayed state of the workspace. Calculated from desired_state+actual_state.' @@ -63,6 +69,12 @@ class WorkspaceType < ::Types::BaseObject field :deployment_resource_version, GraphQL::Types::Int, null: true, description: 'ResourceVersion of the Deployment resource for the workspace.' + + field :created_at, Types::TimeType, + null: false, description: 'Timestamp of workspace creation.' + + field :updated_at, Types::TimeType, + null: false, description: 'Timestamp of last update to any mutable workspace property.' end # rubocop: enable Graphql/AuthorizeTypes end diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index f33203919c1508..0d0912ef52b340 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module RemoteDevelopment - # noinspection RailsParamDefResolve + # noinspection RailsParamDefResolve,RubyResolve class Workspace < ApplicationRecord extend ActiveSupport::Concern @@ -29,15 +29,28 @@ class Workspace < ApplicationRecord scope :with_project, ->(project) { where(project: project) } scope :with_agent, ->(agent) { where(agent: agent) } - scope :with_actual_state_not_matching_desired_state, -> { where('actual_state <> desired_state') } - scope :without_names_in, ->(names_to_not_match) { where.not(name: names_to_not_match) } + scope :with_desired_state_updated_more_recently_than_agent_info_reported, -> do + # noinspection SqlResolve + where('desired_state_updated_at >= agent_info_reported_at').or(where(agent_info_reported_at: nil)) + end + scope :with_ids_in, ->(ids_to_not_match) { where(id: ids_to_not_match) } def displayed_state DisplayedStateCalculator.calculate(desired_state: desired_state, actual_state: actual_state) end + before_save :touch_desired_state_updated_at, if: ->(workspace) do + workspace.new_record? || workspace.desired_state_changed? + end + after_commit :trigger_group_workspaces_updated_subscription + def desired_state_updated_more_recently_than_agent_info_reported? + return true if agent_info_reported_at.nil? + + desired_state_updated_at >= agent_info_reported_at + end + def terminated? actual_state == ::RemoteDevelopment::States::TERMINATED end @@ -47,5 +60,9 @@ def terminated? def trigger_group_workspaces_updated_subscription GraphqlTriggers.group_workspaces_updated(group) end + + def touch_desired_state_updated_at + self.desired_state_updated_at = Time.current + end end end diff --git a/ee/app/services/remote_development/workspaces/state_reconciliation_service.rb b/ee/app/services/remote_development/workspaces/state_reconciliation_service.rb index 230b8452fb9dfc..0b488b63329022 100644 --- a/ee/app/services/remote_development/workspaces/state_reconciliation_service.rb +++ b/ee/app/services/remote_development/workspaces/state_reconciliation_service.rb @@ -5,23 +5,17 @@ module RemoteDevelopment module Workspaces class StateReconciliationService def execute(agent:, params:) - Rails.logger.debug("DEBUGGING: Agent ID #{agent.id}") - - parsed_params, error = RemoteDevelopment::WorkspaceUpdatesParamsParser.new.parse(params: params) - return ServiceResponse.error(message: error.message, reason: error.reason) if error - - workspace_updates_processor = WorkspaceUpdatesProcessor.new - payload, error = workspace_updates_processor.process(**parsed_params) - - return ServiceResponse.error(message: error.message, reason: error.reason) if error - - ServiceResponse.success(payload: payload) + # TODO: 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 err = "Unexpected exception. Exception class: #{e.class}. " \ "Exception message: #{e.message.presence || '(no error message)'}" Rails.logger.error("DEBUGGING: #{err}") - # noinspection RubyNilAnalysis - Rails.logger.error("DEBUGGING: #{e.backtrace.join("\n")}") + Rails.logger.error("DEBUGGING: #{e.full_message}") # TODO: 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, @@ -31,11 +25,28 @@ def execute(agent:, params:) # 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 ServiceResponse.error( message: err, reason: :internal_server_error ) end + + private + + def process(agent, params) + Rails.logger.debug("DEBUGGING: Agent ID #{agent.id}") + + parsed_params, error = RemoteDevelopment::WorkspaceUpdatesParamsParser.new.parse(params: params) + return ServiceResponse.error(message: error.message, reason: error.reason) if error + + workspace_updates_processor = WorkspaceUpdatesProcessor.new + payload, error = workspace_updates_processor.process(**parsed_params) + + return ServiceResponse.error(message: error.message, reason: error.reason) if error + + ServiceResponse.success(payload: payload) + end end end end diff --git a/ee/lib/remote_development/desired_config_generator.rb b/ee/lib/remote_development/desired_config_generator.rb index fe0a1915cbba0c..16e679a2459aad 100644 --- a/ee/lib/remote_development/desired_config_generator.rb +++ b/ee/lib/remote_development/desired_config_generator.rb @@ -4,12 +4,6 @@ module RemoteDevelopment # noinspection RubyResolve class DesiredConfigGenerator def generate_desired_config(workspace:) - # TODO: use processed devfile if present - https://gitlab.com/gitlab-org/gitlab/-/issues/396420 - # For now, the assumption is that a state change is the ONLY thing that can cause the config to change. - # Therefore, we will return a nil config unless the state has changed. - # noinspection RubyResolve - return if workspace.desired_state == workspace.actual_state - name = workspace.name namespace = workspace.namespace agent = workspace.agent diff --git a/ee/lib/remote_development/workspace_agent_info_parser.rb b/ee/lib/remote_development/workspace_agent_info_parser.rb index bf9b81f9ee0836..8c8ad5f79b5f8f 100644 --- a/ee/lib/remote_development/workspace_agent_info_parser.rb +++ b/ee/lib/remote_development/workspace_agent_info_parser.rb @@ -25,7 +25,10 @@ def parse(workspace_agent_info:) unless [States::TERMINATED, States::UNKNOWN].include?(actual_state) # Unless the workspace is already terminated, the other workspace_agent_info entries should be populated info[:namespace] = workspace_agent_info.fetch('namespace') - info[:deployment_resource_version] = latest_k8s_deployment_info.fetch('metadata').fetch('resourceVersion') + deployment_resource_version = latest_k8s_deployment_info.fetch('metadata').fetch('resourceVersion') + # TODO: Remove this presence? check once we convert the resourceVersion to an integer on the agentk side. + # See: https://gitlab.com/gitlab-org/gitlab/-/issues/396882 + info[:deployment_resource_version] = deployment_resource_version if deployment_resource_version.present? end WorkspaceAgentInfo.new(**info) diff --git a/ee/lib/remote_development/workspace_updates_processor.rb b/ee/lib/remote_development/workspace_updates_processor.rb index 359277631fa070..49d639cb1414f2 100644 --- a/ee/lib/remote_development/workspace_updates_processor.rb +++ b/ee/lib/remote_development/workspace_updates_processor.rb @@ -1,79 +1,81 @@ # frozen_string_literal: true # rubocop:disable Gitlab/RailsLogger +# noinspection RubyResolve module RemoteDevelopment class WorkspaceUpdatesProcessor + # rubocop:disable CodeReuse/ActiveRecord def process(workspace_agent_infos:, update_type:) Rails.logger.debug("DEBUGGING: Beginning processing of workspace updates") Rails.logger.debug("DEBUGGING: workspace_agent_infos: #{workspace_agent_infos.inspect}") # parse an array of WorkspaceAgentInfo objects from the workspace_agent_infos array - workspace_agent_infos = workspace_agent_infos.map do |workspace_agent_info| - RemoteDevelopment::WorkspaceAgentInfoParser.new.parse(workspace_agent_info: workspace_agent_info) + workspace_agent_infos_by_name = workspace_agent_infos.each_with_object({}) do |workspace_agent_info, hash| + info = RemoteDevelopment::WorkspaceAgentInfoParser.new.parse(workspace_agent_info: workspace_agent_info) + hash[info.name] = info end + names_from_agent_infos = workspace_agent_infos_by_name.keys - # Update persisted workspaces which match the names of the workspaces in the WorkspaceAgentInfo objects array - workspaces = workspace_agent_infos.filter_map do |workspace_agent_info| - workspace_name = workspace_agent_info.name - workspace = RemoteDevelopment::Workspace.find_by_name(workspace_name) + check_for_orphan_workspaces(names_from_agent_infos) - unless workspace - Rails.logger.error("DEBUGGING: ERROR: Received workspace agent info for workspace '#{workspace_name}' " \ - "but no persisted workspace record exists for this workspace") - next - end + # TODO: SECURITY CONCERN - This needs to be scoped by agent + persisted_workspaces_from_agent_infos = RemoteDevelopment::Workspace.where(name: names_from_agent_infos) - # Update the persisted workspaces with the latest info from the array of WorkspaceAgentInfo objects + # Update persisted workspaces which match the names of the workspaces in the WorkspaceAgentInfo objects array + persisted_workspaces_from_agent_infos.each do |workspace_from_agent_info| + workspace_agent_info = workspace_agent_infos_by_name[workspace_from_agent_info.name] + # Update the persisted workspaces with the latest info from the WorkspaceAgentInfo objects we received update_existing_workspace_with_latest_info( - persisted_workspace: workspace, + persisted_workspace: workspace_from_agent_info, deployment_resource_version: workspace_agent_info.deployment_resource_version, actual_state: workspace_agent_info.actual_state ) - workspace end Rails.logger.debug("DEBUGGING: updated persisted workspaces with actual infos") - # Load workspaces which still have a different desired vs. actual state. These could either of these two cases: - # 1. Workspaces which are newly persisted since the last reconciliation cycle and need to be created and add them - # to the array of existing workspaces. These are identified by the `actual_state` being `Creating` - # 2. Existing Workspaces in which the actual_state does not match the desired_state. - workspaces_query = RemoteDevelopment::Workspace - .with_actual_state_not_matching_desired_state - .without_names_in(workspaces.map(&:name)) - .to_a - Rails.logger.debug("DEBUGGING: update_type: #{update_type}") - if update_type == RemoteDevelopment::WorkspaceUpdatesParamsParser::UPDATE_TYPE_PARTIAL + if update_type == RemoteDevelopment::WorkspaceUpdatesParamsParser::UPDATE_TYPE_FULL + # For a FULL update, return all workspaces for the agent which exist in the database + # TODO: SECURITY CONCERN - This needs to be scoped by agent + workspaces_to_return_in_rails_infos_query = RemoteDevelopment::Workspace.all.to_a + else + # For a PARTIAL update, return: + # 1. Workspaces with_desired_state_updated_more_recently_than_agent_info_reported + # 2. Workspaces which we received from the agent in the agent_infos array # TODO: Implement as described in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116475 + # TODO: SECURITY CONCERN - This needs to be scoped by agent + persisted_workspaces_from_agent_infos_ids = persisted_workspaces_from_agent_infos.map(&:id) + workspaces_to_return_in_rails_infos_query = + RemoteDevelopment::Workspace + .with_desired_state_updated_more_recently_than_agent_info_reported + .or(RemoteDevelopment::Workspace.with_ids_in(persisted_workspaces_from_agent_infos_ids)) end - workspaces += workspaces_query.to_a + 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.map do |workspace| - desired_config_to_apply = - RemoteDevelopment::DesiredConfigGenerator.new.generate_desired_config(workspace: workspace) - - unless desired_config_to_apply.nil? - desired_config_to_apply = desired_config_to_apply.map do |resource| - YAML.dump(resource) - end.join - end - + workspace_rails_infos = workspaces_to_return_in_rails_infos.map do |workspace| deployment_resource_version = workspace.deployment_resource_version - { + workspace_rails_info = { name: workspace.name, namespace: workspace.namespace, # TODO: Dry up disagreement on whether this version is an integer or a string - k8s has it as a string, - # but value is int and that's what we put in the db schema + # but value is int and that's what we put in the db schema. There is a task to convert to an integer + # on the agentk side: https://gitlab.com/gitlab-org/gitlab/-/issues/396882 persisted_deployment_resource_version: deployment_resource_version.blank? ? nil : deployment_resource_version.to_s, persisted_actual_state_is_terminated: workspace.actual_state == States::TERMINATED, # help agentk manage state desired_state_is_terminated: workspace.desired_state == States::TERMINATED, - config_to_apply: desired_config_to_apply + config_to_apply: desired_config_to_apply(workspace) }.compact # remove nil entries + + # Update the agent_info_reported_at at this point, after we have already done all the calculations + # related to state + workspace.touch(:agent_info_reported_at) + + workspace_rails_info end Rails.logger.debug("DEBUGGING: workspace_rails_infos: #{workspace_rails_infos.inspect}") @@ -85,39 +87,49 @@ def process(workspace_agent_infos:, update_type:) private - def update_existing_workspace_with_latest_info( - persisted_workspace:, deployment_resource_version:, actual_state: - ) + def desired_config_to_apply(workspace) + return unless workspace.desired_state_updated_more_recently_than_agent_info_reported? - if actual_state == States::TERMINATED - Rails.logger.debug("DEBUGGING: actual_state was Terminated for '#{persisted_workspace.name}'") - # noinspection RubyResolve - persisted_workspace.actual_state = actual_state - else - # If actual state was not Terminated, update the actual state and other fields - - if actual_state == States::UNKNOWN - Rails.logger.debug("DEBUGGING: Ignoring actual_state=UNKNOWN for workspace '#{persisted_workspace.name}'") - else - # noinspection RubyResolve - persisted_workspace.actual_state = actual_state - end - - # noinspection RubyResolve - # Handle the special case of RESTARTING. desired_state is only set to 'RESTARTING' 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::RESTARTING && actual_state == States::STOPPED - persisted_workspace.desired_state = States::RUNNING - end - - # Update other fields - resource version - # noinspection RubyResolve - persisted_workspace.deployment_resource_version = deployment_resource_version + workspace_resources = + RemoteDevelopment::DesiredConfigGenerator.new.generate_desired_config(workspace: workspace) + + desired_config_to_apply_array = workspace_resources.map do |resource| + YAML.dump(resource) + end + + return unless desired_config_to_apply_array.present? + + desired_config_to_apply_array.join + end + + def check_for_orphan_workspaces(names_from_agent_infos) + found_names = RemoteDevelopment::Workspace.where(name: names_from_agent_infos).pluck(:name) + + orphan_workspaces = names_from_agent_infos - found_names + return unless orphan_workspaces.present? + + Rails.logger.error("DEBUGGING: ERROR: Received workspace agent info for workspaces " \ + "where no persisted workspace record exists: #{orphan_workspaces.inspect}") + end + + def update_existing_workspace_with_latest_info(persisted_workspace:, deployment_resource_version:, actual_state:) + # Handle the special case of RESTARTING. desired_state is only set to 'RESTARTING' 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::RESTARTING && actual_state == States::STOPPED + persisted_workspace.desired_state = States::RUNNING 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 resulted in a Failure or Error. + persisted_workspace.deployment_resource_version = deployment_resource_version if deployment_resource_version + persisted_workspace.save! end + + # rubocop:enable CodeReuse/ActiveRecord end end # rubocop:enable Gitlab/RailsLogger diff --git a/ee/spec/factories/remote_development/workspaces.rb b/ee/spec/factories/remote_development/workspaces.rb index f99b5b23fb756c..a5555e720f6855 100644 --- a/ee/spec/factories/remote_development/workspaces.rb +++ b/ee/spec/factories/remote_development/workspaces.rb @@ -15,7 +15,7 @@ desired_state { RemoteDevelopment::States::STOPPED } actual_state { RemoteDevelopment::States::STOPPED } - deployment_resource_version { 1 } + deployment_resource_version { 2 } editor { 'webide' } url { "https://8000-#{name}.workspaces.localdev.me" } # noinspection RubyResolve @@ -42,9 +42,35 @@ workspace.agent.project.add_developer(user) end + after(:create) do |workspace, _| + if workspace.desired_state == workspace.actual_state + # The most recent activity was a poll that reconciled the desired and actual state. + desired_state_updated_at = 2.seconds.ago + agent_info_reported_at = 1.second.ago + else + # The most recent activity was a user action which updated the desired state to be different + # than the actual state. + desired_state_updated_at = 1.second.ago + agent_info_reported_at = 2.seconds.ago + end + + workspace.update!( + # NOTE: created_at and updated_at are not currently used in any logic, but we set them to be + # before desired_state_updated_at or agent_info_reported_at to ensure the record represents + # a realistic condition. + created_at: 3.seconds.ago, + updated_at: 3.seconds.ago, + + desired_state_updated_at: desired_state_updated_at, + agent_info_reported_at: agent_info_reported_at + ) + end + trait :unprovisioned do desired_state { RemoteDevelopment::States::RUNNING } actual_state { RemoteDevelopment::States::CREATING } + # noinspection RubyResolve + agent_info_reported_at { nil } deployment_resource_version { nil } end end diff --git a/ee/spec/graphql/types/remote_development/workspace_type_spec.rb b/ee/spec/graphql/types/remote_development/workspace_type_spec.rb index e7d0de7b5ac4d0..01f835bebca668 100644 --- a/ee/spec/graphql/types/remote_development/workspace_type_spec.rb +++ b/ee/spec/graphql/types/remote_development/workspace_type_spec.rb @@ -5,8 +5,9 @@ RSpec.describe GitlabSchema.types['Workspace'], feature_category: :remote_development do let(:fields) do %i[ - id cluster_agent group project user name namespace desired_state actual_state displayed_state url editor - devfile_ref devfile_path devfile processed_devfile deployment_resource_version + id cluster_agent group project user name namespace + desired_state desired_state_updated_at actual_state agent_info_reported_at displayed_state + url editor devfile_ref devfile_path devfile processed_devfile deployment_resource_version created_at updated_at ] end diff --git a/ee/spec/lib/remote_development/desired_config_generator_spec.rb b/ee/spec/lib/remote_development/desired_config_generator_spec.rb index 6c46e88a20674b..76501bb6793dff 100644 --- a/ee/spec/lib/remote_development/desired_config_generator_spec.rb +++ b/ee/spec/lib/remote_development/desired_config_generator_spec.rb @@ -9,6 +9,10 @@ let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:agent) { create(:cluster_agent) } + let(:desired_state) { RemoteDevelopment::States::RUNNING } + let(:actual_state) { RemoteDevelopment::States::STOPPED } + let(:deployment_resource_version_from_agent) { workspace.deployment_resource_version.to_s } + let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } let(:workspace) do create( @@ -17,59 +21,41 @@ ) end + let(:expected_config) 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: started + ) + ) + end + subject do described_class.new end - context 'when desired_state is the same as actual_state' do - let(:desired_state) { RemoteDevelopment::States::STOPPED } - let(:actual_state) { RemoteDevelopment::States::STOPPED } + context 'when desired_state results in started=true' do + let(:started) { true } - it 'returns nil' do + it 'returns expected config' do workspace_resources = subject.generate_desired_config(workspace: workspace) - expect(workspace_resources).to be_nil - end - end - - context 'when desired_state is different than actual_state' do - let(:desired_state) { RemoteDevelopment::States::RUNNING } - let(:actual_state) { RemoteDevelopment::States::STOPPED } - let(:deployment_resource_version_from_agent) { workspace.deployment_resource_version.to_s } - let(:owning_inventory) { "#{workspace.name}-workspace-inventory" } - let(:expected_config) 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: started - ) - ) - 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) - - expect(workspace_resources).to eq(expected_config) - end + expect(workspace_resources).to eq(expected_config) end + end - context 'when desired_state results in started=false' do - let(:desired_state) { RemoteDevelopment::States::STOPPED } - let(:actual_state) { RemoteDevelopment::States::RUNNING } - let(:started) { false } + context 'when desired_state results in started=false' do + let(:desired_state) { RemoteDevelopment::States::STOPPED } + let(:started) { false } - it 'returns expected config' do - workspace_resources = subject.generate_desired_config(workspace: workspace) + it 'returns expected config' do + workspace_resources = subject.generate_desired_config(workspace: workspace) - expect(workspace_resources).to eq(expected_config) - end + expect(workspace_resources).to eq(expected_config) end end end diff --git a/ee/spec/lib/remote_development/workspace_create_processor_spec.rb b/ee/spec/lib/remote_development/workspace_create_processor_spec.rb index ffb38a27ff8c83..91cd5d92fb90c9 100644 --- a/ee/spec/lib/remote_development/workspace_create_processor_spec.rb +++ b/ee/spec/lib/remote_development/workspace_create_processor_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ::RemoteDevelopment::WorkspaceCreateProcessor, feature_category: :remote_development do +RSpec.describe ::RemoteDevelopment::WorkspaceCreateProcessor, :freeze_time, feature_category: :remote_development do include_context 'with remote development shared fixtures' let(:fake_devfile) { example_devfile } @@ -50,6 +50,8 @@ expect(workspace.user).to eq(user) expect(workspace.agent).to eq(agent) expect(workspace.desired_state).to eq(RemoteDevelopment::States::RUNNING) + # noinspection RubyResolve + expect(workspace.desired_state_updated_at).to eq(Time.current) expect(workspace.actual_state).to eq(RemoteDevelopment::States::CREATING) expect(workspace.name).to eq("workspace-#{agent.id}-#{user.id}-#{random_string}") expect(workspace.namespace).to eq("gl-rd-ns-#{agent.id}-#{user.id}-#{random_string}") diff --git a/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb b/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb index e7b67a1e9e1d3d..2dc841bdc39897 100644 --- a/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb +++ b/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb @@ -52,12 +52,39 @@ let(:expected_workspace_rails_infos) { [expected_provisioned_workspace_rails_info] } + # rubocop:disable RSpec/ExpectInHook + before do + # Ensure that both desired_state_updated_at and agent_info_reported_at are before Time.current, + # so that we can test for any necessary differences after processing updates them + # noinspection RubyResolve + expect(provisioned_workspace.desired_state_updated_at).to be_before(Time.current) + # noinspection RubyResolve + expect(provisioned_workspace.agent_info_reported_at).to be_before(Time.current) + end + + after do + # After processing, the agent_info_reported_at should always have been updated + provisioned_workspace.reload + # noinspection RubyResolve + expect(provisioned_workspace.agent_info_reported_at) + .not_to be_before(provisioned_workspace.desired_state_updated_at) + end + # rubocop:enable RSpec/ExpectInHook + context 'when desired_state matches actual_state' do let(:provisioned_workspace) do create(:workspace, agent: agent, user: user, group: group, desired_state: desired_state, actual_state: actual_state) end + # rubocop:disable RSpec/ExpectInHook + before do + # noinspection RubyResolve + expect(provisioned_workspace.agent_info_reported_at) + .to be_after(provisioned_workspace.desired_state_updated_at) + end + # rubocop:enable RSpec/ExpectInHook + context 'when state is Stopped' do let(:desired_state) { RemoteDevelopment::States::STOPPED } @@ -148,6 +175,14 @@ let(:expected_workspace_rails_infos) { [expected_workspace_rails_info] } + # rubocop:disable RSpec/ExpectInHook + before do + # noinspection RubyResolve + expect(provisioned_workspace.agent_info_reported_at) + .to be_before(provisioned_workspace.desired_state_updated_at) + end + # rubocop:enable RSpec/ExpectInHook + context 'when desired_state is Running' do let(:desired_state) { RemoteDevelopment::States::RUNNING } let(:actual_state) { RemoteDevelopment::States::STOPPED } @@ -254,11 +289,6 @@ context 'when actual_state is Unknown' do let(:actual_state) { RemoteDevelopment::States::UNKNOWN } - let(:provisioned_workspace) do - create(:workspace, - agent: agent, user: user, group: group, actual_state: actual_state, desired_state: desired_state) - end - it 'has test coverage for logging in conditional' do subject.process(workspace_agent_infos: workspace_agent_infos, update_type: update_type) end diff --git a/ee/spec/models/remote_development/workspace_spec.rb b/ee/spec/models/remote_development/workspace_spec.rb index 670d9e36d21482..c05cd67c183902 100644 --- a/ee/spec/models/remote_development/workspace_spec.rb +++ b/ee/spec/models/remote_development/workspace_spec.rb @@ -38,4 +38,37 @@ expect(subject.terminated?).to eq(true) end end + + describe '.before_save' do + describe 'when creating new record', :freeze_time do + # NOTE: The workspaces factory overrides the desired_state_updated_at to be earlier than + # the current time, so we need to use build here instead of create here to test + # the callback which sets the desired_state_updated_at to current time upon creation. + subject { build(:workspace, user: user, group: group, agent: agent, project: project) } + + it 'sets desired_state_updated_at' do + subject.save! + # noinspection RubyResolve + expect(subject.desired_state_updated_at).to eq(Time.current) + end + end + + describe 'when updating desired_state' do + it 'sets desired_state_updated_at' do + expect { subject.update!(desired_state: ::RemoteDevelopment::States::RUNNING) }.to change { + # noinspection RubyResolve + subject.desired_state_updated_at + } + end + end + + describe 'when updating a field other than desired_state' do + it 'does not set desired_state_updated_at' do + expect { subject.update!(actual_state: ::RemoteDevelopment::States::RUNNING) }.not_to change { + # noinspection RubyResolve + subject.desired_state_updated_at + } + end + end + end end -- GitLab From cd5fc9f29ed8e27a99f0e69ec999a487c250e90e Mon Sep 17 00:00:00 2001 From: Chad Woolley Date: Mon, 10 Apr 2023 02:33:43 -0700 Subject: [PATCH 2/2] Review feedback --- .../20221225010101_create_workspaces_table.rb | 2 +- db/structure.sql | 2 +- doc/api/graphql/reference/index.md | 2 +- .../remote_development/workspace_type.rb | 2 +- ee/app/models/remote_development/workspace.rb | 6 +- .../remote_development/devfile_processor.rb | 4 +- .../workspace_updates_params_parser.rb | 2 + .../workspace_updates_processor.rb | 35 +- .../remote_development/workspaces.rb | 10 +- .../remote_development/workspace_type_spec.rb | 2 +- .../workspace_updates_processor_spec.rb | 595 +++++++++--------- .../remote_development_shared_contexts.rb | 2 + 12 files changed, 342 insertions(+), 322 deletions(-) diff --git a/db/migrate/20221225010101_create_workspaces_table.rb b/db/migrate/20221225010101_create_workspaces_table.rb index ebb3eddc9e0017..4d794a8f2acf3c 100644 --- a/db/migrate/20221225010101_create_workspaces_table.rb +++ b/db/migrate/20221225010101_create_workspaces_table.rb @@ -16,7 +16,7 @@ def up t.text :desired_state, limit: 32, null: false t.datetime_with_timezone :desired_state_updated_at, null: false t.text :actual_state, limit: 32, null: false - t.datetime_with_timezone :agent_info_reported_at + t.datetime_with_timezone :sent_to_agent_at t.text :editor, limit: 256, null: false t.text :devfile_ref, limit: 256, null: false t.text :devfile_path, limit: 2048, null: false diff --git a/db/structure.sql b/db/structure.sql index 0b7009793f8988..66785c2d66a907 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -24303,7 +24303,7 @@ CREATE TABLE workspaces ( desired_state text NOT NULL, desired_state_updated_at timestamp with time zone NOT NULL, actual_state text NOT NULL, - agent_info_reported_at timestamp with time zone, + sent_to_agent_at timestamp with time zone, editor text NOT NULL, devfile_ref text NOT NULL, devfile_path text NOT NULL, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 6a0805360fdb92..9088271e783872 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -22608,7 +22608,6 @@ Represents a remote development workspace. | Name | Type | Description | | ---- | ---- | ----------- | | `actualState` | [`String!`](#string) | Actual state of the workspace. | -| `agentInfoReportedAt` | [`Time`](#time) | Timestamp of last report of desired state from GA4K. | | `clusterAgent` | [`ClusterAgent!`](#clusteragent) | Kubernetes Agent associated with the workspace. | | `createdAt` | [`Time!`](#time) | Timestamp of workspace creation. | | `deploymentResourceVersion` | [`Int`](#int) | ResourceVersion of the Deployment resource for the workspace. | @@ -22625,6 +22624,7 @@ Represents a remote development workspace. | `namespace` | [`String!`](#string) | Namespace of the workspace in Kubernetes. | | `processedDevfile` | [`String!`](#string) | Processed YAML of the devfile used to configure the workspace. | | `project` | [`Project!`](#project) | Project providing the Devfile for the workspace. | +| `sentToAgentAt` | [`Time`](#time) | Timestamp of last report of desired state from GA4K. | | `updatedAt` | [`Time!`](#time) | Timestamp of last update to any mutable workspace property. | | `url` | [`String!`](#string) | URL of the workspace. | | `user` | [`UserCore!`](#usercore) | Owner of the workspace. | diff --git a/ee/app/graphql/types/remote_development/workspace_type.rb b/ee/app/graphql/types/remote_development/workspace_type.rb index 51159f5c3fc8b9..c9e194e570f560 100644 --- a/ee/app/graphql/types/remote_development/workspace_type.rb +++ b/ee/app/graphql/types/remote_development/workspace_type.rb @@ -42,7 +42,7 @@ class WorkspaceType < ::Types::BaseObject field :actual_state, GraphQL::Types::String, null: false, description: 'Actual state of the workspace.' - field :agent_info_reported_at, Types::TimeType, + field :sent_to_agent_at, Types::TimeType, null: true, description: 'Timestamp of last report of desired state from GA4K.' field :displayed_state, GraphQL::Types::String, diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index 0d0912ef52b340..1c83a2dd3b81dd 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -31,7 +31,7 @@ class Workspace < ApplicationRecord scope :with_desired_state_updated_more_recently_than_agent_info_reported, -> do # noinspection SqlResolve - where('desired_state_updated_at >= agent_info_reported_at').or(where(agent_info_reported_at: nil)) + where('desired_state_updated_at >= sent_to_agent_at').or(where(sent_to_agent_at: nil)) end scope :with_ids_in, ->(ids_to_not_match) { where(id: ids_to_not_match) } @@ -46,9 +46,9 @@ def displayed_state after_commit :trigger_group_workspaces_updated_subscription def desired_state_updated_more_recently_than_agent_info_reported? - return true if agent_info_reported_at.nil? + return true if sent_to_agent_at.nil? - desired_state_updated_at >= agent_info_reported_at + desired_state_updated_at >= sent_to_agent_at end def terminated? diff --git a/ee/lib/remote_development/devfile_processor.rb b/ee/lib/remote_development/devfile_processor.rb index 3ae053ec2bdb81..b10e1f0aca753f 100644 --- a/ee/lib/remote_development/devfile_processor.rb +++ b/ee/lib/remote_development/devfile_processor.rb @@ -2,7 +2,6 @@ require 'devfile' -# rubocop:disable Gitlab/RailsLogger module RemoteDevelopment class DevfileProcessor def process(devfile:, editor:) @@ -17,7 +16,7 @@ def process(devfile:, editor:) def add_editor(devfile:, editor:) # TODO: choose image based on which editor is passed. - Rails.logger.debug("DEBUGGING: adding editor #{editor} to devfile") # use editor to avoid unused param warning + raise "Editor is required" unless editor # use editor param to avoid unused param warning in RubyMine for now image_name = 'registry.gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork/web-ide-injector' image_tag = '1.75.1-1.0.0-dev-20230224090559' @@ -119,4 +118,3 @@ def add_editor(devfile:, editor:) # end end end -# rubocop:enable Gitlab/RailsLogger diff --git a/ee/lib/remote_development/workspace_updates_params_parser.rb b/ee/lib/remote_development/workspace_updates_params_parser.rb index 57b2f599269c07..af44e53f9c34bf 100644 --- a/ee/lib/remote_development/workspace_updates_params_parser.rb +++ b/ee/lib/remote_development/workspace_updates_params_parser.rb @@ -4,7 +4,9 @@ module RemoteDevelopment class WorkspaceUpdatesParamsParser + # TODO: Rename message type WORKSPACE_UPDATES to WORKSPACE_RECONCILIATION MESSAGE_TYPE_WORKSPACE_UPDATES = 'workspace_updates' + # TODO: Move these to RemoteDevelopment::UpdateType::PARTIAL and RemoteDevelopment::UpdateType::FULL UPDATE_TYPE_PARTIAL = 'partial' UPDATE_TYPE_FULL = 'full' diff --git a/ee/lib/remote_development/workspace_updates_processor.rb b/ee/lib/remote_development/workspace_updates_processor.rb index 49d639cb1414f2..8d5367429d93ae 100644 --- a/ee/lib/remote_development/workspace_updates_processor.rb +++ b/ee/lib/remote_development/workspace_updates_processor.rb @@ -16,17 +16,17 @@ def process(workspace_agent_infos:, update_type:) end names_from_agent_infos = workspace_agent_infos_by_name.keys - check_for_orphan_workspaces(names_from_agent_infos) - # TODO: SECURITY CONCERN - This needs to be scoped by agent persisted_workspaces_from_agent_infos = RemoteDevelopment::Workspace.where(name: names_from_agent_infos) + check_for_orphan_workspaces(names_from_agent_infos, persisted_workspaces_from_agent_infos.map(&:name)) + # Update persisted workspaces which match the names of the workspaces in the WorkspaceAgentInfo objects array - persisted_workspaces_from_agent_infos.each do |workspace_from_agent_info| - workspace_agent_info = workspace_agent_infos_by_name[workspace_from_agent_info.name] + persisted_workspaces_from_agent_infos.each do |workspace| + workspace_agent_info = workspace_agent_infos_by_name[workspace.name] # Update the persisted workspaces with the latest info from the WorkspaceAgentInfo objects we received update_existing_workspace_with_latest_info( - persisted_workspace: workspace_from_agent_info, + persisted_workspace: workspace, deployment_resource_version: workspace_agent_info.deployment_resource_version, actual_state: workspace_agent_info.actual_state ) @@ -38,7 +38,7 @@ def process(workspace_agent_infos:, update_type:) if update_type == RemoteDevelopment::WorkspaceUpdatesParamsParser::UPDATE_TYPE_FULL # For a FULL update, return all workspaces for the agent which exist in the database # TODO: SECURITY CONCERN - This needs to be scoped by agent - workspaces_to_return_in_rails_infos_query = RemoteDevelopment::Workspace.all.to_a + workspaces_to_return_in_rails_infos_query = RemoteDevelopment::Workspace.all else # For a PARTIAL update, return: # 1. Workspaces with_desired_state_updated_more_recently_than_agent_info_reported @@ -57,23 +57,21 @@ def process(workspace_agent_infos:, update_type:) # 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| - deployment_resource_version = workspace.deployment_resource_version workspace_rails_info = { name: workspace.name, namespace: workspace.namespace, # TODO: Dry up disagreement on whether this version is an integer or a string - k8s has it as a string, # but value is int and that's what we put in the db schema. There is a task to convert to an integer # on the agentk side: https://gitlab.com/gitlab-org/gitlab/-/issues/396882 - persisted_deployment_resource_version: - deployment_resource_version.blank? ? nil : deployment_resource_version.to_s, + persisted_deployment_resource_version: workspace.deployment_resource_version&.to_s, persisted_actual_state_is_terminated: workspace.actual_state == States::TERMINATED, # help agentk manage state desired_state_is_terminated: workspace.desired_state == States::TERMINATED, - config_to_apply: desired_config_to_apply(workspace) + config_to_apply: config_to_apply(workspace: workspace, update_type: update_type) }.compact # remove nil entries - # Update the agent_info_reported_at at this point, after we have already done all the calculations + # Update the sent_to_agent_at at this point, after we have already done all the calculations # related to state - workspace.touch(:agent_info_reported_at) + workspace.touch(:sent_to_agent_at) workspace_rails_info end @@ -87,8 +85,11 @@ def process(workspace_agent_infos:, update_type:) private - def desired_config_to_apply(workspace) - return unless workspace.desired_state_updated_more_recently_than_agent_info_reported? + def config_to_apply(workspace:, update_type:) + # If update_type==FULL, always return the config + # If update_type==PARTIAL, only return config if !desired_state_updated_more_recently_than_agent_info_reported? + return if update_type == RemoteDevelopment::WorkspaceUpdatesParamsParser::UPDATE_TYPE_PARTIAL && + !workspace.desired_state_updated_more_recently_than_agent_info_reported? workspace_resources = RemoteDevelopment::DesiredConfigGenerator.new.generate_desired_config(workspace: workspace) @@ -102,9 +103,7 @@ def desired_config_to_apply(workspace) desired_config_to_apply_array.join end - def check_for_orphan_workspaces(names_from_agent_infos) - found_names = RemoteDevelopment::Workspace.where(name: names_from_agent_infos).pluck(:name) - + def check_for_orphan_workspaces(names_from_agent_infos, found_names) orphan_workspaces = names_from_agent_infos - found_names return unless orphan_workspaces.present? @@ -123,7 +122,7 @@ def update_existing_workspace_with_latest_info(persisted_workspace:, deployment_ 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 resulted in a Failure or Error. + # workspace creation resulted in an Error. persisted_workspace.deployment_resource_version = deployment_resource_version if deployment_resource_version persisted_workspace.save! diff --git a/ee/spec/factories/remote_development/workspaces.rb b/ee/spec/factories/remote_development/workspaces.rb index a5555e720f6855..637e32f7aedec1 100644 --- a/ee/spec/factories/remote_development/workspaces.rb +++ b/ee/spec/factories/remote_development/workspaces.rb @@ -46,23 +46,23 @@ if workspace.desired_state == workspace.actual_state # The most recent activity was a poll that reconciled the desired and actual state. desired_state_updated_at = 2.seconds.ago - agent_info_reported_at = 1.second.ago + sent_to_agent_at = 1.second.ago else # The most recent activity was a user action which updated the desired state to be different # than the actual state. desired_state_updated_at = 1.second.ago - agent_info_reported_at = 2.seconds.ago + sent_to_agent_at = 2.seconds.ago end workspace.update!( # NOTE: created_at and updated_at are not currently used in any logic, but we set them to be - # before desired_state_updated_at or agent_info_reported_at to ensure the record represents + # before desired_state_updated_at or sent_to_agent_at to ensure the record represents # a realistic condition. created_at: 3.seconds.ago, updated_at: 3.seconds.ago, desired_state_updated_at: desired_state_updated_at, - agent_info_reported_at: agent_info_reported_at + sent_to_agent_at: sent_to_agent_at ) end @@ -70,7 +70,7 @@ desired_state { RemoteDevelopment::States::RUNNING } actual_state { RemoteDevelopment::States::CREATING } # noinspection RubyResolve - agent_info_reported_at { nil } + sent_to_agent_at { nil } deployment_resource_version { nil } end end diff --git a/ee/spec/graphql/types/remote_development/workspace_type_spec.rb b/ee/spec/graphql/types/remote_development/workspace_type_spec.rb index 01f835bebca668..26ffd63d8f471c 100644 --- a/ee/spec/graphql/types/remote_development/workspace_type_spec.rb +++ b/ee/spec/graphql/types/remote_development/workspace_type_spec.rb @@ -6,7 +6,7 @@ let(:fields) do %i[ id cluster_agent group project user name namespace - desired_state desired_state_updated_at actual_state agent_info_reported_at displayed_state + desired_state desired_state_updated_at actual_state sent_to_agent_at displayed_state url editor devfile_ref devfile_path devfile processed_devfile deployment_resource_version created_at updated_at ] end diff --git a/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb b/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb index 2dc841bdc39897..6e3f7227958cf0 100644 --- a/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb +++ b/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb @@ -11,248 +11,151 @@ let_it_be(:agent) { create(:cluster_agent) } let(:expected_value_for_desired_state_is_terminated) { false } let(:expected_value_for_started) { true } - let(:update_type) { RemoteDevelopment::WorkspaceUpdatesParamsParser::UPDATE_TYPE_PARTIAL } subject do described_class.new end - context 'when receiving agent updates for a workspace which exists in the db' do - let(:desired_state) { RemoteDevelopment::States::STOPPED } - let(:actual_state) { RemoteDevelopment::States::STOPPED } - let(:deployment_resource_version_from_agent) { '2' } - let(:expected_deployment_resource_version) { deployment_resource_version_from_agent } - let(:owning_inventory) { "#{provisioned_workspace.name}-workspace-inventory" } - - # noinspection RubyResolve - let(:workspace_agent_info) do - create_workspace_agent_info( - workspace_name: provisioned_workspace.name, - workspace_namespace: provisioned_workspace.namespace, - agent_id: provisioned_workspace.agent.id, - owning_inventory: owning_inventory, - resource_version: deployment_resource_version_from_agent, - actual_state: actual_state - ) - end - - let(:workspace_agent_infos) { [workspace_agent_info] } - let(:expected_value_for_persisted_actual_state_is_terminated) { false } - - # noinspection RubyResolve - let(:expected_provisioned_workspace_rails_info) do - { - desired_state_is_terminated: expected_value_for_desired_state_is_terminated, - name: provisioned_workspace.name, - namespace: provisioned_workspace.namespace, - persisted_deployment_resource_version: expected_deployment_resource_version, - persisted_actual_state_is_terminated: expected_value_for_persisted_actual_state_is_terminated - } - end - - let(:expected_workspace_rails_infos) { [expected_provisioned_workspace_rails_info] } - - # rubocop:disable RSpec/ExpectInHook - before do - # Ensure that both desired_state_updated_at and agent_info_reported_at are before Time.current, - # so that we can test for any necessary differences after processing updates them - # noinspection RubyResolve - expect(provisioned_workspace.desired_state_updated_at).to be_before(Time.current) - # noinspection RubyResolve - expect(provisioned_workspace.agent_info_reported_at).to be_before(Time.current) - end - - after do - # After processing, the agent_info_reported_at should always have been updated - provisioned_workspace.reload - # noinspection RubyResolve - expect(provisioned_workspace.agent_info_reported_at) - .not_to be_before(provisioned_workspace.desired_state_updated_at) - end - # rubocop:enable RSpec/ExpectInHook - - context 'when desired_state matches actual_state' do - let(:provisioned_workspace) do - create(:workspace, - agent: agent, user: user, group: group, desired_state: desired_state, actual_state: actual_state) - end - - # rubocop:disable RSpec/ExpectInHook - before do - # noinspection RubyResolve - expect(provisioned_workspace.agent_info_reported_at) - .to be_after(provisioned_workspace.desired_state_updated_at) - end - # rubocop:enable RSpec/ExpectInHook - - context 'when state is Stopped' do - let(:desired_state) { RemoteDevelopment::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(provisioned_workspace.desired_state).to eq(desired_state) - expect(provisioned_workspace.actual_state).to eq(actual_state) - - payload, error = subject.process(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) - - provisioned_workspace.reload - - expect(provisioned_workspace.desired_state).to eq(provisioned_workspace.actual_state) - expect(provisioned_workspace.deployment_resource_version.to_s) - .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::States::TERMINATED } - let(:actual_state) { RemoteDevelopment::States::TERMINATED } - let(:expected_value_for_desired_state_is_terminated) { true } - let(:expected_value_for_persisted_actual_state_is_terminated) { true } - let(:expected_deployment_resource_version) { provisioned_workspace.deployment_resource_version.to_s } - - 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(provisioned_workspace.desired_state).to eq(desired_state) - expect(provisioned_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(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) + context 'when update_type is full' do + let(:update_type) { RemoteDevelopment::WorkspaceUpdatesParamsParser::UPDATE_TYPE_FULL } - provisioned_workspace.reload - - expect(provisioned_workspace.desired_state).to eq(provisioned_workspace.actual_state) - expect(provisioned_workspace.deployment_resource_version.to_s) - .to eq(expected_deployment_resource_version) + it 'updates workspace record and returns proper workspace_rails_info entry' do + create(:workspace, agent: agent, user: user, group: group) + payload, error = subject.process(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 - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end + # 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 desired_state does not match actual_state' do - let(:provisioned_workspace) do - create( - :workspace, agent: agent, user: user, group: group, - desired_state: desired_state, actual_state: actual_state - ) - end + context 'when update_type is partial' do + let(:update_type) { RemoteDevelopment::WorkspaceUpdatesParamsParser::UPDATE_TYPE_PARTIAL } - # noinspection RubyResolve - let(:deployment_resource_version_from_agent) { provisioned_workspace.deployment_resource_version.to_s } - # noinspection RubyResolve + context 'when receiving agent updates for a workspace which exists in the db' do + let(:desired_state) { RemoteDevelopment::States::STOPPED } + let(:actual_state) { RemoteDevelopment::States::STOPPED } + let(:deployment_resource_version_from_agent) { '2' } + let(:expected_deployment_resource_version) { deployment_resource_version_from_agent } let(:owning_inventory) { "#{provisioned_workspace.name}-workspace-inventory" } - # noinspection RubyResolve - let(:expected_config_to_apply) do - create_config_to_apply( - workspace_id: provisioned_workspace.id, + let(:workspace_agent_info) do + create_workspace_agent_info( workspace_name: provisioned_workspace.name, workspace_namespace: provisioned_workspace.namespace, agent_id: provisioned_workspace.agent.id, owning_inventory: owning_inventory, - started: expected_value_for_started + resource_version: deployment_resource_version_from_agent, + actual_state: actual_state ) end - let(:expected_workspace_rails_info) do + let(:workspace_agent_infos) { [workspace_agent_info] } + let(:expected_value_for_persisted_actual_state_is_terminated) { false } + + # noinspection RubyResolve + let(:expected_provisioned_workspace_rails_info) do { desired_state_is_terminated: expected_value_for_desired_state_is_terminated, name: provisioned_workspace.name, namespace: provisioned_workspace.namespace, - config_to_apply: expected_config_to_apply, - persisted_deployment_resource_version: deployment_resource_version_from_agent, - persisted_actual_state_is_terminated: false - }.compact + persisted_deployment_resource_version: expected_deployment_resource_version, + persisted_actual_state_is_terminated: expected_value_for_persisted_actual_state_is_terminated + } end - let(:expected_workspace_rails_infos) { [expected_workspace_rails_info] } + let(:expected_workspace_rails_infos) { [expected_provisioned_workspace_rails_info] } # rubocop:disable RSpec/ExpectInHook before do + # Ensure that both desired_state_updated_at and sent_to_agent_at are before Time.current, + # so that we can test for any necessary differences after processing updates them + # noinspection RubyResolve + expect(provisioned_workspace.desired_state_updated_at).to be_before(Time.current) + # noinspection RubyResolve + expect(provisioned_workspace.sent_to_agent_at).to be_before(Time.current) + end + + after do + # After processing, the sent_to_agent_at should always have been updated + provisioned_workspace.reload # noinspection RubyResolve - expect(provisioned_workspace.agent_info_reported_at) - .to be_before(provisioned_workspace.desired_state_updated_at) + expect(provisioned_workspace.sent_to_agent_at) + .not_to be_before(provisioned_workspace.desired_state_updated_at) end # rubocop:enable RSpec/ExpectInHook - context 'when desired_state is Running' do - let(:desired_state) { RemoteDevelopment::States::RUNNING } - let(:actual_state) { RemoteDevelopment::States::STOPPED } + context 'when desired_state matches actual_state' do + let(:provisioned_workspace) do + create(:workspace, + agent: agent, user: user, group: group, desired_state: desired_state, actual_state: actual_state) + end - # noinspection RubyResolve - 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(provisioned_workspace.desired_state).to eq(desired_state) - expect(provisioned_workspace.actual_state).to eq(actual_state) + # rubocop:disable RSpec/ExpectInHook + before do + # noinspection RubyResolve + expect(provisioned_workspace.sent_to_agent_at) + .to be_after(provisioned_workspace.desired_state_updated_at) + end + # rubocop:enable RSpec/ExpectInHook - payload, error = subject.process(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) + context 'when state is Stopped' do + let(:desired_state) { RemoteDevelopment::States::STOPPED } - provisioned_workspace.reload + 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(provisioned_workspace.desired_state).to eq(desired_state) + expect(provisioned_workspace.actual_state).to eq(actual_state) - expect(provisioned_workspace.deployment_resource_version.to_s) - .to eq(expected_deployment_resource_version) + payload, error = subject.process(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 - provisioned_workspace_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == provisioned_workspace.name } - expect(provisioned_workspace_rails_info.fetch(:config_to_apply)) - .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) + provisioned_workspace.reload - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - end + expect(provisioned_workspace.desired_state).to eq(provisioned_workspace.actual_state) + expect(provisioned_workspace.deployment_resource_version.to_s) + .to eq(expected_deployment_resource_version) - context 'when desired_state is Terminated' do - let(:desired_state) { RemoteDevelopment::States::TERMINATED } - let(:actual_state) { RemoteDevelopment::States::STOPPED } - let(:expected_value_for_desired_state_is_terminated) { true } - let(:expected_value_for_started) { false } + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end + end - # noinspection RubyResolve - 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(provisioned_workspace.desired_state).to eq(desired_state) - expect(provisioned_workspace.actual_state).to eq(actual_state) + context 'when state is Terminated' do + let(:desired_state) { RemoteDevelopment::States::TERMINATED } + let(:actual_state) { RemoteDevelopment::States::TERMINATED } + let(:expected_value_for_desired_state_is_terminated) { true } + let(:expected_value_for_persisted_actual_state_is_terminated) { true } + let(:expected_deployment_resource_version) { provisioned_workspace.deployment_resource_version.to_s } - payload, error = subject.process(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) + 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(provisioned_workspace.desired_state).to eq(desired_state) + expect(provisioned_workspace.actual_state).to eq(actual_state) - provisioned_workspace.reload + # We could do this with a should_not_change block but this reads cleaner IMO + payload, error = subject.process(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) - expect(provisioned_workspace.deployment_resource_version.to_s) - .to eq(expected_deployment_resource_version) + provisioned_workspace.reload - # test the config to apply first to get a more specific diff if it fails - # noinspection RubyResolve - provisioned_workspace_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == provisioned_workspace.name } - expect(provisioned_workspace_rails_info.fetch(:config_to_apply)) - .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) + expect(provisioned_workspace.desired_state).to eq(provisioned_workspace.actual_state) + expect(provisioned_workspace.deployment_resource_version.to_s) + .to eq(expected_deployment_resource_version) - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + end end end - context 'when desired_state is Restarting and actual_state is Stopped' do - let(:desired_state) { RemoteDevelopment::States::RESTARTING } - let(:actual_state) { RemoteDevelopment::States::STOPPED } - + context 'when desired_state does not match actual_state' do let(:provisioned_workspace) do create( :workspace, agent: agent, user: user, group: group, @@ -261,126 +164,242 @@ end # noinspection RubyResolve - it 'changes desired_state to Running' do - # verify initial states in db (sanity check of match between factory and fixtures) - expect(provisioned_workspace.desired_state).to eq(desired_state) - expect(provisioned_workspace.actual_state).to eq(actual_state) + let(:deployment_resource_version_from_agent) { provisioned_workspace.deployment_resource_version.to_s } + # noinspection RubyResolve + let(:owning_inventory) { "#{provisioned_workspace.name}-workspace-inventory" } - payload, error = subject.process(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) + # noinspection RubyResolve + let(:expected_config_to_apply) do + create_config_to_apply( + workspace_id: provisioned_workspace.id, + workspace_name: provisioned_workspace.name, + workspace_namespace: provisioned_workspace.namespace, + agent_id: provisioned_workspace.agent.id, + owning_inventory: owning_inventory, + started: expected_value_for_started + ) + end + + let(:expected_workspace_rails_info) do + { + desired_state_is_terminated: expected_value_for_desired_state_is_terminated, + name: provisioned_workspace.name, + namespace: provisioned_workspace.namespace, + config_to_apply: expected_config_to_apply, + persisted_deployment_resource_version: deployment_resource_version_from_agent, + persisted_actual_state_is_terminated: false + }.compact + end - provisioned_workspace.reload - expect(provisioned_workspace.desired_state).to eq(RemoteDevelopment::States::RUNNING) + let(:expected_workspace_rails_infos) { [expected_workspace_rails_info] } - # test the config to apply first to get a more specific diff if it fails + # rubocop:disable RSpec/ExpectInHook + before do # noinspection RubyResolve - provisioned_workspace_rails_info = - workspace_rails_infos.detect { |info| info.fetch(:name) == provisioned_workspace.name } - expect(provisioned_workspace_rails_info[:config_to_apply]) - .to eq(expected_workspace_rails_info.fetch(:config_to_apply)) + expect(provisioned_workspace.sent_to_agent_at) + .to be_before(provisioned_workspace.desired_state_updated_at) + end + # rubocop:enable RSpec/ExpectInHook + + context 'when desired_state is Running' do + let(:desired_state) { RemoteDevelopment::States::RUNNING } + let(:actual_state) { RemoteDevelopment::States::STOPPED } - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + # noinspection RubyResolve + 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(provisioned_workspace.desired_state).to eq(desired_state) + expect(provisioned_workspace.actual_state).to eq(actual_state) + + payload, error = subject.process(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) + + provisioned_workspace.reload + + expect(provisioned_workspace.deployment_resource_version.to_s) + .to eq(expected_deployment_resource_version) + + # test the config to apply first to get a more specific diff if it fails + # noinspection RubyResolve + provisioned_workspace_rails_info = + workspace_rails_infos.detect { |info| info.fetch(:name) == provisioned_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 - end - context 'when actual_state is Unknown' do - let(:actual_state) { RemoteDevelopment::States::UNKNOWN } + context 'when desired_state is Terminated' do + let(:desired_state) { RemoteDevelopment::States::TERMINATED } + let(:actual_state) { RemoteDevelopment::States::STOPPED } + let(:expected_value_for_desired_state_is_terminated) { true } + let(:expected_value_for_started) { false } + + # noinspection RubyResolve + 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(provisioned_workspace.desired_state).to eq(desired_state) + expect(provisioned_workspace.actual_state).to eq(actual_state) + + payload, error = subject.process(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) + + provisioned_workspace.reload + + expect(provisioned_workspace.deployment_resource_version.to_s) + .to eq(expected_deployment_resource_version) + + # test the config to apply first to get a more specific diff if it fails + # noinspection RubyResolve + provisioned_workspace_rails_info = + workspace_rails_infos.detect { |info| info.fetch(:name) == provisioned_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 Restarting and actual_state is Stopped' do + let(:desired_state) { RemoteDevelopment::States::RESTARTING } + let(:actual_state) { RemoteDevelopment::States::STOPPED } + + let(:provisioned_workspace) do + create( + :workspace, agent: agent, user: user, group: group, + desired_state: desired_state, actual_state: actual_state + ) + end + + # noinspection RubyResolve + it 'changes desired_state to Running' do + # verify initial states in db (sanity check of match between factory and fixtures) + expect(provisioned_workspace.desired_state).to eq(desired_state) + expect(provisioned_workspace.actual_state).to eq(actual_state) + + payload, error = subject.process(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) + + provisioned_workspace.reload + expect(provisioned_workspace.desired_state).to eq(RemoteDevelopment::States::RUNNING) + + # test the config to apply first to get a more specific diff if it fails + # noinspection RubyResolve + provisioned_workspace_rails_info = + workspace_rails_infos.detect { |info| info.fetch(:name) == provisioned_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 - it 'has test coverage for logging in conditional' do - subject.process(workspace_agent_infos: workspace_agent_infos, update_type: update_type) + context 'when actual_state is Unknown' do + let(:actual_state) { RemoteDevelopment::States::UNKNOWN } + + it 'has test coverage for logging in conditional' do + subject.process(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_name: workspace_name, - workspace_namespace: workspace_namespace, - agent_id: '1', - owning_inventory: 'does-not-matter', - resource_version: '42', - actual_state: RemoteDevelopment::States::STOPPED - ) - 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_name: workspace_name, + workspace_namespace: workspace_namespace, + agent_id: '1', + owning_inventory: 'does-not-matter', + resource_version: '42', + actual_state: RemoteDevelopment::States::STOPPED + ) + end - let(:workspace_agent_infos) { [workspace_agent_info] } + let(:workspace_agent_infos) { [workspace_agent_info] } - let(:expected_workspace_rails_infos) { [] } + 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(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 + it 'prints an error and does not attempt to update the workspace in the db' do + payload, error = subject.process(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 - end - context 'when new unprovisioned workspace exists in database"' do - let(:desired_state) { RemoteDevelopment::States::RUNNING } - let(:actual_state) { RemoteDevelopment::States::CREATING } + context 'when new unprovisioned workspace exists in database"' do + let(:desired_state) { RemoteDevelopment::States::RUNNING } + let(:actual_state) { RemoteDevelopment::States::CREATING } - let_it_be(:unprovisioned_workspace) do - create(:workspace, :unprovisioned, agent: agent, user: user, group: group) - end + let_it_be(:unprovisioned_workspace) do + create(:workspace, :unprovisioned, agent: agent, user: user, group: group) + end - let(:workspace_agent_infos) { [] } - - # noinspection RubyResolve - let(:owning_inventory) { "#{unprovisioned_workspace.name}-workspace-inventory" } - - # noinspection RubyResolve - 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 - ) - end + let(:workspace_agent_infos) { [] } - # noinspection RubyResolve - let(:expected_unprovisioned_workspace_rails_info) do - { - desired_state_is_terminated: expected_value_for_desired_state_is_terminated, - name: unprovisioned_workspace.name, - namespace: unprovisioned_workspace.namespace, - config_to_apply: expected_config_to_apply, - persisted_actual_state_is_terminated: false - } - end + # noinspection RubyResolve + let(:owning_inventory) { "#{unprovisioned_workspace.name}-workspace-inventory" } - let(:expected_workspace_rails_infos) { [expected_unprovisioned_workspace_rails_info] } + # noinspection RubyResolve + 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 + ) + end - # noinspection RubyResolve - 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) + # noinspection RubyResolve + let(:expected_unprovisioned_workspace_rails_info) do + { + desired_state_is_terminated: expected_value_for_desired_state_is_terminated, + name: unprovisioned_workspace.name, + namespace: unprovisioned_workspace.namespace, + config_to_apply: expected_config_to_apply, + persisted_actual_state_is_terminated: false + } + end - payload, error = subject.process(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) + let(:expected_workspace_rails_infos) { [expected_unprovisioned_workspace_rails_info] } - # test the config to apply first to get a more specific diff if it fails # noinspection RubyResolve - 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)) + 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(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) - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) + # test the config to apply first to get a more specific diff if it fails + # noinspection RubyResolve + 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/support/shared_contexts/remote_development/remote_development_shared_contexts.rb b/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb index ba9f2787b15489..90e11d8ad77155 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 @@ -9,6 +9,8 @@ def create_workspace_agent_info( resource_version:, actual_state: ) + # TODO: Default some of the parameters which can be derived from others: e.g. owning_inventory, workspace_namespace + info = { 'name' => workspace_name } -- GitLab