Skip to content

Commit

Permalink
fix: prevent re-creation instances when only Configuration metadata o…
Browse files Browse the repository at this point in the history
…r status changes
  • Loading branch information
ammmze committed Sep 4, 2021
1 parent 56ef83e commit 791a33a
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
**/obj
**/bin
**/cobertura.xml
.idea
36 changes: 35 additions & 1 deletion agent/src/util/config_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use akri_shared::{
use futures::{StreamExt, TryStreamExt};
use kube::api::{Api, ListParams, WatchEvent};
use log::{info, trace};
use std::{collections::HashMap, sync::Arc};
use std::{collections::HashMap, option::Option, sync::Arc};
use tokio::sync::{broadcast, mpsc, Mutex};

type ConfigMap = Arc<Mutex<HashMap<String, ConfigInfo>>>;
Expand All @@ -34,6 +34,10 @@ pub struct ConfigInfo {
/// Receives notification that all `DiscoveryOperators` threads have completed and a Configuration's Instances
/// can be safely deleted and the associated `DevicePluginServices` terminated.
finished_discovery_receiver: mpsc::Receiver<()>,
/// Tracks the last generation of the `Configuration` resource (i.e. `.metadata.generation`).
/// This is used to determine if the `Configuration` actually changed, or if only the metadata changed.
/// The `.metadata.generation` value is incremented for all changes, except for changes to `.metadata` or `.status`.
last_generation: Option<i64>,
}

/// This handles pre-existing Configurations and invokes an internal method that watches for Configuration events.
Expand Down Expand Up @@ -150,6 +154,16 @@ async fn handle_config(
}
// If a config is updated, delete all associated instances and device plugins and then recreate them to reflect updated config
WatchEvent::Modified(config) => {
let do_recreate = should_recreate_config(&config, config_map.clone())
.await
.unwrap();
if !do_recreate {
trace!(
"handle_config - config {:?} has not changed. ignoring config modified event.",
config.metadata.name,
);
return Ok(());
}
info!(
"handle_config - modified Configuration {:?}",
config.metadata.name,
Expand Down Expand Up @@ -196,6 +210,7 @@ async fn handle_config_add(
instance_map: instance_map.clone(),
stop_discovery_sender: stop_discovery_sender.clone(),
finished_discovery_receiver,
last_generation: config.metadata.generation,
};
config_map
.lock()
Expand Down Expand Up @@ -276,6 +291,24 @@ async fn handle_config_delete(
Ok(())
}

/// Checks to see if the configuration needs to be recreated.
/// At present, this just checks to see if the `.metadata.generation` has changed.
/// The `.metadata.generation` value is incremented for all changes, except for changes to `.metadata` or `.status`.
async fn should_recreate_config(config: &Configuration, config_map: ConfigMap) -> Result<bool, ()> {
let name = config.metadata.name.clone().unwrap();
let last_generation = config_map.lock().await.get(&name).unwrap().last_generation;
trace!(
"should_recreate_config - checking if config {} needs to be recreated",
name,
);

if config.metadata.generation == last_generation {
return Ok(false);
}

Ok(true)
}

/// This shuts down all a Configuration's Instances and terminates the associated Device Plugins
pub async fn delete_all_instances_in_map(
kube_interface: &impl k8s::KubeInterface,
Expand Down Expand Up @@ -341,6 +374,7 @@ mod config_action_tests {
stop_discovery_sender,
instance_map: instance_map.clone(),
finished_discovery_receiver,
last_generation: config.metadata.generation,
},
);
let config_map: ConfigMap = Arc::new(Mutex::new(map));
Expand Down

0 comments on commit 791a33a

Please sign in to comment.