Remove ReqwestBlockIO and VolumeConstructionRequest::Url

As far as I can tell, nothing uses this anymore. With the exception of a pantry integration test. That test is not interested in the Reqwest part, and is just interested in a read-only parent with random data in it, so I changed that to use a tempfile read-only parent. Propolis will need to be updated. It has exactly one reference to `VolumeConstructionRequest::Url`: bin/propolis-server/src/lib/initializer.rs 385: VolumeConstructionRequest::Url { id, .. } => id.to_string(), This is in some code that's extracting the UUIDs out of all the VCR types, and also doesn't care about the Reqwest functionality.
oxidecomputer · Apr 20, 2024 · b97e434 · b97e434
1 parent 1ef72f3
commit b97e434
Show file tree

Hide file tree

Showing 10 changed files with 18 additions and 373 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crucible-client-types/src/lib.rs b/crucible-client-types/src/lib.rs
@@ -18,11 +18,6 @@ pub enum VolumeConstructionRequest {
         sub_volumes: Vec<VolumeConstructionRequest>,
         read_only_parent: Option<Box<VolumeConstructionRequest>>,
     },
-    Url {
-        id: Uuid,
-        block_size: u64,
-        url: String,
-    },
     Region {
         block_size: u64,
         blocks_per_extent: u64,

diff --git a/integration_tests/src/lib.rs b/integration_tests/src/lib.rs
@@ -19,6 +19,7 @@ mod test {
     use repair_client::Client;
     use sha2::Digest;
     use slog::{info, o, warn, Drain, Logger};
+    use std::io::Write;
     use tempfile::*;
     use tokio::sync::mpsc;
     use uuid::*;
@@ -746,82 +747,6 @@ mod test {
         Ok(())
     }
 
-    #[tokio::test]
-    async fn integration_test_url() -> Result<()> {
-        const BLOCK_SIZE: usize = 512;
-
-        let tds = TestDownstairsSet::small(false).await?;
-        let opts = tds.opts();
-
-        let server = Server::run();
-        server.expect(
-            Expectation::matching(request::method_path("GET", "/ff.raw"))
-                .times(1..)
-                .respond_with(status_code(200).body(vec![0xff; BLOCK_SIZE])),
-        );
-        server.expect(
-            Expectation::matching(request::method_path("HEAD", "/ff.raw"))
-                .times(1..)
-                .respond_with(status_code(200).append_header(
-                    "Content-Length",
-                    format!("{}", BLOCK_SIZE),
-                )),
-        );
-
-        let vcr: VolumeConstructionRequest =
-            VolumeConstructionRequest::Volume {
-                id: Uuid::new_v4(),
-                block_size: BLOCK_SIZE as u64,
-                sub_volumes: vec![VolumeConstructionRequest::Region {
-                    block_size: BLOCK_SIZE as u64,
-                    blocks_per_extent: tds.blocks_per_extent(),
-                    extent_count: tds.extent_count(),
-                    opts,
-                    gen: 1,
-                }],
-                read_only_parent: Some(Box::new(
-                    VolumeConstructionRequest::Volume {
-                        id: Uuid::new_v4(),
-                        block_size: BLOCK_SIZE as u64,
-                        sub_volumes: vec![VolumeConstructionRequest::Url {
-                            id: Uuid::new_v4(),
-                            block_size: BLOCK_SIZE as u64,
-                            url: server.url("/ff.raw").to_string(),
-                        }],
-                        read_only_parent: None,
-                    },
-                )),
-            };
-
-        let volume = Volume::construct(vcr, None, csl()).await?;
-        volume.activate().await?;
-
-        // Read one block: should be all 0xff
-        let mut buffer = Buffer::new(1, BLOCK_SIZE);
-        volume
-            .read(Block::new(0, BLOCK_SIZE.trailing_zeros()), &mut buffer)
-            .await?;
-
-        assert_eq!(vec![0xff; BLOCK_SIZE], &buffer[..]);
-
-        // Write one block full of 0x01
-        volume
-            .write(
-                Block::new(0, BLOCK_SIZE.trailing_zeros()),
-                BytesMut::from(vec![0x01; BLOCK_SIZE].as_slice()),
-            )
-            .await?;
-
-        // Read one block: should be all 0x01
-        let mut buffer = Buffer::new(1, BLOCK_SIZE);
-        volume
-            .read(Block::new(0, BLOCK_SIZE.trailing_zeros()), &mut buffer)
-            .await?;
-
-        assert_eq!(vec![0x01; BLOCK_SIZE], &buffer[..]);
-        Ok(())
-    }
-
     #[tokio::test]
     async fn integration_test_just_read() -> Result<()> {
         // Just do a read of a new volume.
@@ -4998,32 +4923,20 @@ mod test {
 
     #[tokio::test]
     async fn test_pantry_scrub() {
-        // Test scrubbing the OVMF image from a URL
-        // XXX httptest::Server does not support range requests, otherwise that
-        // should be used here instead.
+        // Test scrubbing random data from a file on disk.
 
-        let base_url = "https://oxide-omicron-build.s3.amazonaws.com";
-        let url = format!("{}/OVMF_CODE_20220922.fd", base_url);
+        const BLOCK_SIZE: usize = 512;
 
-        let data = {
-            let dur = std::time::Duration::from_secs(25);
-            let client = reqwest::ClientBuilder::new()
-                .connect_timeout(dur)
-                .timeout(dur)
-                .build()
-                .unwrap();
+        // There's no particular reason for this exact count other than, this
+        // test used to reference OVMF code over HTTP, and that OVMF code was
+        // this many 512-byte blocks in size.
+        const BLOCK_COUNT: usize = 3840;
 
-            client
-                .get(&url)
-                .send()
-                .await
-                .unwrap()
-                .bytes()
-                .await
-                .unwrap()
-        };
+        let mut data = vec![0u8; BLOCK_SIZE * BLOCK_COUNT];
+        rand::thread_rng().fill(data.as_mut_slice());
 
-        const BLOCK_SIZE: usize = 512;
+        let mut parent_file = NamedTempFile::new().unwrap();
+        parent_file.write_all(&data).unwrap();
 
         // Spin off three downstairs, build our Crucible struct (with a
         // read-only parent pointing to the random data above)
@@ -5033,11 +4946,12 @@ mod test {
 
         let volume_id = Uuid::new_v4();
         let rop_id = Uuid::new_v4();
-        let read_only_parent = Some(Box::new(VolumeConstructionRequest::Url {
-            id: rop_id,
-            block_size: BLOCK_SIZE as u64,
-            url: url.clone(),
-        }));
+        let read_only_parent =
+            Some(Box::new(VolumeConstructionRequest::File {
+                id: rop_id,
+                block_size: BLOCK_SIZE as u64,
+                path: parent_file.path().to_string_lossy().to_string(),
+            }));
 
         let vcr: VolumeConstructionRequest =
             VolumeConstructionRequest::Volume {

diff --git a/openapi/crucible-pantry.json b/openapi/crucible-pantry.json
@@ -687,35 +687,6 @@
               "type"
             ]
           },
-          {
-            "type": "object",
-            "properties": {
-              "block_size": {
-                "type": "integer",
-                "format": "uint64",
-                "minimum": 0
-              },
-              "id": {
-                "type": "string",
-                "format": "uuid"
-              },
-              "type": {
-                "type": "string",
-                "enum": [
-                  "url"
-                ]
-              },
-              "url": {
-                "type": "string"
-              }
-            },
-            "required": [
-              "block_size",
-              "id",
-              "type",
-              "url"
-            ]
-          },
           {
             "type": "object",
             "properties": {

diff --git a/tools/dtrace/README.md b/tools/dtrace/README.md
@@ -264,15 +264,6 @@ dtrace: script 'perfgw.d' matched 6 probes
        134217728 |
 ```
 
-## perf-reqwest.d
-This is a simple dtrace script that measures latency times for reads
-to a volume having a read only parent.  The time is from when the
-volume read only parent (ReqwestBlockIO) layer receives a read to when
-that read has been completed.
-```
-pfexec dtrace -s perf-reqwest.d
-```
-
 ## perf-vol.d
 This dtrace script measures latency times for IOs at the volume layer.
 This is essentially where an IO first lands in crucible and is measured

diff --git a/tools/dtrace/perf-reqwest.d b/tools/dtrace/perf-reqwest.d
diff --git a/upstairs/Cargo.toml b/upstairs/Cargo.toml
@@ -52,7 +52,6 @@ usdt.workspace = true
 uuid.workspace = true
 aes-gcm-siv.workspace = true
 rand_chacha.workspace = true
-reqwest.workspace = true
 crucible-workspace-hack.workspace = true
 nexus-client = { workspace = true, optional = true }
 internal-dns = { workspace = true, optional = true }