From c3994541eb66fcd637630341beb987fc9c56e29b Mon Sep 17 00:00:00 2001 From: Alek Westover Date: Fri, 23 Jun 2023 13:26:28 -0400 Subject: [PATCH] add real s3 tests --- compute_tools/src/bin/compute_ctl.rs | 22 +++---- compute_tools/src/extension_server.rs | 1 + test_runner/fixtures/neon_fixtures.py | 1 + .../regress/test_download_extensions.py | 61 +++++++++++-------- 4 files changed, 47 insertions(+), 38 deletions(-) diff --git a/compute_tools/src/bin/compute_ctl.rs b/compute_tools/src/bin/compute_ctl.rs index fae9ac9319..4cc0310995 100644 --- a/compute_tools/src/bin/compute_ctl.rs +++ b/compute_tools/src/bin/compute_ctl.rs @@ -76,8 +76,8 @@ fn main() -> Result<()> { None => None, }; - let rt = Runtime::new().unwrap(); - rt.block_on(async { + let rt0 = Runtime::new().unwrap(); + rt0.block_on(async { download_extension(&ext_remote_storage, ExtensionType::Shared, pgbin) .await .expect("download shared extensions should work"); @@ -181,7 +181,6 @@ fn main() -> Result<()> { } }; - dbg!(&spec); let mut new_state = ComputeState::new(); let spec_set; let tenant_id; @@ -228,14 +227,15 @@ fn main() -> Result<()> { } // Now we have the spec, so we request the tenant specific extensions - if let Some(tenant_id) = tenant_id { - let rt = Runtime::new().unwrap(); - rt.block_on(async { - download_extension(&ext_remote_storage, ExtensionType::Tenant(tenant_id), pgbin) - .await - .expect("download tenant specific extensions should work"); - }); - } + // TODO: this is temporarily disabled + // if let Some(tenant_id) = tenant_id { + // let rt1 = Runtime::new().unwrap(); + // rt1.block_on(async { + // download_extension(&ext_remote_storage, ExtensionType::Tenant(tenant_id), pgbin) + // .await + // .expect("download tenant specific extensions should not return an error"); + // }); + // } // We got all we need, update the state. let mut state = compute.state.lock().unwrap(); diff --git a/compute_tools/src/extension_server.rs b/compute_tools/src/extension_server.rs index 4ee304f583..ea3d2210d7 100644 --- a/compute_tools/src/extension_server.rs +++ b/compute_tools/src/extension_server.rs @@ -52,6 +52,7 @@ async fn download_helper( .download_stream .read_to_end(&mut write_data_buffer) .await?; + dbg!(str::from_utf8(&write_data_buffer)?); let mut output_file = BufWriter::new(File::create(local_path)?); output_file.write_all(&write_data_buffer)?; Ok(()) diff --git a/test_runner/fixtures/neon_fixtures.py b/test_runner/fixtures/neon_fixtures.py index 175e0ab705..c33e31fdb2 100644 --- a/test_runner/fixtures/neon_fixtures.py +++ b/test_runner/fixtures/neon_fixtures.py @@ -767,6 +767,7 @@ class NeonEnvBuilder: ) if enable_remote_extensions: + # TODO: add an env variable for EXT_REMOTE_STORAGE_S3_BUCKET ext_bucket_name = os.getenv("EXT_REMOTE_STORAGE_S3_BUCKET") if ext_bucket_name is None: ext_bucket_name = "neon-dev-extensions" diff --git a/test_runner/regress/test_download_extensions.py b/test_runner/regress/test_download_extensions.py index 833ab9dd65..bca6000bae 100644 --- a/test_runner/regress/test_download_extensions.py +++ b/test_runner/regress/test_download_extensions.py @@ -12,12 +12,18 @@ from fixtures.neon_fixtures import ( """ TODO: -- **add tests with real S3 storage** -- libs/remote_storage/src/s3_bucket.rs TODO // TODO: if bucket prefix is empty, - the folder is prefixed with a "/" I think. Is this desired? +status: +it appears that list_files on a non-existing path is bad whether you are real or mock s3 storage -- test LIBRARY exttensions -- how to add env variable EXT_REMOTE_STORAGE_S3_BUCKET? +1. debug real s3 tests: I think the paths were slightly different than I was expecting +2. Make sure it gracefully is sad when tenant is not found +stderr: command failed: unexpected compute status: Empty + +3. clean up the junk I put in the bucket (one time task) +4. can we simultaneously do MOCK and REAL s3 tests, or are the env vars conflicting/ +5. libs/remote_storage/src/s3_bucket.rs TODO // TODO: if bucket prefix is empty, + the folder is prefixed with a "/" I think. Is this desired? +6. test LIBRARY extensions: maybe Anastasia already did this? """ @@ -31,8 +37,7 @@ relocatable = true""" @pytest.mark.parametrize( - "remote_storage_kind", - [RemoteStorageKind.MOCK_S3], + "remote_storage_kind", [RemoteStorageKind.MOCK_S3, RemoteStorageKind.REAL_S3] ) def test_file_download(neon_env_builder: NeonEnvBuilder, remote_storage_kind: RemoteStorageKind): """ @@ -41,7 +46,6 @@ def test_file_download(neon_env_builder: NeonEnvBuilder, remote_storage_kind: Re Then, we download test_ext.control from the bucket to pg_install/v15/share/postgresql/extension/ Finally, we list available extensions and assert that test_ext is present """ - # RemoteStorageKind.REAL_S3, RemoteStorageKind.LOCAL_FS neon_env_builder.enable_remote_storage( remote_storage_kind=remote_storage_kind, @@ -61,26 +65,27 @@ def test_file_download(neon_env_builder: NeonEnvBuilder, remote_storage_kind: Re BUCKET_PREFIX = "5314225671" # this is the build number cleanup_files = [] - # Upload test_ext{i}.control files to the bucket + # Upload test_ext{i}.control files to the bucket (for MOCK_S3) # Note: In real life this is done by CI/CD for i in range(NUM_EXT): # public extensions public_ext = BytesIO(bytes(ext_contents("public", i), "utf-8")) - remote_name = f"{BUCKET_PREFIX}/{PUB_EXT_ROOT}/test_ext{i}.control" - local_name = f"pg_install/{PUB_EXT_ROOT}/test_ext{i}.control" - env.remote_storage_client.upload_fileobj( - public_ext, env.ext_remote_storage.bucket_name, remote_name - ) - cleanup_files.append(local_name) - + public_remote_name = f"{BUCKET_PREFIX}/{PUB_EXT_ROOT}/test_ext{i}.control" + public_local_name = f"pg_install/{PUB_EXT_ROOT}/test_ext{i}.control" # private extensions private_ext = BytesIO(bytes(ext_contents(str(tenant_id), i), "utf-8")) - remote_name = f"{BUCKET_PREFIX}/{str(tenant_id)}/private_ext{i}.control" - local_name = f"pg_install/{PUB_EXT_ROOT}/private_ext{i}.control" - env.remote_storage_client.upload_fileobj( - private_ext, env.ext_remote_storage.bucket_name, remote_name - ) - cleanup_files.append(local_name) + private_remote_name = f"{BUCKET_PREFIX}/{str(tenant_id)}/private_ext{i}.control" + private_local_name = f"pg_install/{PUB_EXT_ROOT}/private_ext{i}.control" + + cleanup_files += [public_local_name, private_local_name] + + if remote_storage_kind == RemoteStorageKind.MOCK_S3: + env.remote_storage_client.upload_fileobj( + public_ext, env.ext_remote_storage.bucket_name, public_remote_name + ) + env.remote_storage_client.upload_fileobj( + private_ext, env.ext_remote_storage.bucket_name, private_remote_name + ) # Rust will then download the control files from the bucket # our rust code should obtain the same result as the following: @@ -89,10 +94,14 @@ def test_file_download(neon_env_builder: NeonEnvBuilder, remote_storage_kind: Re # Key=os.path.join(BUCKET_PREFIX, PUB_EXT_PATHS[0]) # )["Body"].read() + region = "us-east-1" + if remote_storage_kind == RemoteStorageKind.REAL_S3: + region = "eu-central-1" + remote_ext_config = json.dumps( { "bucket": env.ext_remote_storage.bucket_name, - "region": "us-east-1", + "region": region, "endpoint": env.ext_remote_storage.endpoint, "prefix": BUCKET_PREFIX, } @@ -116,14 +125,12 @@ def test_file_download(neon_env_builder: NeonEnvBuilder, remote_storage_kind: Re log.info(all_extensions) for i in range(NUM_EXT): assert f"test_ext{i}" in all_extensions - assert f"private_ext{i}" in all_extensions + # assert f"private_ext{i}" in all_extensions # TODO: can create extension actually install an extension? # cur.execute("CREATE EXTENSION test_ext0") - # log.info("**" * 100) - # log.info(cur.fetchall()) - # cleanup downloaded extensions (TODO: the file names are quesionable here) + # cleanup downloaded extensions for file in cleanup_files: try: log.info(f"Deleting {file}")