diff --git a/db/migrate/20221225010101_create_workspaces_table.rb b/db/migrate/20221225010101_create_workspaces_table.rb index f4b8edd845ded6bc9ecbe35cfa7c3ac97d751701..4d794a8f2acf3c0e547e65f823999864a928c712 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 :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 a4f6b3a47273943c82824c1c5a2a10fc713df02d..66785c2d66a9072ad47de3edb8f0dd43098c2bbf 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, + 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 e7a96684e39a98ac249aebad26a290cb643eab69..9088271e78387267975f487b014eb04552172e41 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -22609,8 +22609,10 @@ Represents a remote development workspace. | ---- | ---- | ----------- | | `actualState` | [`String!`](#string) | Actual state of the workspace. | | `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 +22624,8 @@ 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 f3d6c849ec556d3439189354b1b5f538995cf013..c9e194e570f560fbdf981cfe5225a14998e39f1e 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 :sent_to_agent_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 f33203919c150859de6e254ff769a39af035d04f..1c83a2dd3b81ddbacc35ea0a3417e1e5bdf6fce0 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 >= 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) } 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 sent_to_agent_at.nil? + + desired_state_updated_at >= sent_to_agent_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 230b8452fb9dfc480a18332ca92dcf033d89e1de..0b488b63329022cda63972490182221d06c7dca9 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 fe0a1915cbba0c9eda0c4e01563cfba392351362..16e679a2459aad5bcc2cae6e21308005b1135ab9 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/devfile_processor.rb b/ee/lib/remote_development/devfile_processor.rb index 3ae053ec2bdb81936f6be8d66ea330af9db862c2..b10e1f0aca753f087622b4190246230f34a19f14 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_agent_info_parser.rb b/ee/lib/remote_development/workspace_agent_info_parser.rb index bf9b81f9ee0836f9c5def109f931f2361b74f036..8c8ad5f79b5f8f1036206a63e4d3133de6498581 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_params_parser.rb b/ee/lib/remote_development/workspace_updates_params_parser.rb index 57b2f599269c076031224940e64e929dd9ceddb6..af44e53f9c34bf2bca45883233fa22ad7fb0237a 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 359277631fa070111d9d81ea6a1c5b7b16e0dd87..8d5367429d93ae6b9f1b9798c4b9f5cb9a41bf40 100644 --- a/ee/lib/remote_development/workspace_updates_processor.rb +++ b/ee/lib/remote_development/workspace_updates_processor.rb @@ -1,79 +1,79 @@ # 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) + # TODO: SECURITY CONCERN - This needs to be scoped by agent + persisted_workspaces_from_agent_infos = RemoteDevelopment::Workspace.where(name: 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 + check_for_orphan_workspaces(names_from_agent_infos, persisted_workspaces_from_agent_infos.map(&:name)) - # 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| + 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, 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 + 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 - - deployment_resource_version = workspace.deployment_resource_version - { + workspace_rails_infos = workspaces_to_return_in_rails_infos.map do |workspace| + 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 - persisted_deployment_resource_version: - deployment_resource_version.blank? ? nil : deployment_resource_version.to_s, + # 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: 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 + config_to_apply: config_to_apply(workspace: workspace, update_type: update_type) }.compact # remove nil entries + + # Update the sent_to_agent_at at this point, after we have already done all the calculations + # related to state + workspace.touch(:sent_to_agent_at) + + workspace_rails_info end Rails.logger.debug("DEBUGGING: workspace_rails_infos: #{workspace_rails_infos.inspect}") @@ -85,39 +85,50 @@ def process(workspace_agent_infos:, update_type:) private - def update_existing_workspace_with_latest_info( - persisted_workspace:, deployment_resource_version:, actual_state: - ) + 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? - 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) + 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 creation resulted in an 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 f99b5b23fb756ccf7877dfcd8e0ae480c996340f..637e32f7aedec1fe0d4507679c6f73554aa8e037 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 + 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 + 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 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, + sent_to_agent_at: sent_to_agent_at + ) + end + trait :unprovisioned do desired_state { RemoteDevelopment::States::RUNNING } actual_state { RemoteDevelopment::States::CREATING } + # noinspection RubyResolve + 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 e7d0de7b5ac4d0f6801f482b01d158a9585f450c..26ffd63d8f471c82e65559dcc2d189821671ba90 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 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/desired_config_generator_spec.rb b/ee/spec/lib/remote_development/desired_config_generator_spec.rb index 6c46e88a20674b3dd267b236ad35eaa515cfd5e8..76501bb6793dffff74754b1f4825b9800f7821e1 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 ffb38a27ff8c83b06a703d216017d0541c235cba..91cd5d92fb90c9a13169fc24b377ff2e8dc3db7d 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 e7b67a1e9e1d3dd48f0fb41d997a0a157e8f1c7b..6e3f7227958cf0ead017d89754c9b0ec0d220b9b 100644 --- a/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb +++ b/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb @@ -11,213 +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] } - - 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 - - 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 update_type is full' do + let(:update_type) { RemoteDevelopment::WorkspaceUpdatesParamsParser::UPDATE_TYPE_FULL } - 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) - - 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] } - context 'when desired_state is Running' do - let(:desired_state) { RemoteDevelopment::States::RUNNING } - let(:actual_state) { RemoteDevelopment::States::STOPPED } + # 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 - 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) + expect(provisioned_workspace.sent_to_agent_at) + .not_to be_before(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 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 - provisioned_workspace.reload + # 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 - expect(provisioned_workspace.deployment_resource_version.to_s) - .to eq(expected_deployment_resource_version) + context 'when state is Stopped' do + let(:desired_state) { RemoteDevelopment::States::STOPPED } - # 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)) + 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) - # then test everything in the infos - expect(workspace_rails_infos).to eq(expected_workspace_rails_infos) - end - 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) - 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 } + provisioned_workspace.reload - # 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) + 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 - 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 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 } - 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) + # 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) - # 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 + + 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, @@ -226,131 +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 } - let(:provisioned_workspace) do - create(:workspace, - agent: agent, user: user, group: group, actual_state: actual_state, desired_state: desired_state) + # 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 - it 'has test coverage for logging in conditional' do - subject.process(workspace_agent_infos: workspace_agent_infos, update_type: update_type) + 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 + + 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/models/remote_development/workspace_spec.rb b/ee/spec/models/remote_development/workspace_spec.rb index 670d9e36d21482a79db23e240c03b1997b636001..c05cd67c18390220c1c957d4676ce20c7119f439 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 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 ba9f2787b1548960924f1ebcb44774641143af56..90e11d8ad77155bdc2f1de1c753985ecb84b806d 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 }