From fa7f328669f7789066bedd1c05693c75195df71c Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Mon, 3 Feb 2020 16:14:54 -0800 Subject: [PATCH 1/6] apiserver: add transaction support --- .../api/apiserver/src/datastore/filesystem.rs | 137 ++++++++++++----- .../api/apiserver/src/datastore/memory.rs | 145 +++++++++++------- workspaces/api/apiserver/src/datastore/mod.rs | 64 +++++--- .../api/apiserver/src/server/controller.rs | 121 ++++++++++----- workspaces/api/apiserver/src/server/mod.rs | 101 ++++++++---- 5 files changed, 375 insertions(+), 193 deletions(-) diff --git a/workspaces/api/apiserver/src/datastore/filesystem.rs b/workspaces/api/apiserver/src/datastore/filesystem.rs index 5d886ed0a5d..d07273f016e 100644 --- a/workspaces/api/apiserver/src/datastore/filesystem.rs +++ b/workspaces/api/apiserver/src/datastore/filesystem.rs @@ -27,27 +27,30 @@ const ENCODE_CHARACTERS: &AsciiSet = &NON_ALPHANUMERIC.remove(b'_').remove(b'-') #[derive(Debug)] pub struct FilesystemDataStore { live_path: PathBuf, - pending_path: PathBuf, + pending_base_path: PathBuf, } impl FilesystemDataStore { pub fn new>(base_path: P) -> FilesystemDataStore { FilesystemDataStore { live_path: base_path.as_ref().join("live"), - pending_path: base_path.as_ref().join("pending"), + pending_base_path: base_path.as_ref().join("pending"), } } /// Returns the appropriate filesystem path for pending or live data. - fn base_path(&self, committed: Committed) -> &PathBuf { + fn base_path(&self, committed: &Committed) -> PathBuf { match committed { - Committed::Pending => &self.pending_path, - Committed::Live => &self.live_path, + Committed::Pending { tx } => { + let encoded = encode_path_component(tx); + self.pending_base_path.join(encoded) + } + Committed::Live => self.live_path.clone(), } } /// Returns the appropriate path on the filesystem for the given data key. - fn data_path(&self, key: &Key, committed: Committed) -> Result { + fn data_path(&self, key: &Key, committed: &Committed) -> Result { let base_path = self.base_path(committed); // Encode key segments so they're filesystem-safe @@ -75,7 +78,7 @@ impl FilesystemDataStore { &self, metadata_key: &Key, data_key: &Key, - committed: Committed, + committed: &Committed, ) -> Result { let path = self.data_path(data_key, committed)?; @@ -109,7 +112,7 @@ impl FilesystemDataStore { /// error for trying to remove an empty directory is not specific, and we don't want to rely /// on platform-specific error codes or the error description. We could check the directory /// contents ourself, but it would be more complex and subject to timing issues.) - fn delete_key_path

(&mut self, path: P, committed: Committed) -> Result<()> + fn delete_key_path

(&mut self, path: P, committed: &Committed) -> Result<()> where P: AsRef, { @@ -314,7 +317,7 @@ fn find_populated_key_paths>( datastore: &FilesystemDataStore, key_type: KeyType, prefix: S, - committed: Committed, + committed: &Committed, ) -> Result> { // Find the base path for our search, and confirm it exists. let base = datastore.base_path(committed); @@ -329,7 +332,7 @@ fn find_populated_key_paths>( .fail() } // No pending keys, OK, return empty set. - Committed::Pending => { + Committed::Pending { .. } => { trace!( "Returning empty list because pending path doesn't exist: {}", base.display() @@ -340,7 +343,7 @@ fn find_populated_key_paths>( } // Walk through the filesystem. - let walker = WalkDir::new(base) + let walker = WalkDir::new(&base) .follow_links(false) // shouldn't be links... .same_file_system(true); // shouldn't be filesystems to cross... @@ -377,7 +380,7 @@ fn find_populated_key_paths>( // TODO: maybe add/strip single newline at end, so file is easier to read impl DataStore for FilesystemDataStore { - fn key_populated(&self, key: &Key, committed: Committed) -> Result { + fn key_populated(&self, key: &Key, committed: &Committed) -> Result { let path = self.data_path(key, committed)?; Ok(path.exists()) @@ -388,7 +391,7 @@ impl DataStore for FilesystemDataStore { fn list_populated_keys>( &self, prefix: S, - committed: Committed, + committed: &Committed, ) -> Result> { let key_paths = find_populated_key_paths(self, KeyType::Data, prefix, committed)?; let keys = key_paths.into_iter().map(|kp| kp.data_key).collect(); @@ -413,7 +416,7 @@ impl DataStore for FilesystemDataStore { S2: AsRef, { // Find metadata key paths on disk - let key_paths = find_populated_key_paths(self, KeyType::Meta, prefix, Committed::Live)?; + let key_paths = find_populated_key_paths(self, KeyType::Meta, prefix, &Committed::Live)?; // For each file on disk, check the user's conditions, and add it to our output let mut result = HashMap::new(); @@ -438,23 +441,23 @@ impl DataStore for FilesystemDataStore { Ok(result) } - fn get_key(&self, key: &Key, committed: Committed) -> Result> { + fn get_key(&self, key: &Key, committed: &Committed) -> Result> { let path = self.data_path(key, committed)?; read_file_for_key(&key, &path) } - fn set_key>(&mut self, key: &Key, value: S, committed: Committed) -> Result<()> { + fn set_key>(&mut self, key: &Key, value: S, committed: &Committed) -> Result<()> { let path = self.data_path(key, committed)?; write_file_mkdir(path, value) } - fn unset_key(&mut self, key: &Key, committed: Committed) -> Result<()> { + fn unset_key(&mut self, key: &Key, committed: &Committed) -> Result<()> { let path = self.data_path(key, committed)?; self.delete_key_path(path, committed) } fn get_metadata_raw(&self, metadata_key: &Key, data_key: &Key) -> Result> { - let path = self.metadata_path(metadata_key, data_key, Committed::Live)?; + let path = self.metadata_path(metadata_key, data_key, &Committed::Live)?; read_file_for_key(&metadata_key, &path) } @@ -464,20 +467,26 @@ impl DataStore for FilesystemDataStore { data_key: &Key, value: S, ) -> Result<()> { - let path = self.metadata_path(metadata_key, data_key, Committed::Live)?; + let path = self.metadata_path(metadata_key, data_key, &Committed::Live)?; write_file_mkdir(path, value) } fn unset_metadata(&mut self, metadata_key: &Key, data_key: &Key) -> Result<()> { - let path = self.metadata_path(metadata_key, data_key, Committed::Live)?; - self.delete_key_path(path, Committed::Live) + let path = self.metadata_path(metadata_key, data_key, &Committed::Live)?; + self.delete_key_path(path, &Committed::Live) } /// We commit by copying pending keys to live, then removing pending. Something smarter (lock, /// atomic flip, etc.) will be required to make the server concurrent. - fn commit(&mut self) -> Result> { + fn commit_transaction(&mut self, transaction: S) -> Result> + where + S: Into + AsRef, + { + let pending = Committed::Pending { + tx: transaction.into(), + }; // Get data for changed keys - let pending_data = self.get_prefix("settings.", Committed::Pending)?; + let pending_data = self.get_prefix("settings.", &pending)?; // Nothing to do if no keys are present in pending if pending_data.is_empty() { @@ -489,33 +498,73 @@ impl DataStore for FilesystemDataStore { // Apply changes to live debug!("Writing pending keys to live"); - self.set_keys(&pending_data, Committed::Live)?; + self.set_keys(&pending_data, &Committed::Live)?; // Remove pending debug!("Removing old pending keys"); - fs::remove_dir_all(&self.pending_path).context(error::Io { - path: &self.pending_path, - })?; + let path = self.base_path(&pending); + fs::remove_dir_all(&path).context(error::Io { path })?; Ok(pending_keys) } - fn delete_pending(&mut self) -> Result> { + fn delete_transaction(&mut self, transaction: S) -> Result> + where + S: Into + AsRef, + { + let pending = Committed::Pending { + tx: transaction.into(), + }; // Get changed keys so we can return the list - let pending_data = self.get_prefix("settings.", Committed::Pending)?; + let pending_data = self.get_prefix("settings.", &pending)?; // Pull out just the keys so we can log them and return them let pending_keys = pending_data.into_iter().map(|(key, _val)| key).collect(); debug!("Found pending keys: {:?}", &pending_keys); // Delete pending from the filesystem, same as a commit - debug!("Removing all pending keys"); - fs::remove_dir_all(&self.pending_path).context(error::Io { - path: &self.pending_path, - })?; + let path = self.base_path(&pending); + debug!("Removing transaction directory {}", path.display()); + if let Err(e) = fs::remove_dir_all(&path) { + // If path doesn't exist, it's fine, we'll just return an empty list. + if e.kind() != io::ErrorKind::NotFound { + return Err(e).context(error::Io { path }); + } + } Ok(pending_keys) } + + /// We store transactions as subdirectories of the pending data store, so to list them we list + /// the names of the subdirectories. + fn list_transactions(&self) -> Result> { + // Any directory under pending should be a transaction name. + let walker = WalkDir::new(&self.pending_base_path) + .min_depth(1) + .max_depth(1); + + let mut transactions = HashSet::new(); + trace!( + "Starting walk of filesystem to list transactions under {}", + self.pending_base_path.display(), + ); + + for entry in walker { + let entry = entry.context(error::ListKeys)?; + if entry.file_type().is_dir() { + // The directory name should be valid UTF-8, encoded by encode_path_component, + // or the data store has been corrupted. + let file_name = entry.file_name().to_str().context(error::Corruption { + msg: "Non-UTF8 path", + path: entry.path(), + })?; + let transaction = decode_path_component(file_name, entry.path())?; + transactions.insert(transaction); + } + } + + Ok(transactions) + } } #[cfg(test)] @@ -527,10 +576,16 @@ mod test { let f = FilesystemDataStore::new("/base"); let key = Key::new(KeyType::Data, "a.b.c").unwrap(); - let pending = f.data_path(&key, Committed::Pending).unwrap(); - assert_eq!(pending.into_os_string(), "/base/pending/a/b/c"); + let tx = "test transaction"; + let pending = f + .data_path(&key, &Committed::Pending { tx: tx.into() }) + .unwrap(); + assert_eq!( + pending.into_os_string(), + "/base/pending/test%20transaction/a/b/c" + ); - let live = f.data_path(&key, Committed::Live).unwrap(); + let live = f.data_path(&key, &Committed::Live).unwrap(); assert_eq!(live.into_os_string(), "/base/live/a/b/c"); } @@ -540,13 +595,17 @@ mod test { let data_key = Key::new(KeyType::Data, "a.b.c").unwrap(); let md_key = Key::new(KeyType::Meta, "my-metadata").unwrap(); + let tx = "test transaction"; let pending = f - .metadata_path(&md_key, &data_key, Committed::Pending) + .metadata_path(&md_key, &data_key, &Committed::Pending { tx: tx.into() }) .unwrap(); - assert_eq!(pending.into_os_string(), "/base/pending/a/b/c.my-metadata"); + assert_eq!( + pending.into_os_string(), + "/base/pending/test%20transaction/a/b/c.my-metadata" + ); let live = f - .metadata_path(&md_key, &data_key, Committed::Live) + .metadata_path(&md_key, &data_key, &Committed::Live) .unwrap(); assert_eq!(live.into_os_string(), "/base/live/a/b/c.my-metadata"); } diff --git a/workspaces/api/apiserver/src/datastore/memory.rs b/workspaces/api/apiserver/src/datastore/memory.rs index 421f156366a..1334ecebdd2 100644 --- a/workspaces/api/apiserver/src/datastore/memory.rs +++ b/workspaces/api/apiserver/src/datastore/memory.rs @@ -4,14 +4,13 @@ //! immediately. use std::collections::{HashMap, HashSet}; -use std::mem; use super::{Committed, DataStore, Key, Result}; #[derive(Debug)] pub struct MemoryDataStore { - // Uncommitted (pending) data. - pending: HashMap, + // Transaction name -> (key -> data) + pending: HashMap>, // Committed (live) data. live: HashMap, // Map of data keys to their metadata, which in turn is a mapping of metadata keys to @@ -28,17 +27,17 @@ impl MemoryDataStore { } } - fn dataset(&self, committed: Committed) -> &HashMap { + fn dataset(&self, committed: &Committed) -> Option<&HashMap> { match committed { - Committed::Live => &self.live, - Committed::Pending => &self.pending, + Committed::Live => Some(&self.live), + Committed::Pending { tx } => self.pending.get(tx) } } - fn dataset_mut(&mut self, committed: Committed) -> &mut HashMap { + fn dataset_mut(&mut self, committed: &Committed) -> &mut HashMap { match committed { Committed::Live => &mut self.live, - Committed::Pending => &mut self.pending, + Committed::Pending { tx } => self.pending.entry(tx.clone()).or_default(), } } } @@ -47,9 +46,10 @@ impl DataStore for MemoryDataStore { fn list_populated_keys>( &self, prefix: S, - committed: Committed, + committed: &Committed, ) -> Result> { - let dataset = self.dataset(committed); + let empty = HashMap::new(); + let dataset = self.dataset(committed).unwrap_or(&empty); Ok(dataset .keys() // Make sure the data keys start with the given prefix. @@ -93,23 +93,27 @@ impl DataStore for MemoryDataStore { Ok(result) } - fn get_key(&self, key: &Key, committed: Committed) -> Result> { - Ok(self.dataset(committed).get(key).cloned()) + fn get_key(&self, key: &Key, committed: &Committed) -> Result> { + let empty = HashMap::new(); + let dataset = self.dataset(committed).unwrap_or(&empty); + Ok(dataset.get(key).cloned()) } - fn set_key>(&mut self, key: &Key, value: S, committed: Committed) -> Result<()> { + fn set_key>(&mut self, key: &Key, value: S, committed: &Committed) -> Result<()> { self.dataset_mut(committed) .insert(key.clone(), value.as_ref().to_owned()); Ok(()) } - fn unset_key(&mut self, key: &Key, committed: Committed) -> Result<()> { + fn unset_key(&mut self, key: &Key, committed: &Committed) -> Result<()> { self.dataset_mut(committed).remove(key); Ok(()) } - fn key_populated(&self, key: &Key, committed: Committed) -> Result { - Ok(self.dataset(committed).contains_key(key)) + fn key_populated(&self, key: &Key, committed: &Committed) -> Result { + let empty = HashMap::new(); + let dataset = self.dataset(committed).unwrap_or(&empty); + Ok(dataset.contains_key(key)) } fn get_metadata_raw(&self, metadata_key: &Key, data_key: &Key) -> Result> { @@ -146,27 +150,36 @@ impl DataStore for MemoryDataStore { Ok(()) } - fn commit(&mut self) -> Result> { - // We need a clone of the pending keys so we can set_keys (which holds &mut self) and we - // have to clone the keys anyway for the return value. - let pending = self.pending.clone(); - - // Apply pending changes to live - self.set_keys(&pending, Committed::Live)?; - - // Remove pending - self.pending = HashMap::new(); - - // Return keys (using into_iter to avoid further clone) - Ok(pending.into_iter().map(|(k, _v)| k).collect()) + fn commit_transaction(&mut self, transaction: S) -> Result> + where + S: Into + AsRef, + { + // Remove anything pending for this transaction + if let Some(pending) = self.pending.remove(transaction.as_ref()) { + // Apply pending changes to live + self.set_keys(&pending, &Committed::Live)?; + // Return keys that were committed + Ok(pending.keys().cloned().collect()) + } else { + Ok(HashSet::new()) + } } - fn delete_pending(&mut self) -> Result> { - // Replace pending with an empty map - let old_pending = mem::replace(&mut self.pending, HashMap::new()); + fn delete_transaction(&mut self, transaction: S) -> Result> + where + S: Into + AsRef, + { + // Remove anything pending for this transaction + if let Some(pending) = self.pending.remove(transaction.as_ref()) { + // Return the old pending keys + Ok(pending.keys().cloned().collect()) + } else { + Ok(HashSet::new()) + } + } - // Return the old pending keys - Ok(old_pending.into_iter().map(|(key, _val)| key).collect()) + fn list_transactions(&self) -> Result> { + Ok(self.pending.keys().cloned().collect()) } } @@ -181,8 +194,8 @@ mod test { let mut m = MemoryDataStore::new(); let k = Key::new(KeyType::Data, "memtest").unwrap(); let v = "memvalue"; - m.set_key(&k, v, Committed::Live).unwrap(); - assert_eq!(m.get_key(&k, Committed::Live).unwrap(), Some(v.to_string())); + m.set_key(&k, v, &Committed::Live).unwrap(); + assert_eq!(m.get_key(&k, &Committed::Live).unwrap(), Some(v.to_string())); let mdkey = Key::new(KeyType::Meta, "testmd").unwrap(); let md = "mdval"; @@ -195,8 +208,8 @@ mod test { m.unset_metadata(&mdkey, &k).unwrap(); assert_eq!(m.get_metadata_raw(&mdkey, &k).unwrap(), None); - m.unset_key(&k, Committed::Live).unwrap(); - assert_eq!(m.get_key(&k, Committed::Live).unwrap(), None); + m.unset_key(&k, &Committed::Live).unwrap(); + assert_eq!(m.get_key(&k, &Committed::Live).unwrap(), None); } #[test] @@ -205,18 +218,18 @@ mod test { let k1 = Key::new(KeyType::Data, "memtest1").unwrap(); let k2 = Key::new(KeyType::Data, "memtest2").unwrap(); let v = "memvalue"; - m.set_key(&k1, v, Committed::Live).unwrap(); - m.set_key(&k2, v, Committed::Live).unwrap(); + m.set_key(&k1, v, &Committed::Live).unwrap(); + m.set_key(&k2, v, &Committed::Live).unwrap(); - assert!(m.key_populated(&k1, Committed::Live).unwrap()); - assert!(m.key_populated(&k2, Committed::Live).unwrap()); + assert!(m.key_populated(&k1, &Committed::Live).unwrap()); + assert!(m.key_populated(&k2, &Committed::Live).unwrap()); assert_eq!( - m.list_populated_keys("", Committed::Live).unwrap(), + m.list_populated_keys("", &Committed::Live).unwrap(), hashset!(k1, k2), ); let bad_key = Key::new(KeyType::Data, "memtest3").unwrap(); - assert!(!m.key_populated(&bad_key, Committed::Live).unwrap()); + assert!(!m.key_populated(&bad_key, &Committed::Live).unwrap()); } #[test] @@ -224,26 +237,40 @@ mod test { let mut m = MemoryDataStore::new(); let k = Key::new(KeyType::Data, "settings.a.b.c").unwrap(); let v = "memvalue"; - m.set_key(&k, v, Committed::Pending).unwrap(); - - assert!(m.key_populated(&k, Committed::Pending).unwrap()); - assert!(!m.key_populated(&k, Committed::Live).unwrap()); - m.commit().unwrap(); - assert!(!m.key_populated(&k, Committed::Pending).unwrap()); - assert!(m.key_populated(&k, Committed::Live).unwrap()); + let tx = "test transaction"; + let pending = Committed::Pending { tx: tx.into() }; + m.set_key(&k, v, &pending).unwrap(); + + assert!(m.key_populated(&k, &pending).unwrap()); + assert!(!m.key_populated(&k, &Committed::Live).unwrap()); + m.commit_transaction(tx).unwrap(); + assert!(!m.key_populated(&k, &pending).unwrap()); + assert!(m.key_populated(&k, &Committed::Live).unwrap()); } #[test] - fn delete_pending() { + fn delete_transaction() { let mut m = MemoryDataStore::new(); let k = Key::new(KeyType::Data, "settings.a.b.c").unwrap(); let v = "memvalue"; - m.set_key(&k, v, Committed::Pending).unwrap(); - - assert!(m.key_populated(&k, Committed::Pending).unwrap()); - assert!(!m.key_populated(&k, Committed::Live).unwrap()); - m.delete_pending().unwrap(); - assert!(!m.key_populated(&k, Committed::Pending).unwrap()); - assert!(!m.key_populated(&k, Committed::Live).unwrap()); + let tx = "test transaction"; + let pending = Committed::Pending { tx: tx.into() }; + m.set_key(&k, v, &pending).unwrap(); + + // Set something in a different transaction to ensure it doesn't get deleted + let k2 = Key::new(KeyType::Data, "settings.x.y.z").unwrap(); + let v2 = "memvalue 2"; + let tx2 = "test transaction 2"; + let pending2 = Committed::Pending { tx: tx2.into() }; + m.set_key(&k2, v2, &pending2).unwrap(); + + assert!(m.key_populated(&k, &pending).unwrap()); + assert!(!m.key_populated(&k, &Committed::Live).unwrap()); + m.delete_transaction(tx).unwrap(); + assert!(!m.key_populated(&k, &pending).unwrap()); + assert!(!m.key_populated(&k, &Committed::Live).unwrap()); + + // Assure other transactions were not deleted + assert!(m.key_populated(&k2, &pending2).unwrap()); } } diff --git a/workspaces/api/apiserver/src/datastore/mod.rs b/workspaces/api/apiserver/src/datastore/mod.rs index a5bac35bfbc..4e5189e9a9e 100644 --- a/workspaces/api/apiserver/src/datastore/mod.rs +++ b/workspaces/api/apiserver/src/datastore/mod.rs @@ -26,21 +26,24 @@ use std::collections::{HashMap, HashSet}; /// Committed represents whether we want to look at pending (uncommitted) or live (committed) data /// in the datastore. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Clone)] pub enum Committed { - Pending, Live, + Pending { + // If the change is pending, we need to know the transaction name. + tx: String, + }, } pub trait DataStore { /// Returns whether a key is present (has a value) in the datastore. - fn key_populated(&self, key: &Key, committed: Committed) -> Result; + fn key_populated(&self, key: &Key, committed: &Committed) -> Result; /// Returns a list of the populated data keys in the datastore whose names start with the given /// prefix. fn list_populated_keys>( &self, prefix: S, - committed: Committed, + committed: &Committed, ) -> Result>; /// Finds all metadata keys that are currently populated in the datastore whose data keys /// start with the given prefix. If you specify metadata_key_name, only metadata keys with @@ -57,13 +60,13 @@ pub trait DataStore { S2: AsRef; /// Retrieve the value for a single data key from the datastore. - fn get_key(&self, key: &Key, committed: Committed) -> Result>; + fn get_key(&self, key: &Key, committed: &Committed) -> Result>; /// Set the value of a single data key in the datastore. - fn set_key>(&mut self, key: &Key, value: S, committed: Committed) -> Result<()>; + fn set_key>(&mut self, key: &Key, value: S, committed: &Committed) -> Result<()>; /// Removes the given data key from the datastore. If we succeeded, we return Ok(()); if /// the key didn't exist, we also return Ok(()); we return Err only if we failed to check /// or remove the key. - fn unset_key(&mut self, key: &Key, committed: Committed) -> Result<()>; + fn unset_key(&mut self, key: &Key, committed: &Committed) -> Result<()>; /// Retrieve the value for a single metadata key from the datastore. Values will inherit from /// earlier in the tree, if more specific values are not found later. @@ -101,17 +104,26 @@ pub trait DataStore { /// Ok(()); we return Err only if we failed to check or remove the key. fn unset_metadata(&mut self, metadata_key: &Key, data_key: &Key) -> Result<()>; - /// Applies pending changes to the live datastore. Returns the list of changed keys. - fn commit(&mut self) -> Result>; + /// Applies pending changes from the given transaction to the live datastore. Returns the + /// list of changed keys. + fn commit_transaction(&mut self, transaction: S) -> Result> + where + S: Into + AsRef; + + /// Remove the given pending transaction from the datastore. Returns the list of removed + /// keys. If the transaction doesn't exist, will return Ok with an empty list. + fn delete_transaction(&mut self, transaction: S) -> Result> + where + S: Into + AsRef; - /// Remove pending changes from the datastore. Returns the list of removed keys. - fn delete_pending(&mut self) -> Result>; + /// Returns a list of the names of any pending transactions in the data store. + fn list_transactions(&self) -> Result>; /// Set multiple data keys at once in the data store. /// /// Implementers can replace the default implementation if there's a faster way than setting /// each key individually. - fn set_keys(&mut self, pairs: &HashMap, committed: Committed) -> Result<()> + fn set_keys(&mut self, pairs: &HashMap, committed: &Committed) -> Result<()> where S: AsRef, { @@ -125,7 +137,7 @@ pub trait DataStore { /// /// Implementers can replace the default implementation if there's a faster way than /// unsetting each key individually. - fn unset_keys(&mut self, keys: &HashSet, committed: Committed) -> Result<()> { + fn unset_keys(&mut self, keys: &HashSet, committed: &Committed) -> Result<()> { for key in keys { trace!("Unsetting data key {}", key.name()); self.unset_key(key, committed)?; @@ -139,7 +151,7 @@ pub trait DataStore { fn get_prefix>( &self, find_prefix: S, - committed: Committed, + committed: &Committed, ) -> Result> { let keys = self.list_populated_keys(&find_prefix, committed)?; trace!("Found populated keys: {:?}", keys); @@ -273,18 +285,20 @@ mod test { k3.clone() => &v3, ); - m.set_keys(&data, Committed::Pending).unwrap(); + let tx = "test transaction"; + let pending = Committed::Pending { tx: tx.into() }; + m.set_keys(&data, &pending).unwrap(); - assert_eq!(m.get_key(&k1, Committed::Pending).unwrap(), Some(v1)); - assert_eq!(m.get_key(&k2, Committed::Pending).unwrap(), Some(v2)); - assert_eq!(m.get_key(&k3, Committed::Pending).unwrap(), Some(v3.clone())); + assert_eq!(m.get_key(&k1, &pending).unwrap(), Some(v1)); + assert_eq!(m.get_key(&k2, &pending).unwrap(), Some(v2)); + assert_eq!(m.get_key(&k3, &pending).unwrap(), Some(v3.clone())); let unset = hashset!(k1.clone(), k2.clone()); - m.unset_keys(&unset, Committed::Pending).unwrap(); + m.unset_keys(&unset, &pending).unwrap(); - assert_eq!(m.get_key(&k1, Committed::Pending).unwrap(), None); - assert_eq!(m.get_key(&k2, Committed::Pending).unwrap(), None); - assert_eq!(m.get_key(&k3, Committed::Pending).unwrap(), Some(v3)); + assert_eq!(m.get_key(&k1, &pending).unwrap(), None); + assert_eq!(m.get_key(&k2, &pending).unwrap(), None); + assert_eq!(m.get_key(&k3, &pending).unwrap(), Some(v3)); } #[test] @@ -314,10 +328,12 @@ mod test { Key::new(KeyType::Data, "x.2").unwrap() => "x2".to_string(), Key::new(KeyType::Data, "y.3").unwrap() => "y3".to_string(), ); - m.set_keys(&data, Committed::Pending).unwrap(); + let tx = "test transaction"; + let pending = Committed::Pending { tx: tx.into() }; + m.set_keys(&data, &pending).unwrap(); assert_eq!( - m.get_prefix("x.", Committed::Pending).unwrap(), + m.get_prefix("x.", &pending).unwrap(), hashmap!(Key::new(KeyType::Data, "x.1").unwrap() => "x1".to_string(), Key::new(KeyType::Data, "x.2").unwrap() => "x2".to_string()) ); diff --git a/workspaces/api/apiserver/src/server/controller.rs b/workspaces/api/apiserver/src/server/controller.rs index 960d5d0c186..dd4ac68baeb 100644 --- a/workspaces/api/apiserver/src/server/controller.rs +++ b/workspaces/api/apiserver/src/server/controller.rs @@ -15,22 +15,45 @@ use crate::datastore::{ use crate::server::error::{self, Result}; use model::{ConfigurationFiles, Services, Settings}; +/// List the open transactions from the data store. +pub(crate) fn list_transactions(datastore: &D) -> Result> +where + D: DataStore, +{ + datastore.list_transactions().context(error::DataStore { + op: "list_transactions", + }) +} + /// Build a Settings based on pending data in the datastore; the Settings will be empty if there /// are no pending settings. -pub(crate) fn get_pending_settings(datastore: &D) -> Result { - get_prefix(datastore, Committed::Pending, "settings.", None) +pub(crate) fn get_transaction(datastore: &D, transaction: S) -> Result +where + D: DataStore, + S: Into, +{ + let pending = Committed::Pending { + tx: transaction.into(), + }; + get_prefix(datastore, &pending, "settings.", None) .map(|maybe_settings| maybe_settings.unwrap_or_else(Settings::default)) } -/// Delete any settings that have been received but not committed -pub(crate) fn delete_pending_settings(datastore: &mut D) -> Result> { - datastore.delete_pending().context(error::DataStore { - op: "delete_pending", - }) +/// Deletes the transaction from the data store, removing any uncommitted settings under that +/// transaction name. +pub(crate) fn delete_transaction( + datastore: &mut D, + transaction: &str, +) -> Result> { + datastore + .delete_transaction(transaction) + .context(error::DataStore { + op: "delete_pending", + }) } /// Build a Settings based on the data in the datastore. Errors if no settings are found. -pub(crate) fn get_settings(datastore: &D, committed: Committed) -> Result { +pub(crate) fn get_settings(datastore: &D, committed: &Committed) -> Result { get_prefix(datastore, committed, "settings.", None) .transpose() // None is not OK here - we always have *some* settings @@ -41,7 +64,7 @@ pub(crate) fn get_settings(datastore: &D, committed: Committed) -> pub(crate) fn get_settings_prefix>( datastore: &D, prefix: S, - committed: Committed, + committed: &Committed, ) -> Result { let prefix = "settings.".to_string() + prefix.as_ref(); get_prefix(datastore, committed, &prefix, None) @@ -54,7 +77,7 @@ pub(crate) fn get_settings_prefix>( pub(crate) fn get_services(datastore: &D) -> Result { get_prefix( datastore, - Committed::Live, + &Committed::Live, "services.", Some("services".to_string()), ) @@ -67,7 +90,7 @@ pub(crate) fn get_services(datastore: &D) -> Result { pub(crate) fn get_configuration_files(datastore: &D) -> Result { get_prefix( datastore, - Committed::Live, + &Committed::Live, "configuration-files", Some("configuration-files".to_string()), ) @@ -84,7 +107,7 @@ pub(crate) fn get_configuration_files(datastore: &D) -> Result( datastore: &D, - committed: Committed, + committed: &Committed, find_prefix: S, map_prefix: Option, ) -> Result> @@ -111,7 +134,7 @@ where pub(crate) fn get_settings_keys( datastore: &D, keys: &HashSet<&str>, - committed: Committed, + committed: &Committed, ) -> Result { let mut data = HashMap::new(); for key_str in keys { @@ -141,7 +164,7 @@ pub(crate) fn get_settings_keys( pub(crate) fn get_services_names<'a, D: DataStore>( datastore: &D, names: &'a HashSet<&str>, - committed: Committed, + committed: &Committed, ) -> Result { get_map_from_prefix(datastore, "services.".to_string(), names, committed) } @@ -151,7 +174,7 @@ pub(crate) fn get_services_names<'a, D: DataStore>( pub(crate) fn get_configuration_files_names( datastore: &D, names: &HashSet<&str>, - committed: Committed, + committed: &Committed, ) -> Result { get_map_from_prefix( datastore, @@ -169,7 +192,7 @@ fn get_map_from_prefix( datastore: &D, prefix: String, names: &HashSet<&str>, - committed: Committed, + committed: &Committed, ) -> Result> where T: DeserializeOwned, @@ -200,11 +223,18 @@ where } /// Given a Settings, takes any Some values and updates them in the datastore. -pub(crate) fn set_settings(datastore: &mut D, settings: &Settings) -> Result<()> { +pub(crate) fn set_settings( + datastore: &mut D, + settings: &Settings, + transaction: &str, +) -> Result<()> { trace!("Serializing Settings to write to data store"); let pairs = to_pairs(settings).context(error::DataStoreSerialization { given: "Settings" })?; + let pending = Committed::Pending { + tx: transaction.into(), + }; datastore - .set_keys(&pairs, Committed::Pending) + .set_keys(&pairs, &pending) .context(error::DataStore { op: "set_keys" }) } @@ -276,15 +306,18 @@ pub(crate) fn get_metadata_for_all_data_keys>( } /// Makes live any pending settings in the datastore, returning the changed keys. -pub(crate) fn commit(datastore: &mut D) -> Result> { +pub(crate) fn commit_transaction(datastore: &mut D, transaction: &str) -> Result> +where + D: DataStore, +{ datastore - .commit() + .commit_transaction(transaction) .context(error::DataStore { op: "commit" }) } /// Launches the config applier to make appropriate changes to the system based on any settings -/// that have changed. Can be called after a commit, with the keys that changed in that commit, -/// or called on its own to reset configuration state with all known keys. +/// that have been committed. Can be called after a commit, with the keys that changed in that +/// commit, or called on its own to reset configuration state with all known keys. /// /// If `keys_limit` is Some, gives those keys to the applier so only changes relevant to those /// keys are made. Otherwise, tells the applier to apply changes for all known keys. @@ -350,12 +383,12 @@ mod test { ds.set_key( &Key::new(KeyType::Data, "settings.hostname").unwrap(), "\"json string\"", - Committed::Live, + &Committed::Live, ) .unwrap(); // Retrieve with helper - let settings = get_settings(&ds, Committed::Live).unwrap(); + let settings = get_settings(&ds, &Committed::Live).unwrap(); assert_eq!(settings.hostname, Some("json string".try_into().unwrap())); } @@ -366,18 +399,18 @@ mod test { ds.set_key( &Key::new(KeyType::Data, "settings.timezone").unwrap(), "\"json string\"", - Committed::Live, + &Committed::Live, ) .unwrap(); // Retrieve with helper - let settings = get_settings_prefix(&ds, "", Committed::Live).unwrap(); + let settings = get_settings_prefix(&ds, "", &Committed::Live).unwrap(); assert_eq!(settings.timezone, Some("json string".try_into().unwrap())); - let settings = get_settings_prefix(&ds, "tim", Committed::Live).unwrap(); + let settings = get_settings_prefix(&ds, "tim", &Committed::Live).unwrap(); assert_eq!(settings.timezone, Some("json string".try_into().unwrap())); - let settings = get_settings_prefix(&ds, "timbits", Committed::Live).unwrap(); + let settings = get_settings_prefix(&ds, "timbits", &Committed::Live).unwrap(); assert_eq!(settings.timezone, None); } @@ -388,20 +421,20 @@ mod test { ds.set_key( &Key::new(KeyType::Data, "settings.timezone").unwrap(), "\"json string 1\"", - Committed::Live, + &Committed::Live, ) .unwrap(); ds.set_key( &Key::new(KeyType::Data, "settings.hostname").unwrap(), "\"json string 2\"", - Committed::Live, + &Committed::Live, ) .unwrap(); // Retrieve with helper let settings = - get_settings_keys(&ds, &hashset!("settings.timezone"), Committed::Live).unwrap(); + get_settings_keys(&ds, &hashset!("settings.timezone"), &Committed::Live).unwrap(); assert_eq!(settings.timezone, Some("json string 1".try_into().unwrap())); assert_eq!(settings.hostname, None); } @@ -413,19 +446,19 @@ mod test { ds.set_key( &Key::new(KeyType::Data, "services.foo.configuration-files").unwrap(), "[\"file1\"]", - Committed::Pending, + &Committed::Live, ) .unwrap(); ds.set_key( &Key::new(KeyType::Data, "services.foo.restart-commands").unwrap(), "[\"echo hi\"]", - Committed::Pending, + &Committed::Live, ) .unwrap(); // Retrieve built service let names = hashset!("foo"); - let services = get_services_names(&ds, &names, Committed::Pending).unwrap(); + let services = get_services_names(&ds, &names, &Committed::Live).unwrap(); assert_eq!( services, hashmap!("foo".to_string() => Service { @@ -442,13 +475,15 @@ mod test { // Set with helper let mut ds = MemoryDataStore::new(); - set_settings(&mut ds, &settings).unwrap(); + let tx = "test transaction"; + let pending = Committed::Pending { tx: tx.into() }; + set_settings(&mut ds, &settings, tx).unwrap(); // Retrieve directly let key = Key::new(KeyType::Data, "settings.timezone").unwrap(); assert_eq!( Some("\"tz\"".to_string()), - ds.get_key(&key, Committed::Pending).unwrap() + ds.get_key(&key, &pending).unwrap() ); } @@ -503,26 +538,28 @@ mod test { fn commit_works() { // Set directly with data store let mut ds = MemoryDataStore::new(); + let tx = "test transaction"; + let pending = Committed::Pending { tx: tx.into() }; ds.set_key( &Key::new(KeyType::Data, "settings.hostname").unwrap(), "\"json string\"", - Committed::Pending, + &pending, ) .unwrap(); // Confirm pending - let settings = get_settings(&ds, Committed::Pending).unwrap(); + let settings = get_settings(&ds, &pending).unwrap(); assert_eq!(settings.hostname, Some("json string".try_into().unwrap())); // No live settings yet - get_settings(&ds, Committed::Live).unwrap_err(); + get_settings(&ds, &Committed::Live).unwrap_err(); // Commit, pending -> live - commit(&mut ds).unwrap(); + commit_transaction(&mut ds, tx).unwrap(); // No more pending settings - get_settings(&ds, Committed::Pending).unwrap_err(); + get_settings(&ds, &pending).unwrap_err(); // Confirm live - let settings = get_settings(&ds, Committed::Live).unwrap(); + let settings = get_settings(&ds, &Committed::Live).unwrap(); assert_eq!(settings.hostname, Some("json string".try_into().unwrap())); } } diff --git a/workspaces/api/apiserver/src/server/mod.rs b/workspaces/api/apiserver/src/server/mod.rs index 87602f0c852..bb17d462b98 100644 --- a/workspaces/api/apiserver/src/server/mod.rs +++ b/workspaces/api/apiserver/src/server/mod.rs @@ -68,14 +68,19 @@ where .service( web::scope("/settings") .route("", web::get().to(get_settings)) - .route("", web::patch().to(patch_settings)) - .route("/pending", web::get().to(get_pending_settings)) - .route("/pending", web::delete().to(delete_pending_settings)) - .route("/commit", web::post().to(commit_settings)) - .route("/apply", web::post().to(apply_settings)) + .route("", web::patch().to(patch_settings)), + ) + .service( + // Transaction support + web::scope("/tx") + .route("/list", web::get().to(get_transaction_list)) + .route("", web::get().to(get_transaction)) + .route("", web::delete().to(delete_transaction)) + .route("/commit", web::post().to(commit_transaction)) + .route("/apply", web::post().to(apply_changes)) .route( "/commit_and_apply", - web::post().to(commit_and_apply_settings), + web::post().to(commit_transaction_and_apply), ), ) .service( @@ -128,15 +133,15 @@ fn get_settings( let settings = if let Some(keys_str) = query.get("keys") { let keys = comma_separated("keys", keys_str)?; - controller::get_settings_keys(&*datastore, &keys, Committed::Live) + controller::get_settings_keys(&*datastore, &keys, &Committed::Live) } else if let Some(prefix_str) = query.get("prefix") { if prefix_str.is_empty() { return error::EmptyInput { input: "prefix" }.fail(); } // Note: the prefix should not include "settings." - controller::get_settings_prefix(&*datastore, prefix_str, Committed::Live) + controller::get_settings_prefix(&*datastore, prefix_str, &Committed::Live) } else { - controller::get_settings(&*datastore, Committed::Live) + controller::get_settings(&*datastore, &Committed::Live) }?; Ok(SettingsResponse(settings)) @@ -145,32 +150,53 @@ fn get_settings( /// Apply the requested settings to the pending data store fn patch_settings( settings: web::Json, + query: web::Query>, data: web::Data, ) -> Result { + let transaction = transaction_name(&query); let mut datastore = data.ds.write().ok().context(error::DataStoreLock)?; - controller::set_settings(&mut *datastore, &settings)?; + controller::set_settings(&mut *datastore, &settings, transaction)?; Ok(HttpResponse::NoContent().finish()) // 204 } -/// Return any settings that have been received but not committed -fn get_pending_settings(data: web::Data) -> Result { +fn get_transaction_list(data: web::Data) -> Result { let datastore = data.ds.read().ok().context(error::DataStoreLock)?; - let settings = controller::get_pending_settings(&*datastore)?; - Ok(SettingsResponse(settings)) + let data = controller::list_transactions(&*datastore)?; + Ok(TransactionListResponse(data)) } -/// Delete any settings that have been received but not committed -fn delete_pending_settings(data: web::Data) -> Result { +/// Get any pending settings in the given transaction, or the "default" transaction if unspecified. +fn get_transaction( + query: web::Query>, + data: web::Data, +) -> Result { + let transaction = transaction_name(&query); + let datastore = data.ds.read().ok().context(error::DataStoreLock)?; + let data = controller::get_transaction(&*datastore, transaction)?; + Ok(SettingsResponse(data)) +} + +/// Delete the given transaction, or the "default" transaction if unspecified. +fn delete_transaction( + query: web::Query>, + data: web::Data, +) -> Result { + let transaction = transaction_name(&query); let mut datastore = data.ds.write().ok().context(error::DataStoreLock)?; - let deleted = controller::delete_pending_settings(&mut *datastore)?; + let deleted = controller::delete_transaction(&mut *datastore, transaction)?; Ok(ChangedKeysResponse(deleted)) } -/// Save settings changes to the main data store and kick off appliers. -fn commit_settings(data: web::Data) -> Result { +/// Save settings changes from the given transaction, or the "default" transaction if unspecified, +/// to the live data store. Returns the list of changed keys. +fn commit_transaction( + query: web::Query>, + data: web::Data, +) -> Result { + let transaction = transaction_name(&query); let mut datastore = data.ds.write().ok().context(error::DataStoreLock)?; - let changes = controller::commit(&mut *datastore)?; + let changes = controller::commit_transaction(&mut *datastore, transaction)?; if changes.is_empty() { return error::CommitWithNoPending.fail(); @@ -179,8 +205,9 @@ fn commit_settings(data: web::Data) -> Result>) -> Result { +/// Starts settings appliers for any changes that have been committed to the data store. This +/// updates config files, runs restart commands, etc. +fn apply_changes(query: web::Query>) -> Result { if let Some(keys_str) = query.get("keys") { let keys = comma_separated("keys", keys_str)?; controller::apply_changes(Some(&keys))?; @@ -191,11 +218,17 @@ fn apply_settings(query: web::Query>) -> Result) -> Result { +/// Usually you want to apply settings changes you've committed, so this is a convenience method to +/// perform both a commit and an apply. Commits the given transaction, or the "default" +/// transaction if unspecified. +fn commit_transaction_and_apply( + query: web::Query>, + data: web::Data, +) -> Result { + let transaction = transaction_name(&query); let mut datastore = data.ds.write().ok().context(error::DataStoreLock)?; - let changes = controller::commit(&mut *datastore)?; + let changes = controller::commit_transaction(&mut *datastore, transaction)?; if changes.is_empty() { return error::CommitWithNoPending.fail(); @@ -239,8 +272,7 @@ fn get_templates( if let Some(keys_str) = query.get("keys") { let data_keys = comma_separated("keys", keys_str)?; let datastore = data.ds.read().ok().context(error::DataStoreLock)?; - let resp = - controller::get_metadata_for_data_keys(&*datastore, "template", &data_keys)?; + let resp = controller::get_metadata_for_data_keys(&*datastore, "template", &data_keys)?; Ok(MetadataResponse(resp)) } else { @@ -257,7 +289,7 @@ fn get_services( let resp = if let Some(names_str) = query.get("names") { let names = comma_separated("names", names_str)?; - controller::get_services_names(&*datastore, &names, Committed::Live) + controller::get_services_names(&*datastore, &names, &Committed::Live) } else { controller::get_services(&*datastore) }?; @@ -274,7 +306,7 @@ fn get_configuration_files( let resp = if let Some(names_str) = query.get("names") { let names = comma_separated("names", names_str)?; - controller::get_configuration_files_names(&*datastore, &names, Committed::Live) + controller::get_configuration_files_names(&*datastore, &names, &Committed::Live) } else { controller::get_configuration_files(&*datastore) }?; @@ -293,6 +325,14 @@ fn comma_separated<'a>(key_name: &'static str, input: &'a str) -> Result>) -> &str { + if let Some(name_str) = query.get("tx") { + name_str + } else { + "default" + } +} + // Can also override `render_response` if we want to change headers, content type, etc. impl ResponseError for error::Error { /// Maps our error types to the HTTP error code they should return. @@ -378,3 +418,6 @@ impl_responder_for!(ConfigurationFilesResponse, self, self.0); struct ChangedKeysResponse(HashSet); impl_responder_for!(ChangedKeysResponse, self, self.0); + +struct TransactionListResponse(HashSet); +impl_responder_for!(TransactionListResponse, self, self.0); From 3db580ff9252f68f811dd589cc6e4771a04f9c61 Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Wed, 5 Feb 2020 14:55:55 -0800 Subject: [PATCH 2/6] Update OpenAPI spec to match API changes --- workspaces/api/openapi.yaml | 92 ++++++++++++++++++++++++++++++++----- 1 file changed, 80 insertions(+), 12 deletions(-) diff --git a/workspaces/api/openapi.yaml b/workspaces/api/openapi.yaml index c93661a14c9..30271c254e6 100644 --- a/workspaces/api/openapi.yaml +++ b/workspaces/api/openapi.yaml @@ -47,6 +47,13 @@ paths: patch: summary: "Update settings" operationId: "set_settings" + parameters: + - in: query + name: tx + description: "Transaction in which to update settings; defaults to user 'default' transaction" + schema: + type: string + required: false requestBody: required: true content: @@ -61,10 +68,17 @@ paths: 500: description: "Server error" - /settings/pending: + /tx: get: - summary: "Get pending settings" - operationId: "get_pending_settings" + summary: "Get pending settings in a transaction" + operationId: "get_tx" + parameters: + - in: query + name: tx + description: "Transaction for which to retrieve pending settings; defaults to user 'default' transaction" + schema: + type: string + required: false responses: 200: description: "Successful request" @@ -75,28 +89,58 @@ paths: 500: description: "Server error" delete: - summary: "Delete pending settings" - operationId: "delete_pending_settings" + summary: "Delete transaction" + operationId: "delete_tx" + parameters: + - in: query + name: tx + description: "Transaction to delete; defaults to user 'default' transaction" + schema: + type: string + required: false responses: 200: description: "Successful deleted pending settings - deleted keys are returned" 500: description: "Server error" - /settings/commit: + /tx/list: + get: + summary: "List names of pending transactions" + operationId: "list_tx" + responses: + 200: + description: "Successful request" + content: + application/json: + schema: + type: array + items: + type: string + 500: + description: "Server error" + + /tx/commit: post: summary: "Commit pending settings, without applying changes to config files or restarting services" - operationId: "commit_settings" + operationId: "commit_tx" + parameters: + - in: query + name: tx + description: "Transaction to commit; defaults to user 'default' transaction" + schema: + type: string + required: false responses: 200: description: "Successfully Staged settings - changed keys are returned" 500: description: "Server error" - /settings/apply: + /tx/apply: post: summary: "Apply changes to config files and restart services" - operationId: "apply_settings" + operationId: "apply" parameters: - in: query name: keys @@ -115,10 +159,17 @@ paths: 500: description: "Server error" - /settings/commit_and_apply: + /tx/commit_and_apply: post: - summary: "Commit pending settings, and apply changes to relevant config files and services" - operationId: "commit_and_apply_settings" + summary: "Commit transaction, and apply any committed changes to relevant config files and services" + operationId: "commit_tx_and_apply" + parameters: + - in: query + name: tx + description: "Transaction to commit; defaults to user 'default' transaction" + schema: + type: string + required: false responses: 200: description: "Successful settings update, committed keys are returned" @@ -177,6 +228,23 @@ paths: 500: description: "Server error" + /metadata/templates: + get: + summary: "Get template strings for dynamically generated settings" + operationId: "get_templates" + responses: + 200: + description: "Successful request" + content: + application/json: + # The response is a hashmap of string to string. Example: + # { "settings.foobar": "hi {{ key }}" } + schema: + type: object + additionalProperties: + type: string + 500: + description: "Server error" /services: get: From 8dd0ae76072205cc7bbb821234dc14bea790a39d Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Fri, 7 Feb 2020 16:35:42 -0800 Subject: [PATCH 3/6] Update system tools to use transaction API --- workspaces/api/moondog/src/main.rs | 21 +++---- workspaces/api/settings-committer/README.md | 7 ++- workspaces/api/settings-committer/src/main.rs | 49 ++++++++++----- workspaces/api/storewolf/src/main.rs | 23 ++++--- workspaces/api/sundog/src/main.rs | 61 +++++++++---------- 5 files changed, 88 insertions(+), 73 deletions(-) diff --git a/workspaces/api/moondog/src/main.rs b/workspaces/api/moondog/src/main.rs index abef34c2860..f9abb165877 100644 --- a/workspaces/api/moondog/src/main.rs +++ b/workspaces/api/moondog/src/main.rs @@ -30,6 +30,8 @@ use std::{env, fs, process}; // FIXME Get these from configuration in the future const DEFAULT_API_SOCKET: &str = "/run/api.sock"; const API_SETTINGS_URI: &str = "/settings"; +// We change settings in the shared transaction used by boot-time services. +const TRANSACTION: &str = "thar-boot"; // We only want to run moondog once, at first boot. Our systemd unit file has a // ConditionPathExists that will prevent it from running again if this file exists. @@ -362,24 +364,19 @@ fn run() -> Result<()> { let data_provider = find_provider()?; info!("Retrieving platform-specific data"); + let uri = &format!("{}?tx={}", API_SETTINGS_URI, TRANSACTION); + let method = "PATCH"; for settings_json in data_provider.platform_data()? { info!("Sending {} to API", settings_json.desc); trace!("Request body: {}", settings_json.json); - let (code, response_body) = apiclient::raw_request( - &args.socket_path, - API_SETTINGS_URI, - "PATCH", - Some(settings_json.json), - ) - .context(error::APIRequest { - method: "PATCH", - uri: API_SETTINGS_URI, - })?; + let (code, response_body) = + apiclient::raw_request(&args.socket_path, uri, method, Some(settings_json.json)) + .context(error::APIRequest { method, uri })?; ensure!( code.is_success(), error::Response { - method: "PATCH", - uri: API_SETTINGS_URI, + method, + uri, code, response_body, } diff --git a/workspaces/api/settings-committer/README.md b/workspaces/api/settings-committer/README.md index 150c8511543..091aacf14db 100644 --- a/workspaces/api/settings-committer/README.md +++ b/workspaces/api/settings-committer/README.md @@ -4,11 +4,12 @@ Current version: 0.1.0 ## Introduction -settings-committer can be called to commit any pending settings in the API. +settings-committer can be called to commit a pending transaction in the API. It logs any pending settings, then commits them to live. -This is typically run during startup as a pre-exec command by any services that depend on settings -changes from previous services. +By default, it commits the 'thar-boot' transaction, which is used to organize boot-time services - this program is typically run as a pre-exec command by any services that depend on settings changes from previous services. + +The `--transaction` argument can be used to specify another transaction. ## Colophon diff --git a/workspaces/api/settings-committer/src/main.rs b/workspaces/api/settings-committer/src/main.rs index b65393fa2d0..845c5d5bd26 100644 --- a/workspaces/api/settings-committer/src/main.rs +++ b/workspaces/api/settings-committer/src/main.rs @@ -1,11 +1,12 @@ /*! # Introduction -settings-committer can be called to commit any pending settings in the API. +settings-committer can be called to commit a pending transaction in the API. It logs any pending settings, then commits them to live. -This is typically run during startup as a pre-exec command by any services that depend on settings -changes from previous services. +By default, it commits the 'thar-boot' transaction, which is used to organize boot-time services - this program is typically run as a pre-exec command by any services that depend on settings changes from previous services. + +The `--transaction` argument can be used to specify another transaction. */ #![deny(rust_2018_idioms)] @@ -18,8 +19,10 @@ use std::str::FromStr; use std::{collections::HashMap, env, process}; const DEFAULT_API_SOCKET: &str = "/run/api.sock"; -const API_PENDING_URI: &str = "/settings/pending"; -const API_COMMIT_URI: &str = "/settings/commit"; +const API_PENDING_URI_BASE: &str = "/tx"; +const API_COMMIT_URI_BASE: &str = "/tx/commit"; +// By default we commit settings from a shared transaction used by boot-time services. +const DEFAULT_TRANSACTION: &str = "thar-boot"; type Result = std::result::Result; @@ -55,11 +58,11 @@ mod error { /// commit if there's a blip in retrieval or parsing of the pending /// settings. We know the system won't be functional without a commit, /// but we can live without logging what was committed. -fn check_pending_settings>(socket_path: S) { - let uri = API_PENDING_URI; +fn check_pending_settings>(socket_path: S, transaction: &str) { + let uri = format!("{}?tx={}", API_PENDING_URI_BASE, transaction); debug!("GET-ing {} to determine if there are pending settings", uri); - let get_result = apiclient::raw_request(socket_path.as_ref(), uri, "GET", None); + let get_result = apiclient::raw_request(socket_path.as_ref(), &uri, "GET", None); let response_body = match get_result { Ok((code, response_body)) => { if !code.is_success() { @@ -81,7 +84,7 @@ fn check_pending_settings>(socket_path: S) { serde_json::from_str(&response_body); match pending_result { Ok(pending) => { - debug!("Pending settings: {:?}", &pending); + debug!("Pending settings for tx {}: {:?}", transaction, &pending); } Err(err) => { warn!("Failed to parse response from {}: {}", uri, err); @@ -90,11 +93,11 @@ fn check_pending_settings>(socket_path: S) { } /// Commits pending settings to live. -fn commit_pending_settings>(socket_path: S) -> Result<()> { - let uri = API_COMMIT_URI; +fn commit_pending_settings>(socket_path: S, transaction: &str) -> Result<()> { + let uri = format!("{}?tx={}", API_COMMIT_URI_BASE, transaction); debug!("POST-ing to {} to move pending settings to live", uri); - if let Err(e) = apiclient::raw_request(socket_path.as_ref(), uri, "POST", None) { + if let Err(e) = apiclient::raw_request(socket_path.as_ref(), &uri, "POST", None) { match e { // Some types of response errors are OK for this use. apiclient::Error::ResponseStatus { code, body, .. } => { @@ -107,7 +110,8 @@ fn commit_pending_settings>(socket_path: S) -> Result<()> { uri, code, response_body: body, - }.fail(); + } + .fail(); } } // Any other type of error means we couldn't even make the request. @@ -124,6 +128,7 @@ fn commit_pending_settings>(socket_path: S) -> Result<()> { /// Store the args we receive on the command line struct Args { + transaction: String, log_level: LevelFilter, socket_path: String, } @@ -133,11 +138,13 @@ fn usage() -> ! { let program_name = env::args().next().unwrap_or_else(|| "program".to_string()); eprintln!( r"Usage: {} + [ --transaction TX ] [ --socket-path PATH ] [ --log-level trace|debug|info|warn|error ] + Transaction defaults to {} Socket path defaults to {}", - program_name, DEFAULT_API_SOCKET + program_name, DEFAULT_TRANSACTION, DEFAULT_API_SOCKET ); process::exit(2); } @@ -150,12 +157,20 @@ fn usage_msg>(msg: S) -> ! { /// Parse the args to the program and return an Args struct fn parse_args(args: env::Args) -> Args { + let mut transaction = None; let mut log_level = None; let mut socket_path = None; let mut iter = args.skip(1); while let Some(arg) = iter.next() { match arg.as_ref() { + "--transaction" => { + transaction = Some( + iter.next() + .unwrap_or_else(|| usage_msg("Did not give argument to --transaction")), + ) + } + "--log-level" => { let log_level_str = iter .next() @@ -171,11 +186,13 @@ fn parse_args(args: env::Args) -> Args { .unwrap_or_else(|| usage_msg("Did not give argument to --socket-path")), ) } + _ => usage(), } } Args { + transaction: transaction.unwrap_or_else(|| DEFAULT_TRANSACTION.to_string()), log_level: log_level.unwrap_or_else(|| LevelFilter::Info), socket_path: socket_path.unwrap_or_else(|| DEFAULT_API_SOCKET.to_string()), } @@ -190,10 +207,10 @@ fn run() -> Result<()> { .context(error::Logger)?; info!("Checking pending settings."); - check_pending_settings(&args.socket_path); + check_pending_settings(&args.socket_path, &args.transaction); info!("Committing settings."); - commit_pending_settings(&args.socket_path)?; + commit_pending_settings(&args.socket_path, &args.transaction)?; Ok(()) } diff --git a/workspaces/api/storewolf/src/main.rs b/workspaces/api/storewolf/src/main.rs index fe967d7b4d8..07b0839c27b 100644 --- a/workspaces/api/storewolf/src/main.rs +++ b/workspaces/api/storewolf/src/main.rs @@ -31,6 +31,8 @@ use model::modeled_types::SingleLineString; // FIXME Get these from configuration in the future const DATASTORE_VERSION_FILE: &str = "/usr/share/thar/data-store-version"; +// Shared transaction used by boot-time services. +const TRANSACTION: &str = "thar-boot"; mod error { use std::io; @@ -46,7 +48,7 @@ mod error { #[derive(Debug, Snafu)] #[snafu(visibility = "pub(super)")] pub(super) enum StorewolfError { - #[snafu(display("Unable to clear pending settings: {}", source))] + #[snafu(display("Unable to clear pending transactions: {}", source))] DeletePending { source: io::Error }, #[snafu(display("Unable to create datastore at '{}': {}", path.display(), source))] @@ -359,7 +361,7 @@ fn populate_default_datastore>( .list_populated_metadata("", &None as &Option<&str>) .context(error::QueryMetadata)?; existing_data = datastore - .list_populated_keys("", datastore::Committed::Live) + .list_populated_keys("", &datastore::Committed::Live) .context(error::QueryData)?; } else { info!("Creating datastore at: {}", &live_path.display()); @@ -390,10 +392,9 @@ fn populate_default_datastore>( let maybe_metadata_val = table.remove("metadata"); let maybe_settings_val = table.remove("settings"); - // If there are default settings, write them to the datastore in Pending - // state. This ensures the settings will go through a commit cycle when - // first-boot services run, which will create config files for default - // keys that require them. + // If there are default settings, write them to the datastore in the shared pending + // transaction. This ensures the settings will go through a commit cycle when first-boot + // services run, which will create config files for default keys that require them. if let Some(def_settings_val) = maybe_settings_val { debug!("Serializing default settings and writing new ones to datastore"); let def_settings_table = def_settings_val @@ -424,8 +425,11 @@ fn populate_default_datastore>( "Writing default settings to datastore: {:#?}", &settings_to_write ); + let pending = datastore::Committed::Pending { + tx: TRANSACTION.to_string(), + }; datastore - .set_keys(&settings_to_write, datastore::Committed::Pending) + .set_keys(&settings_to_write, &pending) .context(error::WriteKeys)?; } @@ -507,7 +511,7 @@ fn populate_default_datastore>( &other_defaults_to_write ); datastore - .set_keys(&other_defaults_to_write, datastore::Committed::Live) + .set_keys(&other_defaults_to_write, &datastore::Committed::Live) .context(error::WriteKeys)?; } Ok(()) @@ -598,8 +602,7 @@ fn run() -> Result<()> { info!("Storewolf started"); - // If anything exists in Pending state, delete it - info!("Deleting pending settings"); + info!("Deleting pending transactions"); let pending_path = Path::new(&args.data_store_base_path) .join("current") .join("pending"); diff --git a/workspaces/api/sundog/src/main.rs b/workspaces/api/sundog/src/main.rs index 51e8b2c822c..2b48935209a 100644 --- a/workspaces/api/sundog/src/main.rs +++ b/workspaces/api/sundog/src/main.rs @@ -26,8 +26,9 @@ use apiserver::datastore::{self, deserialization, Key, KeyType}; // FIXME Get from configuration in the future const DEFAULT_API_SOCKET: &str = "/run/api.sock"; const API_SETTINGS_URI: &str = "/settings"; -const API_PENDING_SETTINGS_URI: &str = "/settings/pending"; const API_SETTING_GENERATORS_URI: &str = "/metadata/setting-generators"; +// We change settings in the shared transaction used by boot-time services. +const TRANSACTION: &str = "thar-boot"; /// Potential errors during Sundog execution mod error { @@ -182,8 +183,7 @@ where Ok(generators) } -/// Given a list of settings, query the API for any that are currently -/// set or are in pending state. +/// Given a list of settings, query the API for any that are currently set. fn get_populated_settings

(socket_path: P, to_query: Vec<&str>) -> Result> where P: AsRef, @@ -192,39 +192,36 @@ where let mut populated_settings = HashSet::new(); - // Build the query string and the URI containing that query. Currently - // the API doesn't suport queries for the '/settings/pending' endpoint, - // so we dont' build the string for it. + // Build the query string and the URI containing that query. let query = to_query.join(","); - let committed_uri = format!("{}?keys={}", API_SETTINGS_URI, query); - - for uri in &[committed_uri, API_PENDING_SETTINGS_URI.to_string()] { - let (code, response_body) = apiclient::raw_request(socket_path.as_ref(), &uri, "GET", None) - .context(error::APIRequest { method: "GET", uri })?; - ensure!( - code.is_success(), - error::APIResponse { - method: "GET", - uri, - code, - response_body, - } - ); + let uri = &format!("{}?keys={}", API_SETTINGS_URI, query); + + let (code, response_body) = apiclient::raw_request(socket_path.as_ref(), uri, "GET", None) + .context(error::APIRequest { method: "GET", uri })?; + ensure!( + code.is_success(), + error::APIResponse { + method: "GET", + uri, + code, + response_body, + } + ); - // Build a Settings struct from the response. - let settings: model::Settings = serde_json::from_str(&response_body) - .context(error::ResponseJson { method: "GET", uri })?; + // Build a Settings struct from the response. + let settings: model::Settings = + serde_json::from_str(&response_body).context(error::ResponseJson { method: "GET", uri })?; - // Serialize the Settings struct into key/value pairs. This builds the dotted - // string representation of the setting - let settings_keypairs = - to_pairs_with_prefix("settings", &settings).context(error::SerializeSettings)?; + // Serialize the Settings struct into key/value pairs. This builds the dotted + // string representation of the setting + let settings_keypairs = + to_pairs_with_prefix("settings", &settings).context(error::SerializeSettings)?; - // Put the setting into our hashset of populated keys - for (k, _) in settings_keypairs { - populated_settings.insert(k); - } + // Put the setting into our hashset of populated keys + for (k, _) in settings_keypairs { + populated_settings.insert(k); } + trace!("Found populated settings: {:#?}", &populated_settings); Ok(populated_settings) } @@ -363,7 +360,7 @@ where // Serialize our Settings struct to the JSON wire format let request_body = serde_json::to_string(&settings).context(error::SerializeRequest)?; - let uri = API_SETTINGS_URI; + let uri = &format!("{}?tx={}", API_SETTINGS_URI, TRANSACTION); let method = "PATCH"; trace!("Settings to {} to {}: {}", method, uri, &request_body); let (code, response_body) = From 0d5a0e73d47f2c73707c9a56cfd52c472eb4c9b3 Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Fri, 7 Feb 2020 16:35:47 -0800 Subject: [PATCH 4/6] Update control container to use API transactions --- .../thar-control/enable-admin-container | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/extras/host-containers/thar-control/enable-admin-container b/extras/host-containers/thar-control/enable-admin-container index b1f0fbbad0b..0ec15b2db27 100644 --- a/extras/host-containers/thar-control/enable-admin-container +++ b/extras/host-containers/thar-control/enable-admin-container @@ -5,7 +5,7 @@ error() { cat >&2 <<-EOF You can manually enable the admin container like this: apiclient -u /settings -m PATCH -d '{"host-containers": {"admin": {"enabled": true}}}' - apiclient -u /settings/commit_and_apply -m POST + apiclient -u /tx/commit_and_apply -m POST EOF exit 1 } @@ -14,22 +14,16 @@ if ! command -v apiclient >/dev/null 2>&1; then error "can't find 'apiclient'" fi -echo "Checking whether there are pending settings; we don't want to silently commit other changes" -PENDING="$(apiclient -u /settings/pending)" -rc="${?}" -if [ "${rc}" -ne 0 ]; then - error "apiclient returned ${rc} - couldn't check pending settings, so we don't know if it's safe to commit changes for enabling admin container.\nTry to check what's pending with \`apiclient -u /settings/pending\`" -elif [ "${PENDING}" != "{}" ]; then - error "found pending settings in API, cowardly refusing to commit them.\nYou can commit them yourself with \`apiclient -u /settings/commit_and_apply -m POST\` and try again.\nPending settings: ${PENDING}" -fi +# Use our own transaction so we don't interfere with other changes +TX="enable-admin-container" echo "Setting admin container to enabled" -if ! apiclient -v -u /settings -m PATCH -d '{"host-containers": {"admin": {"enabled": true}}}'; then +if ! apiclient -v -u "/settings?tx=${TX}" -m PATCH -d '{"host-containers": {"admin": {"enabled": true}}}'; then error "failed to change enabled setting of admin container" fi echo "Committing and applying changes" -if ! apiclient -v -u /settings/commit_and_apply -m POST; then +if ! apiclient -v -u "/tx/commit_and_apply?tx=${TX}" -m POST; then error "failed to commit and apply settings" fi From 82b6959af01a794b7602c2290a46afad84c090e1 Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Wed, 5 Feb 2020 15:01:53 -0800 Subject: [PATCH 5/6] migration-helpers: update to understand transactions --- .../migration-helpers/src/datastore.rs | 6 +++--- .../migration/migration-helpers/src/error.rs | 3 +++ .../migration/migration-helpers/src/lib.rs | 21 ++++++++++++------- .../migration-helpers/src/workarounds.rs | 18 ++++++++-------- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/workspaces/api/migration/migration-helpers/src/datastore.rs b/workspaces/api/migration/migration-helpers/src/datastore.rs index 473d7b9620a..0b77b6ee960 100644 --- a/workspaces/api/migration/migration-helpers/src/datastore.rs +++ b/workspaces/api/migration/migration-helpers/src/datastore.rs @@ -16,11 +16,11 @@ use apiserver::datastore::{ /// Retrieves data from the specified data store in a consistent format for easy modification. pub(crate) fn get_input_data( datastore: &D, - committed: Committed, + committed: &Committed, ) -> Result { let raw_data = datastore .get_prefix("", committed) - .context(error::GetData { committed })?; + .with_context(|| error::GetData { committed: committed.clone() })?; let mut data = HashMap::new(); for (data_key, value_str) in raw_data.into_iter() { @@ -66,7 +66,7 @@ pub(crate) fn get_input_data( pub(crate) fn set_output_data( datastore: &mut D, input: &MigrationData, - committed: Committed, + committed: &Committed, ) -> Result<()> { // Prepare serialized data let mut data = HashMap::new(); diff --git a/workspaces/api/migration/migration-helpers/src/error.rs b/workspaces/api/migration/migration-helpers/src/error.rs index 5f743441136..0a336b3fc95 100644 --- a/workspaces/api/migration/migration-helpers/src/error.rs +++ b/workspaces/api/migration/migration-helpers/src/error.rs @@ -52,6 +52,9 @@ pub enum Error { key: String, source: datastore::Error, }, + + #[snafu(display("Unable to list transactions in data store: {}", source))] + ListTransactions { source: datastore::Error }, } /// Result alias containing our Error type. diff --git a/workspaces/api/migration/migration-helpers/src/lib.rs b/workspaces/api/migration/migration-helpers/src/lib.rs index d7dcfb90559..dfbb6dddf6e 100644 --- a/workspaces/api/migration/migration-helpers/src/lib.rs +++ b/workspaces/api/migration/migration-helpers/src/lib.rs @@ -17,6 +17,7 @@ mod datastore; pub mod error; mod workarounds; +use snafu::ResultExt; use std::collections::HashMap; use std::env; use std::fmt; @@ -40,9 +41,10 @@ type DataStoreImplementation = FilesystemDataStore; /// necessary. /// /// Migrations must not assume any key will exist because they're run on pending data as well as -/// live, and pending may be empty. For the same reason, migrations must not add a key in all -/// cases if it's missing, because you could add a key to the pending data when the user didn't -/// have any pending data. Instead, make sure you're adding a key to an existing structure. +/// live, and pending transactions usually do not impact all keys. For the same reason, migrations +/// must not add a key in all cases if it's missing, because you could be adding the key to an +/// unrelated pending transaction. Instead, make sure you're adding a key to an existing +/// structure. pub trait Migration { /// Migrates data forward from the prior version to the version specified in the migration /// name. @@ -90,8 +92,13 @@ pub fn run_migration(mut migration: impl Migration, args: &Args) -> Result<()> { let source = DataStoreImplementation::new(&args.source_datastore); let mut target = DataStoreImplementation::new(&args.target_datastore); - for committed in &[Committed::Live, Committed::Pending] { - let input = get_input_data(&source, *committed)?; + // Run for live data and for each pending transaction + let mut committeds = vec![Committed::Live]; + let transactions = source.list_transactions().context(error::ListTransactions)?; + committeds.extend(transactions.into_iter().map(|tx| Committed::Pending { tx })); + + for committed in committeds { + let input = get_input_data(&source, &committed)?; let mut migrated = input.clone(); migrated = match args.migration_type { @@ -104,13 +111,13 @@ pub fn run_migration(mut migration: impl Migration, args: &Args) -> Result<()> { &mut migrated, &source, &mut target, - *committed, + &committed, &args, )?; validate_migrated_data(&migrated)?; - set_output_data(&mut target, &migrated, *committed)?; + set_output_data(&mut target, &migrated, &committed)?; } Ok(()) } diff --git a/workspaces/api/migration/migration-helpers/src/workarounds.rs b/workspaces/api/migration/migration-helpers/src/workarounds.rs index 420668ae9ce..fb825bf3d59 100644 --- a/workspaces/api/migration/migration-helpers/src/workarounds.rs +++ b/workspaces/api/migration/migration-helpers/src/workarounds.rs @@ -11,7 +11,7 @@ pub(crate) fn fix_migrated_data( output: &MigrationData, _source_datastore: &D, target_datastore: &mut D, - committed: Committed, + committed: &Committed, args: &Args, ) -> Result<()> { // If the source and target data store path are the same, we're using the old migrator @@ -33,7 +33,7 @@ pub(crate) fn fix_migrated_data( key: *removed_key_str, })?; target_datastore - .unset_key(&removed_key, committed) + .unset_key(&removed_key, &committed) .context(error::DataStoreRemove { key: *removed_key_str, })?; @@ -122,19 +122,19 @@ mod test { // The point of the workaround is affecting the data store directly, so make test stores let mut source = MemoryDataStore::new(); - set_output_data(&mut source, &input, Committed::Live).unwrap(); + set_output_data(&mut source, &input, &Committed::Live).unwrap(); // To replicate old interface, the target data store starts with the input data, and // we're going to confirm that removed values are actually removed let mut target = MemoryDataStore::new(); - set_output_data(&mut target, &input, Committed::Live).unwrap(); + set_output_data(&mut target, &input, &Committed::Live).unwrap(); // Ensure values are there at the start let kept_data = Key::new(KeyType::Data, "keepdata").unwrap(); let removed_data = Key::new(KeyType::Data, "removedata").unwrap(); let kept_meta = Key::new(KeyType::Meta, "keepmeta").unwrap(); let removed_meta = Key::new(KeyType::Meta, "removemeta").unwrap(); - assert_eq!(target.get_key(&kept_data, Committed::Live).unwrap(), Some("\"hi\"".into())); - assert_eq!(target.get_key(&removed_data, Committed::Live).unwrap(), Some("\"sup\"".into())); + assert_eq!(target.get_key(&kept_data, &Committed::Live).unwrap(), Some("\"hi\"".into())); + assert_eq!(target.get_key(&removed_data, &Committed::Live).unwrap(), Some("\"sup\"".into())); assert_eq!(target.get_metadata(&kept_meta, &kept_data).unwrap(), Some("\"howdy\"".into())); assert_eq!(target.get_metadata(&kept_meta, &removed_data).unwrap(), Some("\"hello\"".into())); assert_eq!(target.get_metadata(&removed_meta, &kept_data).unwrap(), Some("\"yo\"".into())); @@ -151,17 +151,17 @@ mod test { &expected, &source, &mut target, - Committed::Live, + &Committed::Live, &args, ) .unwrap(); // Ensure unaffected values were kept - assert_eq!(target.get_key(&kept_data, Committed::Live).unwrap(), Some("\"hi\"".into())); + assert_eq!(target.get_key(&kept_data, &Committed::Live).unwrap(), Some("\"hi\"".into())); assert_eq!(target.get_metadata(&kept_meta, &kept_data).unwrap(), Some("\"howdy\"".into())); assert_eq!(target.get_metadata(&kept_meta, &removed_data).unwrap(), Some("\"hello\"".into())); // Ensure removed values were removed - assert_eq!(target.get_key(&removed_data, Committed::Live).unwrap(), None); + assert_eq!(target.get_key(&removed_data, &Committed::Live).unwrap(), None); assert_eq!(target.get_metadata(&removed_meta, &kept_data).unwrap(), None); assert_eq!(target.get_metadata(&removed_meta, &removed_data).unwrap(), None); } From ccd00a06d5e259b62cc45594d08cdd6ecd73e690 Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Wed, 5 Feb 2020 15:02:11 -0800 Subject: [PATCH 6/6] docs: update to describe transactions --- README.md | 17 +++++++++++++---- workspaces/api/README.md | 8 ++++---- workspaces/api/apiclient/README.md | 11 +++++++++-- workspaces/api/apiclient/README.tpl | 11 +++++++++-- workspaces/api/apiserver/README.md | 14 +++++++++----- workspaces/api/apiserver/src/lib.rs | 14 +++++++++----- 6 files changed, 53 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 6915a08a3cf..3f200a0ecf0 100644 --- a/README.md +++ b/README.md @@ -197,17 +197,26 @@ This can include any number of settings changes. apiclient -m PATCH -u /settings -d '{"timezone": "America/Thunder_Bay"}' ``` -This will *stage* the setting in a "pending" area. -You can see all the pending settings like this: +This will *stage* the setting in a "pending" area - a transaction. +You can see all your pending settings like this: ``` -apiclient -u /settings/pending +apiclient -u /tx ``` To *commit* the settings, and let the system apply them to any relevant configuration files or services, do this: ``` -apiclient -m POST -u /settings/commit_and_apply +apiclient -m POST -u /tx/commit_and_apply ``` +Behind the scenes, these commands are working with the "default" transaction. +This keeps the interface simple. +System services use their own transactions, so you don't have to worry about conflicts. +For example, there's a "thar-boot" transaction used to coordinate changes at startup. + +If you want to group sets of changes yourself, pick a transaction name and append a `tx` parameter to the URLs above. +For example, if you want the name "FOO", you can `PATCH` to `/settings?tx=FOO` and `POST` to `/tx/commit_and_apply?tx=FOO`. +(Transactions are created automatically when used, and are cleaned up on reboot.) + For more details on using the client, see the [apiclient documentation](workspaces/api/apiclient/). #### Using user data diff --git a/workspaces/api/README.md b/workspaces/api/README.md index dafdfece3fd..02922071423 100644 --- a/workspaces/api/README.md +++ b/workspaces/api/README.md @@ -53,10 +53,10 @@ storewolf ensures the default values (defined in [defaults.toml](../models/defau First, it has to create the data store directories and symlinks if they don’t exist. Then, it goes key-by-key through the defaults, and if a key isn’t already set, sets it with the default value. -The settings are written to the *pending* section of the data store, meaning they’re not available until committed later by [settings-committer](#settings-committer). +The settings are written to the *pending* section of the data store, in a "thar-boot" transaction, which is used for startup coordination. +This means they’re not available until committed later by [settings-committer](#settings-committer). -If there are any pending settings in the data store, they’re discarded. -We’re unable to guarantee users that any pending settings they haven’t committed will survive a reboot, because we have to be able to commit changes ourselves during the boot process (see later services), and we don’t yet have a way of separating transactions. +If there are any pending transactions in the data store when storewolf starts, they’re discarded. ### apiserver @@ -101,7 +101,7 @@ Pluto generates settings needed for Kubernetes configuration, for example cluste [Further docs](settings-committer/) -This binary sends a commit request to the API, which moves all the pending settings from the above services into the live part of the data store. +This binary sends a commit request to the API (by default for the "thar-boot" transaction) which moves all the pending settings from the above services into the live part of the data store. It's called as a prerequisite of other services, like [sundog](#sundog) and [settings-applier](#settings-applier), that rely on settings being committed. ### settings-applier diff --git a/workspaces/api/apiclient/README.md b/workspaces/api/apiclient/README.md index 757a84c7c19..36d1c3bc93d 100644 --- a/workspaces/api/apiclient/README.md +++ b/workspaces/api/apiclient/README.md @@ -25,16 +25,23 @@ Getting settings: ``` apiclient -m GET -u /settings -apiclient -m GET -u /settings/pending ``` Changing settings: ``` apiclient -X PATCH -u /settings -d '{"timezone": "OldLosAngeles"}' -apiclient -m POST -u /settings/commit_and_apply +apiclient -m POST -u /tx/commit_and_apply ``` +You can also check what you've changed but not commited by looking at the pending transaction: + +``` +apiclient -m GET -u /tx +``` + +(You can group changes into transactions by adding a parameter like `?tx=FOO` to the calls above.) + ## apiclient library The apiclient library provides simple, synchronous methods to query an HTTP API over a diff --git a/workspaces/api/apiclient/README.tpl b/workspaces/api/apiclient/README.tpl index 128a611e7b6..f666ad911f0 100644 --- a/workspaces/api/apiclient/README.tpl +++ b/workspaces/api/apiclient/README.tpl @@ -25,16 +25,23 @@ Getting settings: ``` apiclient -m GET -u /settings -apiclient -m GET -u /settings/pending ``` Changing settings: ``` apiclient -X PATCH -u /settings -d '{"timezone": "OldLosAngeles"}' -apiclient -m POST -u /settings/commit_and_apply +apiclient -m POST -u /tx/commit_and_apply ``` +You can also check what you've changed but not commited by looking at the pending transaction: + +``` +apiclient -m GET -u /tx +``` + +(You can group changes into transactions by adding a parameter like `?tx=FOO` to the calls above.) + ## apiclient library {{readme}} diff --git a/workspaces/api/apiserver/README.md b/workspaces/api/apiserver/README.md index 4d4f1fc3503..33e23c84396 100644 --- a/workspaces/api/apiserver/README.md +++ b/workspaces/api/apiserver/README.md @@ -21,12 +21,16 @@ The interface is documented in [OpenAPI format](https://swagger.io/docs/specific The Settings APIs are particularly important. You can GET settings from the `/settings` endpoint. You can also PATCH changes to the `/settings` endpoint. -Settings are stored as pending until a commit API is called. -Pending settings can be retrieved from `/settings/pending` to see what will change. +Settings are stored as a pending transaction until a commit API is called. +Pending settings can be retrieved from `/tx` to see what will change. -Upon making a `commit` API call, pending settings are made live. -Upon making an `apply` API call, an external settings applier tool is called to apply the changes to the system and restart services as necessary. -There's also a `commit_and_apply` API to do both, which is the most common case. +Upon making a `/tx/commit` POST call, the pending transaction is made live. +Upon making an `/tx/apply` POST call, an external settings applier tool is called to apply the changes to the system and restart services as necessary. +There's also `/tx/commit_and_apply` to do both, which is the most common case. + +If you don't specify a transaction, the "default" transaction is used, so you usually don't have to think about it. +If you want to group changes into transactions yourself, you can add a `tx` parameter to the APIs mentioned above. +For example, if you want the name "FOO", you can `PATCH` to `/settings?tx=FOO` and `POST` to `/tx/commit_and_apply?tx=FOO`. Requests are directed by `server::router`. `server::controller` maps requests into our data model. diff --git a/workspaces/api/apiserver/src/lib.rs b/workspaces/api/apiserver/src/lib.rs index f81668dfe08..079444d08f3 100644 --- a/workspaces/api/apiserver/src/lib.rs +++ b/workspaces/api/apiserver/src/lib.rs @@ -18,12 +18,16 @@ The interface is documented in [OpenAPI format](https://swagger.io/docs/specific The Settings APIs are particularly important. You can GET settings from the `/settings` endpoint. You can also PATCH changes to the `/settings` endpoint. -Settings are stored as pending until a commit API is called. -Pending settings can be retrieved from `/settings/pending` to see what will change. +Settings are stored as a pending transaction until a commit API is called. +Pending settings can be retrieved from `/tx` to see what will change. -Upon making a `commit` API call, pending settings are made live. -Upon making an `apply` API call, an external settings applier tool is called to apply the changes to the system and restart services as necessary. -There's also a `commit_and_apply` API to do both, which is the most common case. +Upon making a `/tx/commit` POST call, the pending transaction is made live. +Upon making an `/tx/apply` POST call, an external settings applier tool is called to apply the changes to the system and restart services as necessary. +There's also `/tx/commit_and_apply` to do both, which is the most common case. + +If you don't specify a transaction, the "default" transaction is used, so you usually don't have to think about it. +If you want to group changes into transactions yourself, you can add a `tx` parameter to the APIs mentioned above. +For example, if you want the name "FOO", you can `PATCH` to `/settings?tx=FOO` and `POST` to `/tx/commit_and_apply?tx=FOO`. Requests are directed by `server::router`. `server::controller` maps requests into our data model.