-
Notifications
You must be signed in to change notification settings - Fork 109
PBM-1618: Minio as storage type for Custom-S3 storages #1203
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
Conversation
It's not used anymore.
Intention is to use minio driver as the alternative for accessing S3 compatible storages or minio storage itself.
- Adding handling for ErrNotExist resolutin during Copy and Delete. - Handle prefix during the List operation
PBM propagates LogEvent by injecting it in types and methods. Custom logging typically requires logger instance. Therefore it's exposed in LogEvent interface.
It allows turning debug trace on/off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements MinIO as a new storage type for Custom-S3 storages in the Percona Backup for MongoDB (PBM) system. The implementation provides MinIO-specific storage functionality while maintaining compatibility with the existing storage interface.
Key changes:
- Added new
mio
package with MinIO-specific storage implementation - Integrated MinIO storage type into the existing storage configuration and factory system
- Refactored S3/GCS constants to avoid duplication
Reviewed Changes
Copilot reviewed 13 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
pbm/util/storage.go | Added factory methods for creating MinIO storage instances |
pbm/storage/storage.go | Added MinIO as a new storage type constant and parsing support |
pbm/storage/s3/s3.go | Removed duplicate GCS endpoint constant and provider field |
pbm/storage/mio/minio.go | Core MinIO storage implementation with upload, download, and file operations |
pbm/storage/mio/download.go | MinIO-specific download functionality with concurrent chunk handling |
pbm/storage/mio/config.go | Configuration structure and validation for MinIO storage |
pbm/storage/mio/config_test.go | Unit tests for MinIO configuration functionality |
pbm/storage/mio/minio_test.go | Integration tests using testcontainers for MinIO validation |
pbm/storage/gcs/gcs.go | Added GCS endpoint constant to replace removed duplicate |
pbm/storage/gcs/hmac_client.go | Updated to use the new GCS endpoint constant |
pbm/config/config.go | Integrated MinIO configuration into the main config structure |
pbm/config/config_test.go | Added tests for MinIO configuration handling |
go.mod | Added google/go-cmp dependency for test comparisons |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Minio library supports changing this only on the global level. For PBM that means data races or performance degradation. MaxRetries should be enough.
The Region info was displayed just for S3 type of storage. Storage config is expanded with the method that resolves the Region field for any type of the storage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 18 out of 39 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
7f8db59
to
8bdddda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 18 out of 39 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
a3280cb
to
4df0ee6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 18 out of 39 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
pbm/storage/mio/minio.go:1
- The hardcoded 60-second timeout should be configurable or defined as a constant to improve maintainability and allow for different timeout requirements.
package mio
pbm/storage/mio/minio.go:1
- The buffer size of 32KB should be defined as a named constant to improve code readability and maintainability.
package mio
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 18 out of 39 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
PR started as the bug fix for PBM-1618.
PR implements the feature described in PBM-1631.
PR adds a new storage type called
minio
, which uses minio client and enables targeting any custom S3-compatible storage.PBM config for
minio
storage type looks like:Default values are specified within example above.
Besides standard configuration parameters, the following are specific to
minio
:secure
: usinghttp
orhttps
credentials.signature-ver
: using latest AWS Signature Version 4 or the previous one Version 2 (V2
should be used in config)debugTrace
: iftrue
it'll output all http communication trace in pbm's log