hmon: Cargo fix post-review fixes

- Less restrictive feature flags.
- Additional small code changes.

Author: arkjedrz

.github/workflows/lint_clippy.yml

@@ -48,14 +48,8 @@ jobs:
           override: true
           components: clippy
 
-      - name: check clippy errors (with "--features stub_supervisor_api_client")
+      - name: check clippy errors
         uses: actions-rs/cargo@v1
         with:
           command: clippy
-          args: --features stub_supervisor_api_client --all-targets -- -D warnings
-
-      - name: check clippy errors (with "--features score_supervisor_api_client")
-        uses: actions-rs/cargo@v1
-        with:
-          command: clippy
-          args: --features score_supervisor_api_client --no-default-features --all-targets -- -D warnings
+          args: --all-features --all-targets --workspace -- -D warnings

.vscode/settings.json

@@ -9,9 +9,6 @@
     "rust-analyzer.cargo.cfgs": [
         "!miri"
     ],
-    "rust-analyzer.cargo.features": [
-        "stub_supervisor_api_client"
-    ],
     "rust-analyzer.check.command": "clippy",
     "rust-analyzer.rustfmt.overrideCommand": [
         "${workspaceFolder}/.vscode/rustfmt.sh"

src/health_monitoring_lib/rust/common.rs

@@ -39,6 +39,9 @@ pub(crate) enum MonitorEvaluationError {
 
 /// Trait for evaluating monitors and reporting errors to be used by HealthMonitor.
 pub(crate) trait MonitorEvaluator {
+    /// Run monitor evaluation.
+    ///
+    /// - `on_error` - error handling, containing tag of failing object and error code.
     fn evaluate(&self, on_error: &mut dyn FnMut(&MonitorTag, MonitorEvaluationError));
 }
 

src/health_monitoring_lib/rust/lib.rs

@@ -22,7 +22,6 @@ mod worker;
 pub mod deadline;
 
 use crate::common::MonitorEvalHandle;
-use crate::supervisor_api_client::SupervisorAPIClientImpl;
 pub use common::TimeRange;
 use containers::fixed_capacity::FixedCapacityVec;
 use core::time::Duration;
@@ -216,8 +215,17 @@ impl HealthMonitor {
     }
 
     pub(crate) fn start_internal(&mut self, monitors: FixedCapacityVec<MonitorEvalHandle>) {
-        let monitoring_logic =
-            worker::MonitoringLogic::new(monitors, self.supervisor_api_cycle, SupervisorAPIClientImpl::new());
+        let monitoring_logic = worker::MonitoringLogic::new(
+            monitors,
+            self.supervisor_api_cycle,
+            #[cfg(all(not(test), feature = "score_supervisor_api_client"))]
+            supervisor_api_client::score_supervisor_api_client::ScoreSupervisorAPIClient::new(),
+            #[cfg(any(
+                test,
+                all(feature = "stub_supervisor_api_client", not(feature = "score_supervisor_api_client"))
+            ))]
+            supervisor_api_client::stub_supervisor_api_client::StubSupervisorAPIClient::new(),
+        );
 
         self.worker.start(monitoring_logic)
     }

src/health_monitoring_lib/rust/supervisor_api_client/mod.rs

@@ -11,7 +11,7 @@
 // SPDX-License-Identifier: Apache-2.0
 // *******************************************************************************
 
-//! Module for selecting [`SupervisorAPIClient`] implementation.
+//! Module providing [`SupervisorAPIClient`] implementations.
 //! Currently `ScoreSupervisorAPIClient` and `StubSupervisorAPIClient` are supported.
 //! The latter is meant for testing purposes.
 
@@ -20,19 +20,9 @@ pub trait SupervisorAPIClient {
     fn notify_alive(&self);
 }
 
-// Disallow both and none features.
-#[cfg(any(
-    all(feature = "score_supervisor_api_client", feature = "stub_supervisor_api_client"),
-    not(any(feature = "score_supervisor_api_client", feature = "stub_supervisor_api_client"))
-))]
-compile_error!("Either 'score_supervisor_api_client' or 'stub_supervisor_api_client' must be enabled!");
+// NOTE: various implementations are not mutually exclusive.
 
 #[cfg(feature = "score_supervisor_api_client")]
-mod score_supervisor_api_client;
+pub mod score_supervisor_api_client;
 #[cfg(feature = "stub_supervisor_api_client")]
-mod stub_supervisor_api_client;
-
-#[cfg(feature = "score_supervisor_api_client")]
-pub use score_supervisor_api_client::ScoreSupervisorAPIClient as SupervisorAPIClientImpl;
-#[cfg(feature = "stub_supervisor_api_client")]
-pub use stub_supervisor_api_client::StubSupervisorAPIClient as SupervisorAPIClientImpl;
+pub mod stub_supervisor_api_client;

src/health_monitoring_lib/rust/supervisor_api_client/score_supervisor_api_client.rs

@@ -11,18 +11,18 @@
 // SPDX-License-Identifier: Apache-2.0
 // *******************************************************************************
 
+#![allow(dead_code)]
+
 use crate::log::debug;
 use crate::supervisor_api_client::SupervisorAPIClient;
 use crate::worker::Checks;
 
-#[allow(dead_code)]
 pub struct ScoreSupervisorAPIClient {
     supervisor_link: monitor_rs::Monitor<Checks>,
 }
 
 unsafe impl Send for ScoreSupervisorAPIClient {} // Just assuming it's safe to send across threads, this is a temporary solution
 
-#[allow(dead_code)]
 impl ScoreSupervisorAPIClient {
     pub fn new() -> Self {
         let value = std::env::var("IDENTIFIER").expect("IDENTIFIER env not set");

src/health_monitoring_lib/rust/supervisor_api_client/stub_supervisor_api_client.rs

@@ -11,11 +11,12 @@
 // SPDX-License-Identifier: Apache-2.0
 // *******************************************************************************
 
+#![allow(dead_code)]
+
 use crate::log::warn;
 use crate::supervisor_api_client::SupervisorAPIClient;
 
 /// A stub implementation of the SupervisorAPIClient that logs alive notifications.
-#[allow(dead_code)]
 pub struct StubSupervisorAPIClient;
 
 impl StubSupervisorAPIClient {
@@ -24,7 +25,6 @@ impl StubSupervisorAPIClient {
     }
 }
 
-#[allow(dead_code)]
 impl SupervisorAPIClient for StubSupervisorAPIClient {
     fn notify_alive(&self) {
         warn!("StubSupervisorAPIClient: notify_alive called");

src/health_monitoring_lib/rust/worker.rs

@@ -14,12 +14,15 @@ use crate::common::{MonitorEvalHandle, MonitorEvaluator};
 use crate::log::{info, warn};
 use crate::supervisor_api_client::SupervisorAPIClient;
 use containers::fixed_capacity::FixedCapacityVec;
+use core::sync::atomic::{AtomicBool, Ordering};
 use core::time::Duration;
+use std::sync::Arc;
+use std::time::Instant;
 
 pub(super) struct MonitoringLogic<T: SupervisorAPIClient> {
     monitors: FixedCapacityVec<MonitorEvalHandle>,
     client: T,
-    last_notification: std::time::Instant,
+    last_notification: Instant,
     supervisor_api_cycle: Duration,
 }
 
@@ -38,7 +41,7 @@ impl<T: SupervisorAPIClient> MonitoringLogic<T> {
             monitors,
             client,
             supervisor_api_cycle,
-            last_notification: std::time::Instant::now(),
+            last_notification: Instant::now(),
         }
     }
 
@@ -55,7 +58,7 @@ impl<T: SupervisorAPIClient> MonitoringLogic<T> {
 
         if !has_any_error {
             if self.last_notification.elapsed() > self.supervisor_api_cycle {
-                self.last_notification = std::time::Instant::now();
+                self.last_notification = Instant::now();
                 self.client.notify_alive();
             }
         } else {
@@ -70,15 +73,15 @@ impl<T: SupervisorAPIClient> MonitoringLogic<T> {
 /// A struct that manages a unique thread for running monitoring logic periodically.
 pub struct UniqueThreadRunner {
     handle: Option<std::thread::JoinHandle<()>>,
-    should_stop: std::sync::Arc<core::sync::atomic::AtomicBool>,
+    should_stop: Arc<AtomicBool>,
     internal_duration_cycle: Duration,
 }
 
 impl UniqueThreadRunner {
     pub(super) fn new(internal_duration_cycle: Duration) -> Self {
         Self {
             handle: None,
-            should_stop: std::sync::Arc::new(core::sync::atomic::AtomicBool::new(false)),
+            should_stop: Arc::new(AtomicBool::new(false)),
             internal_duration_cycle,
         }
     }
@@ -96,10 +99,10 @@ impl UniqueThreadRunner {
                 let mut next_sleep_time = interval;
 
                 // TODO Add some checks and log if cyclicly here is not met.
-                while !should_stop.load(core::sync::atomic::Ordering::Relaxed) {
+                while !should_stop.load(Ordering::Relaxed) {
                     std::thread::sleep(next_sleep_time);
 
-                    let now = std::time::Instant::now();
+                    let now = Instant::now();
 
                     if !monitoring_logic.run() {
                         info!("Monitoring logic failed, stopping thread.");
@@ -115,7 +118,7 @@ impl UniqueThreadRunner {
     }
 
     pub fn join(&mut self) {
-        self.should_stop.store(true, core::sync::atomic::Ordering::Relaxed);
+        self.should_stop.store(true, Ordering::Relaxed);
         if let Some(handle) = self.handle.take() {
             let _ = handle.join();
         }
@@ -152,28 +155,30 @@ mod tests {
     use crate::worker::{MonitoringLogic, UniqueThreadRunner};
     use crate::TimeRange;
     use containers::fixed_capacity::FixedCapacityVec;
+    use core::sync::atomic::{AtomicUsize, Ordering};
     use core::time::Duration;
+    use std::sync::Arc;
 
     #[derive(Clone)]
     struct MockSupervisorAPIClient {
-        pub notify_called: std::sync::Arc<core::sync::atomic::AtomicUsize>,
+        pub notify_called: Arc<AtomicUsize>,
     }
 
     impl MockSupervisorAPIClient {
         pub fn new() -> Self {
             Self {
-                notify_called: std::sync::Arc::new(core::sync::atomic::AtomicUsize::new(0)),
+                notify_called: Arc::new(AtomicUsize::new(0)),
             }
         }
 
         fn get_notify_count(&self) -> usize {
-            self.notify_called.load(core::sync::atomic::Ordering::Acquire)
+            self.notify_called.load(Ordering::Acquire)
         }
     }
 
     impl SupervisorAPIClient for MockSupervisorAPIClient {
         fn notify_alive(&self) {
-            self.notify_called.fetch_add(1, core::sync::atomic::Ordering::AcqRel);
+            self.notify_called.fetch_add(1, Ordering::AcqRel);
         }
     }