controller: Stop panicking when unable to create pod. (#615)

* controller: Stop panicking when unable to create pod. Currently the controller panic if it encounters an error while trying to create a pod/job for an instance, thus causing an impact far wider than just the failing instance. This should only have impact on the said instance, so as a first step, this commit stop the controller from panicking in this case and just output a log entry. Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> * Update patch version Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com> --------- Signed-off-by: Nicolas Belouin <nicolas.belouin@suse.com>
project-akri · Jul 24, 2023 · 51b3a05 · 51b3a05
1 parent cc14ba6
commit 51b3a05
Show file tree

Hide file tree

Showing 19 changed files with 77 additions and 41 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/agent/Cargo.toml b/agent/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "agent"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>", "<bfjelds@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/controller/Cargo.toml b/controller/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "controller"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["<bfjelds@microsoft.com>", "<kagold@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/controller/src/util/instance_action.rs b/controller/src/util/instance_action.rs
@@ -419,32 +419,34 @@ pub async fn handle_instance_change(
  }
  };
  if let Some(broker_spec) = &configuration.spec.broker_spec {
- match broker_spec {
+ let instance_change_result = match broker_spec {
  BrokerSpec::BrokerPodSpec(p) => {
  handle_instance_change_pod(instance, p, action, kube_interface).await
  }
  BrokerSpec::BrokerJobSpec(j) => {
  handle_instance_change_job(
- instance.clone(),
+ instance,
  *configuration.metadata.generation.as_ref().unwrap(),
  j,
  action,
  kube_interface,
  )
  .await
  }
+ };
+ if let Err(e) = instance_change_result {
+ error!("Unable to handle Broker action: {:?}", e);
  }
- } else {
- Ok(())
  }
+ Ok(())
 }
 
 /// Called when an Instance has changed that requires a Job broker. Action determined by InstanceAction.
 /// InstanceAction::Add => Deploy a Job with JobSpec from Configuration. Label with Instance name.
 /// InstanceAction::Remove => Delete all Jobs labeled with the Instance name
 /// InstanceAction::Update => No nothing
 pub async fn handle_instance_change_job(
- instance: Instance,
+ instance: &Instance,
  config_generation: i64,
  job_spec: &JobSpec,
  action: &InstanceAction,
@@ -472,7 +474,7 @@ pub async fn handle_instance_change_job(
  trace!("handle_instance_change_job - instance added");
  let capability_id = format!("{}/{}", AKRI_PREFIX, instance_name);
  let new_job = job::create_new_job_from_spec(
- &instance,
+ instance,
  OwnershipInfo::new(
  OwnershipType::Instance,
  instance_name.to_string(),
@@ -841,13 +843,15 @@ mod handle_instance_tests {
  new_pod_names: Vec<&'static str>,
  new_pod_instance_names: Vec<&'static str>,
  new_pod_namespaces: Vec<&'static str>,
+ new_pod_error: Vec<bool>,
  }
 
  fn configure_add_shared_config_a_359973(pod_name: &'static str) -> HandleAdditionWork {
  HandleAdditionWork {
  new_pod_names: vec![pod_name],
  new_pod_instance_names: vec!["config-a-359973"],
  new_pod_namespaces: vec!["config-a-namespace"],
+ new_pod_error: vec![false],
  }
  }
  fn get_config_work() -> HandleConfigWork {
@@ -857,11 +861,12 @@ mod handle_instance_tests {
  find_config_result: "../test/json/config-a.json",
  }
  }
- fn configure_add_local_config_a_b494b6() -> HandleAdditionWork {
+ fn configure_add_local_config_a_b494b6(error: bool) -> HandleAdditionWork {
  HandleAdditionWork {
  new_pod_names: vec!["config-a-b494b6-pod"],
  new_pod_instance_names: vec!["config-a-b494b6"],
  new_pod_namespaces: vec!["config-a-namespace"],
+ new_pod_error: vec![error],
  }
  }
 
@@ -873,6 +878,7 @@ mod handle_instance_tests {
  work.new_pod_namespaces[i],
  AKRI_INSTANCE_LABEL_NAME,
  work.new_pod_instance_names[i],
+ work.new_pod_error[i],
  );
  }
  }
@@ -944,7 +950,33 @@ mod handle_instance_tests {
  find_pods_delete_start_time: false,
  config_work: get_config_work(),
  deletion_work: None,
- addition_work: Some(configure_add_local_config_a_b494b6()),
+ addition_work: Some(configure_add_local_config_a_b494b6(false)),
+ },
+ );
+ run_handle_instance_change_test(
+ &mut mock,
+ "../test/json/local-instance.json",
+ &InstanceAction::Add,
+ )
+ .await;
+ }
+
+ #[tokio::test]
+ async fn test_handle_instance_change_for_add_new_local_instance_error() {
+ let _ = env_logger::builder().is_test(true).try_init();
+
+ let mut mock = MockKubeInterface::new();
+ configure_for_handle_instance_change(
+ &mut mock,
+ &HandleInstanceWork {
+ find_pods_selector: "akri.sh/instance=config-a-b494b6",
+ find_pods_result: "../test/json/empty-list.json",
+ find_pods_phase: None,
+ find_pods_start_time: None,
+ find_pods_delete_start_time: false,
+ config_work: get_config_work(),
+ deletion_work: None,
+ addition_work: Some(configure_add_local_config_a_b494b6(true)),
  },
  );
  run_handle_instance_change_test(
@@ -1139,7 +1171,7 @@ mod handle_instance_tests {
  find_pods_delete_start_time: false,
  config_work: get_config_work(),
  deletion_work: None,
- addition_work: Some(configure_add_local_config_a_b494b6()),
+ addition_work: Some(configure_add_local_config_a_b494b6(false)),
  },
  );
  run_handle_instance_change_test(

diff --git a/controller/src/util/shared_test_utils.rs b/controller/src/util/shared_test_utils.rs
@@ -253,6 +253,7 @@ pub mod config_for_tests {
  pod_namespace: &'static str,
  label_id: &'static str,
  label_value: &'static str,
+ error: bool,
  ) {
  trace!("mock.expect_create_pod pod_name:{}", pod_name);
  mock.expect_create_pod()
@@ -269,7 +270,10 @@ pub mod config_for_tests {
  == label_value
  && namespace == pod_namespace
  })
- .returning(move |_, _| Ok(()));
+ .returning(move |_, _| match error {
+ false => Ok(()),
+ true => Err(anyhow::format_err!("create pod error")),
+ });
  }
 
  pub fn configure_remove_pod(

diff --git a/deployment/helm/Chart.yaml b/deployment/helm/Chart.yaml
@@ -16,9 +16,9 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 0.11.0
+version: 0.11.1
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
-appVersion: 0.11.0
+appVersion: 0.11.1
diff --git a/discovery-handler-modules/debug-echo-discovery-handler/Cargo.toml b/discovery-handler-modules/debug-echo-discovery-handler/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "debug-echo-discovery-handler"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/discovery-handler-modules/onvif-discovery-handler/Cargo.toml b/discovery-handler-modules/onvif-discovery-handler/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "onvif-discovery-handler"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/discovery-handler-modules/opcua-discovery-handler/Cargo.toml b/discovery-handler-modules/opcua-discovery-handler/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "opcua-discovery-handler"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/discovery-handler-modules/udev-discovery-handler/Cargo.toml b/discovery-handler-modules/udev-discovery-handler/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "udev-discovery-handler"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/discovery-handlers/debug-echo/Cargo.toml b/discovery-handlers/debug-echo/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "akri-debug-echo"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/discovery-handlers/onvif/Cargo.toml b/discovery-handlers/onvif/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "akri-onvif"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/discovery-handlers/opcua/Cargo.toml b/discovery-handlers/opcua/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "akri-opcua"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/discovery-handlers/udev/Cargo.toml b/discovery-handlers/udev/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "akri-udev"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/discovery-utils/Cargo.toml b/discovery-utils/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "akri-discovery-utils"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/samples/brokers/udev-video-broker/Cargo.toml b/samples/brokers/udev-video-broker/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "udev-video-broker"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["Kate Goldenring <kate.goldenring@microsoft.com>", "<bfjelds@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/shared/Cargo.toml b/shared/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "akri-shared"
-version = "0.11.0"
+version = "0.11.1"
 authors = ["<bfjelds@microsoft.com>"]
 edition = "2018"
 rust-version = "1.68.1"

diff --git a/version.txt b/version.txt
@@ -1 +1 @@
-0.11.0
+0.11.1