revert reload feature

Author: FedericoPonzi

README.md

@@ -136,9 +136,6 @@ horustctl --uds-folder-path /tmp stop myapp.toml
 
 # Restart a service (stop then start)
 horustctl --uds-folder-path /tmp restart myapp.toml
-
-# Reload service directories to pick up new service definitions
-horustctl --uds-folder-path /tmp reload
 ```
 
 ## Quick tutorial

commands/src/client.rs

@@ -1,8 +1,8 @@
 use crate::proto::messages::horust_msg_message::MessageType;
 use crate::proto::messages::{
-    HorustMsgAllServicesStatusRequest, HorustMsgMessage, HorustMsgReloadRequest, HorustMsgRequest,
-    HorustMsgRestartRequest, HorustMsgServiceChangeRequest, HorustMsgServiceStatusRequest,
-    horust_msg_request, horust_msg_response,
+    HorustMsgAllServicesStatusRequest, HorustMsgMessage, HorustMsgRequest, HorustMsgRestartRequest,
+    HorustMsgServiceChangeRequest, HorustMsgServiceStatusRequest, horust_msg_request,
+    horust_msg_response,
 };
 use crate::{HorustMsgServiceStatus, UdsConnectionHandler};
 use anyhow::{Context, anyhow};
@@ -106,18 +106,6 @@ impl ClientHandler {
         }
     }
 
-    pub fn send_reload_request(&mut self) -> Result<(bool, Vec<String>)> {
-        let msg = new_request(horust_msg_request::Request::ReloadRequest(
-            HorustMsgReloadRequest {},
-        ));
-        let response = send_and_receive(&mut self.uds_connection_handler, msg)?;
-        if let horust_msg_response::Response::ReloadResponse(resp) = response {
-            Ok((resp.accepted, resp.new_services))
-        } else {
-            bail!("Invalid response received: {:?}", response);
-        }
-    }
-
     pub fn send_all_status_request(&mut self) -> Result<Vec<(String, HorustMsgServiceStatus)>> {
         let msg = new_request(horust_msg_request::Request::AllStatusRequest(
             HorustMsgAllServicesStatusRequest {},

commands/src/commands.proto

@@ -12,7 +12,6 @@ message HorustMsgRequest {
     HorustMsgServiceStatusRequest status_request = 1;
     HorustMsgServiceChangeRequest change_request = 2;
     HorustMsgRestartRequest restart_request = 3;
-    HorustMsgReloadRequest reload_request = 4;
     HorustMsgAllServicesStatusRequest all_status_request = 5;
   }
 }
@@ -22,8 +21,7 @@ message HorustMsgResponse {
     HorustMsgServiceStatusResponse status_response = 2;
     HorustMsgServiceChangeResponse change_response = 3;
     HorustMsgRestartResponse restart_response = 4;
-    HorustMsgReloadResponse reload_response = 5;
-    HorustMsgAllServicesStatusResponse all_status_response = 6;
+    HorustMsgAllServicesStatusResponse all_status_response = 5;
   }
 }
 
@@ -61,15 +59,6 @@ message HorustMsgRestartResponse {
   bool accepted = 2;
 }
 
-// Reload service directories for new service definitions.
-message HorustMsgReloadRequest {
-}
-
-message HorustMsgReloadResponse {
-  bool accepted = 1;
-  repeated string new_services = 2;
-}
-
 // Request status of all services.
 message HorustMsgAllServicesStatusRequest {
 }

commands/src/proto/messages.rs

@@ -16,7 +16,7 @@ pub mod horust_msg_message {
 }
 #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
 pub struct HorustMsgRequest {
-    #[prost(oneof = "horust_msg_request::Request", tags = "1, 2, 3, 4, 5")]
+    #[prost(oneof = "horust_msg_request::Request", tags = "1, 2, 3, 5")]
     pub request: ::core::option::Option<horust_msg_request::Request>,
 }
 /// Nested message and enum types in `HorustMsgRequest`.
@@ -29,15 +29,13 @@ pub mod horust_msg_request {
         ChangeRequest(super::HorustMsgServiceChangeRequest),
         #[prost(message, tag = "3")]
         RestartRequest(super::HorustMsgRestartRequest),
-        #[prost(message, tag = "4")]
-        ReloadRequest(super::HorustMsgReloadRequest),
         #[prost(message, tag = "5")]
         AllStatusRequest(super::HorustMsgAllServicesStatusRequest),
     }
 }
 #[derive(Clone, PartialEq, ::prost::Message)]
 pub struct HorustMsgResponse {
-    #[prost(oneof = "horust_msg_response::Response", tags = "1, 2, 3, 4, 5, 6")]
+    #[prost(oneof = "horust_msg_response::Response", tags = "1, 2, 3, 4, 5")]
     pub response: ::core::option::Option<horust_msg_response::Response>,
 }
 /// Nested message and enum types in `HorustMsgResponse`.
@@ -53,8 +51,6 @@ pub mod horust_msg_response {
         #[prost(message, tag = "4")]
         RestartResponse(super::HorustMsgRestartResponse),
         #[prost(message, tag = "5")]
-        ReloadResponse(super::HorustMsgReloadResponse),
-        #[prost(message, tag = "6")]
         AllStatusResponse(super::HorustMsgAllServicesStatusResponse),
     }
 }
@@ -103,16 +99,6 @@ pub struct HorustMsgRestartResponse {
     #[prost(bool, tag = "2")]
     pub accepted: bool,
 }
-/// Reload service directories for new service definitions.
-#[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)]
-pub struct HorustMsgReloadRequest {}
-#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
-pub struct HorustMsgReloadResponse {
-    #[prost(bool, tag = "1")]
-    pub accepted: bool,
-    #[prost(string, repeated, tag = "2")]
-    pub new_services: ::prost::alloc::vec::Vec<::prost::alloc::string::String>,
-}
 /// Request status of all services.
 #[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)]
 pub struct HorustMsgAllServicesStatusRequest {}

commands/src/server.rs

@@ -1,8 +1,8 @@
 use crate::UdsConnectionHandler;
 use crate::proto::messages::horust_msg_message::MessageType::Request;
 use crate::proto::messages::{
-    HorustMsgAllServicesStatusResponse, HorustMsgError, HorustMsgMessage, HorustMsgReloadResponse,
-    HorustMsgRequest, HorustMsgResponse, HorustMsgRestartResponse, HorustMsgServiceChangeResponse,
+    HorustMsgAllServicesStatusResponse, HorustMsgError, HorustMsgMessage, HorustMsgRequest,
+    HorustMsgResponse, HorustMsgRestartResponse, HorustMsgServiceChangeResponse,
     HorustMsgServiceStatus, HorustMsgServiceStatusEntry, HorustMsgServiceStatusResponse,
     horust_msg_message, horust_msg_request, horust_msg_response,
 };
@@ -100,15 +100,6 @@ pub trait CommandsHandlerTrait {
                         )),
                     }
                 }
-                horust_msg_request::Request::ReloadRequest(_) => {
-                    info!("Requested service reload");
-                    match self.reload_services() {
-                        Ok(new_services) => new_horust_msg_reload_response(true, new_services),
-                        Err(err) => new_horust_msg_error_response(format!(
-                            "Error from reload handler: {err}"
-                        )),
-                    }
-                }
                 horust_msg_request::Request::AllStatusRequest(_) => {
                     info!("Requested all services status");
                     let statuses = self.get_all_service_statuses();
@@ -129,7 +120,6 @@ pub trait CommandsHandlerTrait {
         new_status: HorustMsgServiceStatus,
     ) -> Result<()>;
     fn restart_service(&self, service_name: &str) -> Result<()>;
-    fn reload_services(&self) -> Result<Vec<String>>;
     fn get_all_service_statuses(&self) -> Vec<(String, HorustMsgServiceStatus)>;
 }
 
@@ -179,15 +169,6 @@ fn new_horust_msg_restart_response(service_name: String, accepted: bool) -> Horu
     ))
 }
 
-fn new_horust_msg_reload_response(accepted: bool, new_services: Vec<String>) -> HorustMsgMessage {
-    wrap_response(horust_msg_response::Response::ReloadResponse(
-        HorustMsgReloadResponse {
-            accepted,
-            new_services,
-        },
-    ))
-}
-
 fn new_horust_msg_all_status_response(
     statuses: Vec<(String, HorustMsgServiceStatus)>,
 ) -> HorustMsgMessage {

commands/tests/simple.rs

@@ -55,10 +55,6 @@ impl CommandsHandlerTrait for MockCommandsHandler {
         Ok(())
     }
 
-    fn reload_services(&self) -> Result<Vec<String>> {
-        Ok(vec!["new-service.toml".to_string()])
-    }
-
     fn get_all_service_statuses(&self) -> Vec<(String, HorustMsgServiceStatus)> {
         vec![
             ("Running".to_string(), HorustMsgServiceStatus::Running),
@@ -162,35 +158,6 @@ fn test_restart_request() -> Result<()> {
     Ok(())
 }
 
-#[test]
-fn test_reload_request() -> Result<()> {
-    init();
-    let socket_path: PathBuf = "/tmp/test_reload.sock".into();
-    if socket_path.exists() {
-        std::fs::remove_file(&socket_path)?;
-    }
-    let socket_path2 = socket_path.clone();
-    let barrier = Arc::new(Barrier::new(2));
-    let barrier2 = Arc::clone(&barrier);
-
-    let s_handle = thread::spawn(move || {
-        let mut uds = MockCommandsHandler::new(socket_path2);
-        barrier.wait();
-        uds.accept().unwrap();
-    });
-
-    let c_handle = thread::spawn(move || {
-        barrier2.wait();
-        let mut client = ClientHandler::new_client(&socket_path).unwrap();
-        let (accepted, new_services) = client.send_reload_request().unwrap();
-        assert!(accepted);
-        assert_eq!(new_services, vec!["new-service.toml"]);
-    });
-    s_handle.join().unwrap();
-    c_handle.join().unwrap();
-    Ok(())
-}
-
 #[test]
 fn test_all_status_request() -> Result<()> {
     init();

docs/README.md

@@ -339,9 +339,6 @@ horustctl --uds-folder-path /tmp stop myapp.toml
 
 # Restart a service (stop then start)
 horustctl --uds-folder-path /tmp restart myapp.toml
-
-# Reload service directories to pick up new service definitions
-horustctl --uds-folder-path /tmp reload
 ```
 
 ### Permissions

horust/src/horust/commands_handler.rs

@@ -1,6 +1,6 @@
 use crate::horust::Event;
 use crate::horust::bus::BusConnector;
-use crate::horust::formats::{Service, ServiceName, ServiceStatus, User};
+use crate::horust::formats::{ServiceName, ServiceStatus, User};
 use anyhow::{Result, anyhow, bail};
 use horust_commands_lib::{CommandsHandlerTrait, HorustMsgServiceStatus, UdsConnectionHandler};
 use std::collections::HashMap;
@@ -14,10 +14,9 @@ pub fn spawn(
     bus: BusConnector<Event>,
     uds_path: PathBuf,
     services: Vec<(ServiceName, User)>,
-    services_paths: Vec<PathBuf>,
 ) -> JoinHandle<()> {
     thread::spawn(move || {
-        let mut commands_handler = CommandsHandler::new(bus, uds_path, services, services_paths);
+        let mut commands_handler = CommandsHandler::new(bus, uds_path, services);
         commands_handler.run();
     })
 }
@@ -28,7 +27,6 @@ struct CommandsHandler {
     service_users: HashMap<ServiceName, User>,
     uds_listener: UnixListener,
     uds_path: PathBuf,
-    services_paths: Vec<PathBuf>,
     /// Peer UID of the current connection being handled (set during accept).
     current_peer_uid: Option<u32>,
 }
@@ -38,7 +36,6 @@ impl CommandsHandler {
         bus: BusConnector<Event>,
         uds_path: PathBuf,
         services: Vec<(ServiceName, User)>,
-        services_paths: Vec<PathBuf>,
     ) -> Self {
         let uds_listener = UnixListener::bind(&uds_path).unwrap();
         uds_listener.set_nonblocking(true).unwrap();
@@ -50,7 +47,6 @@ impl CommandsHandler {
             bus,
             uds_path,
             uds_listener,
-            services_paths,
             service_users,
             current_peer_uid: None,
             services: services
@@ -69,12 +65,6 @@ impl CommandsHandler {
                             *k = status;
                         }
                     }
-                    Event::ServiceAdded(ref service) => {
-                        self.services
-                            .insert(service.name.clone(), ServiceStatus::Initial);
-                        self.service_users
-                            .insert(service.name.clone(), service.user.clone());
-                    }
                     Event::ShuttingDownInitiated(_) => {
                         fs::remove_file(&self.uds_path).unwrap();
                         return;
@@ -87,51 +77,6 @@ impl CommandsHandler {
         }
     }
 
-    fn load_services_from_paths(paths: &[PathBuf]) -> Result<Vec<Service>> {
-        let mut services = Vec::new();
-        for path in paths {
-            if !path.exists() {
-                continue;
-            }
-            let entries = if path.is_file() {
-                vec![path.to_path_buf()]
-            } else {
-                fs::read_dir(path)?
-                    .filter_map(Result::ok)
-                    .map(|e| e.path())
-                    .filter(|p| {
-                        p.is_file()
-                            && p.extension()
-                                .and_then(|ext| ext.to_str())
-                                .is_some_and(|ext| ext == "toml")
-                    })
-                    .collect()
-            };
-            for entry in entries {
-                match Service::from_file(&entry) {
-                    Ok(mut svc) => {
-                        if svc.name.is_empty() {
-                            svc.name = entry
-                                .file_name()
-                                .unwrap_or_default()
-                                .to_string_lossy()
-                                .into_owned();
-                        }
-                        if svc.name.is_empty() {
-                            error!("Skipping service with empty name from {:?}", entry);
-                            continue;
-                        }
-                        services.push(svc);
-                    }
-                    Err(err) => {
-                        error!("Failed to load service from {:?}: {}", entry, err);
-                    }
-                }
-            }
-        }
-        Ok(services)
-    }
-
     /// Validate that a service exists and the peer has permission to manage it.
     fn validate_and_authorize(&self, service_name: &str) -> Result<()> {
         if !self.services.contains_key(service_name) {
@@ -262,18 +207,6 @@ impl CommandsHandlerTrait for CommandsHandler {
             .send_event(Event::Restart(service_name.to_string()));
         Ok(())
     }
-    fn reload_services(&self) -> Result<Vec<String>> {
-        let all_on_disk = Self::load_services_from_paths(&self.services_paths)?;
-        let new_services: Vec<Service> = all_on_disk
-            .into_iter()
-            .filter(|s| !self.services.contains_key(&s.name))
-            .collect();
-        let new_names: Vec<String> = new_services.iter().map(|s| s.name.clone()).collect();
-        for service in new_services {
-            self.bus.send_event(Event::ServiceAdded(service));
-        }
-        Ok(new_names)
-    }
     fn get_all_service_statuses(&self) -> Vec<(String, HorustMsgServiceStatus)> {
         let mut statuses: Vec<(String, HorustMsgServiceStatus)> = self
             .services
@@ -334,7 +267,6 @@ mod tests {
             service_users,
             uds_listener,
             uds_path: socket_path,
-            services_paths: vec![],
             current_peer_uid: peer_uid,
         };
         handler

horust/src/horust/formats/mod.rs

@@ -28,8 +28,6 @@ pub enum Event {
     HealthCheck(ServiceName, HealthinessStatus),
     /// Request to restart a specific service (kill if alive, then set to Initial).
     Restart(ServiceName),
-    /// A new service definition to add at runtime (consumed by supervisor, healthcheck, commands_handler).
-    ServiceAdded(Service),
 }
 
 impl Event {