feat: add S3 chunk delimiter config to support MinIO running on Windows #16319
Conversation
when the chunk_delimiter is specified then the objectkey will be converted prior to sending a request to the S3 (MinIO) storage provider. The list() response creates StorageObjects so convert the key back to normal again so that downstream logic still works.
github-actions
bot
added
the
type/docs
|
Makes sense though you can't change this midflight. We should probably document it. |
adding the flag mapping as well as updating the documentation.
|
@cyriltovena added flag and updated documentation |
|
@cyriltovena can you let me know if the PR is fine with the comments handled? |
configuration.md is auto generated and usage is coming from the RegisterFlagsWithPrefix definition.
|
@cyriltovena @JStickler comments have been addressed. Please let me know if there is anything else to address. |
|
@Notedop, since you added a flag, you need to update the generated configuration documentation by running 'make doc' (singular) from the root directory (/loki) and commit the generated changes. Then we should be ready to merge this. |
|
@JStickler I ran |
There was a problem hiding this comment.
lgtm!
Ideally we should also support this option for new storage clients introduced in 3.4 release but it should not block us from merging this
https://grafana.com/docs/loki/latest/setup/migrate/migrate-storage-clients/
|
@ashwanthgoli So there's one test failing but I don't see how it is related to this change. What would be best method to address this? Update: After rerunning the previously failing test is now passing (promtail test was failing). But still the golangcilint check is failing. |
What this PR does / why we need it:
When MinIO runs on Windows, it cannot handle the colon ":" chunk delimiter and any put of chunks will be rejected.
Confirmed by MinIO here
This change allows the user to override the chunk delimiter similar as we allow for Azure storage configuration.
Default chunk delimiter remains unchanged for backwards compatibility.
Delimiter is converted before sending out a chunk (put) as well as when listing chunks (list) to continue to support any downstream logic.
Which issue(s) this PR fixes:
Fixes #14600
Special notes for your reviewer:
Only updated the documented example and added a comment for it's specific usage
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mddeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR