Skip to content

Commit

Permalink
Revert "Added tests for Accesspoints and transfer acceleration"
Browse files Browse the repository at this point in the history
This reverts commit 9532ea8.
  • Loading branch information
Ankit Saurabh committed Sep 14, 2023
1 parent 9532ea8 commit bb8d6a6
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 86 deletions.
55 changes: 12 additions & 43 deletions mountpoint-s3-client/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,54 +21,24 @@ fn init_tracing_subscriber() {
let _ = tracing_subscriber::fmt::try_init();
}

pub enum AccessPointType {
SingleRegion,
ObjectLambda,
MultiRegion,
pub fn get_test_client() -> S3CrtClient {
let endpoint_config = EndpointConfig::new(&get_test_region());
S3CrtClient::new(S3ClientConfig::new().endpoint_config(endpoint_config)).expect("could not create test client")
}

pub fn get_unique_test_prefix(test_name: &str) -> String {
// Prefix always has a trailing "/" to keep meaning in sync with the S3 API.
let prefix = std::env::var("S3_BUCKET_TEST_PREFIX").unwrap_or(String::from("mountpoint-test/"));
assert!(prefix.ends_with('/'), "S3_BUCKET_TEST_PREFIX should end in '/'");
pub fn get_test_bucket_and_prefix(test_name: &str) -> (String, String) {
let bucket = std::env::var("S3_BUCKET_NAME").expect("Set S3_BUCKET_NAME to run integration tests");

// Generate a random nonce to make sure this prefix is truly unique
let nonce = OsRng.next_u64();
let prefix = format!("{prefix}{test_name}/{nonce}/");
prefix
}

pub fn get_test_client() -> S3CrtClient {
let endpoint_config = EndpointConfig::new(&get_test_region());
S3CrtClient::new(S3ClientConfig::new().endpoint_config(endpoint_config)).expect("could not create test client")
}
// Prefix always has a trailing "/" to keep meaning in sync with the S3 API.
let prefix = std::env::var("S3_BUCKET_TEST_PREFIX").unwrap_or(String::from("mountpoint-test/"));
assert!(prefix.ends_with('/'), "S3_BUCKET_TEST_PREFIX should end in '/'");

pub fn get_test_bucket() -> String {
std::env::var("S3_BUCKET_NAME").expect("Set S3_BUCKET_NAME to run integration tests")
}
let prefix = format!("{prefix}{test_name}/{nonce}/");

pub fn get_test_access_point_alias(arn: bool, access_point_type: AccessPointType) -> String {
match access_point_type {
AccessPointType::SingleRegion => {
if arn {
std::env::var("S3_ACCESS_POINT_ARN").expect("Set S3_ACCESS_POINT_ALIAS to run integration tests")
} else {
std::env::var("S3_ACCESS_POINT_ALIAS").expect("Set S3_ACCESS_POINT_ALIAS to run integration tests")
}
}
AccessPointType::ObjectLambda => {
if arn {
std::env::var("S3_OLAP_ARN").expect("Set S3_OLAP_ALIAS to run integration tests")
} else {
std::env::var("S3_OLAP_ALIAS").expect("Set S3_OLAP_ALIAS to run integration tests")
}
}
AccessPointType::MultiRegion => {
// Multi region accesspoints should only be accessed using their ARN
// (since endpoint for alias needs to be in format `<mrap-alias>.accesspoint.s3-global.amazonaws.com`. But this endpoint could not be formed using
// CRT endpoint resolver any bucket alias with '.' in it will be resolved in path style addressing. Similar is the case with CLI)
std::env::var("S3_MRAP_ARN").expect("Set S3_MRAP_ALIAS to run integration tests")
}
}
(bucket, prefix)
}

pub fn get_test_region() -> String {
Expand Down Expand Up @@ -133,8 +103,7 @@ pub async fn get_mpu_count_for_key(
#[tokio::test]
async fn test_sdk_create_object() {
let sdk_client = get_test_sdk_client().await;
let bucket = get_test_bucket();
let prefix = get_unique_test_prefix("test_sdk_create_object");
let (bucket, prefix) = get_test_bucket_and_prefix("test_sdk_create_object");

let response = sdk_client
.put_object()
Expand Down
51 changes: 14 additions & 37 deletions mountpoint-s3-client/tests/endpoint_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ use common::*;
use mountpoint_s3_client::{AddressingStyle, EndpointConfig, ObjectClient, S3ClientConfig, S3CrtClient};
use test_case::test_case;

async fn run_test<F: FnOnce(&str) -> EndpointConfig>(f: F, prefix: &str, bucket: &str) {
async fn run_test<F: FnOnce(&str) -> EndpointConfig>(f: F) {
let sdk_client = get_test_sdk_client().await;
let (bucket, prefix) = get_test_bucket_and_prefix("test_region");

// Create one object named "hello"
let key = format!("{prefix}hello");
let key = format!("{prefix}/hello");
let body = b"hello world!";
sdk_client
.put_object()
Expand All @@ -35,57 +36,33 @@ async fn run_test<F: FnOnce(&str) -> EndpointConfig>(f: F, prefix: &str, bucket:
check_get_result(result, None, &body[..]).await;
}

#[test_case(AddressingStyle::Automatic, "test_default_addressing_style")]
#[test_case(AddressingStyle::Path, "test_path_addressing_style")]
#[test_case(AddressingStyle::Automatic)]
#[test_case(AddressingStyle::Path)]
#[tokio::test]
async fn test_addressing_style_region(addressing_style: AddressingStyle, prefix: &str) {
run_test(
|region| EndpointConfig::new(region).addressing_style(addressing_style),
&get_unique_test_prefix(prefix),
&get_test_bucket()
)
.await;
async fn test_addressing_style_region(addressing_style: AddressingStyle) {
run_test(|region| EndpointConfig::new(region).addressing_style(addressing_style)).await;
}

#[cfg(feature = "fips_tests")]
#[tokio::test]
async fn test_fips_mount_option() {
let prefix = get_unique_test_prefix("test_fips");
run_test(|region| EndpointConfig::new(region).use_fips(true), &prefix, &get_test_bucket()).await;
run_test(|region| EndpointConfig::new(region).use_fips(true)).await;
}

#[test_case(AddressingStyle::Automatic)]
#[test_case(AddressingStyle::Path)]
#[tokio::test]
async fn test_addressing_style_dualstack_option(addressing_style: AddressingStyle) {
let prefix = get_unique_test_prefix("test_dual_stack");
run_test(
|region| {
EndpointConfig::new(region)
.addressing_style(addressing_style)
.use_dual_stack(true)
},
&prefix,
&get_test_bucket(),
)
run_test(|region| {
EndpointConfig::new(region)
.addressing_style(addressing_style)
.use_dual_stack(true)
})
.await;
}

#[cfg(feature = "fips_tests")]
#[tokio::test]
async fn test_fips_dual_stack_mount_option() {
let prefix = get_unique_test_prefix("test_fips_dual_stack");
run_test(
|region| EndpointConfig::new(region).use_fips(true).use_dual_stack(true),
&prefix,
&get_test_bucket(),
)
.await;
run_test(|region| EndpointConfig::new(region).use_fips(true).use_dual_stack(true)).await;
}

#[test_case(AddressingStyle::Automatic)]
#[test_case(AddressingStyle::Path)]
#[tokio::test]
async fn test_single_region_access_point(addressing_style: AddressingStyle, arn: ) {

}
12 changes: 6 additions & 6 deletions mountpoint-s3/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl<Client: ObjectClient, Runtime> FileHandleType<Client, Runtime> {
let key = lookup.inode.full_key();
let handle = match fs.uploader.put(&fs.bucket, key).await {
Err(e) => {
return Err(err!(libc::EIO, source: e, "put failed to start"));
return Err(err!(libc::EIO, source:e, "put failed to start"));
}
Ok(request) => FileHandleType::Write(UploadState::InProgress { request, handle }.into()),
};
Expand Down Expand Up @@ -172,7 +172,7 @@ impl<Client: ObjectClient> UploadState<Client> {
debug!(key, size, "put succeeded");
Ok(())
}
Err(e) => Err(err!(libc::EIO, source: e, "put failed")),
Err(e) => Err(err!(libc::EIO, source:e, "put failed")),
};
if let Err(err) = handle.finish_writing() {
// Log the issue but still return put_result.
Expand Down Expand Up @@ -540,15 +540,15 @@ where
match request.as_mut().unwrap().read(offset as u64, size as usize).await {
Ok(checksummed_bytes) => match checksummed_bytes.into_bytes() {
Ok(bytes) => reply.data(&bytes),
Err(e) => reply.error(err!(libc::EIO, source: e, "integrity error")),
Err(e) => reply.error(err!(libc::EIO, source:e, "integrity error")),
},
Err(PrefetchReadError::GetRequestFailed(ObjectClientError::ServiceError(
GetObjectError::PreconditionFailed,
))) => reply.error(err!(libc::ESTALE, "object was mutated remotely")),
Err(PrefetchReadError::Integrity(e)) => reply.error(err!(libc::EIO, source: e, "integrity error")),
Err(PrefetchReadError::Integrity(e)) => reply.error(err!(libc::EIO, source:e, "integrity error")),
Err(e @ PrefetchReadError::GetRequestFailed(_))
| Err(e @ PrefetchReadError::GetRequestTerminatedUnexpectedly) => {
reply.error(err!(libc::EIO, source: e, "get request failed"))
reply.error(err!(libc::EIO, source:e, "get request failed"))
}
}
}
Expand Down Expand Up @@ -765,7 +765,7 @@ where
match request.complete(&file_handle.full_key).await {
// According to the `fsync` man page we should return ENOSPC instead of EFBIG if it's a
// space-related failure.
Err(e) if e.to_errno() == libc::EFBIG => Err(err!(libc::ENOSPC, source: e, "object too big")),
Err(e) if e.to_errno() == libc::EFBIG => Err(err!(libc::ENOSPC, source:e, "object too big")),
ret => ret,
}
}
Expand Down

0 comments on commit bb8d6a6

Please sign in to comment.