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 f52e492f2..94cd65d18 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 @@ -37,8 +37,9 @@ namespace /// @param config The configuration containing components and run targets. /// @param process_handling The interfaces used to start, stop and report on the OS processes. /// @param run_target_map Map to keep the translation between IDHash to Index -/// @return A populated dependency graph with all components and run targets. -void CreateDependencyGraph( +/// @return Success once populated with all components and run targets, or the error from the first +/// component whose ProcessInfoNode failed to construct. +score::cpp::expected_blank CreateDependencyGraph( DependencyGraph& graph, GraphConfig& config, ProcessHandling process_handling, @@ -55,8 +56,14 @@ void CreateDependencyGraph( const auto name = component_config.name; auto depends_on = std::move(component_config.component_properties.depends_on); - const auto index = graph.try_emplace( - IdentifierHash{name}, std::in_place_type, std::move(component_config), process_handling); + auto node_res = ProcessInfoNode::Create(std::move(component_config), process_handling); + if (!node_res.has_value()) + { + return score::cpp::make_unexpected(node_res.error()); + } + + const auto index = + graph.try_emplace(IdentifierHash{name}, std::in_place_type, std::move(node_res).value()); LM_LOG_DEBUG() << "Creating component node:" << name; pending_dependencies.emplace_back(index, std::move(depends_on)); @@ -107,10 +114,32 @@ void CreateDependencyGraph( } LM_LOG_DEBUG() << "Created dependency graph with" << graph.size() << "total nodes"; + return {}; } } // anonymous namespace +score::cpp::expected, IComponent::ComponentError> Graph::Create( + uint32_t max_num_nodes, + GraphConfig& configuration, + std::shared_ptr job_queue, + ProcessHandling process_handling, + ITransitionResultPublisher* transition_result_receiver) +{ + // std::unique_ptr rather than std::make_unique since the constructor is private. + std::unique_ptr graph{ + new Graph(max_num_nodes, configuration, job_queue, std::move(process_handling), transition_result_receiver)}; + + auto res = CreateDependencyGraph( + graph->nodes_, graph->configuration_, graph->process_handling_, graph->off_state_transition_timeout_); + if (!res.has_value()) + { + return score::cpp::make_unexpected(res.error()); + } + + return graph; +} + Graph::Graph( uint32_t max_num_nodes, GraphConfig& configuration, @@ -128,7 +157,6 @@ Graph::Graph( last_state_manager_.process_identifier_ = IdentifierHash{""}; // an invalid state manager last_state_manager_.process_group_index_ = 0xFFFFU; cancel_message_.request_or_response_ = ControlClientCode::kNotSet; - CreateDependencyGraph(nodes_, configuration_, process_handling_, off_state_transition_timeout_); } Graph::~Graph() 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 27454d905..32c20647a 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 @@ -33,12 +33,14 @@ #include "score/mw/launch_manager/process_group_manager/details/component_of.hpp" #include "score/mw/launch_manager/process_group_manager/details/component_task.hpp" #include "score/mw/launch_manager/process_group_manager/details/dependency_graph.hpp" +#include "score/mw/launch_manager/process_group_manager/details/icomponent.hpp" #include "score/mw/launch_manager/process_group_manager/details/itransition_result_publisher.hpp" #include "score/mw/launch_manager/process_group_manager/details/process_handling.hpp" #include "score/mw/launch_manager/process_group_manager/details/process_info_node.hpp" #include "score/mw/launch_manager/process_group_manager/details/run_target.hpp" #include "score/mw/launch_manager/process_group_manager/details/transition.hpp" #include "score/mw/launch_manager/process_group_manager/iprocess.hpp" +#include #include namespace score::mw::lifecycle::internal @@ -159,13 +161,15 @@ class Graph final static constexpr std::string_view off_state_name{"Off"}; static constexpr std::string_view recovery_state_name{"fallback"}; - /// @brief Constructor to initialize a Graph object. + /// @brief Creates a Graph, building all of its ProcessInfoNode and RunTarget nodes from @p configuration. /// @param max_num_nodes Maximum number of nodes this graph can hold. /// @param configuration Configuration containing run target and component information. /// @param job_queue Queue to push component jobs to for multithreaded processing. /// @param process_handling The interfaces used to start, stop and report on the OS processes. /// @param transition_result_receiver Object to notify when the initial transition is complete. - Graph( + /// @return The constructed Graph, or an error if any component node failed to construct (e.g. alive + /// supervision setup failed). + [[nodiscard]] static score::cpp::expected, IComponent::ComponentError> Create( uint32_t max_num_nodes, GraphConfig& configuration, std::shared_ptr job_queue, @@ -298,6 +302,19 @@ class Graph final std::chrono::milliseconds getOffStateTransitionTimeout() const; private: + /// @brief Constructs a Graph without populating its nodes. Use Create() instead. + /// @param max_num_nodes Maximum number of nodes this graph can hold. + /// @param configuration Configuration containing run target and component information. + /// @param job_queue Queue to push component jobs to for multithreaded processing. + /// @param process_handling The interfaces used to start, stop and report on the OS processes. + /// @param transition_result_receiver Object to notify when the initial transition is complete. + Graph( + uint32_t max_num_nodes, + GraphConfig& configuration, + std::shared_ptr job_queue, + ProcessHandling process_handling, + ITransitionResultPublisher* transition_result_receiver); + /// @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); 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 a04936615..c6cd750a0 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 @@ -60,12 +60,14 @@ class GraphTest : public ::testing::Test // The Graph builds its nodes from the configuration, so it can only be created once the // (fixture-specific) config is in place. - graph_ = std::make_unique( + auto graph_res = Graph::Create( 10U, graph_config_, job_queue_, ProcessHandling{&process_interface_, mock_process_map, nullptr, mock_factory_}, &mock_transition_result_publisher_); + ASSERT_THAT(graph_res.has_value(), IsTrue()); + graph_ = std::move(graph_res).value(); } virtual void SetConfig() @@ -207,6 +209,58 @@ class GraphTest : public ::testing::Test }; }; +// Deliberately does not derive from GraphTest: that fixture's SetUp() asserts Graph::Create() succeeds, +// which is exactly what this test needs to fail. +class GraphCreateFailureTest : public ::testing::Test +{ + protected: + GraphConfig graph_config_{}; + std::shared_ptr job_queue_ = std::make_shared(); + StrictMock process_interface_{}; + std::shared_ptr mock_process_map = std::make_shared(); + MockSupervisionFactory mock_factory_{}; + MockTransitionResultPublisher mock_transition_result_publisher_{}; +}; + +TEST_F(GraphCreateFailureTest, ComponentSupervisionConstructionFailurePreventsGraphCreation) +{ + RecordProperty( + "Description", + "If a component's alive supervision fails to construct, Graph::Create() fails instead of silently " + "building a graph containing a partially-initialized node, so process group startup is correctly " + "escalated."); + + ComponentConfig supervised_component{}; + supervised_component.name = "supervised_process"; + supervised_component.component_properties.ready_condition = ReadyCondition{configuration::ProcessState::Running}; + supervised_component.component_properties.application_profile.application_type = + ApplicationType::ReportingAndSupervised; + supervised_component.component_properties.application_profile.alive_supervision = ComponentAliveSupervision{ + .reporting_cycle_ms = 10, .failed_cycles_tolerance = 1, .min_indications = 0, .max_indications = 0}; + + std::vector components{}; + components.push_back(std::move(supervised_component)); + + std::vector run_targets{}; + run_targets.push_back(RunTargetConfig{"Startup", "", {}, 10, {}}); + run_targets.push_back(RunTargetConfig{"Off", "", {}, 10, {}}); + + graph_config_ = + GraphConfig{std::move(components), std::move(run_targets), FallbackRunTargetConfig{"", {}, 10}, "Startup"}; + + EXPECT_CALL(mock_factory_, constructSupervision).WillOnce(Return(nullptr)); + + auto result = Graph::Create( + 3U, + graph_config_, + job_queue_, + ProcessHandling{&process_interface_, mock_process_map, nullptr, mock_factory_}, + &mock_transition_result_publisher_); + + ASSERT_THAT(result.has_value(), IsFalse()); + EXPECT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); +} + class GraphOrdinaryTransitionTest : public GraphTest { protected: 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 4c3aacab1..3acc00ff6 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 @@ -24,6 +24,20 @@ namespace score::mw::lifecycle::internal { +score::cpp::expected ProcessInfoNode::Create( + configuration::ComponentConfig&& config, + ProcessHandling process_handling) +{ + ProcessInfoNode node{std::move(config), std::move(process_handling)}; + + if (auto res = node.setupAliveSupervision(); !res.has_value()) + { + return score::cpp::make_unexpected(res.error()); + } + + return node; +} + ProcessInfoNode::ProcessInfoNode(configuration::ComponentConfig&& config, ProcessHandling process_handling) : terminator_(), has_semaphore_(false), @@ -37,32 +51,38 @@ ProcessInfoNode::ProcessInfoNode(configuration::ComponentConfig&& config, Proces { start_tries_ = config_.deployment_config.ready_recovery_action->number_of_attempts + 1; } +} +score::cpp::expected_blank ProcessInfoNode::setupAliveSupervision() +{ const configuration::ApplicationProfile& app_profile = config_.component_properties.application_profile; - if (app_profile.application_type == configuration::ApplicationType::ReportingAndSupervised) + if (app_profile.application_type != configuration::ApplicationType::ReportingAndSupervised) { - SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE( - app_profile.alive_supervision.has_value(), "Supervised process did not have alive supervision config"); - const uid_t uid = config_.deployment_config.sandbox.uid; + return {}; + } - LM_LOG_DEBUG() << "Setting up alive supervision for" << identifier_; + SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE( + app_profile.alive_supervision.has_value(), "Supervised process did not have alive supervision config"); + const uid_t uid = config_.deployment_config.sandbox.uid; - supervision_handle_ = process_handling_.supervision_factory.constructSupervision( - identifier_, uid, app_profile.alive_supervision.value()); + LM_LOG_DEBUG() << "Setting up alive supervision for" << identifier_; - if (!supervision_handle_) - { - LM_LOG_ERROR() << "Failed to set up alive supervision for" << identifier_; - } - else - { - LM_LOG_DEBUG() << "Successfully set up alive supervision for" << identifier_; - } + supervision_handle_ = process_handling_.supervision_factory.constructSupervision( + identifier_, uid, app_profile.alive_supervision.value()); - config_.deployment_config.environmental_variables.add( - "LCM_ALIVE_INTERFACE_PATH", supervision_handle_->getConnectionId()); + if (!supervision_handle_) + { + LM_LOG_ERROR() << "Failed to set up alive supervision for" << identifier_; + return score::cpp::make_unexpected(ComponentError::kErrorBeforeReady); } + + LM_LOG_DEBUG() << "Successfully set up alive supervision for" << identifier_; + + config_.deployment_config.environmental_variables.add( + "LCM_ALIVE_INTERFACE_PATH", supervision_handle_->getConnectionId()); + + return {}; } IComponent::RequestResult ProcessInfoNode::tryReportCompletion(score::mw::lifecycle::ProcessState new_state) 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 b13407650..366d1fcf7 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 @@ -22,6 +22,7 @@ #include "score/mw/launch_manager/process_group_manager/details/process_handling.hpp" #include "score/mw/launch_manager/process_group_manager/details/safe_process_map.hpp" #include "score/mw/launch_manager/process_group_manager/process_state.hpp" +#include #include #include #include @@ -52,10 +53,13 @@ class ProcessInfoNode final : public IComponent }; public: - /// @brief Constructs a ProcessInfoNode. + /// @brief Constructs a ProcessInfoNode, setting up alive supervision if configured. /// @param config Configuration for the OS process. /// @param process_handling The interfaces used to start, stop and report on the OS process. - ProcessInfoNode(configuration::ComponentConfig&& config, ProcessHandling process_handling); + /// @return The constructed node, or kErrorBeforeReady if alive supervision construction failed. + [[nodiscard]] static score::cpp::expected Create( + configuration::ComponentConfig&& config, + ProcessHandling process_handling); /// @brief Explicit move constructor required due to atomics. PIN must be moveable to exist in the graph ProcessInfoNode(ProcessInfoNode&& other) noexcept @@ -70,7 +74,9 @@ class ProcessInfoNode final : public IComponent sync_(std::move(other.sync_)), process_handling_(std::move(other.process_handling_)), supervision_handle_(std::move(other.supervision_handle_)), - identifier_(other.identifier_) + start_tries_(other.start_tries_), + identifier_(other.identifier_), + termination_result_(other.termination_result_) { } @@ -102,6 +108,16 @@ class ProcessInfoNode final : public IComponent [[nodiscard]] ControlClientChannelP getControlClientChannel() const; private: + /// @brief Constructs a ProcessInfoNode without setting up alive supervision. Use Create() instead. + /// @param config Configuration for the OS process. + /// @param process_handling The interfaces used to start, stop and report on the OS process. + ProcessInfoNode(configuration::ComponentConfig&& config, ProcessHandling process_handling); + + /// @brief Sets up alive supervision for a Reporting_And_Supervised process. A no-op for any other application + /// type. + /// @return Success, or kErrorBeforeReady if construction of the supervision handle failed. + [[nodiscard]] score::cpp::expected_blank setupAliveSupervision(); + /// @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; 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 940effaa7..3467b255e 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 @@ -106,8 +106,10 @@ class ProcessInfoNodeFixture : public ::testing::Test config.component_properties.application_profile.alive_supervision = alive; } - return std::make_unique( + auto result = ProcessInfoNode::Create( std::move(config), ProcessHandling{&mock_processIf_, process_map_, nullptr, mock_factory_}); + EXPECT_THAT(result.has_value(), IsTrue()); + return std::make_unique(std::move(result).value()); } /// @brief Helper method to create a ProcessInfoNode with a FileState ready condition. @@ -127,8 +129,10 @@ class ProcessInfoNodeFixture : public ::testing::Test config.deployment_config.ready_timeout_ms = static_cast(ready_timeout.count()); config.deployment_config.shutdown_timeout_ms = shutdown_timeout_ms_; - return std::make_unique( + auto result = ProcessInfoNode::Create( std::move(config), ProcessHandling{&mock_processIf_, process_map_, &mock_file_waiter_, mock_factory_}); + EXPECT_THAT(result.has_value(), IsTrue()); + return std::make_unique(std::move(result).value()); } /// @brief Helper method to create a ProcessInfoNode that is self-terminating. @@ -188,6 +192,30 @@ class ProcessInfoNodeFixture : public ::testing::Test NiceMock mock_factory_{}; }; +TEST_F(ProcessInfoNodeFixture, CreateFailsWhenAliveSupervisionConstructionFails) +{ + RecordProperty( + "Description", + "Create() returns kErrorBeforeReady, instead of silently continuing, when the supervision factory fails to " + "construct a supervision handle."); + EXPECT_CALL(mock_factory_, constructSupervision).WillOnce(Return(nullptr)); + + configuration::ComponentConfig config{}; + config.name = kProcessName; + config.component_properties.binary_name = kProcessName; + config.component_properties.application_profile.application_type = + configuration::ApplicationType::ReportingAndSupervised; + config.component_properties.application_profile.alive_supervision = configuration::ComponentAliveSupervision{ + .reporting_cycle_ms = 10, .failed_cycles_tolerance = 1, .min_indications = 0, .max_indications = 0}; + config.component_properties.ready_condition = configuration::ReadyCondition{configuration::ProcessState::Running}; + + auto result = ProcessInfoNode::Create( + std::move(config), ProcessHandling{&mock_processIf_, process_map_, nullptr, mock_factory_}); + + ASSERT_THAT(result.has_value(), IsFalse()); + EXPECT_THAT(result.error(), Eq(IComponent::ComponentError::kErrorBeforeReady)); +} + // Bundles different cases for activate() that occur during startup, before the ready condition is reached. class ProcessInfoNodeStartupTest : public ProcessInfoNodeFixture { diff --git a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp index 306c40e09..c4cda6cbe 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp @@ -206,7 +206,7 @@ bool ProcessGroupManager::initializeControlClientHandler() bool ProcessGroupManager::initializeProcessGroups() { - graph_ = std::make_shared( + auto graph_res = Graph::Create( // size is +2 for fallback + off configuration_.components_.size() + configuration_.run_targets_.size() + 2, configuration_, @@ -214,6 +214,14 @@ bool ProcessGroupManager::initializeProcessGroups() ProcessHandling{&process_interface_, process_map_, &file_waiter_, alive_monitor_->getSupervisionFactory()}, this); + if (!graph_res.has_value()) + { + LM_LOG_ERROR() << "Failed to initialize process group"; + return false; + } + + graph_ = std::move(graph_res).value(); + LM_LOG_DEBUG() << "Process group initialized successfully"; return true; }