Merge pull request #1217 from tjkirch/sysctl-migr-metadata

sysctl migration: also remove services.sysctl

sources/api/migration/migration-helpers/src/common_migrations.rs

@@ -54,18 +54,19 @@ impl Migration for AddSettingMigration {
 
 // =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=
 
-/// We use this migration when we add a cluster of settings under a prefix and want to make sure
-/// they're removed before we go back to old versions that don't understand them.  Normally you'd
-/// use AddSettingsMigration since you know the key names, but this is useful for user-defined
-/// keys, for example in a map like settings.kernel.sysctl or settings.host-containers.
-pub struct AddPrefixMigration(pub &'static str);
-
-impl Migration for AddPrefixMigration {
+/// We use this migration when we add a cluster of settings under known prefixes and want to make
+/// sure they're removed before we go back to old versions that don't understand them.  Normally
+/// you'd use AddSettingsMigration since you know the key names, but this is useful for
+/// user-defined keys, for example in a map like settings.kernel.sysctl or
+/// settings.host-containers.
+pub struct AddPrefixesMigration(pub Vec<&'static str>);
+
+impl Migration for AddPrefixesMigration {
     /// New versions must either have a default for the settings or generate them; we don't need to
     /// do anything.
     fn forward(&mut self, input: MigrationData) -> Result<MigrationData> {
         println!(
-            "AddPrefixMigration({:?}) has no work to do on upgrade.",
+            "AddPrefixesMigration({:?}) has no work to do on upgrade.",
             self.0
         );
         Ok(input)
@@ -78,7 +79,7 @@ impl Migration for AddPrefixMigration {
         let settings = input
             .data
             .keys()
-            .filter(|k| k.starts_with(self.0))
+            .filter(|k| self.0.iter().any(|prefix| k.starts_with(prefix)))
             .cloned()
             .collect::<Vec<_>>();
         for setting in settings {
@@ -91,14 +92,14 @@ impl Migration for AddPrefixMigration {
 }
 
 #[cfg(test)]
-mod test_add_prefix_migration {
-    use super::AddPrefixMigration;
+mod test_add_prefixes_migration {
+    use super::AddPrefixesMigration;
     use crate::{Migration, MigrationData};
     use maplit::hashmap;
     use std::collections::HashMap;
 
     #[test]
-    fn works() {
+    fn single() {
         let data = MigrationData {
             data: hashmap! {
                 "keep.me.a".into() => 0.into(),
@@ -109,7 +110,33 @@ mod test_add_prefix_migration {
             metadata: HashMap::new(),
         };
         // Run backward, e.g. downgrade, to test that the right keys are removed
-        let result = AddPrefixMigration("remove.me").backward(data).unwrap();
+        let result = AddPrefixesMigration(vec!["remove.me"])
+            .backward(data)
+            .unwrap();
+        assert_eq!(
+            result.data,
+            hashmap! {
+                "keep.me.a".into() => 0.into(),
+                "keep.this.c".into() => 0.into(),
+            }
+        );
+    }
+
+    #[test]
+    fn multiple() {
+        let data = MigrationData {
+            data: hashmap! {
+                "keep.me.a".into() => 0.into(),
+                "remove.me.b".into() => 0.into(),
+                "keep.this.c".into() => 0.into(),
+                "remove.this.d.e".into() => 0.into(),
+            },
+            metadata: HashMap::new(),
+        };
+        // Run backward, e.g. downgrade, to test that the right keys are removed
+        let result = AddPrefixesMigration(vec!["remove.me", "remove.this"])
+            .backward(data)
+            .unwrap();
         assert_eq!(
             result.data,
             hashmap! {
@@ -118,6 +145,32 @@ mod test_add_prefix_migration {
             }
         );
     }
+
+    #[test]
+    fn no_match() {
+        let data = MigrationData {
+            data: hashmap! {
+                "keep.me.a".into() => 0.into(),
+                "remove.me.b".into() => 0.into(),
+                "keep.this.c".into() => 0.into(),
+                "remove.this.d.e".into() => 0.into(),
+            },
+            metadata: HashMap::new(),
+        };
+        // Run backward, e.g. downgrade, to test that the right keys are removed
+        let result = AddPrefixesMigration(vec!["not.found", "nor.this"])
+            .backward(data)
+            .unwrap();
+        assert_eq!(
+            result.data,
+            hashmap! {
+                "keep.me.a".into() => 0.into(),
+                "remove.me.b".into() => 0.into(),
+                "keep.this.c".into() => 0.into(),
+                "remove.this.d.e".into() => 0.into(),
+            }
+        );
+    }
 }
 
 // =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=   =^..^=

sources/api/migration/migrations/v1.0.3/add-sysctl/src/main.rs

@@ -1,13 +1,16 @@
 #![deny(rust_2018_idioms)]
 
-use migration_helpers::common_migrations::AddPrefixMigration;
+use migration_helpers::common_migrations::AddPrefixesMigration;
 use migration_helpers::{migrate, Result};
 use std::process;
 
 /// We added the ability to set sysctl keys via API settings.  We don't want to track all possible
 /// Linux sysctl keys, so we remove the whole prefix if we downgrade.
 fn run() -> Result<()> {
-    migrate(AddPrefixMigration("settings.kernel.sysctl"))
+    migrate(AddPrefixesMigration(vec![
+        "settings.kernel.sysctl",
+        "services.sysctl",
+    ]))
 }
 
 // Returning a Result from main makes it print a Debug representation of the error, but with Snafu