mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-25 05:26:50 +00:00
refactor: return embedded identity from startup (#3666)
This commit is contained in:
@@ -5,17 +5,15 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
|||||||
## Current Context
|
## Current Context
|
||||||
|
|
||||||
- Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660)
|
- Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660)
|
||||||
- Branch: `overtrue/arch-embedded-builder-shell`
|
- Branch: `overtrue/arch-embedded-startup-identity-flow`
|
||||||
- Baseline: completed `R-042/R-043`.
|
- Baseline: completed `R-044/R-045`.
|
||||||
- Stacked on: embedded handle boundary slice.
|
- Stacked on: embedded builder shell slice.
|
||||||
- PR type for this branch: `pure-move`
|
- PR type for this branch: `pure-move`
|
||||||
- Runtime behavior changes: none.
|
- Runtime behavior changes: none.
|
||||||
- Rust code changes: keep the public embedded builder as a thin shell over
|
- Rust code changes: hide embedded startup argument fields behind crate-only
|
||||||
crate-only startup arguments and move embedded port probing behind
|
setters and return public server identity from the startup result.
|
||||||
startup-server helpers.
|
- CI/script changes: none.
|
||||||
- CI/script changes: make the unsafe allowance checker avoid a local
|
- Docs changes: record the embedded startup identity slices.
|
||||||
`pipefail` false positive after `rg -q` matches nearby `SAFETY:` comments.
|
|
||||||
- Docs changes: record the embedded builder shell slices.
|
|
||||||
|
|
||||||
## Phase 0 Tasks
|
## Phase 0 Tasks
|
||||||
|
|
||||||
@@ -2367,6 +2365,29 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
|||||||
migration/layer guards, formatting, diff hygiene, Rust risk scan, branch
|
migration/layer guards, formatting, diff hygiene, Rust risk scan, branch
|
||||||
freshness check, pre-commit quality gate, and three-expert review.
|
freshness check, pre-commit quality gate, and three-expert review.
|
||||||
|
|
||||||
|
- [x] `R-046` Encapsulate embedded startup argument mutation.
|
||||||
|
- Do: hide embedded startup argument fields behind crate-only setter methods
|
||||||
|
used by the public builder.
|
||||||
|
- Acceptance: public builder fluent methods still apply the same address,
|
||||||
|
credential, region, and volume values in the same order.
|
||||||
|
- Must preserve: builder method signatures, default values, `volume` append
|
||||||
|
semantics, `volumes` replacement semantics, and startup input ownership.
|
||||||
|
- Verification: focused startup-embedded and embedded checks, RustFS lib
|
||||||
|
check, migration/layer guards, formatting, diff hygiene, Rust risk scan,
|
||||||
|
branch freshness check, pre-commit quality gate, and three-expert review.
|
||||||
|
|
||||||
|
- [x] `R-047` Return embedded server identity from startup result.
|
||||||
|
- Do: let the crate-only startup result carry the access key, secret key, and
|
||||||
|
region used by the public server handle.
|
||||||
|
- Acceptance: public server accessors still expose the configured values
|
||||||
|
without the public builder duplicating identity assembly.
|
||||||
|
- Must preserve: startup error mapping, readiness logging order, endpoint
|
||||||
|
address handling, shutdown handle ownership, and no public API signature
|
||||||
|
changes.
|
||||||
|
- Verification: focused startup-embedded and embedded checks, RustFS lib
|
||||||
|
check, migration/layer guards, formatting, diff hygiene, Rust risk scan,
|
||||||
|
branch freshness check, pre-commit quality gate, and three-expert review.
|
||||||
|
|
||||||
- [x] `E-001/E-SET-001` Add ECStore layout skeleton and set-layout boundary.
|
- [x] `E-001/E-SET-001` Add ECStore layout skeleton and set-layout boundary.
|
||||||
- Do: create the ECStore internal layout ownership buckets and pin static set
|
- Do: create the ECStore internal layout ownership buckets and pin static set
|
||||||
layout versus runtime `Sets`/`SetDisks` orchestration boundaries before any
|
layout versus runtime `Sets`/`SetDisks` orchestration boundaries before any
|
||||||
@@ -2609,21 +2630,35 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
|||||||
|
|
||||||
## Next PRs
|
## Next PRs
|
||||||
|
|
||||||
1. `pure-move`: continue pruning residual embedded builder/startup-only
|
1. `pure-move`: continue pruning residual embedded handle/startup-only
|
||||||
boundaries after the embedded builder shell slice lands.
|
boundaries after the embedded startup identity slice lands.
|
||||||
|
|
||||||
## Pre-Push Review Log
|
## Pre-Push Review Log
|
||||||
|
|
||||||
| Expert | Status | Notes |
|
| Expert | Status | Notes |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
| Quality/architecture | passed | R-044 and R-045 keep the public embedded builder as a thin shell over crate-only startup args and move port probing behind startup-server helpers without widening the public API. |
|
| Quality/architecture | passed | R-046 and R-047 hide startup argument mutation behind crate-only methods and move public server identity assembly into the startup result without widening the public API. |
|
||||||
| Migration preservation | passed | Builder defaults, fluent setters, volume replacement, credential/region accessors, public port helper signature, and localhost ephemeral port behavior stay unchanged. |
|
| Migration preservation | passed | Builder setter semantics, public server identity accessors, endpoint handling, startup error mapping, and shutdown ownership stay unchanged. |
|
||||||
| Testing/verification | passed | Focused embedded/startup-embedded/startup-server checks, RustFS lib check, architecture/layer/unsafe guards, formatting, diff hygiene, Rust risk scan, and pre-commit gate passed. |
|
| Testing/verification | passed | Focused startup-embedded/embedded checks, RustFS lib check, architecture/layer/unsafe guards, formatting, diff hygiene, Rust risk scan, and pre-commit gate passed. |
|
||||||
|
|
||||||
## Verification Notes
|
## Verification Notes
|
||||||
|
|
||||||
Passed before push:
|
Passed before push:
|
||||||
|
|
||||||
|
- Issue #660 R-046/R-047 current slice:
|
||||||
|
- `cargo test -p rustfs --lib startup_embedded -- --nocapture`: passed.
|
||||||
|
- `cargo test -p rustfs --lib embedded -- --nocapture`: passed.
|
||||||
|
- `cargo check -p rustfs --lib`: passed.
|
||||||
|
- `cargo fmt --all --check`: passed.
|
||||||
|
- `git diff --check`: passed.
|
||||||
|
- `./scripts/check_architecture_migration_rules.sh`: passed.
|
||||||
|
- `./scripts/check_layer_dependencies.sh`: passed.
|
||||||
|
- `./scripts/check_unsafe_code_allowances.sh`: passed.
|
||||||
|
- Rust risk scan on changed Rust files: passed; matches were limited to
|
||||||
|
existing embedded doc examples.
|
||||||
|
- `make pre-commit`: passed; nextest ran 6324 tests with 6324 passed and
|
||||||
|
111 skipped, and doctests passed.
|
||||||
|
|
||||||
- Issue #660 R-044/R-045 current slice:
|
- Issue #660 R-044/R-045 current slice:
|
||||||
- `cargo test -p rustfs --lib embedded -- --nocapture`: passed.
|
- `cargo test -p rustfs --lib embedded -- --nocapture`: passed.
|
||||||
- `cargo test -p rustfs --lib startup_embedded -- --nocapture`: passed.
|
- `cargo test -p rustfs --lib startup_embedded -- --nocapture`: passed.
|
||||||
|
|||||||
+10
-14
@@ -160,25 +160,25 @@ impl RustFSServerBuilder {
|
|||||||
/// [`build`](Self::build), but that is too late for the earlier
|
/// [`build`](Self::build), but that is too late for the earlier
|
||||||
/// initialization that depends on the configured address.
|
/// initialization that depends on the configured address.
|
||||||
pub fn address(mut self, addr: impl Into<String>) -> Self {
|
pub fn address(mut self, addr: impl Into<String>) -> Self {
|
||||||
self.startup_args.address = addr.into();
|
self.startup_args.set_address(addr.into());
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Set the S3 access key (default: `"rustfsadmin"`).
|
/// Set the S3 access key (default: `"rustfsadmin"`).
|
||||||
pub fn access_key(mut self, key: impl Into<String>) -> Self {
|
pub fn access_key(mut self, key: impl Into<String>) -> Self {
|
||||||
self.startup_args.access_key = key.into();
|
self.startup_args.set_access_key(key.into());
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Set the S3 secret key (default: `"rustfsadmin"`).
|
/// Set the S3 secret key (default: `"rustfsadmin"`).
|
||||||
pub fn secret_key(mut self, key: impl Into<String>) -> Self {
|
pub fn secret_key(mut self, key: impl Into<String>) -> Self {
|
||||||
self.startup_args.secret_key = key.into();
|
self.startup_args.set_secret_key(key.into());
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Set the AWS region (default: `"us-east-1"`).
|
/// Set the AWS region (default: `"us-east-1"`).
|
||||||
pub fn region(mut self, region: impl Into<String>) -> Self {
|
pub fn region(mut self, region: impl Into<String>) -> Self {
|
||||||
self.startup_args.region = region.into();
|
self.startup_args.set_region(region.into());
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -187,13 +187,13 @@ impl RustFSServerBuilder {
|
|||||||
/// If no volumes are added, a temporary directory with a single drive is
|
/// If no volumes are added, a temporary directory with a single drive is
|
||||||
/// created automatically (and cleaned up on [`RustFSServer::shutdown`]).
|
/// created automatically (and cleaned up on [`RustFSServer::shutdown`]).
|
||||||
pub fn volume(mut self, path: impl Into<String>) -> Self {
|
pub fn volume(mut self, path: impl Into<String>) -> Self {
|
||||||
self.startup_args.volumes.push(path.into());
|
self.startup_args.push_volume(path.into());
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Set multiple volume paths at once, replacing any previously set volumes.
|
/// Set multiple volume paths at once, replacing any previously set volumes.
|
||||||
pub fn volumes(mut self, paths: Vec<String>) -> Self {
|
pub fn volumes(mut self, paths: Vec<String>) -> Self {
|
||||||
self.startup_args.volumes = paths;
|
self.startup_args.set_volumes(paths);
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -212,17 +212,13 @@ impl RustFSServerBuilder {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async fn do_build(&mut self) -> Result<RustFSServer, ServerError> {
|
async fn do_build(&mut self) -> Result<RustFSServer, ServerError> {
|
||||||
let startup_args = self.startup_args.clone();
|
let started = run_embedded_startup(self.startup_args.clone()).await?;
|
||||||
let access_key = startup_args.access_key.clone();
|
|
||||||
let secret_key = startup_args.secret_key.clone();
|
|
||||||
let region = startup_args.region.clone();
|
|
||||||
let started = run_embedded_startup(startup_args).await?;
|
|
||||||
|
|
||||||
let server = RustFSServer {
|
let server = RustFSServer {
|
||||||
address: started.bound_addr,
|
address: started.bound_addr,
|
||||||
access_key,
|
access_key: started.access_key,
|
||||||
secret_key,
|
secret_key: started.secret_key,
|
||||||
region,
|
region: started.region,
|
||||||
shutdown_handle: Some(started.shutdown_handle),
|
shutdown_handle: Some(started.shutdown_handle),
|
||||||
cancel_token: started.cancel_token,
|
cancel_token: started.cancel_token,
|
||||||
temp_dir: started.temp_dir,
|
temp_dir: started.temp_dir,
|
||||||
|
|||||||
@@ -28,11 +28,11 @@ use tokio_util::sync::CancellationToken;
|
|||||||
|
|
||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
pub(crate) struct EmbeddedStartupArgs {
|
pub(crate) struct EmbeddedStartupArgs {
|
||||||
pub address: String,
|
address: String,
|
||||||
pub access_key: String,
|
access_key: String,
|
||||||
pub secret_key: String,
|
secret_key: String,
|
||||||
pub volumes: Vec<String>,
|
volumes: Vec<String>,
|
||||||
pub region: String,
|
region: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl EmbeddedStartupArgs {
|
impl EmbeddedStartupArgs {
|
||||||
@@ -45,10 +45,37 @@ impl EmbeddedStartupArgs {
|
|||||||
region: rustfs_config::RUSTFS_REGION.to_string(),
|
region: rustfs_config::RUSTFS_REGION.to_string(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) fn set_address(&mut self, address: String) {
|
||||||
|
self.address = address;
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn set_access_key(&mut self, access_key: String) {
|
||||||
|
self.access_key = access_key;
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn set_secret_key(&mut self, secret_key: String) {
|
||||||
|
self.secret_key = secret_key;
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn set_region(&mut self, region: String) {
|
||||||
|
self.region = region;
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn push_volume(&mut self, path: String) {
|
||||||
|
self.volumes.push(path);
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn set_volumes(&mut self, paths: Vec<String>) {
|
||||||
|
self.volumes = paths;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) struct EmbeddedStartedServer {
|
pub(crate) struct EmbeddedStartedServer {
|
||||||
pub bound_addr: SocketAddr,
|
pub bound_addr: SocketAddr,
|
||||||
|
pub access_key: String,
|
||||||
|
pub secret_key: String,
|
||||||
|
pub region: String,
|
||||||
pub shutdown_handle: ShutdownHandle,
|
pub shutdown_handle: ShutdownHandle,
|
||||||
pub cancel_token: CancellationToken,
|
pub cancel_token: CancellationToken,
|
||||||
pub temp_dir: Option<PathBuf>,
|
pub temp_dir: Option<PathBuf>,
|
||||||
@@ -68,13 +95,24 @@ impl From<io::Error> for EmbeddedStartupError {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) async fn run_embedded_startup(args: EmbeddedStartupArgs) -> Result<EmbeddedStartedServer, EmbeddedStartupError> {
|
pub(crate) async fn run_embedded_startup(args: EmbeddedStartupArgs) -> Result<EmbeddedStartedServer, EmbeddedStartupError> {
|
||||||
|
let EmbeddedStartupArgs {
|
||||||
|
address,
|
||||||
|
access_key,
|
||||||
|
secret_key,
|
||||||
|
volumes,
|
||||||
|
region,
|
||||||
|
} = args;
|
||||||
|
let server_access_key = access_key.clone();
|
||||||
|
let server_secret_key = secret_key.clone();
|
||||||
|
let server_region = region.clone();
|
||||||
|
|
||||||
// Build is allowed to fail before irreversible global initialization
|
// Build is allowed to fail before irreversible global initialization
|
||||||
// (for example on temporary I/O or directory setup errors), and in that
|
// (for example on temporary I/O or directory setup errors), and in that
|
||||||
// case callers can retry.
|
// case callers can retry.
|
||||||
let mut startup_guard = EmbeddedStartupGuard::new();
|
let mut startup_guard = EmbeddedStartupGuard::new();
|
||||||
|
|
||||||
let EmbeddedStartupConfig { config, temp_dir_guard } =
|
let EmbeddedStartupConfig { config, temp_dir_guard } =
|
||||||
prepare_embedded_startup_config(args.address, args.access_key, args.secret_key, args.volumes, args.region)
|
prepare_embedded_startup_config(address, access_key, secret_key, volumes, region)
|
||||||
.await
|
.await
|
||||||
.map_err(init_error)?;
|
.map_err(init_error)?;
|
||||||
|
|
||||||
@@ -134,6 +172,9 @@ pub(crate) async fn run_embedded_startup(args: EmbeddedStartupArgs) -> Result<Em
|
|||||||
|
|
||||||
Ok(EmbeddedStartedServer {
|
Ok(EmbeddedStartedServer {
|
||||||
bound_addr,
|
bound_addr,
|
||||||
|
access_key: server_access_key,
|
||||||
|
secret_key: server_secret_key,
|
||||||
|
region: server_region,
|
||||||
shutdown_handle,
|
shutdown_handle,
|
||||||
cancel_token,
|
cancel_token,
|
||||||
temp_dir: temp_dir_guard.map(|guard| guard.keep()),
|
temp_dir: temp_dir_guard.map(|guard| guard.keep()),
|
||||||
@@ -158,4 +199,22 @@ mod tests {
|
|||||||
assert_eq!(args.region, rustfs_config::RUSTFS_REGION);
|
assert_eq!(args.region, rustfs_config::RUSTFS_REGION);
|
||||||
assert!(args.volumes.is_empty());
|
assert!(args.volumes.is_empty());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn embedded_startup_args_setters_preserve_public_builder_inputs() {
|
||||||
|
let mut args = EmbeddedStartupArgs::new_default();
|
||||||
|
|
||||||
|
args.set_address("127.0.0.1:9100".to_string());
|
||||||
|
args.set_access_key("custom-access".to_string());
|
||||||
|
args.set_secret_key("custom-secret".to_string());
|
||||||
|
args.set_region("test-region".to_string());
|
||||||
|
args.push_volume("/tmp/one".to_string());
|
||||||
|
args.set_volumes(vec!["/tmp/two".to_string(), "/tmp/three".to_string()]);
|
||||||
|
|
||||||
|
assert_eq!(args.address, "127.0.0.1:9100");
|
||||||
|
assert_eq!(args.access_key, "custom-access");
|
||||||
|
assert_eq!(args.secret_key, "custom-secret");
|
||||||
|
assert_eq!(args.region, "test-region");
|
||||||
|
assert_eq!(args.volumes, vec!["/tmp/two", "/tmp/three"]);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user