Skip to content

[bundles] Add range request support to other support bundle APIs #8202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
May 23, 2025
Merged

[bundles] Add range request support to other support bundle APIs #8202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b3580dd
Update to dropshot 0.16.1
smklein May 16, 2025
c5a9fea
workspace hack
smklein May 16, 2025
62a3561
[wip] Working towards range requests for download API
smklein May 16, 2025
27d1af2
Proxying range requests into sled agent
smklein May 19, 2025
52ccb16
Merge branch 'main' into dropshot-update
smklein May 20, 2025
4bda871
Merge branch 'dropshot-update' into range-request-sb
smklein May 20, 2025
71f9fd1
add tests
smklein May 21, 2025
0b80eb6
Upstream dropshot, omdb, openapi spec internal/external API
smklein May 21, 2025
54e9da1
clippy
smklein May 21, 2025
7a714e6
rust docs
smklein May 21, 2025
761d5a0
feedback
smklein May 21, 2025
4896684
doctest
smklein May 22, 2025
2128d79
wip
smklein May 22, 2025
a4b0bf6
openapi
smklein May 22, 2025
222ccc6
Merge branch 'range-request-sb' into more-rr
smklein May 22, 2025
1343cf0
[bundles] Add range request support to other support bundle download …
smklein May 22, 2025
50160a8
More text
smklein May 22, 2025
7d946ae
Merge branch 'range-request-sb' into more-rr
smklein May 22, 2025
1ed79bf
Merge branch 'main' into range-request-sb
smklein May 22, 2025
2e3dfea
Merge branch 'range-request-sb' into more-rr
smklein May 22, 2025
a3a017e
doc changes again
smklein May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions dev-tools/omdb/src/bin/omdb/nexus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3934,7 +3934,7 @@ async fn cmd_nexus_support_bundles_download(
args: &SupportBundleDownloadArgs,
) -> Result<(), anyhow::Error> {
let total_length = client
.support_bundle_head(args.id.as_untyped_uuid())
.support_bundle_head(args.id.as_untyped_uuid(), None)
.await?
.content_length()
.ok_or_else(|| anyhow::anyhow!("No content length"))?;
Expand Down Expand Up @@ -3972,7 +3972,7 @@ async fn cmd_nexus_support_bundles_get_index(
args: &SupportBundleIndexArgs,
) -> Result<(), anyhow::Error> {
let stream = client
.support_bundle_index(args.id.as_untyped_uuid())
.support_bundle_index(args.id.as_untyped_uuid(), None)
.await
.with_context(|| {
format!("downloading support bundle index {}", args.id)
Expand All @@ -3995,6 +3995,7 @@ async fn cmd_nexus_support_bundles_get_file(
.support_bundle_download_file(
args.id.as_untyped_uuid(),
args.path.as_str(),
None,
)
.await
.with_context(|| {
Expand Down
6 changes: 3 additions & 3 deletions dev-tools/omdb/src/bin/omdb/support_bundle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ impl<'a> StreamedFile<'a> {
//
// This means that we would potentially want to restart the stream with a different position.
async fn start_stream(&mut self) -> anyhow::Result<()> {
// TODO: Add range headers, for range requests? Though this
// will require adding support to Progenitor + Nexus too.
// TODO: Add range headers, for range requests?
let stream = self
.client
.support_bundle_download_file(
self.id.as_untyped_uuid(),
self.path.as_str(),
None,
)
.await
.with_context(|| {
Expand Down Expand Up @@ -142,7 +142,7 @@ impl<'c> SupportBundleAccessor for InternalApiAccess<'c> {
async fn get_index(&self) -> anyhow::Result<SupportBundleIndex> {
let stream = self
.client
.support_bundle_index(self.id.as_untyped_uuid())
.support_bundle_index(self.id.as_untyped_uuid(), None)
.await
.with_context(|| {
format!("downloading support bundle index {}", self.id)
Expand Down
4 changes: 4 additions & 0 deletions nexus/external-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3157,6 +3157,7 @@ pub trait NexusExternalApi {
}]
async fn support_bundle_index(
rqctx: RequestContext<Self::Context>,
headers: Header<headers::RangeRequest>,
path_params: Path<params::SupportBundlePath>,
) -> Result<Response<Body>, HttpError>;

Expand All @@ -3180,6 +3181,7 @@ pub trait NexusExternalApi {
}]
async fn support_bundle_download_file(
rqctx: RequestContext<Self::Context>,
headers: Header<headers::RangeRequest>,
path_params: Path<params::SupportBundleFilePath>,
) -> Result<Response<Body>, HttpError>;

Expand All @@ -3191,6 +3193,7 @@ pub trait NexusExternalApi {
}]
async fn support_bundle_head(
rqctx: RequestContext<Self::Context>,
headers: Header<headers::RangeRequest>,
path_params: Path<params::SupportBundlePath>,
) -> Result<Response<Body>, HttpError>;

Expand All @@ -3202,6 +3205,7 @@ pub trait NexusExternalApi {
}]
async fn support_bundle_head_file(
rqctx: RequestContext<Self::Context>,
headers: Header<headers::RangeRequest>,
path_params: Path<params::SupportBundleFilePath>,
) -> Result<Response<Body>, HttpError>;

Expand Down
4 changes: 4 additions & 0 deletions nexus/internal-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ pub trait NexusInternalApi {
}]
async fn support_bundle_index(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<params::SupportBundlePath>,
) -> Result<Response<Body>, HttpError>;

Expand All @@ -591,6 +592,7 @@ pub trait NexusInternalApi {
}]
async fn support_bundle_download_file(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<params::SupportBundleFilePath>,
) -> Result<Response<Body>, HttpError>;

Expand All @@ -601,6 +603,7 @@ pub trait NexusInternalApi {
}]
async fn support_bundle_head(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<params::SupportBundlePath>,
) -> Result<Response<Body>, HttpError>;

Expand All @@ -611,6 +614,7 @@ pub trait NexusInternalApi {
}]
async fn support_bundle_head_file(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<params::SupportBundleFilePath>,
) -> Result<Response<Body>, HttpError>;

Expand Down
5 changes: 5 additions & 0 deletions nexus/src/app/support_bundles.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ impl super::Nexus {
&ZpoolUuid::from(bundle.zpool_id),
&DatasetUuid::from(bundle.dataset_id),
&SupportBundleUuid::from(bundle.id),
range,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a minor change, but this part is actually critical: for all these APIs, where Nexus was proxying requests to the sled agent over progenitor APIs, it was previously unable to forward range request headers.

Now, with the new progenitor work, it can do so, for all bundle-related APIs.

(This is wiring up the same thing we did already in #8183 , but for other more granular bundle access)

)
.await
}
Expand All @@ -142,6 +143,7 @@ impl super::Nexus {
&ZpoolUuid::from(bundle.zpool_id),
&DatasetUuid::from(bundle.dataset_id),
&SupportBundleUuid::from(bundle.id),
range,
)
.await
}
Expand All @@ -151,6 +153,7 @@ impl super::Nexus {
&ZpoolUuid::from(bundle.zpool_id),
&DatasetUuid::from(bundle.dataset_id),
&SupportBundleUuid::from(bundle.id),
range,
)
.await
}
Expand All @@ -161,6 +164,7 @@ impl super::Nexus {
&DatasetUuid::from(bundle.dataset_id),
&SupportBundleUuid::from(bundle.id),
&file_path,
range,
)
.await
}
Expand All @@ -171,6 +175,7 @@ impl super::Nexus {
&DatasetUuid::from(bundle.dataset_id),
&SupportBundleUuid::from(bundle.id),
&file_path,
range,
)
.await
}
Expand Down
25 changes: 20 additions & 5 deletions nexus/src/external_api/http_entrypoints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ use propolis_client::support::tungstenite::protocol::{
CloseFrame, Role as WebSocketRole,
};
use range_requests::PotentialRange;
use range_requests::RequestContextEx;
use ref_cast::RefCast;

type NexusApiDescription = ApiDescription<ApiContext>;
Expand Down Expand Up @@ -7078,6 +7077,7 @@ impl NexusExternalApi for NexusExternalApiImpl {

async fn support_bundle_index(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<params::SupportBundlePath>,
) -> Result<Response<Body>, HttpError> {
let apictx = rqctx.context();
Expand All @@ -7088,7 +7088,10 @@ impl NexusExternalApi for NexusExternalApiImpl {
crate::context::op_context_for_external_api(&rqctx).await?;

let head = false;
let range = rqctx.range();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see here, we actually were parsing the range request headers before - just not visibly through the API spec.

let range = headers
.into_inner()
.range
.map(|r| PotentialRange::new(r.as_bytes()));

let body = nexus
.support_bundle_download(
Expand Down Expand Up @@ -7146,6 +7149,7 @@ impl NexusExternalApi for NexusExternalApiImpl {

async fn support_bundle_download_file(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<params::SupportBundleFilePath>,
) -> Result<Response<Body>, HttpError> {
let apictx = rqctx.context();
Expand All @@ -7155,7 +7159,10 @@ impl NexusExternalApi for NexusExternalApiImpl {
let opctx =
crate::context::op_context_for_external_api(&rqctx).await?;
let head = false;
let range = rqctx.range();
let range = headers
.into_inner()
.range
.map(|r| PotentialRange::new(r.as_bytes()));

let body = nexus
.support_bundle_download(
Expand All @@ -7177,6 +7184,7 @@ impl NexusExternalApi for NexusExternalApiImpl {

async fn support_bundle_head(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<params::SupportBundlePath>,
) -> Result<Response<Body>, HttpError> {
let apictx = rqctx.context();
Expand All @@ -7186,7 +7194,10 @@ impl NexusExternalApi for NexusExternalApiImpl {
let opctx =
crate::context::op_context_for_external_api(&rqctx).await?;
let head = true;
let range = rqctx.range();
let range = headers
.into_inner()
.range
.map(|r| PotentialRange::new(r.as_bytes()));

let body = nexus
.support_bundle_download(
Expand All @@ -7208,6 +7219,7 @@ impl NexusExternalApi for NexusExternalApiImpl {

async fn support_bundle_head_file(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<params::SupportBundleFilePath>,
) -> Result<Response<Body>, HttpError> {
let apictx = rqctx.context();
Expand All @@ -7217,7 +7229,10 @@ impl NexusExternalApi for NexusExternalApiImpl {
let opctx =
crate::context::op_context_for_external_api(&rqctx).await?;
let head = true;
let range = rqctx.range();
let range = headers
.into_inner()
.range
.map(|r| PotentialRange::new(r.as_bytes()));

let body = nexus
.support_bundle_download(
Expand Down
25 changes: 20 additions & 5 deletions nexus/src/internal_api/http_entrypoints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::InstanceUuid;
use omicron_uuid_kinds::SupportBundleUuid;
use range_requests::PotentialRange;
use range_requests::RequestContextEx;
use std::collections::BTreeMap;

type NexusApiDescription = ApiDescription<ApiContext>;
Expand Down Expand Up @@ -997,6 +996,7 @@ impl NexusInternalApi for NexusInternalApiImpl {

async fn support_bundle_index(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<SupportBundlePath>,
) -> Result<Response<Body>, HttpError> {
let apictx = rqctx.context();
Expand All @@ -1007,7 +1007,10 @@ impl NexusInternalApi for NexusInternalApiImpl {
crate::context::op_context_for_internal_api(&rqctx).await;

let head = false;
let range = rqctx.range();
let range = headers
.into_inner()
.range
.map(|r| PotentialRange::new(r.as_bytes()));

let body = nexus
.support_bundle_download(
Expand Down Expand Up @@ -1065,6 +1068,7 @@ impl NexusInternalApi for NexusInternalApiImpl {

async fn support_bundle_download_file(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<SupportBundleFilePath>,
) -> Result<Response<Body>, HttpError> {
let apictx = rqctx.context();
Expand All @@ -1074,7 +1078,10 @@ impl NexusInternalApi for NexusInternalApiImpl {
let opctx =
crate::context::op_context_for_internal_api(&rqctx).await;
let head = false;
let range = rqctx.range();
let range = headers
.into_inner()
.range
.map(|r| PotentialRange::new(r.as_bytes()));

let body = nexus
.support_bundle_download(
Expand All @@ -1096,6 +1103,7 @@ impl NexusInternalApi for NexusInternalApiImpl {

async fn support_bundle_head(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<SupportBundlePath>,
) -> Result<Response<Body>, HttpError> {
let apictx = rqctx.context();
Expand All @@ -1105,7 +1113,10 @@ impl NexusInternalApi for NexusInternalApiImpl {
let opctx =
crate::context::op_context_for_internal_api(&rqctx).await;
let head = true;
let range = rqctx.range();
let range = headers
.into_inner()
.range
.map(|r| PotentialRange::new(r.as_bytes()));

let body = nexus
.support_bundle_download(
Expand All @@ -1127,6 +1138,7 @@ impl NexusInternalApi for NexusInternalApiImpl {

async fn support_bundle_head_file(
rqctx: RequestContext<Self::Context>,
headers: Header<RangeRequest>,
path_params: Path<SupportBundleFilePath>,
) -> Result<Response<Body>, HttpError> {
let apictx = rqctx.context();
Expand All @@ -1136,7 +1148,10 @@ impl NexusInternalApi for NexusInternalApiImpl {
let opctx =
crate::context::op_context_for_internal_api(&rqctx).await;
let head = true;
let range = rqctx.range();
let range = headers
.into_inner()
.range
.map(|r| PotentialRange::new(r.as_bytes()));

let body = nexus
.support_bundle_download(
Expand Down
32 changes: 32 additions & 0 deletions openapi/nexus-internal.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,14 @@
"summary": "Download the metadata of a support bundle",
"operationId": "support_bundle_head",
"parameters": [
{
"in": "header",
"name": "range",
"description": "A request to access a portion of the resource, such as:\n\n```text bytes=0-499 ```\n\n<https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Range>",
"schema": {
"type": "string"
}
},
{
"in": "path",
"name": "bundle_id",
Expand Down Expand Up @@ -953,6 +961,14 @@
"summary": "Download a file within a support bundle",
"operationId": "support_bundle_download_file",
"parameters": [
{
"in": "header",
"name": "range",
"description": "A request to access a portion of the resource, such as:\n\n```text bytes=0-499 ```\n\n<https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Range>",
"schema": {
"type": "string"
}
},
{
"in": "path",
"name": "bundle_id",
Expand Down Expand Up @@ -988,6 +1004,14 @@
"summary": "Download the metadata of a file within the support bundle",
"operationId": "support_bundle_head_file",
"parameters": [
{
"in": "header",
"name": "range",
"description": "A request to access a portion of the resource, such as:\n\n```text bytes=0-499 ```\n\n<https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Range>",
"schema": {
"type": "string"
}
},
{
"in": "path",
"name": "bundle_id",
Expand Down Expand Up @@ -1025,6 +1049,14 @@
"summary": "Download the index of a support bundle",
"operationId": "support_bundle_index",
"parameters": [
{
"in": "header",
"name": "range",
"description": "A request to access a portion of the resource, such as:\n\n```text bytes=0-499 ```\n\n<https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Range>",
"schema": {
"type": "string"
}
},
{
"in": "path",
"name": "bundle_id",
Expand Down
Loading
Loading