Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New S3 Flag to configure ListObjectsVersion #5099

Merged
merged 4 commits into from
Jun 27, 2023

Conversation

shaardie
Copy link
Contributor

This patch make it possible to configure the S3 ListObjectsVersion, which is already configurable in the thanos-io S3 Client.

What this PR does

This PR introduce a new configuration flag -<prefix>.s3.list-objects-version to be able to configure the used S3 ListObjectsVersion as already possible in the underlying thanos-io S3 Client.

This helps, e.g. with older versions from Ceph, which only give 1000 Objects, if this is not configured to v1.

Which issue(s) this PR fixes or relates to

No issue for this, I directly created this Pull Request.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@shaardie shaardie requested review from a team as code owners May 30, 2023 10:01
@CLAassistant
Copy link

CLAassistant commented May 30, 2023

CLA assistant check
All committers have signed the CLA.

@flxbk
Copy link
Contributor

flxbk commented Jun 5, 2023

The CHANGELOG has just been cut to prepare for the next Mimir release. Please rebase main and eventually move the CHANGELOG entry added / updated in this PR to the top of the CHANGELOG document. Thanks!

This patch make it possible to configure the S3 ListObjectsVersion, which is
already configurable in the [thanos-io S3
Client](https://github.com/thanos-io/objstore/blob/23ebe2eacadd89cf23bf0fbd931352112b4c846d/providers/s3/s3.go#LL137C47-L137C67).

Signed-off-by: Sven Haardiek <sven@haardiek.de>
@shaardie shaardie force-pushed the s3-list-objects-version branch from dec4225 to 28815f7 Compare June 16, 2023 14:05
@shaardie
Copy link
Contributor Author

The CHANGELOG has just been cut to prepare for the next Mimir release. Please rebase main and eventually move the CHANGELOG entry added / updated in this PR to the top of the CHANGELOG document. Thanks!

Done.

Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change LGTM. Thanks and sorry for the delay reviewing!

pkg/storage/bucket/s3/config.go Outdated Show resolved Hide resolved
shaardie and others added 2 commits June 27, 2023 16:27
This patch adds the suggestions from the PR

Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>
@56quarters
Copy link
Contributor

Looks like the docs need to be regenerated based on the CLI flag change. make docs and make reference-help should do it, then I can merge. Thanks!

Signed-off-by: Sven Haardiek <sven@haardiek.de>
@shaardie
Copy link
Contributor Author

Looks like the docs need to be regenerated based on the CLI flag change. make docs and make reference-help should do it, then I can merge. Thanks!

Done and great that this can be merged!

@56quarters 56quarters merged commit 6a76b73 into grafana:main Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants