diff --git a/ee/lib/remote_development/workspace_updates_processor.rb b/ee/lib/remote_development/workspace_updates_processor.rb index bd3019bd92e465539458a931a5ff2c3c581116d4..4036ecd793b86ca1ebe22435b97c09736d3839a4 100644 --- a/ee/lib/remote_development/workspace_updates_processor.rb +++ b/ee/lib/remote_development/workspace_updates_processor.rb @@ -67,7 +67,7 @@ def process(workspace_agent_infos:, update_type:) 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: config_to_apply(workspace: workspace, update_type: update_type) - }.compact # remove nil entries + } workspace_rails_info end diff --git a/ee/spec/features/remote_development/workspaces_spec.rb b/ee/spec/features/remote_development/workspaces_spec.rb index 187f1f72b066a9785161b1033a7b8ca8ddcf0081..48615c2ce41be2047fb1eca8fc2a5b3bf5fa548a 100644 --- a/ee/spec/features/remote_development/workspaces_spec.rb +++ b/ee/spec/features/remote_development/workspaces_spec.rb @@ -65,7 +65,7 @@ workspaces = RemoteDevelopment::Workspace.all.to_a expect(workspaces.length).to eq(1) - workspace = workspaces.first + workspace = workspaces[0] id = workspace.id name = workspace.name namespace = workspace.namespace @@ -90,6 +90,8 @@ force_page_refresh # There's better ways to make this locator, this is just a quick hack to get it working on the placeholder UI page.find('[data-test-column-name="displayed-state"]', text: RemoteDevelopment::States::RUNNING) + + # TODO: Add state transition to TERMINATED end def simulate_first_poll(id:, name:, namespace:) @@ -108,7 +110,7 @@ def simulate_first_poll(id:, name:, namespace:) expect(info.fetch(:namespace)).to eq(namespace) expect(info.fetch(:desired_state_is_terminated)).to eq(false) expect(info.fetch(:persisted_actual_state_is_terminated)).to eq(false) - expect(info.has_key?(:persisted_deployment_resource_version)).to eq(false) + expect(info.fetch(:persisted_deployment_resource_version)).to be_nil expected_config_to_apply = create_config_to_apply( workspace_id: id, @@ -152,7 +154,7 @@ def simulate_second_poll(name:, namespace:) expect(info.fetch(:persisted_deployment_resource_version)).to eq(resource_version.to_s) expect(info.fetch(:desired_state_is_terminated)).to eq(false) expect(info.fetch(:persisted_actual_state_is_terminated)).to eq(false) - expect(info.has_key?(:config_to_apply)).to eq(false) + expect(info.fetch(:config_to_apply)).to be_nil end def force_page_refresh diff --git a/ee/spec/lib/remote_development/workspace_updates_processor_reconciliation_scenarios_spec.rb b/ee/spec/lib/remote_development/workspace_updates_processor_reconciliation_scenarios_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f0681a46d17d813e547eb79486d28ba4f99f1325 --- /dev/null +++ b/ee/spec/lib/remote_development/workspace_updates_processor_reconciliation_scenarios_spec.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# TODO: This spec is dense and cryptic. Make it better. +RSpec.describe ::RemoteDevelopment::WorkspaceUpdatesProcessor, 'Reconciliation Scenarios', feature_category: :remote_development do + include_context 'with remote development shared fixtures' + + # See following documentation for details on all scenarios: + # + # https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/-/blob/main/doc/workspace-updates.md + # + # Columns: + # + # initial_db_state: Initial state of workspace in DB. nil for new workspace, 2-tuple of [desired_state, actual_state] + # for existing workspace. + # + # user_desired_state_update: Optional first request event. nil if there is no user action, symbol for state if there + # is a user action. + # + # agent_actual_state_updates: Array of actual state updates from agent. nil if agent reports no info for workspace, + # otherwise an array of [previous_actual_state, current_actual_state, workspace_exists] + # to be used as args when calling #create_workspace_agent_info to generate the workspace agent info fixture. + # + # response_expectations: Array corresponding to entries in agent_actual_state_updates, representing + # expected rails_info hash response to agent for the workspace. Array is a 2-tuple of booleans for + # [config_to_apply_present?, persisted_deployment_resource_version_present?]. + # + # db_expectations: Array corresponding to entries in + # (initial_db_state + user_desired_state_update + agent_actual_state_updates). + # Array entry is nil, or 2-tuple of symbols for [desired_state, actual_state]. + + # rubocop:disable Layout/LineLength + where(:initial_db_state, :user_desired_state_update, :agent_actual_state_updates, :response_expectations, :db_expectations) do + [ + # + # actual:Creating* / desired: Running -> actual: Running / desired: Running + [nil, :running, [nil, [:creating, :starting, false], [:starting, :running, true]], [[true, false], [false, true], [false, true]], [[:running, :creating], [:running, :creating], [:running, :starting], [:running, :running]]], + # + # actual:Running / desired: Running -> actual: Stopped / desired: Stopped + [[:running, :running], :stopped, [nil, [:running, :stopping, true], [:stopping, :stopped, false]], [[true, true], [false, true], [false, true]], [[:running, :running], [:stopped, :running], [:stopped, :running], [:stopped, :stopping], [:stopped, :stopped]]] + ] + end + # rubocop:enable Layout/LineLength + + with_them do + it 'behaves as expected' do + workspace = nil + db_expectation_index = 0 + initial_resource_version = 1 + + # Handle initial db state, if necessary + # noinspection RubyResolve + if initial_db_state + workspace = create( + :workspace, + desired_state: initial_db_state[0].to_s.capitalize, + actual_state: initial_db_state[1].to_s.capitalize, + deployment_resource_version: initial_resource_version + ) + + # assert on the workspace state in the db after initial creation + expect(workspace.slice(:desired_state, :actual_state).values) + .to eq(db_expectations[db_expectation_index].map(&:to_s).map(&:capitalize)) + db_expectation_index += 1 + end + + # handle user desired state update, if necessary + # noinspection RubyResolve + if user_desired_state_update + if workspace + # noinspection RubyResolve + workspace.update!(desired_state: user_desired_state_update.capitalize) + else + workspace = create(:workspace, :unprovisioned) + end + + # assert on the workspace state in the db after user desired state update + expect(workspace.slice(:desired_state, :actual_state).values) + .to eq(db_expectations[db_expectation_index].map(&:to_s).map(&:capitalize)) + db_expectation_index += 1 + end + + raise 'Must have workspace by now, either from initial_db_state or user_desired_state_update' unless workspace + + # Handle agent updates + # noinspection RubyResolve + agent_actual_state_updates.each_with_index do |actual_state_update_fixture_args, response_expectations_index| + update_type = RemoteDevelopment::WorkspaceUpdatesParamsParser::UPDATE_TYPE_PARTIAL + deployment_resource_version_from_agent ||= initial_resource_version + + workspace_agent_infos = + if actual_state_update_fixture_args + previous_actual_state = actual_state_update_fixture_args[0].to_s.capitalize + current_actual_state = actual_state_update_fixture_args[1].to_s.capitalize + workspace_exists = actual_state_update_fixture_args[2] + [ + create_workspace_agent_info( + workspace_name: workspace.name, + workspace_namespace: workspace.namespace, + agent_id: workspace.agent.id, + owning_inventory: "#{workspace.name}-workspace-inventory", + resource_version: deployment_resource_version_from_agent += 1, + current_actual_state: current_actual_state, + previous_actual_state: previous_actual_state, + workspace_exists: workspace_exists + ) + ] + else + [] + end + + result = described_class.new.process(workspace_agent_infos: workspace_agent_infos, update_type: update_type) + workspace_rails_infos = result[0].fetch(:workspace_rails_infos) + + # assert on the rails_info response to the agent + expect(workspace_rails_infos.size).to eq(1) + response_expectation = response_expectations[response_expectations_index] + # assert on the config_to_apply presence + expected_config_to_apply_present = response_expectation[0] + expect(workspace_rails_infos[0].fetch(:config_to_apply).present?).to eq(expected_config_to_apply_present) + # assert on the persisted_deployment_resource_version presence/value + expected_persisted_deployment_resource_version = + response_expectation[1] ? deployment_resource_version_from_agent.to_s : nil + persisted_deployment_resource_version = workspace_rails_infos[0].fetch(:persisted_deployment_resource_version) + expect(persisted_deployment_resource_version).to eq(expected_persisted_deployment_resource_version) + + # assert on the workspace state in the db after processing the agent update + expect(workspace.reload.slice(:desired_state, :actual_state).values) + .to eq(db_expectations[db_expectation_index].map(&:to_s).map(&:capitalize)) + db_expectation_index += 1 + end + end + end +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 d7b8f4ec6e023cf79217302a506131ef33695aa1..45ad4145d882ee0a6403ba2017c5aa5bd26f10d9 100644 --- a/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb +++ b/ee/spec/lib/remote_development/workspace_updates_processor_spec.rb @@ -69,7 +69,8 @@ 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 + persisted_actual_state_is_terminated: expected_value_for_persisted_actual_state_is_terminated, + config_to_apply: nil } end @@ -378,7 +379,8 @@ name: unprovisioned_workspace.name, namespace: unprovisioned_workspace.namespace, config_to_apply: expected_config_to_apply, - persisted_actual_state_is_terminated: false + persisted_actual_state_is_terminated: false, + persisted_deployment_resource_version: nil } end