merge: #2681

2681: fix(si): Ensure that running an update when no containers installed doesn't trigger start r=fnichol a=stack72 ENG-1886 ENG-1887 - feat(si): Ensure we capture the name of the containe engine when tracking usage - fix(si): Ensure that running an update when no containers installed doesn't trigger start Co-authored-by: stack72 <[email protected]>
systeminit · Aug 23, 2023 · de21ad9 · de21ad9
2 parents 8a42363 + efbab64
commit de21ad9
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 12 deletions.
diff --git a/lib/si-cli/src/cmd/stop.rs b/lib/si-cli/src/cmd/stop.rs
@@ -32,6 +32,7 @@ async fn invoke(app: &AppState, is_preview: bool) -> CliResult<()> {
             .get_existing_container(container_identifier.clone())
             .await?;
         if existing.is_some() {
+            println!("Stopping container {}", container_identifier.clone());
             app.container_engine()
                 .stop_container(existing.unwrap().id.unwrap().to_string())
                 .await?;

diff --git a/lib/si-cli/src/cmd/update.rs b/lib/si-cli/src/cmd/update.rs
@@ -114,21 +114,23 @@ impl AppState {
         let current_containers = self.container_engine().get_container_details().await?;
 
         let mut containers = Vec::new();
-        let latest_containers: Vec<LatestContainer> = req.json().await?;
-        'outer: for latest in latest_containers {
-            for current in &current_containers {
-                if current.image != format!("{}/{}", latest.namespace, latest.repository) {
-                    continue;
-                }
+        if !current_containers.is_empty() {
+            let latest_containers: Vec<LatestContainer> = req.json().await?;
+            'outer: for latest in latest_containers {
+                for current in &current_containers {
+                    if current.image != format!("{}/{}", latest.namespace, latest.repository) {
+                        continue;
+                    }
 
-                if current.version != latest.digest {
-                    containers.push(latest);
+                    if current.version != latest.digest {
+                        containers.push(latest);
+                    }
+                    continue 'outer;
                 }
-                continue 'outer;
-            }
 
-            // If we don't have the container locally we should fetch it
-            containers.push(latest);
+                // If we don't have the container locally we should fetch it
+                containers.push(latest);
+            }
         }
 
         let req = reqwest::get(format!("{host}/github/releases/latest")).await?;

diff --git a/lib/si-cli/src/engine.rs b/lib/si-cli/src/engine.rs
@@ -8,6 +8,7 @@ pub mod podman_engine;
 
 #[async_trait]
 pub trait ContainerEngine {
+    fn get_engine_identifier(&self) -> String;
     async fn ping(&self) -> CliResult<()>;
     async fn missing_containers(&self) -> Result<Vec<String>, SiCliError>;
     async fn download_missing_containers(&self, missing_containers: Vec<String>) -> CliResult<()>;

diff --git a/lib/si-cli/src/engine/docker_engine.rs b/lib/si-cli/src/engine/docker_engine.rs
@@ -57,6 +57,10 @@ impl DockerEngine {
 
 #[async_trait]
 impl ContainerEngine for DockerEngine {
+    fn get_engine_identifier(&self) -> String {
+        "docker".to_string()
+    }
+
     async fn ping(&self) -> CliResult<()> {
         self.docker.ping().await?;
         Ok(())

diff --git a/lib/si-cli/src/engine/podman_engine.rs b/lib/si-cli/src/engine/podman_engine.rs
@@ -18,6 +18,10 @@ impl PodmanEngine {
 
 #[async_trait]
 impl ContainerEngine for PodmanEngine {
+    fn get_engine_identifier(&self) -> String {
+        "podman".to_string()
+    }
+
     async fn ping(&self) -> CliResult<()> {
         let ping_info = self.podman.ping().await?;
         dbg!(&ping_info);

diff --git a/lib/si-cli/src/state.rs b/lib/si-cli/src/state.rs
@@ -91,6 +91,10 @@ impl AppState {
         prop_map.insert("mode".to_string(), serde_json::json!(self.mode()));
         prop_map.insert("os".to_string(), serde_json::json!(env::consts::OS));
         prop_map.insert("arch".to_string(), serde_json::json!(env::consts::ARCH));
+        prop_map.insert(
+            "container_engine".to_string(),
+            serde_json::json!(self.container_engine().get_engine_identifier()),
+        );
 
         ph_client
             .capture("si-command", distinct_id, properties)