mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-24 13:16:28 +00:00
refactor: use relative storage compat consumers (#3727)
This commit is contained in:
@@ -5,17 +5,17 @@ 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-rustfs-local-compat-consumers`
|
- Branch: `overtrue/arch-storage-rpc-s3-local-compat-consumers`
|
||||||
- Baseline: completed `C-011/C-012/C-013/API-055/API-059/API-079/API-080/API-081/API-082/API-083/API-084/API-085/API-086/API-087/API-088/API-089/API-090/API-091/API-092/API-093/API-094/API-095/API-096/API-097/API-098/API-099/API-100/API-101/API-102/API-103/API-104/API-105/API-106/API-107/API-108/API-109/API-110`.
|
- Baseline: completed `C-011/C-012/C-013/API-055/API-059/API-079/API-080/API-081/API-082/API-083/API-084/API-085/API-086/API-087/API-088/API-089/API-090/API-091/API-092/API-093/API-094/API-095/API-096/API-097/API-098/API-099/API-100/API-101/API-102/API-103/API-104/API-105/API-106/API-107/API-108/API-109/API-110/API-111`.
|
||||||
- Based on: API-109 slice.
|
- Based on: API-110 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: collapse crate-qualified RustFS app/admin/storage local
|
- Rust code changes: collapse crate-qualified storage RPC and S3 API local
|
||||||
compatibility consumer paths into relative `super::`/`super::super::` paths.
|
compatibility consumer paths into relative `super::`/`super::super::` paths.
|
||||||
- CI/script changes: guard selected RustFS app/admin/storage local consumers
|
- CI/script changes: guard selected storage RPC and S3 API local consumers
|
||||||
against crate-qualified compatibility paths.
|
against crate-qualified compatibility paths.
|
||||||
- Docs changes: record the API-110 relative local compatibility consumer
|
- Docs changes: record the API-111 relative storage RPC/S3 API local
|
||||||
cleanup.
|
compatibility consumer cleanup.
|
||||||
|
|
||||||
## Phase 0 Tasks
|
## Phase 0 Tasks
|
||||||
|
|
||||||
@@ -571,6 +571,19 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
|||||||
- Verification: RustFS test-target compile coverage, local compatibility
|
- Verification: RustFS test-target compile coverage, local compatibility
|
||||||
consumer residual scan, migration and layer guards, formatting, diff
|
consumer residual scan, migration and layer guards, formatting, diff
|
||||||
hygiene, Rust risk scan, pre-commit quality gate, and three-expert review.
|
hygiene, Rust risk scan, pre-commit quality gate, and three-expert review.
|
||||||
|
- [x] `API-111` Collapse storage RPC and S3 API local compatibility consumers.
|
||||||
|
- Completed slice: replace crate-qualified storage RPC and S3 API local
|
||||||
|
compatibility consumers with relative owner paths.
|
||||||
|
- Acceptance: selected storage RPC and S3 API modules no longer point back to
|
||||||
|
local compatibility facades through crate-qualified paths; migration rules
|
||||||
|
reject regressions.
|
||||||
|
- Must preserve: internode RPC request handling, node service helper tests,
|
||||||
|
S3 list bucket output mapping, multipart listing output mapping, and ETag
|
||||||
|
helper behavior.
|
||||||
|
- Verification: RustFS test-target compile coverage, storage RPC/S3 API
|
||||||
|
local compatibility consumer residual scan, migration and layer guards,
|
||||||
|
formatting, diff hygiene, Rust risk scan, pre-commit quality gate, and
|
||||||
|
three-expert review.
|
||||||
- [x] `G-012` Inventory placement and repair invariants.
|
- [x] `G-012` Inventory placement and repair invariants.
|
||||||
- Acceptance:
|
- Acceptance:
|
||||||
[`placement-repair-invariants.md`](placement-repair-invariants.md) records
|
[`placement-repair-invariants.md`](placement-repair-invariants.md) records
|
||||||
@@ -3604,14 +3617,26 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
|||||||
|
|
||||||
| Expert | Status | Notes |
|
| Expert | Status | Notes |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
| Quality/architecture | pass | API-110 keeps selected app/admin/storage local compatibility consumers owner-relative by replacing crate-qualified compatibility paths with scoped `super::` or `super::super::` paths. |
|
| Quality/architecture | pass | API-111 keeps selected storage RPC and S3 API local compatibility consumers owner-relative by replacing crate-qualified compatibility paths with scoped `super::` or `super::super::` paths. |
|
||||||
| Migration preservation | pass | The new guard rejects crate-qualified app/admin/storage local compatibility consumer paths while preserving the same facade names and aliases. |
|
| Migration preservation | pass | The new guard rejects crate-qualified storage RPC/S3 API local compatibility consumer paths while preserving the same facade names and aliases. |
|
||||||
| Testing/verification | pass | Focused compile, local compatibility consumer residual scan, migration guard, layer guard, formatting, diff hygiene, risk scan, and full pre-commit passed. |
|
| Testing/verification | pass | Focused compile, storage RPC/S3 API local compatibility consumer residual scan, migration guard, layer guard, formatting, diff hygiene, risk scan, and full pre-commit passed. |
|
||||||
|
|
||||||
## Verification Notes
|
## Verification Notes
|
||||||
|
|
||||||
Passed before push:
|
Passed before push:
|
||||||
|
|
||||||
|
- Issue #660 API-111 current slice:
|
||||||
|
- `cargo check -p rustfs --tests`: passed.
|
||||||
|
- `cargo fmt --all`: passed.
|
||||||
|
- `cargo fmt --all --check`: passed.
|
||||||
|
- `git diff --check`: passed.
|
||||||
|
- `bash -n scripts/check_architecture_migration_rules.sh`: passed.
|
||||||
|
- `./scripts/check_architecture_migration_rules.sh`: passed.
|
||||||
|
- `./scripts/check_layer_dependencies.sh`: passed.
|
||||||
|
- Storage RPC/S3 API local compatibility consumer residual scan: passed.
|
||||||
|
- Rust risk scan on changed Rust files and guard script: passed.
|
||||||
|
- `make pre-commit`: passed.
|
||||||
|
|
||||||
- Issue #660 API-110 current slice:
|
- Issue #660 API-110 current slice:
|
||||||
- `cargo check -p rustfs --tests`: passed.
|
- `cargo check -p rustfs --tests`: passed.
|
||||||
- `cargo fmt --all`: passed.
|
- `cargo fmt --all`: passed.
|
||||||
|
|||||||
@@ -12,13 +12,13 @@
|
|||||||
// See the License for the specific language governing permissions and
|
// See the License for the specific language governing permissions and
|
||||||
// limitations under the License.
|
// limitations under the License.
|
||||||
|
|
||||||
|
use super::storage_compat::DEFAULT_READ_BUFFER_SIZE;
|
||||||
|
use super::storage_compat::StorageDiskRpcExt as _;
|
||||||
|
use super::storage_compat::WalkDirOptions;
|
||||||
|
use super::storage_compat::find_local_disk_by_ref;
|
||||||
|
use super::storage_compat::verify_rpc_signature;
|
||||||
use crate::server::RPC_PREFIX;
|
use crate::server::RPC_PREFIX;
|
||||||
use crate::storage::request_context::spawn_traced;
|
use crate::storage::request_context::spawn_traced;
|
||||||
use crate::storage::rpc::storage_compat::DEFAULT_READ_BUFFER_SIZE;
|
|
||||||
use crate::storage::rpc::storage_compat::StorageDiskRpcExt as _;
|
|
||||||
use crate::storage::rpc::storage_compat::WalkDirOptions;
|
|
||||||
use crate::storage::rpc::storage_compat::find_local_disk_by_ref;
|
|
||||||
use crate::storage::rpc::storage_compat::verify_rpc_signature;
|
|
||||||
use bytes::{Bytes, BytesMut};
|
use bytes::{Bytes, BytesMut};
|
||||||
use futures_util::TryStreamExt;
|
use futures_util::TryStreamExt;
|
||||||
use http::{HeaderMap, Method, Request, Response, StatusCode, Uri};
|
use http::{HeaderMap, Method, Request, Response, StatusCode, Uri};
|
||||||
|
|||||||
@@ -12,17 +12,17 @@
|
|||||||
// See the License for the specific language governing permissions and
|
// See the License for the specific language governing permissions and
|
||||||
// limitations under the License.
|
// limitations under the License.
|
||||||
|
|
||||||
use crate::admin::service::{
|
use super::storage_compat::{
|
||||||
config::{reload_dynamic_config_runtime_state, reload_runtime_config_snapshot},
|
|
||||||
site_replication::reload_site_replication_runtime_state,
|
|
||||||
};
|
|
||||||
use crate::storage::rpc::storage_compat::{
|
|
||||||
CollectMetricsOpts, DeleteOptions, DiskError, DiskInfoOptions, DiskStore, FileInfoVersions, LocalPeerS3Client, MetricType,
|
CollectMetricsOpts, DeleteOptions, DiskError, DiskInfoOptions, DiskStore, FileInfoVersions, LocalPeerS3Client, MetricType,
|
||||||
PEER_RESTSIGNAL, PEER_RESTSUB_SYS, ReadMultipleReq, ReadMultipleResp, ReadOptions, SERVICE_SIGNAL_REFRESH_CONFIG,
|
PEER_RESTSIGNAL, PEER_RESTSUB_SYS, ReadMultipleReq, ReadMultipleResp, ReadOptions, SERVICE_SIGNAL_REFRESH_CONFIG,
|
||||||
SERVICE_SIGNAL_RELOAD_DYNAMIC, StorageDiskRpcExt as _, StoragePeerS3ClientExt as _, UpdateMetadataOpts, all_local_disk_path,
|
SERVICE_SIGNAL_RELOAD_DYNAMIC, StorageDiskRpcExt as _, StoragePeerS3ClientExt as _, UpdateMetadataOpts, all_local_disk_path,
|
||||||
collect_local_metrics, find_local_disk_by_ref, get_global_lock_client, get_local_server_property, load_bucket_metadata,
|
collect_local_metrics, find_local_disk_by_ref, get_global_lock_client, get_local_server_property, load_bucket_metadata,
|
||||||
reload_transition_tier_config, resolve_object_store_handle, set_bucket_metadata,
|
reload_transition_tier_config, resolve_object_store_handle, set_bucket_metadata,
|
||||||
};
|
};
|
||||||
|
use crate::admin::service::{
|
||||||
|
config::{reload_dynamic_config_runtime_state, reload_runtime_config_snapshot},
|
||||||
|
site_replication::reload_site_replication_runtime_state,
|
||||||
|
};
|
||||||
use bytes::Bytes;
|
use bytes::Bytes;
|
||||||
use futures::Stream;
|
use futures::Stream;
|
||||||
use futures_util::future::join_all;
|
use futures_util::future::join_all;
|
||||||
@@ -122,11 +122,11 @@ fn unimplemented_rpc(method: &str) -> Status {
|
|||||||
Status::unimplemented(format!("{method} is not implemented"))
|
Status::unimplemented(format!("{method} is not implemented"))
|
||||||
}
|
}
|
||||||
|
|
||||||
fn background_rebalance_start_error_message(result: crate::storage::rpc::storage_compat::Result<()>) -> Option<String> {
|
fn background_rebalance_start_error_message(result: super::storage_compat::Result<()>) -> Option<String> {
|
||||||
result.err().map(|err| format!("start_rebalance failed: {err}"))
|
result.err().map(|err| format!("start_rebalance failed: {err}"))
|
||||||
}
|
}
|
||||||
|
|
||||||
fn stop_rebalance_response(result: crate::storage::rpc::storage_compat::Result<()>) -> StopRebalanceResponse {
|
fn stop_rebalance_response(result: super::storage_compat::Result<()>) -> StopRebalanceResponse {
|
||||||
match result {
|
match result {
|
||||||
Ok(_) => StopRebalanceResponse {
|
Ok(_) => StopRebalanceResponse {
|
||||||
success: true,
|
success: true,
|
||||||
@@ -2389,7 +2389,7 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_background_rebalance_start_error_message_formats_error() {
|
fn test_background_rebalance_start_error_message_formats_error() {
|
||||||
let message = background_rebalance_start_error_message(Err(crate::storage::rpc::storage_compat::Error::other("boom")))
|
let message = background_rebalance_start_error_message(Err(super::super::storage_compat::Error::other("boom")))
|
||||||
.expect("background rebalance start failure should be formatted");
|
.expect("background rebalance start failure should be formatted");
|
||||||
|
|
||||||
assert!(message.contains("start_rebalance failed"));
|
assert!(message.contains("start_rebalance failed"));
|
||||||
@@ -2398,7 +2398,7 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_stop_rebalance_response_reports_local_stop_error() {
|
fn test_stop_rebalance_response_reports_local_stop_error() {
|
||||||
let response = stop_rebalance_response(Err(crate::storage::rpc::storage_compat::Error::other("boom")));
|
let response = stop_rebalance_response(Err(super::super::storage_compat::Error::other("boom")));
|
||||||
|
|
||||||
assert!(!response.success);
|
assert!(!response.success);
|
||||||
assert!(response.error_info.as_deref().is_some_and(|message| message.contains("boom")));
|
assert!(response.error_info.as_deref().is_some_and(|message| message.contains("boom")));
|
||||||
@@ -2736,7 +2736,7 @@ mod tests {
|
|||||||
vars.insert(PEER_RESTSIGNAL.to_string(), SERVICE_SIGNAL_RELOAD_DYNAMIC.to_string());
|
vars.insert(PEER_RESTSIGNAL.to_string(), SERVICE_SIGNAL_RELOAD_DYNAMIC.to_string());
|
||||||
vars.insert(
|
vars.insert(
|
||||||
PEER_RESTSUB_SYS.to_string(),
|
PEER_RESTSUB_SYS.to_string(),
|
||||||
crate::storage::rpc::storage_compat::STORAGE_CLASS_SUB_SYS.to_string(),
|
super::super::storage_compat::STORAGE_CLASS_SUB_SYS.to_string(),
|
||||||
);
|
);
|
||||||
|
|
||||||
let request = Request::new(SignalServiceRequest {
|
let request = Request::new(SignalServiceRequest {
|
||||||
|
|||||||
@@ -12,8 +12,8 @@
|
|||||||
// See the License for the specific language governing permissions and
|
// See the License for the specific language governing permissions and
|
||||||
// limitations under the License.
|
// limitations under the License.
|
||||||
|
|
||||||
|
use super::storage_compat::to_s3s_etag;
|
||||||
use crate::storage::s3_api::common::rustfs_owner;
|
use crate::storage::s3_api::common::rustfs_owner;
|
||||||
use crate::storage::s3_api::storage_compat::to_s3s_etag;
|
|
||||||
use percent_encoding::percent_decode_str;
|
use percent_encoding::percent_decode_str;
|
||||||
use rustfs_storage_api::{
|
use rustfs_storage_api::{
|
||||||
BucketInfo, ListObjectVersionsInfo as StorageListObjectVersionsInfo, ListObjectsV2Info as StorageListObjectsV2Info,
|
BucketInfo, ListObjectVersionsInfo as StorageListObjectVersionsInfo, ListObjectsV2Info as StorageListObjectsV2Info,
|
||||||
|
|||||||
@@ -12,8 +12,8 @@
|
|||||||
// See the License for the specific language governing permissions and
|
// See the License for the specific language governing permissions and
|
||||||
// limitations under the License.
|
// limitations under the License.
|
||||||
|
|
||||||
|
use super::storage_compat::to_s3s_etag;
|
||||||
use crate::storage::s3_api::common::{rustfs_initiator, rustfs_owner};
|
use crate::storage::s3_api::common::{rustfs_initiator, rustfs_owner};
|
||||||
use crate::storage::s3_api::storage_compat::to_s3s_etag;
|
|
||||||
use rustfs_storage_api::{ListMultipartsInfo, ListPartsInfo};
|
use rustfs_storage_api::{ListMultipartsInfo, ListPartsInfo};
|
||||||
use s3s::dto::{CommonPrefix, ListMultipartUploadsOutput, ListPartsOutput, MultipartUpload, Part, Timestamp};
|
use s3s::dto::{CommonPrefix, ListMultipartUploadsOutput, ListPartsOutput, MultipartUpload, Part, Timestamp};
|
||||||
use s3s::{S3Error, S3ErrorCode};
|
use s3s::{S3Error, S3ErrorCode};
|
||||||
@@ -186,12 +186,12 @@ pub(crate) fn build_list_multipart_uploads_output(
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
|
use super::super::storage_compat::to_s3s_etag;
|
||||||
use super::{
|
use super::{
|
||||||
MAX_MULTIPART_UPLOADS_LIST, build_list_multipart_uploads_output, build_list_parts_output,
|
MAX_MULTIPART_UPLOADS_LIST, build_list_multipart_uploads_output, build_list_parts_output,
|
||||||
parse_list_multipart_uploads_params, parse_list_parts_params, parse_upload_part_number,
|
parse_list_multipart_uploads_params, parse_list_parts_params, parse_upload_part_number,
|
||||||
};
|
};
|
||||||
use crate::storage::s3_api::common::{rustfs_initiator, rustfs_owner};
|
use crate::storage::s3_api::common::{rustfs_initiator, rustfs_owner};
|
||||||
use crate::storage::s3_api::storage_compat::to_s3s_etag;
|
|
||||||
use rustfs_storage_api::{ListMultipartsInfo, ListPartsInfo, MultipartInfo, PartInfo};
|
use rustfs_storage_api::{ListMultipartsInfo, ListPartsInfo, MultipartInfo, PartInfo};
|
||||||
use s3s::S3ErrorCode;
|
use s3s::S3ErrorCode;
|
||||||
use s3s::dto::Timestamp;
|
use s3s::dto::Timestamp;
|
||||||
|
|||||||
@@ -102,6 +102,7 @@ RUSTFS_LOCAL_COMPAT_OWNER_SELF_PATH_HITS_FILE="${TMP_DIR}/rustfs_local_compat_ow
|
|||||||
RUSTFS_ROOT_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_root_compat_relative_consumer_hits.txt"
|
RUSTFS_ROOT_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_root_compat_relative_consumer_hits.txt"
|
||||||
RUSTFS_STORAGE_CORE_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_storage_core_compat_relative_consumer_hits.txt"
|
RUSTFS_STORAGE_CORE_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_storage_core_compat_relative_consumer_hits.txt"
|
||||||
RUSTFS_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_local_compat_relative_consumer_hits.txt"
|
RUSTFS_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_local_compat_relative_consumer_hits.txt"
|
||||||
|
RUSTFS_STORAGE_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_storage_local_compat_relative_consumer_hits.txt"
|
||||||
SCANNER_BUCKET_STORAGE_COMPAT_MODULE_HITS_FILE="${TMP_DIR}/scanner_bucket_storage_compat_module_hits.txt"
|
SCANNER_BUCKET_STORAGE_COMPAT_MODULE_HITS_FILE="${TMP_DIR}/scanner_bucket_storage_compat_module_hits.txt"
|
||||||
NOTIFY_STORAGE_COMPAT_MODULE_HITS_FILE="${TMP_DIR}/notify_storage_compat_module_hits.txt"
|
NOTIFY_STORAGE_COMPAT_MODULE_HITS_FILE="${TMP_DIR}/notify_storage_compat_module_hits.txt"
|
||||||
OBS_STORAGE_COMPAT_PASSTHROUGH_HITS_FILE="${TMP_DIR}/obs_storage_compat_passthrough_hits.txt"
|
OBS_STORAGE_COMPAT_PASSTHROUGH_HITS_FILE="${TMP_DIR}/obs_storage_compat_passthrough_hits.txt"
|
||||||
@@ -1099,6 +1100,22 @@ if [[ -s "$RUSTFS_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE" ]]; then
|
|||||||
report_failure "RustFS local compatibility consumers must use relative owner paths instead of crate-qualified local compatibility paths: $(paste -sd '; ' "$RUSTFS_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE")"
|
report_failure "RustFS local compatibility consumers must use relative owner paths instead of crate-qualified local compatibility paths: $(paste -sd '; ' "$RUSTFS_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE")"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
(
|
||||||
|
cd "$ROOT_DIR"
|
||||||
|
{
|
||||||
|
rg -n --with-filename 'crate::storage::rpc::storage_compat' \
|
||||||
|
rustfs/src/storage/rpc/http_service.rs \
|
||||||
|
rustfs/src/storage/rpc/node_service.rs || true
|
||||||
|
rg -n --with-filename 'crate::storage::s3_api::storage_compat' \
|
||||||
|
rustfs/src/storage/s3_api/bucket.rs \
|
||||||
|
rustfs/src/storage/s3_api/multipart.rs || true
|
||||||
|
}
|
||||||
|
) >"$RUSTFS_STORAGE_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE"
|
||||||
|
|
||||||
|
if [[ -s "$RUSTFS_STORAGE_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE" ]]; then
|
||||||
|
report_failure "RustFS storage RPC/S3 API compatibility consumers must use relative owner paths instead of crate-qualified local compatibility paths: $(paste -sd '; ' "$RUSTFS_STORAGE_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE")"
|
||||||
|
fi
|
||||||
|
|
||||||
(
|
(
|
||||||
cd "$ROOT_DIR"
|
cd "$ROOT_DIR"
|
||||||
rg -n --no-heading 'pub\(crate\)\s+use rustfs_ecstore::api::bucket::\{[^}]*\b(?:bucket_target_sys|lifecycle|metadata_sys|replication|versioning|versioning_sys)\b[^}]*\}\s*;' \
|
rg -n --no-heading 'pub\(crate\)\s+use rustfs_ecstore::api::bucket::\{[^}]*\b(?:bucket_target_sys|lifecycle|metadata_sys|replication|versioning|versioning_sys)\b[^}]*\}\s*;' \
|
||||||
|
|||||||
Reference in New Issue
Block a user