Merge pull request #115 from Synicix/pod_model_hash

Update pod hashing to remove non-replication concerns from hash

Author: eywalker

.github/workflows/tests.yaml

@@ -22,7 +22,7 @@ jobs:
       - name: Install code coverage
         uses: taiki-e/install-action@cargo-llvm-cov
       - name: Run syntax and style tests
-        run: cargo clippy --no-default-features --features=test --all-targets -- -D warnings
+        run: cargo clippy --all-targets -- -D warnings
       - name: Run format test
         run: cargo fmt --check
       - name: Run integration tests w/ coverage report
@@ -57,15 +57,11 @@ jobs:
           uv pip install eclipse-zenoh -p ~/.local/share/base
           . ~/.local/share/base/bin/activate
           maturin develop --uv
-      - name: Run smoke test
+      - name: Run Python tests
         env:
           RUST_BACKTRACE: full
         run: |
           . ~/.local/share/base/bin/activate
-          python tests/extra/python/smoke_test.py -- tests/.tmp
-      - name: Run agent test
-        env:
-          RUST_BACKTRACE: full
-        run: |
-          . ~/.local/share/base/bin/activate
-          python tests/extra/python/agent_test.py -- tests/.tmp
+          for py_file in tests/extra/python/*.py; do
+          python "$py_file" -- tests/.tmp
+          done

notebook.ipynb

@@ -121,6 +121,7 @@ impl fmt::Debug for OrcaError {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         match &self.kind {
             Kind::AgentCommunicationFailure { backtrace, .. }
+            | Kind::EmptyDir { backtrace, .. }
             | Kind::FailedToStartPod { backtrace, .. }
             | Kind::FailedToExtractRunInfo { backtrace, .. }
             | Kind::IncompletePacket { backtrace, .. }

src/core/error.rs

@@ -8,28 +8,33 @@ use std::{
     hash::BuildHasher,
     result,
 };
-/// Converts a model instance into a consistent yaml.
-///
-/// # Errors
-///
-/// Will return `Err` if there is an issue converting an `instance` into YAML (w/o annotation).
-pub fn to_yaml<T: Serialize>(instance: &T) -> Result<String> {
-    let mapping: IndexMap<String, Value> = serde_yaml::from_str(&serde_yaml::to_string(instance)?)?; // cast to map
-    let mut yaml = serde_yaml::to_string(
-        &mapping
-            .iter()
-            .filter_map(|(k, v)| match &**k {
-                "annotation" | "hash" => None,
-                "pod" | "pod_job" => Some((k, v["hash"].clone())),
-                _ => Some((k, v.clone())),
-            })
-            .collect::<IndexMap<_, _>>(),
-    )?; // skip fields and convert refs to hash pointers
-    yaml.insert_str(
-        0,
-        &format!("class: {}\n", get_type_name::<T>().to_snake_case()),
-    ); // replace class at top
-    Ok(yaml)
+
+/// Trait to handle serialization to yaml for `OrcaPod` models
+pub trait ToYaml: Serialize + Sized {
+    /// Serializes the instance to a YAML string.
+    /// # Errors
+    /// Will return `Err` if it fail to serialize instance to string
+    fn to_yaml(&self) -> Result<String> {
+        let mapping: IndexMap<String, Value> = serde_yaml::from_str(&serde_yaml::to_string(self)?)?; // cast to map
+        let mut yaml = serde_yaml::to_string(
+            &mapping
+                .iter()
+                .filter_map(|(k, v)| Self::process_field(k, v))
+                .collect::<IndexMap<_, _>>(),
+        )?; // skip fields and convert refs to hash pointers
+        yaml.insert_str(
+            0,
+            &format!("class: {}\n", get_type_name::<Self>().to_snake_case()),
+        ); // replace class at top
+        Ok(yaml)
+    }
+
+    /// Filter out which field to serialize and which to omit
+    ///
+    /// # Returns
+    /// (`field_name`, `field_value`): to be pass to `to_yaml` for serialization
+    /// None: to skip
+    fn process_field(field_name: &str, field_value: &Value) -> Option<(String, Value)>;
 }
 
 pub(crate) fn serialize_hashmap<S, K: Ord + Serialize, V: Serialize, BH: BuildHasher>(

src/core/model/mod.rs

@@ -653,8 +653,8 @@ impl PodProcessor {
                 )
                 .into(),
             },
-            pod.recommended_cpus,
-            pod.recommended_memory,
+            pod.recommend_specs.cpus,
+            pod.recommend_specs.memory,
             None,
             &pipeline_run.namespace_lookup,
         )?;

src/core/pipeline_runner.rs

@@ -1,6 +1,6 @@
 use crate::{
     core::{
-        model::to_yaml,
+        model::ToYaml,
         store::MODEL_NAMESPACE,
         util::{get_type_name, parse_debug_name},
     },
@@ -111,7 +111,7 @@ impl LocalFileStore {
         Ok(model_info.hash)
     }
 
-    fn save_file(file: impl AsRef<Path>, content: impl AsRef<[u8]>) -> Result<()> {
+    pub(crate) fn save_file(file: impl AsRef<Path>, content: impl AsRef<[u8]>) -> Result<()> {
         if let Some(parent) = file.as_ref().parent() {
             fs::create_dir_all(parent)?;
         }
@@ -123,7 +123,7 @@ impl LocalFileStore {
     /// # Errors
     ///
     /// Will return `Err` if there is an issue storing the model.
-    pub(crate) fn save_model<T: Serialize + fmt::Debug>(
+    pub(crate) fn save_model<T: Serialize + fmt::Debug + ToYaml>(
         &self,
         model: &T,
         hash: &str,
@@ -174,7 +174,7 @@ impl LocalFileStore {
                 .yellow(),
             );
         } else {
-            Self::save_file(spec_file, to_yaml(model)?)?;
+            Self::save_file(spec_file, model.to_yaml()?)?;
         }
         Ok(())
     }

src/core/store/filestore.rs

@@ -31,6 +31,11 @@ pub(crate) enum Kind {
         source: Box<dyn Error + Send + Sync>,
         backtrace: Option<Backtrace>,
     },
+    #[snafu(display("Empty directory: {dir:?}, where they should be files"))]
+    EmptyDir {
+        dir: PathBuf,
+        backtrace: Option<Backtrace>,
+    },
     #[snafu(display(
         "Failed to extract run info from the container image file: {container_name}."
     ))]