Mgmt cleanup part 2

config/src/errors.rs

@@ -16,7 +16,7 @@ use net::eth::mac::Mac;
 use thiserror::Error;
 
 /// The reasons why we may reject a configuration
-#[derive(Debug, Error, PartialEq)]
+#[derive(Clone, Debug, Error, PartialEq)]
 pub enum ConfigError {
     #[error("A VPC with name '{0}' already exists")]
     DuplicateVpcName(String),

config/src/gwconfig.rs

@@ -3,7 +3,7 @@
 
 //! Top-level configuration object for the dataplane
 
-use crate::errors::ConfigResult;
+use crate::errors::{ConfigError, ConfigResult};
 use crate::external::{ExternalConfig, GenId};
 use crate::internal::InternalConfig;
 use std::time::SystemTime;
@@ -12,43 +12,47 @@ use tracing::debug;
 /// Metadata associated to a gateway configuration
 #[derive(Clone, Debug)]
 pub struct GwConfigMeta {
-    pub create_t: SystemTime,        /* time when config was built (received) */
-    pub apply_t: Option<SystemTime>, /* last time when config was applied successfully */
-    pub replace_t: Option<SystemTime>, /* time when config was un-applied */
-    pub replacement: Option<GenId>,  /* Id of config that replaced this one */
-    pub is_applied: bool,            /* True if the config is currently applied */
+    // generation Id of a config
+    pub genid: GenId,
+
+    // time when a config was learnt
+    pub create_t: SystemTime,
+
+    // time when a config was applied
+    pub apply_t: Option<SystemTime>,
+
+    // error if configuration could not be applied
+    pub error: Option<ConfigError>,
 }
 impl GwConfigMeta {
     ////////////////////////////////////////////////////////////////////////////////
-    /// Build config metadata. This is automatically built when creating a `GwConfig
+    /// Build config metadata. This is automatically built when creating a `GwConfig`
     ////////////////////////////////////////////////////////////////////////////////
     #[must_use]
-    fn new() -> Self {
+    fn new(genid: GenId) -> Self {
         Self {
+            genid,
             create_t: SystemTime::now(),
             apply_t: None,
-            replace_t: None,
-            replacement: None,
-            is_applied: false,
+            error: None,
         }
     }
     ////////////////////////////////////////////////////////////////////////////////
-    /// Set the state of this config. The management processor will always be responsible
-    /// for setting this, regardless of how it stores the configurations. The metadata
-    /// is included here in case other components needed some of its data.
+    /// Set the time when attempting to apply a configuration finished, whether it
+    /// succeeded or not.
+    ////////////////////////////////////////////////////////////////////////////////
+    pub fn apply_time(&mut self) {
+        self.apply_t = Some(SystemTime::now());
+    }
+
+    ////////////////////////////////////////////////////////////////////////////////
+    /// Set the `ConfigError` if a configuration failed to be applied.
     ////////////////////////////////////////////////////////////////////////////////
-    pub fn set_state(&mut self, genid: GenId, value: bool, replacement: Option<GenId>) {
-        if value {
-            self.apply_t = Some(SystemTime::now());
-            self.replace_t.take();
-            self.replacement.take();
-            debug!("Config {genid} has been marked as active");
-        } else {
-            self.replace_t = Some(SystemTime::now());
-            self.replacement = replacement;
-            debug!("Config {genid} has been marked as inactive");
+    pub fn error(&mut self, result: &ConfigResult) {
+        self.error.take();
+        if let Err(e) = result {
+            self.error = Some(e.clone());
         }
-        self.is_applied = value;
     }
 }
 
@@ -66,7 +70,7 @@ impl GwConfig {
     #[must_use]
     pub fn new(external: ExternalConfig) -> Self {
         Self {
-            meta: GwConfigMeta::new(),
+            meta: GwConfigMeta::new(external.genid),
             external,
             internal: None,
         }

mgmt/src/processor/display.rs

@@ -12,72 +12,55 @@ use config::{ExternalConfig, GenId, GwConfig, GwConfigMeta, InternalConfig};
 
 macro_rules! CONFIGDB_TBL_FMT {
     () => {
-        " {:>6} {:<25} {:<25} {:<25} {:>6} {:<10}"
+        " {:>6} {:<25} {:<25} {}"
     };
 }
 
 fn fmt_configdb_summary_heading(f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
     writeln!(
         f,
         "{}",
-        format_args!(
-            CONFIGDB_TBL_FMT!(),
-            "GenId", "created", "applied", "replaced", "by", "active"
-        )
+        format_args!(CONFIGDB_TBL_FMT!(), "GenId", "created", "applied", "error")
     )
 }
 
-fn fmt_gwconfig_summary(
-    meta: &GwConfigMeta,
-    genid: GenId,
-    f: &mut std::fmt::Formatter<'_>,
-) -> std::fmt::Result {
+fn fmt_gwconfig_summary(meta: &GwConfigMeta, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
     let created = DateTime::<Utc>::from(meta.create_t).format("%H:%M:%S on %Y/%m/%d");
     let apply_time = if let Some(time) = meta.apply_t {
         let time = DateTime::<Utc>::from(time).format("%H:%M:%S on %Y/%m/%d");
         format!("{time}")
     } else {
         "--".to_string()
     };
-    let replace_time = if let Some(time) = meta.replace_t {
-        let time = DateTime::<Utc>::from(time).format("%H:%M:%S on %Y/%m/%d");
-        format!("{time}")
-    } else {
-        "--".to_string()
-    };
 
-    let applied = if meta.is_applied { "yes" } else { "no" };
-    let replacement = meta
-        .replacement
-        .map(|genid| genid.to_string())
-        .unwrap_or("--".to_string());
+    let error = meta
+        .error
+        .as_ref()
+        .map(|e| e.to_string())
+        .unwrap_or("none".to_string());
+
     writeln!(
         f,
         "{}",
-        format_args!(
-            CONFIGDB_TBL_FMT!(),
-            genid, created, apply_time, replace_time, replacement, applied
-        )
+        format_args!(CONFIGDB_TBL_FMT!(), meta.genid, created, apply_time, error)
     )
 }
 
-pub struct GwConfigDatabaseSummary<'a>(pub &'a GwConfigDatabase);
+pub struct ConfigHistory<'a>(pub &'a GwConfigDatabase);
 
-impl Display for GwConfigDatabaseSummary<'_> {
+impl Display for ConfigHistory<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        Heading(format!(
-            "Configuration database summary ({} configs)",
-            self.0.len()
-        ))
-        .fmt(f)?;
+        Heading("Configuration history".to_string()).fmt(f)?;
+
         if let Some(curr) = self.0.get_current_gen() {
             writeln!(f, " current generation: {curr}")?;
         } else {
             writeln!(f, " current generation: --")?;
         }
+
         fmt_configdb_summary_heading(f)?;
-        for (genid, gwconfig) in self.0.iter() {
-            fmt_gwconfig_summary(&gwconfig.meta, *genid, f)?;
+        for meta in self.0.history() {
+            fmt_gwconfig_summary(meta, f)?;
         }
         Ok(())
     }

mgmt/src/processor/gwconfigdb.rs

@@ -1,100 +1,56 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Open Network Fabric Authors
 
-//! Configuration database: entity able to store multiple gateway configurations
+//! Configuration database
 
-use config::{ConfigError, ConfigResult, ExternalConfig, GenId, GwConfig};
-use std::collections::BTreeMap;
-use tracing::{debug, error, info};
+use config::{GenId, GwConfig, GwConfigMeta};
+use tracing::{debug, info};
 
 /// Configuration database, keeps a set of [`GwConfig`]s keyed by generation id [`GenId`]
 #[derive(Default)]
 pub(crate) struct GwConfigDatabase {
-    configs: BTreeMap<GenId, GwConfig>, /* collection of configs */
-    current: Option<GenId>,             /* [`GenId`] of currently applied config */
+    applied: Option<GwConfig>,  /* Currently applied config */
+    history: Vec<GwConfigMeta>, /* event history */
 }
 
 impl GwConfigDatabase {
     #[must_use]
     pub fn new() -> Self {
         debug!("Building config database...");
         let mut configdb = Self::default();
-        configdb.add(GwConfig::blank());
+        configdb.store(GwConfig::blank());
         configdb
     }
-
-    pub fn add(&mut self, config: GwConfig) {
-        debug!("Storing config '{}' in config db...", config.genid());
-        self.configs.insert(config.external.genid, config);
-    }
-
-    #[allow(clippy::len_without_is_empty)]
     #[must_use]
-    pub fn len(&self) -> usize {
-        self.configs.len()
+    pub fn history(&self) -> &Vec<GwConfigMeta> {
+        &self.history
     }
-
-    pub fn iter(&self) -> impl Iterator<Item = (&GenId, &GwConfig)> {
-        self.configs.iter()
-    }
-
     #[must_use]
-    pub fn get(&self, genid: GenId) -> Option<&GwConfig> {
-        self.configs.get(&genid)
-    }
-
-    #[must_use]
-    pub fn contains(&self, genid: GenId) -> bool {
-        self.configs.contains_key(&genid)
-    }
-
-    #[must_use]
-    pub fn get_mut(&mut self, generation: GenId) -> Option<&mut GwConfig> {
-        self.configs.get_mut(&generation)
-    }
-    #[allow(unused)]
-    pub fn remove(&mut self, genid: GenId) -> ConfigResult {
-        if genid == ExternalConfig::BLANK_GENID {
-            debug!("Will not remove config {genid} as it is protected");
-            return Ok(());
-        }
-        debug!("Removing config '{genid}' from config db...");
-        if let Some(config) = &self.configs.get(&genid) {
-            if config.meta.is_applied {
-                error!("Can't remove config {genid}: in use");
-                Err(ConfigError::Forbidden("In use"))
-            } else {
-                debug!("Successfully removed config '{genid}'");
-                self.configs.remove(&genid);
-                Ok(())
-            }
-        } else {
-            error!("Can't remove config {genid}: not found");
-            Err(ConfigError::NoSuchConfig(genid))
-        }
+    pub fn history_mut(&mut self) -> &mut Vec<GwConfigMeta> {
+        &mut self.history
     }
 
-    /// Set the current generation id
-    pub fn set_current_gen(&mut self, genid: GenId) {
-        info!("Config with genid '{genid}' is now the current");
-        self.current = Some(genid);
+    /// Store the given config
+    pub fn store(&mut self, config: GwConfig) {
+        info!("Storing config for generation '{}' in db", config.genid());
+        self.applied = Some(config);
     }
 
     /// Get the generation Id of the currently applied config, if any.
     #[must_use]
     pub fn get_current_gen(&self) -> Option<GenId> {
-        self.current
+        self.applied.as_ref().map(|c| c.genid())
     }
 
     /// Get a reference to the config currently applied, if any.
     #[must_use]
     pub fn get_current_config(&self) -> Option<&GwConfig> {
-        self.current.and_then(|genid| self.get(genid))
+        self.applied.as_ref()
     }
 
     /// Get a mutable reference to the config currently applied, if any.
     #[must_use]
     pub fn get_current_config_mut(&mut self) -> Option<&mut GwConfig> {
-        self.current.and_then(|genid| self.get_mut(genid))
+        self.applied.as_mut()
     }
 }

mgmt/src/processor/k8s_client.rs

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Open Network Fabric Authors
 
+//! K8s client that glues the gateway to k8s
+
 use std::sync::Arc;
 use std::time::SystemTime;
 

mgmt/src/processor/k8s_less_client.rs

@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Open Network Fabric Authors
 
+//! "Kubeless" client that learns configs from a directory and requests
+//! the configuration processor to apply them.
+
 use config::{ExternalConfig, GwConfig};
 use futures::TryFutureExt;
 use k8s_less::kubeless_watch_gateway_agent_crd;

mgmt/src/processor/launch.rs

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Open Network Fabric Authors
 
+//! The configuration processor
+
 use crate::processor::k8s_client::{K8sClient, K8sClientError};
 use crate::processor::k8s_less_client::{K8sLess, K8sLessError};
 use crate::processor::proc::ConfigProcessor;