Adds testing to bytestream backwards compatibility (TraceMachina#1979)

Author: palfrey

nativelink-config/src/backcompat.rs

@@ -190,4 +190,45 @@ mod tests {
         let value: FullConfig = serde_json::from_value(json).unwrap();
         assert_eq!(value.cas, None);
     }
+
+    #[derive(Debug, Deserialize, Serialize, PartialEq)]
+    struct FullBytestreamConfig {
+        #[serde(default, deserialize_with = "opt_bytestream")]
+        pub bytestream: Option<Vec<WithInstanceName<ByteStreamConfig>>>,
+    }
+
+    #[test]
+    #[traced_test]
+    fn test_bytestream_old_config() {
+        let old_format = json!({
+            "bytestream": {
+                "cas_stores": {
+                    "": "WORKER_FAST_SLOW_STORE"
+            }}
+        });
+
+        let new_format = json!({
+            "bytestream": [
+                {
+                    "cas_store": "WORKER_FAST_SLOW_STORE",
+                },
+            ],
+        });
+
+        let old_format: FullBytestreamConfig = serde_json::from_value(old_format).unwrap();
+        let new_format: FullBytestreamConfig = serde_json::from_value(new_format).unwrap();
+
+        assert_eq!(old_format, new_format);
+
+        logs_assert(|lines: &[&str]| {
+            if lines.len() != 1 {
+                return Err(format!("Expected 1 log line, got: {lines:?}"));
+            }
+            let line = lines[0];
+            // TODO(palfrey): we should be checking the whole thing, but tracing-test is broken with multi-line items
+            // See https://github.com/dbrgn/tracing-test/issues/48
+            assert!(line.ends_with("WARNING: Using deprecated map format for services. Please migrate to the new array format:"));
+            Ok(())
+        });
+    }
 }

nativelink-config/src/cas_server.rs

@@ -176,7 +176,12 @@ pub struct PushConfig {
     pub read_only: bool,
 }
 
-#[derive(Deserialize, Serialize, Debug, Default)]
+// From https://github.com/serde-rs/serde/issues/818#issuecomment-287438544
+fn default<T: Default + PartialEq>(t: &T) -> bool {
+    *t == Default::default()
+}
+
+#[derive(Deserialize, Serialize, Debug, Default, PartialEq, Eq)]
 #[serde(deny_unknown_fields)]
 pub struct ByteStreamConfig {
     /// Name of the store in the "stores" configuration.
@@ -188,7 +193,11 @@ pub struct ByteStreamConfig {
     ///
     ///
     /// Default: 64KiB
-    #[serde(default, deserialize_with = "convert_data_size_with_shellexpand")]
+    #[serde(
+        default,
+        deserialize_with = "convert_data_size_with_shellexpand",
+        skip_serializing_if = "default"
+    )]
     pub max_bytes_per_stream: usize,
 
     /// In the event a client disconnects while uploading a blob, we will hold
@@ -197,7 +206,11 @@ pub struct ByteStreamConfig {
     /// the same blob.
     ///
     /// Default: 10 (seconds)
-    #[serde(default, deserialize_with = "convert_duration_with_shellexpand")]
+    #[serde(
+        default,
+        deserialize_with = "convert_duration_with_shellexpand",
+        skip_serializing_if = "default"
+    )]
     pub persist_stream_on_disconnect_timeout: usize,
 }
 
@@ -208,11 +221,23 @@ pub struct ByteStreamConfig {
 #[serde(deny_unknown_fields)]
 pub struct OldByteStreamConfig {
     pub cas_stores: HashMap<InstanceName, StoreRefName>,
-    #[serde(default, deserialize_with = "convert_data_size_with_shellexpand")]
+    #[serde(
+        default,
+        deserialize_with = "convert_data_size_with_shellexpand",
+        skip_serializing_if = "default"
+    )]
     pub max_bytes_per_stream: usize,
-    #[serde(default, deserialize_with = "convert_data_size_with_shellexpand")]
+    #[serde(
+        default,
+        deserialize_with = "convert_data_size_with_shellexpand",
+        skip_serializing_if = "default"
+    )]
     pub max_decoding_message_size: usize,
-    #[serde(default, deserialize_with = "convert_duration_with_shellexpand")]
+    #[serde(
+        default,
+        deserialize_with = "convert_duration_with_shellexpand",
+        skip_serializing_if = "default"
+    )]
     pub persist_stream_on_disconnect_timeout: usize,
 }