`cloud_storage_clients`: add `cloud_storage_backend::oracle` and fix `s3_client::self_configure()` #22902

WillemKauf · 2024-08-15T19:16:07Z

As a first step in improving compatibility with OCI, help the s3_client properly self-configure itself for the newly added cloud_storage_backend::oracle value.

The s3_client::self_configure() function performs a check with a ListObjects() request that currently succeeds for OCI, even in the unsupported virtual_host mode. This would result in improper self-configuration, and every other cloud storage API call made to OCI in this configuration fails.

Correct this behavior by inferring the oraclecloud backend through the access_point_uri used, and always self-configure for path-style requests when it is in use.

Backports Required

Release Notes

Improvements

Adds the cloud_storage_backend::oracle value, and helps the s3_client properly configure for OCI storage.

vbotbuildovich · 2024-08-15T21:49:52Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/53004#019157c3-0cbb-46bd-8bf6-f5a220bed02f

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/53018#01915901-0e01-498d-9160-4f6127dfc5b3

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/53039#01915b6e-702b-42a5-8af1-219b3091ed4b

tests/rptest/tests/cluster_self_config_test.py

vbotbuildovich · 2024-08-15T22:20:26Z

new failures in https://buildkite.com/redpanda/redpanda/builds/53004#019157d3-e46d-4571-b35b-1e11f47f1303:

"rptest.tests.scaling_up_test.ScalingUpTest.test_moves_with_local_retention.use_topic_property=True"

new failures in https://buildkite.com/redpanda/redpanda/builds/53004#019157d3-e46a-4c57-9d15-4a3f5ee769f8:

"rptest.tests.audit_log_test.AuditLogTestsAppLifecycle.test_recovery_mode"

WillemKauf · 2024-08-16T01:14:38Z

Force push to

Fix conflict introduced by other recent change to ClusterSelfConfigTest (NFC, comment syntax)
Fix check for si_settings.bypass_bucket_creation in RedpandaService.clean()
Correct comment per docs request

src/v/cloud_storage/remote.cc

src/v/model/metadata.h

src/v/cloud_storage_clients/s3_client.cc

In order to provide future support for OCI. Adds a new backend value to the `model::cloud_storage_backend` enum.

In the case of S3-compatible providers, we determine the backend through the access-point/uri if we are already unable to determine the backend from credential sources. Split this logic into its own function. It will see future use in the `s3_client` to assist in self-configuration for required platform specific configs.

Previously, OCI would mistakenly self-configure itself for the `virtual-host` style, due to a check with a `ListObjects` request that succeeds. Oracle states that they only support path-style requests, so this behavior is strange: https://www.oracle.com/ca-en/cloud/storage/object-storage/faq/#category-amazon We now use `infer_backend_from_uri()` to check for the `oracle_s3_compat` backend, and return `s3_url_style::path` if this backend is detected.

`si_settings.bypass_bucket_creation` allows ducktape users to skip the creation of a bucket in cloud storage. However, this value was not considered during the deletion of a bucket in `RedpandaService.clean()`. Change `RedpandaService.clean()` to consider this value, and potentially return early/no-op.

To assert that the `oracle` backend will always configure itself for `path`-style requests.

WillemKauf · 2024-08-16T12:29:41Z

Force push to:

Rename cloud_storage_backend::oracle to cloud_storage_backend::oracle_s3_compat.
Fix support for BulkDelete

nvartolomei

neat!

vbotbuildovich · 2024-08-16T16:11:04Z

/backport v24.2.x

vbotbuildovich · 2024-08-16T16:12:06Z

Failed to create a backport PR to v24.2.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-22902-v24.2.x-196 remotes/upstream/v24.2.x
git cherry-pick -x 8039ed575505a0bf593ce3165a96bbb46562be6c 4921a3240ec66546ce8cd46ac2f92300e654ab0d 229dc0d9c08246e7e7917737240464ee6af7b96d 70d88d5ece4c2eb4040e2a469203e698146ec6fb b198688407acef96c56688994c6a1367cab0bca9

Workflow run logs.

dotnwat · 2024-08-20T20:16:13Z

src/v/model/metadata.h

-    unknown = 4,
+    oracle_s3_compat = 4,
+    unknown


are names or enum numeric values stored in the configuration system? if the later then it seems problematic to assign a different enum to the 4 value.

The enum type is used to template property<T>, which also uses the enum type and not the underlying_type.

what i mean is: if the underlying numeric enum value is stored in the controller log, then it'd be a problem. does the controller log store the stringified name or the underlying numeric vavlue?

From what I am reading, keys and values are left in raw string format when applying updates in the config_manager state machine

I now see the reason for concern though. Persisting this configuration in raft0 as a the raw underlying_type didn't cross my mind when making this change- I'll keep staring at it and revert the change if it ends up being dangerous.

thanks for looking!

dotnwat · 2024-08-20T20:17:29Z

src/v/cloud_storage_clients/configuration.cc

@@ -327,6 +327,21 @@ operator<<(std::ostream& o, const client_self_configuration_output& r) {
      r);
 }

+model::cloud_storage_backend
+infer_backend_from_uri(const access_point_uri& uri) {


WillemKauf requested review from nvartolomei and mattschumpert August 15, 2024 19:16

WillemKauf requested a review from a team as a code owner August 15, 2024 19:16

github-actions bot added the area/redpanda label Aug 15, 2024

mattschumpert previously approved these changes Aug 15, 2024

View reviewed changes

Feediver1 reviewed Aug 15, 2024

View reviewed changes

tests/rptest/tests/cluster_self_config_test.py Outdated Show resolved Hide resolved

WillemKauf dismissed mattschumpert’s stale review via 837b25c August 16, 2024 01:12

WillemKauf force-pushed the oracle_cloud_storage_backend branch from 4434262 to 837b25c Compare August 16, 2024 01:12

nvartolomei reviewed Aug 16, 2024

View reviewed changes

src/v/cloud_storage/remote.cc Outdated Show resolved Hide resolved

nvartolomei reviewed Aug 16, 2024

View reviewed changes

src/v/model/metadata.h Outdated Show resolved Hide resolved

nvartolomei reviewed Aug 16, 2024

View reviewed changes

src/v/cloud_storage_clients/s3_client.cc Show resolved Hide resolved

WillemKauf added 5 commits August 16, 2024 07:07

model: add cloud_storage_backend::oracle_s3_compat

8039ed5

In order to provide future support for OCI. Adds a new backend value to the `model::cloud_storage_backend` enum.

rptest: add test_s3_oracle_self_config

b198688

To assert that the `oracle` backend will always configure itself for `path`-style requests.

WillemKauf force-pushed the oracle_cloud_storage_backend branch from 837b25c to b198688 Compare August 16, 2024 12:29

WillemKauf requested a review from nvartolomei August 16, 2024 12:30

nvartolomei approved these changes Aug 16, 2024

View reviewed changes

WillemKauf merged commit 441d3fe into redpanda-data:dev Aug 16, 2024
18 checks passed

vbotbuildovich mentioned this pull request Aug 16, 2024

[v24.2.x] cloud_storage_clients: add cloud_storage_backend::oracle and fix s3_client::self_configure() #22915

Closed

WillemKauf mentioned this pull request Aug 16, 2024

[v24.2.x] cloud_storage_clients: add cloud_storage_backend::oracle and fix s3_client::self_configure() (manual backport) #22916

Merged

7 tasks

dotnwat reviewed Aug 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`cloud_storage_clients`: add `cloud_storage_backend::oracle` and fix `s3_client::self_configure()` #22902

`cloud_storage_clients`: add `cloud_storage_backend::oracle` and fix `s3_client::self_configure()` #22902

WillemKauf commented Aug 15, 2024 •

edited

Loading

vbotbuildovich commented Aug 15, 2024 •

edited

Loading

vbotbuildovich commented Aug 15, 2024 •

edited

Loading

WillemKauf commented Aug 16, 2024

WillemKauf commented Aug 16, 2024

nvartolomei left a comment

vbotbuildovich commented Aug 16, 2024

vbotbuildovich commented Aug 16, 2024

dotnwat Aug 20, 2024

WillemKauf Aug 20, 2024

dotnwat Aug 21, 2024

WillemKauf Aug 21, 2024 •

edited

Loading

dotnwat Aug 27, 2024 •

edited

Loading

dotnwat Aug 20, 2024

cloud_storage_clients: add cloud_storage_backend::oracle and fix s3_client::self_configure() #22902

cloud_storage_clients: add cloud_storage_backend::oracle and fix s3_client::self_configure() #22902

Conversation

WillemKauf commented Aug 15, 2024 • edited Loading

Backports Required

Release Notes

Improvements

vbotbuildovich commented Aug 15, 2024 • edited Loading

vbotbuildovich commented Aug 15, 2024 • edited Loading

WillemKauf commented Aug 16, 2024

WillemKauf commented Aug 16, 2024

nvartolomei left a comment

Choose a reason for hiding this comment

vbotbuildovich commented Aug 16, 2024

vbotbuildovich commented Aug 16, 2024

dotnwat Aug 20, 2024

Choose a reason for hiding this comment

WillemKauf Aug 20, 2024

Choose a reason for hiding this comment

dotnwat Aug 21, 2024

Choose a reason for hiding this comment

WillemKauf Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

dotnwat Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

dotnwat Aug 20, 2024

Choose a reason for hiding this comment

`cloud_storage_clients`: add `cloud_storage_backend::oracle` and fix `s3_client::self_configure()` #22902

`cloud_storage_clients`: add `cloud_storage_backend::oracle` and fix `s3_client::self_configure()` #22902

WillemKauf commented Aug 15, 2024 •

edited

Loading

vbotbuildovich commented Aug 15, 2024 •

edited

Loading

vbotbuildovich commented Aug 15, 2024 •

edited

Loading

WillemKauf Aug 21, 2024 •

edited

Loading

dotnwat Aug 27, 2024 •

edited

Loading