diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp index 6ea9cd2e2..884c691fa 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp @@ -37,8 +37,8 @@ struct [[nodiscard]] ActivationFailed { /// @brief The identifier of the node that failed. IdentifierHash node_identifier; - /// @brief Description of the error. - IComponent::ComponentError reason; + /// @brief Why the activation failed, either kFailure or kTimeout. + IComponent::RequestResult reason; }; /// @brief A node finished deactivating. @@ -53,8 +53,6 @@ struct [[nodiscard]] UnexpectedTermination { /// @brief The identifier of the node that terminated. IdentifierHash node_identifier; - /// @brief Description of the error. - IComponent::ComponentError reason; }; /// @brief A job was queued but cancelled by the time it was processed diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp index 67d5e0c13..bc1839f57 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp @@ -54,14 +54,14 @@ TEST_F(ComponentEventQueueTest, GetNextEventReturnsPushedEventWithPayloadIntact) { RecordProperty("Description", "Verify a pushed event is returned by getNextEvent with its payload preserved."); const IdentifierHash process_identifier{"payload"}; - EXPECT_TRUE(queue_.push(ActivationFailed{process_identifier, IComponent::ComponentError::kErrorBeforeReady})); + EXPECT_TRUE(queue_.push(ActivationFailed{process_identifier, IComponent::RequestResult::kFailure})); auto event = queue_.getNextEvent(); ASSERT_TRUE(event.has_value()); ASSERT_TRUE(std::holds_alternative(*event)); const auto& failed = std::get(*event); EXPECT_EQ(failed.node_identifier, process_identifier); - EXPECT_EQ(failed.reason, IComponent::ComponentError::kErrorBeforeReady); + EXPECT_EQ(failed.reason, IComponent::RequestResult::kFailure); } TEST_F(ComponentEventQueueTest, GetNextEventReturnsSupervisionFailureWithPayloadIntact) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp index 846190da0..eb4a9c10c 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp @@ -175,11 +175,13 @@ void Graph::updateRunTargetInPlace(RunTarget& run_target, ComponentTaskType task // RunTargets are updated in place, no need to queue them on the thread pool. if (task_type == ComponentTaskType::kActivate) { - run_target.activate(stop_source_.get_token()); + const IComponent::RequestResult result = run_target.activate(stop_source_.get_token()); + SCORE_LANGUAGE_FUTURECPP_ASSERT(result == IComponent::RequestResult::kSuccess); } else { - run_target.deactivate(stop_source_.get_token()); + const IComponent::RequestResult result = run_target.deactivate(stop_source_.get_token()); + SCORE_LANGUAGE_FUTURECPP_ASSERT(result == IComponent::RequestResult::kSuccess); } current_transition_->onNodeFinished(run_target.getIdentifier()); } @@ -347,11 +349,11 @@ void Graph::handleComponentEvent(const ComponentEvent& event) << (std::is_same_v ? std::string_view("activation") : std::string_view("deactivation")) << "successfully"; - nodeExecuted(data.node_identifier, {}); + nodeExecuted(data.node_identifier, IComponent::RequestResult::kSuccess); } else if constexpr (std::is_same_v) { - nodeExecuted(data.node_identifier, score::cpp::make_unexpected(data.reason)); + nodeExecuted(data.node_identifier, data.reason); } else if constexpr (std::is_same_v) { @@ -368,17 +370,17 @@ void Graph::handleComponentEvent(const ComponentEvent& event) } else if constexpr (std::is_same_v) { - nodeExecuted(data.node_identifier, {}); + nodeExecuted(data.node_identifier, IComponent::RequestResult::kSuccess); } }, event); } -void Graph::nodeExecuted(IdentifierHash node, score::cpp::expected_blank error) +void Graph::nodeExecuted(IdentifierHash node, IComponent::RequestResult result) { bool was_last_in_queue = --jobs_in_progress_ == 0; - if (!error.has_value()) + if (result != IComponent::RequestResult::kSuccess) { setState(GraphState::kAborting); } diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp index 76423b2e7..9769aba73 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp @@ -279,7 +279,7 @@ class Graph final private: /// @brief Reports that a node has finished executing, enqueuing successors or updating the graph state if a /// transition has finished. - void nodeExecuted(IdentifierHash node, score::cpp::expected_blank error); + void nodeExecuted(IdentifierHash node, IComponent::RequestResult result); /// @brief Sets the current state of the graph. /// @param new_state The new state to set for the graph. diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp index 441569105..a28866e61 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp @@ -140,8 +140,7 @@ class GraphTest : public ::testing::Test res = job.component.get().deactivate(job.stop_token); } - ASSERT_TRUE(res.has_value()); - ASSERT_EQ(res.value(), IComponent::RequestState::kSuccess); + ASSERT_EQ(res, IComponent::RequestResult::kSuccess); } void failActivationJob(const ComponentTask& job) @@ -149,7 +148,7 @@ class GraphTest : public ::testing::Test EXPECT_CALL(process_interface_, startProcess).WillOnce(Return(osal::OsalReturnType::kFail)); const auto res = job.component.get().activate(job.stop_token); - ASSERT_FALSE(res.has_value()); + ASSERT_EQ(res, IComponent::RequestResult::kFailure); } /// @brief Execute a Run Target activation that activates or deactivates a single node @@ -345,7 +344,7 @@ TEST_F(GraphInitialTransitionTest, jobFailure) const auto job = job_queue_->pop()->value(); failActivationJob(job); graph_->handleComponentEvent( - ActivationFailed{IdentifierHash{process_name(0)}, IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{IdentifierHash{process_name(0)}, IComponent::RequestResult::kFailure}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState); } @@ -548,8 +547,7 @@ TEST_F(GraphHandleComponentEventTest, failedFirstDuringTransition) // Fail the first job const auto first_job = job_queue_->pop(); graph_->handleComponentEvent( - ActivationFailed{ - first_job->value().component.get().getIdentifier(), IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{first_job->value().component.get().getIdentifier(), IComponent::RequestResult::kFailure}); const auto second_job = job_queue_->pop(); @@ -567,8 +565,7 @@ TEST_F(GraphHandleComponentEventTest, failureFollowedBySuccessFails) // Fail the first job const auto first_job = job_queue_->pop(); graph_->handleComponentEvent( - ActivationFailed{ - first_job->value().component.get().getIdentifier(), IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{first_job->value().component.get().getIdentifier(), IComponent::RequestResult::kFailure}); const auto second_job = job_queue_->pop(); executeJobSuccessfully(second_job->value()); @@ -589,8 +586,7 @@ TEST_F(GraphHandleComponentEventTest, cancelAfterFailureStaysAborting) // Fail the first job const auto first_job = job_queue_->pop(); graph_->handleComponentEvent( - ActivationFailed{ - first_job->value().component.get().getIdentifier(), IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{first_job->value().component.get().getIdentifier(), IComponent::RequestResult::kFailure}); EXPECT_EQ(graph_->getState(), GraphState::kAborting); @@ -623,8 +619,7 @@ TEST_F(GraphHandleComponentEventTest, unexpectedTerminationDuringSuccess) }), Return(osal::OsalReturnType::kSuccess))); - graph_->handleComponentEvent( - UnexpectedTermination{component->getIdentifier(), IComponent::ComponentError::kErrorAfterReady}); + graph_->handleComponentEvent(UnexpectedTermination{component->getIdentifier()}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState); } @@ -652,7 +647,7 @@ TEST_F(GraphHandleComponentEventTest, unexpectedTerminationDuringTransition) Return(osal::OsalReturnType::kSuccess))); // The active component then crashes - graph_->handleComponentEvent(UnexpectedTermination{component_index, IComponent::ComponentError::kErrorAfterReady}); + graph_->handleComponentEvent(UnexpectedTermination{component_index}); const auto second_job = job_queue_->pop(); executeJobSuccessfully(second_job->value()); @@ -679,7 +674,7 @@ TEST_F(GraphTransitionFailuresTest, UnusualOrderOfFailures) .WillOnce(Return(osal::OsalReturnType::kSuccess)); // Process is already gone, semaphore will time out EXPECT_CALL(process_interface_, forceTermination).WillOnce(Return(osal::OsalReturnType::kFail)); - graph_->handleComponentEvent(UnexpectedTermination{component_id, IComponent::ComponentError::kErrorAfterReady}); + graph_->handleComponentEvent(UnexpectedTermination{component_id}); graph_->handleComponentEvent(ActivationSuccessful{component_id}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState) << "Graph should be in a final state"; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp index 80df89424..bc586b7e8 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp @@ -31,42 +31,37 @@ namespace score::mw::lifecycle::internal class IComponent { public: - enum class ComponentError : uint8_t + enum class [[nodiscard]] RequestResult : std::uint8_t { - /// @brief An error occurred during startup, before the component was considered ready. - kErrorBeforeReady, - /// @brief An error occurred any time after the component was considered ready. - kErrorAfterReady, - /// @brief An error occurred during startup. Specifically, a timeout was reached. - kActivationTimedOut, + /// @brief The operation succeeded. + kSuccess, + + /// @brief The operation timed out. + kTimeout, + + /// @brief The operation failed. + kFailure }; - enum class RequestState : uint8_t + enum class [[nodiscard]] TerminationResult : std::uint8_t { - /// @brief Activation was successful and the component is now ready. - kSuccess, - /// @brief Activation is waiting on a notification from another thread. The component may not be ready. - kWaiting - }; + /// @brief The termination does not affect the state of the component. + kIgnore, - using RequestResult = score::cpp::expected; + /// @brief The component failed. + kFailure + }; /// @brief Begin activation of the component. /// @p stop_token Token that can be stopped to exit the activation early. - /// @returns kSuccess if the component is now ready, kWaiting if the component is waiting for a notification, or an - /// error. - [[nodiscard]] virtual RequestResult activate(score::cpp::stop_token stop_token) = 0; + virtual RequestResult activate(score::cpp::stop_token stop_token) = 0; /// @brief Begin deactivation of the component. /// @p stop_token Token that can be stopped to exit the deactivation early. - /// @returns kSuccess if the component is now ready, kWaiting if the component is waiting for a notification, or an - /// error. - [[nodiscard]] virtual RequestResult deactivate(score::cpp::stop_token stop_token) = 0; + virtual RequestResult deactivate(score::cpp::stop_token stop_token) = 0; /// @brief Notify the component that it has terminated with status @p status - /// @returns kSuccess if the component is now ready, kWaiting if the component is waiting for a notification, or an - /// error if the termination was not expected. - [[nodiscard]] virtual RequestResult tryHandleTermination(int32_t status) = 0; + virtual TerminationResult tryHandleTermination(int32_t status) = 0; /// @returns the index of the component in the graph. [[nodiscard]] virtual IdentifierHash getIdentifier() const = 0; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp index 3c0413884..8af474a3f 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp @@ -24,7 +24,7 @@ class MockComponent : public IComponent public: MOCK_METHOD(RequestResult, activate, (score::cpp::stop_token stop_token), (override)); MOCK_METHOD(RequestResult, deactivate, (score::cpp::stop_token stop_token), (override)); - MOCK_METHOD(RequestResult, tryHandleTermination, (int32_t status), (override)); + MOCK_METHOD(TerminationResult, tryHandleTermination, (int32_t status), (override)); MOCK_METHOD(IdentifierHash, getIdentifier, (), (override, const)); MOCK_METHOD(bool, active, (), (override, const)); }; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp index 138f68892..c4943c16e 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp @@ -97,13 +97,13 @@ IComponent::RequestResult ProcessInfoNode::tryReportCompletion(score::mw::lifecy if (new_state == ProcessState::kFailed) { // Didn't reach running or startup - return tryReportError(ComponentError::kErrorBeforeReady); + return tryReportError(RequestResult::kFailure); } if (new_state == desired_state) { return tryReportSuccess(); } - return {IComponent::RequestState::kWaiting}; + return RequestResult::kFailure; } IComponent::RequestResult ProcessInfoNode::tryReportSuccess() @@ -112,9 +112,9 @@ IComponent::RequestResult ProcessInfoNode::tryReportSuccess() { reached_ready_.store(true); - return {RequestState::kSuccess}; + return RequestResult::kSuccess; } - return {IComponent::RequestState::kWaiting}; + return RequestResult::kFailure; } std::optional ProcessInfoNode::getTimeForAliveState() const @@ -129,14 +129,14 @@ std::optional ProcessInfoNode::getTimeForAliveState() const return std::nullopt; } -IComponent::RequestResult ProcessInfoNode::tryReportError(ComponentError error) +IComponent::RequestResult ProcessInfoNode::tryReportError(RequestResult error) { if (!success_returned_.test_and_set()) { // Activation failed to reach its ready condition. - return score::cpp::make_unexpected(error); + return error; } - return {IComponent::RequestState::kWaiting}; + return RequestResult::kFailure; } bool ProcessInfoNode::setState(score::mw::lifecycle::ProcessState new_state) @@ -173,20 +173,20 @@ void ProcessInfoNode::unblockSync() } } -IComponent::RequestResult ProcessInfoNode::tryHandleTermination(int32_t process_status) +IComponent::TerminationResult ProcessInfoNode::tryHandleTermination(int32_t process_status) { LM_LOG_DEBUG() << "Process" << identifier_ << "( pid" << pid_ << ") terminated with status" << process_status; exit_status_ = process_status; - IComponent::RequestResult res = {IComponent::RequestState::kWaiting}; + TerminationResult res = TerminationResult::kIgnore; ProcessState starting = ProcessState::kStarting; if (config_.component_properties.application_profile.is_self_terminating && process_status == 0) { - termination_result_ = TeminationResult::kOk; + termination_result_ = TerminationResult::kIgnore; } else { - termination_result_ = TeminationResult::kError; + termination_result_ = TerminationResult::kFailure; } if (termination_requested_.exchange(false)) // Termination was requested @@ -206,15 +206,18 @@ IComponent::RequestResult ProcessInfoNode::tryHandleTermination(int32_t process_ { setState(ProcessState::kTerminated); - if (termination_result_ == TeminationResult::kOk) + if (termination_result_ == TerminationResult::kIgnore) { - res = tryReportCompletion(ProcessState::kTerminated); + if (tryReportCompletion(ProcessState::kTerminated) == RequestResult::kSuccess) + { + res = TerminationResult::kIgnore; + } } else { LM_LOG_WARN() << "unexpected termination of process" << identifier_ << "( pid" << pid_ << "status" << exit_status_ << ")"; - res = score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady); + res = TerminationResult::kFailure; } } @@ -240,7 +243,7 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s LM_LOG_DEBUG() << "Starting process (" << identifier_ << ") from executable" << config_.deployment_config.executable_path; - std::optional error; + std::optional error; const std::chrono::time_point initial_time = std::chrono::steady_clock::now(); for (std::uint16_t attempts = 0U; attempts <= start_retries_; attempts++) @@ -255,14 +258,14 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s if (!setState(score::mw::lifecycle::ProcessState::kIdle)) { LM_LOG_WARN() << "Starting process" << this << "failed: termination in progress"; - error = ComponentError::kErrorBeforeReady; + error = RequestResult::kFailure; break; } pid_ = 0; exit_status_ = 0; error = std::nullopt; - termination_result_ = TeminationResult::kNone; + termination_result_ = std::nullopt; termination_waiter_->reset(); static_cast(setState(score::mw::lifecycle::ProcessState::kStarting)); // Cannot fail by design @@ -293,7 +296,7 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s else { setState(score::mw::lifecycle::ProcessState::kFailed); - error = ComponentError::kErrorBeforeReady; + error = RequestResult::kFailure; break; } @@ -315,38 +318,20 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s // We have terminated after starting up SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE( - termination_result_ != TerminationResult::kNone, "setState(kRunning) failed without a termination result"); + termination_result_.has_value(), "setState(kRunning) failed without a termination result"); // Assuming we have already waited for all ready conditions that require waiting, we have now started up and // terminated. Therefore, as long as the termination was valid, we have satisfied any running or terminated // condition. - if (termination_result_ == TeminationResult::kOk) + if (termination_result_ == TerminationResult::kIgnore) { return tryReportSuccess(); } - // We successfully reached kRunning, but reached kTerminated in error. This affects whether the error - // occurred before or after reaching the ready state - return tryReportError(getErrorAfterState(ProcessState::kRunning)); -} - -IComponent::ComponentError ProcessInfoNode::getErrorAfterState(ProcessState state_reached) const -{ - if (const configuration::ProcessState* state_condition = - std::get_if(&config_.component_properties.ready_condition)) - { - if (*state_condition == configuration::ProcessState::Terminated && state_reached == ProcessState::kRunning) - { - return IComponent::ComponentError::kErrorBeforeReady; - } - } - if (state_reached < ProcessState::kRunning) - { - return IComponent::ComponentError::kErrorBeforeReady; - } - return IComponent::ComponentError::kErrorAfterReady; + // We successfully reached kRunning, but reached kTerminated in error. + return tryReportError(RequestResult::kFailure); } -score::cpp::expected_blank ProcessInfoNode::handleProcessStillStarting( +score::cpp::expected_blank ProcessInfoNode::handleProcessStillStarting( const score::cpp::stop_token& stop_token) { const bool startup_condition_met = std::visit( @@ -434,26 +419,26 @@ score::cpp::expected_blank ProcessInfoNode::handlePr if (getState() == ProcessState::kTerminated) { - return score::cpp::make_unexpected(ComponentError::kErrorBeforeReady); + return score::cpp::make_unexpected(RequestResult::kFailure); } LM_LOG_WARN() << "Process (" << identifier_ << ") did not meet its ready condition within the ready timeout"; terminateProcess(stop_token); - return score::cpp::make_unexpected(ComponentError::kActivationTimedOut); + return score::cpp::make_unexpected(RequestResult::kTimeout); } -score::cpp::expected_blank ProcessInfoNode::handleProcessAlreadyTerminated() +score::cpp::expected_blank ProcessInfoNode::handleProcessAlreadyTerminated() { // The process did start successfully, but didn't report properly. if (isReporting()) { - return score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady); + return score::cpp::make_unexpected(IComponent::RequestResult::kFailure); } // In all other cases, the process did successfully reach the running state (because pid_ was set). return {}; } -score::cpp::expected, IComponent::ComponentError> +score::cpp::expected, IComponent::RequestResult> ProcessInfoNode::handleProcessStarted(const score::cpp::stop_token& stop_token) { switch (process_handling_.process_map_->insertIfNotTerminated(pid_, this)) @@ -470,7 +455,7 @@ ProcessInfoNode::handleProcessStarted(const score::cpp::stop_token& stop_token) // really bad fatal error, should not happen, treat as a failure to set the state & kill the process LM_LOG_ERROR() << "Could not add PID to map!"; terminateProcess(stop_token); - return score::cpp::make_unexpected(ComponentError::kErrorBeforeReady); + return score::cpp::make_unexpected(RequestResult::kFailure); } } @@ -541,7 +526,7 @@ IComponent::RequestResult ProcessInfoNode::activate(score::cpp::stop_token stop_ return tryReportSuccess(); } auto res = startProcess(std::move(stop_token)); - if (res.has_value()) + if (res == RequestResult::kSuccess) { if (auto time = getTimeForAliveState()) { @@ -561,7 +546,7 @@ IComponent::RequestResult ProcessInfoNode::deactivate(score::cpp::stop_token sto } terminateProcess(stop_token); setState(ProcessState::kIdle); - return IComponent::RequestState::kSuccess; + return RequestResult::kSuccess; } bool ProcessInfoNode::active() const diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp index 298fc942c..153ddf36e 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp @@ -25,6 +25,7 @@ #include #include #include +#include namespace score::mw::lifecycle::internal { @@ -40,17 +41,6 @@ namespace score::mw::lifecycle::internal /// In the future, this class shall be split up to properly separate Component and Process lifecycle. class ProcessInfoNode final : public IComponent { - /// @brief Enum representing different outcomes of a termination. - enum class TeminationResult : uint8_t - { - /// @brief Since the last startup, the process has not terminated. - kNone, - /// @brief The last termination was acceptable. - kOk, - /// @brief The last termination was invalid/unexpected. - kError, - }; - public: /// @brief Constructs a ProcessInfoNode. /// @param config Configuration for the OS process. @@ -92,7 +82,7 @@ class ProcessInfoNode final : public IComponent RequestResult deactivate(score::cpp::stop_token stop_token) override; - RequestResult tryHandleTermination(int32_t process_status) override; + TerminationResult tryHandleTermination(int32_t process_status) override; [[nodiscard]] bool active() const override; @@ -106,10 +96,6 @@ class ProcessInfoNode final : public IComponent std::chrono::milliseconds getTerminationTimeout() const; private: - /// @brief Given that an error has occurred after the process has reached state @p state_reached, return an error - /// indicating whether this was an error before the ready condition was satisfied, or after. - ComponentError getErrorAfterState(ProcessState state_reached) const; - /// @brief Returns true if the process is configured to report kRunning bool isReporting() const; @@ -130,14 +116,14 @@ class ProcessInfoNode final : public IComponent /// @brief Get the request result corresponding to the new state reached. For example, if the ready state is /// terminated, the function will only return kSuccess if the new state is kTerminated. - /// @return Success if the ready condition is satisfied and completion is not already reported, an error if the - /// state is unrecoverable, waiting otherwise. + /// @return kSuccess if the ready condition is satisfied and completion is not already reported, kFailure if the + /// state is unrecoverable, kWaitForTermination otherwise. RequestResult tryReportCompletion(score::mw::lifecycle::ProcessState new_state); - /// @return The provided error if the result has not been reported yet. A waiting result otherwise. - RequestResult tryReportError(ComponentError error); + /// @return @p error (kFailure or kTimeout) if the result has not been reported yet, kWaitForTermination otherwise. + RequestResult tryReportError(RequestResult error); - /// @return Success state if success has not been returned yet, waiting result otherwise. + /// @return kSuccess if success has not been returned yet, kWaitForTermination otherwise. RequestResult tryReportSuccess(); /// @brief Requests the OS to terminate this process and waits for it to exit. @@ -145,24 +131,25 @@ class ProcessInfoNode final : public IComponent void terminateProcess(const score::cpp::stop_token& stop_token); /// @brief Starts the OS process, retrying up to the configured restart count on failure. - /// @return A success if the process reached its ready conditon, an error if startup failed, waiting otherwise. + /// @return kSuccess if the process reached its ready conditon, kFailure or kTimeout if startup failed, + /// kWaitForTermination otherwise. RequestResult startProcess(score::cpp::stop_token stop_token); /// @brief Handles the result of inserting the process into the process map after the OS /// process has been created. Routes to the appropriate handler based on whether the /// process is still running or has already terminated. /// @returns An error if the process map error is unrecoverable, the result from the relevant handler otherwise. - score::cpp::expected, ComponentError> handleProcessStarted( + score::cpp::expected, RequestResult> handleProcessStarted( const score::cpp::stop_token& stop_token); /// @brief Waits for the process to report kRunning, terminating the process if it times out. /// @post Process state is either running or terminated. /// @returns an error if the startup times out. - score::cpp::expected_blank handleProcessStillStarting(const score::cpp::stop_token& stop_token); + score::cpp::expected_blank handleProcessStillStarting(const score::cpp::stop_token& stop_token); /// @brief Handles the case where the process exited before the map insertion completed. /// @returns success only for self-terminating, non-reporting processes with exit status 0. - score::cpp::expected_blank handleProcessAlreadyTerminated(); + score::cpp::expected_blank handleProcessAlreadyTerminated(); /// @brief Logs that the process has reached the running state. void handleProcessRunning(); @@ -211,8 +198,8 @@ class ProcessInfoNode final : public IComponent /// @brief Unique hash to identify this node. IdentifierHash identifier_; - /// @brief The result of the last termination since the process started. - TeminationResult termination_result_{}; + /// @brief The result of the last termination since the process started, empty if it has not terminated. + std::optional termination_result_{}; /// @brief Waits for a process to terminate for kTerminated ready condition std::unique_ptr termination_waiter_; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp index d51f42abe..a3b6a36bb 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp @@ -220,8 +220,7 @@ TEST_F(ProcessInfoNodeStartupTest, CanStartNonReportingProcess) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } @@ -236,32 +235,30 @@ TEST_F(ProcessInfoNodeStartupTest, CanStartReportingProcess_ReportsRunningInTime auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } -TEST_F(ProcessInfoNodeStartupTest, OsForkFails_ReturnsErrorBeforeReady) +TEST_F(ProcessInfoNodeStartupTest, OsForkFails_ReturnsFailure) { RecordProperty( "Description", - "If the OS fails to fork the process, activate() returns kErrorBeforeReady and the node ends up in kFailed."); + "If the OS fails to fork the process, activate() returns kFailure and the node ends up in kFailed."); auto node = createProcessInfoNode(configuration::ApplicationType::Native); EXPECT_CALL(mock_processIf_, startProcess(_, _, _)).WillOnce(Return(osal::OsalReturnType::kFail)); auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kFailed)); } -TEST_F(ProcessInfoNodeStartupTest, MapInsertError_ReturnsErrorBeforeReady) +TEST_F(ProcessInfoNodeStartupTest, MapInsertError_ReturnsFailure) { RecordProperty( "Description", - "If the process map insertion fails with an error, activate() returns kErrorBeforeReady and the " + "If the process map insertion fails with an error, activate() returns kFailure and the " "node ends up in kFailed."); auto node = createProcessInfoNode(configuration::ApplicationType::Native); @@ -273,8 +270,7 @@ TEST_F(ProcessInfoNodeStartupTest, MapInsertError_ReturnsErrorBeforeReady) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kFailed)); } @@ -298,8 +294,7 @@ TEST_F(ProcessInfoNodeStartupTest, SelfTerminating_ExitsBeforeMapInsert_ReturnsS auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -313,13 +308,12 @@ TEST_F(ProcessInfoNodeStartupTest, ActivateAlreadyActiveNode_ReturnsSuccess) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->active(), IsTrue()); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } -TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_StopRequestedDuringStartup_ReturnsActivationTimedOut) +TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_StopRequestedDuringStartup_ReturnsTimeout) { RecordProperty( "Description", @@ -338,12 +332,11 @@ TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_StopRequestedDuringS auto result = node->activate(stop_source_.get_token()); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kActivationTimedOut)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kTimeout)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } -TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_NoTerminationInTime_ReturnsActivationTimedOut) +TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_NoTerminationInTime_ReturnsTimeout) { RecordProperty( "Description", @@ -359,8 +352,7 @@ TEST_F(ProcessInfoNodeStartupTest, TerminatedReadyCondition_NoTerminationInTime_ auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kActivationTimedOut)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kTimeout)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -391,7 +383,7 @@ TEST_P(ProcessInfoNodeMapYieldTest, InsertReturnsYield) RecordProperty("Description", GetParam().description_); auto node = createProcessInfoNode(GetParam().app_type_, 0, GetParam().is_self_terminating_, GetParam().condition_); - IComponent::RequestResult tryHandleTerminationResult; + IComponent::TerminationResult tryHandleTerminationResult{}; auto status = GetParam().status_to_exit_with_; EXPECT_CALL(mock_processIf_, startProcess).WillOnce(Return(osal::OsalReturnType::kSuccess)); @@ -406,18 +398,8 @@ TEST_P(ProcessInfoNodeMapYieldTest, InsertReturnsYield) auto activation_result_ = node->activate(score::cpp::stop_token{}); - ASSERT_EQ(activation_result_.has_value(), GetParam().expected_activation_result_.has_value()); - if (GetParam().expected_activation_result_.has_value()) - { - EXPECT_EQ(activation_result_.value(), GetParam().expected_activation_result_.value()); - } - else - { - EXPECT_EQ(activation_result_.error(), GetParam().expected_activation_result_.error()); - } - - ASSERT_TRUE(tryHandleTerminationResult.has_value()); - EXPECT_EQ(tryHandleTerminationResult.value(), IComponent::RequestState::kWaiting) + EXPECT_EQ(activation_result_, GetParam().expected_activation_result_); + EXPECT_EQ(tryHandleTerminationResult, IComponent::TerminationResult::kIgnore) << "An error occurring during startup should never be reported by tryHandleTermination"; EXPECT_EQ(node->getState(), ProcessState::kTerminated); // Not kFailed, the posix process did start successfully } @@ -431,14 +413,14 @@ INSTANTIATE_TEST_SUITE_P( configuration::ProcessState::Running, 0, true, - {IComponent::RequestState::kSuccess}, + IComponent::RequestResult::kSuccess, "A native, self-terminating process exiting quickly with status 0 should report a successful activation"}, YieldTestCasesData{ configuration::ApplicationType::Native, configuration::ProcessState::Running, 111, true, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady), + IComponent::RequestResult::kFailure, "A native, self-terminating process exiting quickly with a non-zero status should report a failure to " "activate"}, YieldTestCasesData{ @@ -446,14 +428,14 @@ INSTANTIATE_TEST_SUITE_P( configuration::ProcessState::Running, 0, false, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady), + IComponent::RequestResult::kFailure, "A native, non-self-terminating process exiting quickly should report a failure to activate"}, YieldTestCasesData{ configuration::ApplicationType::Reporting, configuration::ProcessState::Running, 0, true, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady), + IComponent::RequestResult::kFailure, "A reporting process exiting quickly (i.e. without waiting for a response from launch manager) should " "report a failure to activate"}, @@ -462,14 +444,14 @@ INSTANTIATE_TEST_SUITE_P( configuration::ProcessState::Terminated, 0, true, - {IComponent::RequestState::kSuccess}, + IComponent::RequestResult::kSuccess, "A native, self-terminating process exiting quickly with status 0 should report a successful activation"}, YieldTestCasesData{ configuration::ApplicationType::Native, configuration::ProcessState::Terminated, 111, true, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady), + IComponent::RequestResult::kFailure, "A native, self-terminating process exiting quickly with a non-zero status should report a failure to " "activate"}, YieldTestCasesData{ @@ -477,14 +459,14 @@ INSTANTIATE_TEST_SUITE_P( configuration::ProcessState::Terminated, 0, false, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady), + IComponent::RequestResult::kFailure, "A native, non-self-terminating process exiting quickly should report a failure to activate"}, YieldTestCasesData{ configuration::ApplicationType::Reporting, configuration::ProcessState::Terminated, 0, true, - score::cpp::make_unexpected(IComponent::ComponentError::kErrorBeforeReady), + IComponent::RequestResult::kFailure, "A reporting process exiting quickly (i.e. without waiting for a response from launch manager) should " "report a failure to activate"})); @@ -496,8 +478,7 @@ class ProcessInfoNodeStartupCrashTest : public ProcessInfoNodeFixture TEST_F(ProcessInfoNodeStartupCrashTest, ProcesssTerminated_OnWaitForkRunningTimeout) { RecordProperty( - "Description", - "If waitForkRunning times out, the process reports kActivationTimedOut and ends up in state kTerminated."); + "Description", "If waitForkRunning times out, the process reports kTimeout and ends up in state kTerminated."); expectActivationReport(0); auto node = createProcessInfoNode(configuration::ApplicationType::ReportingAndSupervised); @@ -508,8 +489,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ProcesssTerminated_OnWaitForkRunningTime auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kActivationTimedOut)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kTimeout)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -517,7 +497,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ReportingProcess_CrashesBeforeReady_NoRe { RecordProperty( "Description", - "Process returns kErrorBeforeReady when crashing before reaching its ready condition (kRunning) with 0 restart " + "Process returns kFailure when crashing before reaching its ready condition (kRunning) with 0 restart " "attempts"); expectActivationReport(0); @@ -533,8 +513,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ReportingProcess_CrashesBeforeReady_NoRe auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -542,7 +521,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ReportingProcess_CrashesBeforeReady_With { RecordProperty( "Description", - "Process returns kErrorBeforeReady when crashing before reaching its ready condition (kRunning) with 3 restart " + "Process returns kFailure when crashing before reaching its ready condition (kRunning) with 3 restart " "attempts"); expectActivationReport(0); @@ -568,8 +547,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, ReportingProcess_CrashesBeforeReady_With auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -578,7 +556,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, NonReportingProcess_CrashesBeforeReady_N RecordProperty( "Description", "A non-reporting process that crashes (non-zero status) between map insertion and the startup thread's status " - "check returns kErrorBeforeReady."); + "check returns kFailure."); auto node = createProcessInfoNode(configuration::ApplicationType::Native); EXPECT_CALL(mock_processIf_, startProcess(_, _, _)).WillOnce(Return(osal::OsalReturnType::kSuccess)); @@ -592,8 +570,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, NonReportingProcess_CrashesBeforeReady_N auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -602,7 +579,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, NonReportingProcess_CrashesBeforeReady_W RecordProperty( "Description", "A non-reporting process that crashes before ready on every attempt exhausts all restart attempts and returns " - "kErrorBeforeReady."); + "kFailure."); constexpr uint32_t kRestartAttempts = 2; constexpr uint32_t kTotalAttempts = kRestartAttempts + 1; @@ -621,8 +598,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, NonReportingProcess_CrashesBeforeReady_W auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -649,8 +625,7 @@ TEST_F(ProcessInfoNodeStartupCrashTest, TimeoutThenSuccess_WithRestarts) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } @@ -707,8 +682,7 @@ TEST_P( auto result = node->activate(score::cpp::stop_token{}); // Then - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -728,14 +702,13 @@ class ProcessInfoNodeUnexpectedTerminationTest : public ProcessInfoNodeFixture TEST_F(ProcessInfoNodeUnexpectedTerminationTest, ProcesssCrashed_AfterReadyCondition) { RecordProperty( - "Description", "Process returns kErrorAfterReady when crashing after reaching its ready condition (kRunning)."); + "Description", "Process returns kFailure when crashing after reaching its ready condition (kRunning)."); auto node = createRunningProcessInfoNode(configuration::ApplicationType::Native); auto result = node->tryHandleTermination(-1); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorAfterReady)); + ASSERT_THAT(result, Eq(IComponent::TerminationResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -751,8 +724,7 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminatingProcess_ExitsWit auto result = node->tryHandleTermination(0); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kWaiting)); + ASSERT_THAT(result, Eq(IComponent::TerminationResult::kIgnore)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -760,8 +732,8 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_TerminatedReady { RecordProperty( "Description", - "A self-terminating process with ReadyCondition::kTerminated returns kSuccess from tryHandleTermination() when " - "it exits cleanly, since its exit is the event that satisfies the ready condition."); + "A self-terminating process with ReadyCondition::kTerminated exits cleanly while activate() is waiting for " + "it, so activate() returns kSuccess and tryHandleTermination() returns kIgnore."); auto node = createProcessInfoNode( configuration::ApplicationType::Native, @@ -770,7 +742,7 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_TerminatedReady configuration::ProcessState::Terminated /*ready condition*/); // std::future to hold the asynchronous termination handler execution - std::future termination_future; + std::future termination_future; EXPECT_CALL(mock_processIf_, startProcess(_, _, _)).WillOnce(Return(osal::OsalReturnType::kSuccess)); @@ -785,21 +757,19 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_TerminatedReady // activate() returns kSuccess because kTerminated (the ready condition) is reached within the timeout auto activate_result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(activate_result.has_value(), IsTrue()); - ASSERT_THAT(activate_result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(activate_result, Eq(IComponent::RequestResult::kSuccess)); auto termination_result = termination_future.get(); - ASSERT_THAT(termination_result.has_value(), IsTrue()); - ASSERT_THAT(termination_result.value(), Eq(IComponent::RequestState::kWaiting)); + ASSERT_THAT(termination_result, Eq(IComponent::TerminationResult::kIgnore)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } -TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_CrashAfterReady_ReturnsErrorAfterReady) +TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_CrashAfterReady_ReturnsFailure) { RecordProperty( "Description", "A self-terminating process that crashes (non-zero exit status) after reaching its ready condition returns " - "kErrorAfterReady, just like a non-self-terminating process crash."); + "kFailure, just like a non-self-terminating process crash."); auto node = createSelfTerminatingProcessInfoNode(configuration::ApplicationType::Native); expectSuccessfulProcessLaunch(); @@ -807,8 +777,7 @@ TEST_F(ProcessInfoNodeUnexpectedTerminationTest, SelfTerminating_CrashAfterReady auto result = node->tryHandleTermination(-1); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorAfterReady)); + ASSERT_THAT(result, Eq(IComponent::TerminationResult::kFailure)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } @@ -833,8 +802,7 @@ TEST_F(ProcessInfoNodeDeactivationTest, CanTerminateNonSelfTerminatingProcess) auto result = node->deactivate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->active(), IsFalse()); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kIdle)); } @@ -900,8 +868,7 @@ TEST_F(ProcessInfoNodeDeactivationTest, ProcessIgnoresSigterm_ForcedWithSigkill) auto result = node->deactivate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->active(), IsFalse()); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kIdle)); } @@ -937,8 +904,7 @@ TEST_F(ProcessInfoNodeFileStateTest, ConditionAlreadyMet_ReturnsSuccess) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } @@ -957,8 +923,7 @@ TEST_F(ProcessInfoNodeFileStateTest, NotExistingCondition_ReturnsSuccess) auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } @@ -973,16 +938,14 @@ TEST_F(ProcessInfoNodeFileStateTest, NativeApplication_DoesNotIgnoreRunning_Retu auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsTrue()); - ASSERT_THAT(result.value(), Eq(IComponent::RequestState::kSuccess)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kSuccess)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); } -TEST_F(ProcessInfoNodeFileStateTest, WaitForFileTimesOut_ReturnsActivationTimedOut) +TEST_F(ProcessInfoNodeFileStateTest, WaitForFileTimesOut_ReturnsTimeout) { RecordProperty( - "Description", - "If waitForFile() times out, activate() returns kActivationTimedOut and the process ends up terminated."); + "Description", "If waitForFile() times out, activate() returns kTimeout and the process ends up terminated."); auto node = createFileStateProcessInfoNode("/var/run/ready", configuration::FileExistenceState::Exists); expectSuccessfulProcessLaunch(); @@ -993,8 +956,7 @@ TEST_F(ProcessInfoNodeFileStateTest, WaitForFileTimesOut_ReturnsActivationTimedO auto result = node->activate(score::cpp::stop_token{}); - ASSERT_THAT(result.has_value(), IsFalse()); - ASSERT_THAT(result.error(), Eq(IComponent::ComponentError::kActivationTimedOut)); + ASSERT_THAT(result, Eq(IComponent::RequestResult::kTimeout)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kTerminated)); } diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp index 3f8e6a79d..a856fe3d9 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp @@ -62,7 +62,7 @@ void ProcessMonitor::doWork(ComponentTask&& task) } }; - auto handle_failure = [&](IComponent::ComponentError& error) { + auto handle_failure = [&](IComponent::RequestResult error) { const IdentifierHash node_identifier = task.component.get().getIdentifier(); switch (task.type) @@ -78,17 +78,6 @@ void ProcessMonitor::doWork(ComponentTask&& task) } }; - auto handle_state = [&](IComponent::RequestState& state) { - switch (state) - { - case IComponent::RequestState::kSuccess: - handle_success(); - break; - case IComponent::RequestState::kWaiting: - break; - } - }; - if (task.stop_token.stop_requested()) { if (!event_queue_.push(JobSkipped{task.component.get().getIdentifier()})) @@ -98,29 +87,31 @@ void ProcessMonitor::doWork(ComponentTask&& task) return; } - auto result = run_task(); + const IComponent::RequestResult result = run_task(); - if (result.has_value()) - { - handle_state(result.value()); - } - else + switch (result) { - handle_failure(result.error()); + case IComponent::RequestResult::kSuccess: + handle_success(); + break; + case IComponent::RequestResult::kTimeout: + case IComponent::RequestResult::kFailure: + handle_failure(result); + break; } } void ProcessMonitor::terminated(IComponent& component, int32_t status) { - auto res = component.tryHandleTermination(status); bool push_res = true; - if (!res.has_value()) - { - push_res = event_queue_.push(UnexpectedTermination{component.getIdentifier(), res.error()}); - } - else if (res.value() != IComponent::RequestState::kWaiting) + switch (component.tryHandleTermination(status)) { - push_res = event_queue_.push(ActivationSuccessful{component.getIdentifier()}); + case IComponent::TerminationResult::kFailure: + push_res = event_queue_.push(UnexpectedTermination{component.getIdentifier()}); + break; + case IComponent::TerminationResult::kIgnore: + LM_LOG_DEBUG() << "Termination of component" << component.getIdentifier() << "is expected"; + break; } if (!push_res) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp index b4cde272c..0e9988a7b 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp @@ -44,7 +44,7 @@ TEST_F(ProcessMonitorTest, DoWorkNormalActivation) RecordProperty("Description", "Verify that the process monitor activates components during activate tasks"); // Given a valid activate task // Then - EXPECT_CALL(mock_component, activate).WillOnce(Return(IComponent::RequestState::kSuccess)); + EXPECT_CALL(mock_component, activate).WillOnce(Return(IComponent::RequestResult::kSuccess)); EXPECT_CALL( mock_queue, push(VariantWith(Field(&ActivationSuccessful::node_identifier, kDefaultIdentifier)))) @@ -58,7 +58,7 @@ TEST_F(ProcessMonitorTest, DoWorkNormalDeactivation) RecordProperty("Description", "Verify that the process monitor deactivates components during deactivate tasks"); // Given a valid activate task // Then - EXPECT_CALL(mock_component, deactivate).WillOnce(Return(IComponent::RequestState::kSuccess)); + EXPECT_CALL(mock_component, deactivate).WillOnce(Return(IComponent::RequestResult::kSuccess)); EXPECT_CALL( mock_queue, push(VariantWith(Field(&DeactivationComplete::node_identifier, kDefaultIdentifier)))) @@ -67,25 +67,6 @@ TEST_F(ProcessMonitorTest, DoWorkNormalDeactivation) process_monitor.doWork(ComponentTask{ComponentTaskType::kDeactivate, mock_component, stop_source.get_token()}); } -TEST_F(ProcessMonitorTest, DoWorkOnTerminationDepProcess) -{ - RecordProperty( - "Description", "Verify that a process activated by its own termination recieves the correct instructions"); - // Given a task for starting a component that only reaches active when it terminates - // Then - // The component is neither complete nor failed - EXPECT_CALL(mock_component, activate).WillOnce(Return(IComponent::RequestState::kWaiting)); - EXPECT_CALL(mock_component, tryHandleTermination).WillOnce(Return(IComponent::RequestState::kSuccess)); - EXPECT_CALL( - mock_queue, - push(VariantWith(Field(&ActivationSuccessful::node_identifier, kDefaultIdentifier)))) - .Times(1); - // When - process_monitor.doWork(ComponentTask{ComponentTaskType::kActivate, mock_component, stop_source.get_token()}); - // The OS thread detects the termination: - process_monitor.terminated(mock_component, 0); -} - TEST_F(ProcessMonitorTest, TerminatedUnexpectedly) { RecordProperty( @@ -94,8 +75,7 @@ TEST_F(ProcessMonitorTest, TerminatedUnexpectedly) // Given a terminated signal on a component that is not expected to terminate: // Then - EXPECT_CALL(mock_component, tryHandleTermination) - .WillOnce(Return(score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady))); + EXPECT_CALL(mock_component, tryHandleTermination).WillOnce(Return(IComponent::TerminationResult::kFailure)); EXPECT_CALL( mock_queue, push(VariantWith(Field(&UnexpectedTermination::node_identifier, kDefaultIdentifier)))) @@ -111,9 +91,9 @@ TEST_F(ProcessMonitorTest, ActivationFailed) "Verify that when a component activation fails, the correct error and data is pushed to the event queue"); // Given a task that will fail to activate - auto errc = IComponent::ComponentError::kActivationTimedOut; + auto errc = IComponent::RequestResult::kTimeout; // Then - EXPECT_CALL(mock_component, activate).WillOnce(Return(score::cpp::make_unexpected(errc))); + EXPECT_CALL(mock_component, activate).WillOnce(Return(errc)); EXPECT_CALL(mock_queue, push(VariantWith(Field(&ActivationFailed::reason, errc)))).Times(1); // When process_monitor.doWork(ComponentTask{ComponentTaskType::kActivate, mock_component, stop_source.get_token()}); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp index 1cfd51b83..2653921c8 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp @@ -42,18 +42,18 @@ class RunTarget final : public IComponent RequestResult activate(score::cpp::stop_token) override { active_ = true; - return RequestState::kSuccess; + return RequestResult::kSuccess; } RequestResult deactivate(score::cpp::stop_token) override { active_ = false; - return RequestState::kSuccess; + return RequestResult::kSuccess; } - RequestResult tryHandleTermination(int32_t) override + TerminationResult tryHandleTermination(int32_t) override { - return RequestState::kSuccess; + return TerminationResult::kIgnore; } IdentifierHash getIdentifier() const override diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp index cace64d1e..8da95aded 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp @@ -47,7 +47,7 @@ class MockComponent : public IComponent public: MOCK_METHOD(RequestResult, activate, (score::cpp::stop_token stop_token), (override)); MOCK_METHOD(RequestResult, deactivate, (score::cpp::stop_token stop_token), (override)); - MOCK_METHOD(RequestResult, tryHandleTermination, (int32_t status), (override)); + MOCK_METHOD(TerminationResult, tryHandleTermination, (int32_t status), (override)); MOCK_METHOD(bool, active, (), (const override)); MOCK_METHOD(bool, stopped, (), (const override)); MOCK_METHOD(score::mw::lifecycle::IdentifierHash, getIdentifier, (), (const override)); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp index 0fb818895..40cff6061 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp @@ -40,7 +40,7 @@ class MockComponent : public IComponent MOCK_METHOD(RequestResult, activate, (score::cpp::stop_token), (override)); MOCK_METHOD(RequestResult, deactivate, (score::cpp::stop_token), (override)); - MOCK_METHOD(RequestResult, tryHandleTermination, (int32_t), (override)); + MOCK_METHOD(TerminationResult, tryHandleTermination, (int32_t), (override)); MOCK_METHOD(IdentifierHash, getIdentifier, (), (const, override)); MOCK_METHOD(bool, active, (), (const, override));