Rustfs#907
Open
desmax74 wants to merge 1 commit into
Open
Conversation
Reviewer's GuideAdds optional RustFS S3-compatible storage backend support for E2E Minikube workflows and development/demo setups, replacing filesystem PVC storage where appropriate and updating fixtures, documentation, and diagnostics accordingly. Sequence diagram for E2E Minikube workflow with selectable RustFS storage backendsequenceDiagram
participant GitHubActions
participant MinikubeCluster
participant RustFS
participant MinIOmc
participant TrustedProfileAnalyzer
GitHubActions->>GitHubActions: set STORAGE_TYPE from workflow_dispatch
alt STORAGE_TYPE == rustfs
GitHubActions->>MinikubeCluster: kubectl apply -f test/e2e/fixtures/rustfs.yaml
MinikubeCluster-->>RustFS: create deployment rustfs
GitHubActions->>MinikubeCluster: kubectl wait deployment/rustfs
GitHubActions->>MinikubeCluster: kubectl run create-bucket (minio/mc)
MinikubeCluster-->>MinIOmc: start create-bucket pod
MinIOmc->>RustFS: mc alias set rustfs
MinIOmc->>RustFS: mc mb rustfs/trustify
GitHubActions->>MinikubeCluster: envsubst < e2e-minikube-rustfs-cr.yaml | kubectl apply -f -
MinikubeCluster-->>TrustedProfileAnalyzer: create CR with storage type s3
else STORAGE_TYPE == filesystem
GitHubActions->>MinikubeCluster: rely on PVC storage
GitHubActions->>MinikubeCluster: envsubst < e2e-minikube-cr.yaml | kubectl apply -f -
MinikubeCluster-->>TrustedProfileAnalyzer: create CR with storage type filesystem
end
GitHubActions->>MinikubeCluster: run status and diagnostics
alt STORAGE_TYPE == rustfs
GitHubActions->>MinikubeCluster: get deployment rustfs
else STORAGE_TYPE == filesystem
GitHubActions->>MinikubeCluster: get pvc storage
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
storage_typeinput description says 'filesystem (default)' but the actual default is set to 'rustfs'; consider aligning the default value with the description to avoid confusion. - The RustFS deployment uses the
rustfs/rustfs:latestimage tag, which can introduce non-reproducible behavior; prefer pinning to a specific version or digest for stability in e2e runs. - RustFS credentials (
rustfsadmin/rustfsadmin) are hard-coded in both the deployment and CR fixture; consider parameterizing these via secrets or environment variables to reduce credential duplication and future changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `storage_type` input description says 'filesystem (default)' but the actual default is set to 'rustfs'; consider aligning the default value with the description to avoid confusion.
- The RustFS deployment uses the `rustfs/rustfs:latest` image tag, which can introduce non-reproducible behavior; prefer pinning to a specific version or digest for stability in e2e runs.
- RustFS credentials (`rustfsadmin`/`rustfsadmin`) are hard-coded in both the deployment and CR fixture; consider parameterizing these via secrets or environment variables to reduce credential duplication and future changes.
## Individual Comments
### Comment 1
<location path=".github/workflows/e2e-minikube.yml" line_range="6-9" />
<code_context>
on:
+ workflow_dispatch:
+ inputs:
+ storage_type:
+ description: 'Storage backend: filesystem (default) or rustfs (S3-compatible)'
+ required: false
+ default: 'rustfs'
+ type: choice
+ options:
</code_context>
<issue_to_address>
**nitpick (bug_risk):** Align the input description with the actual default value for storage_type.
The description lists `filesystem` as the default, but the actual default is `rustfs`. This mismatch can cause someone triggering the workflow to select an unintended backend. Please update either the description or the default so they match.
</issue_to_address>
### Comment 2
<location path=".github/workflows/e2e-minikube.yml" line_range="97-101" />
<code_context>
+ kubectl apply -f test/e2e/fixtures/rustfs.yaml
+ kubectl -n e2e-test wait --for=condition=available deployment/rustfs --timeout=120s
+ echo "RustFS is ready, creating bucket..."
+ kubectl -n e2e-test run create-bucket --rm -i --restart=Never \
+ --image=minio/mc:latest \
+ --command -- sh -c '
+ mc alias set rustfs http://rustfs.e2e-test.svc.cluster.local:9000 rustfsadmin rustfsadmin &&
+ mc mb rustfs/trustify &&
+ echo "Bucket trustify created successfully"
+ '
</code_context>
<issue_to_address>
**issue:** Handle the case where the RustFS bucket already exists to avoid step failures.
If the `trustify` bucket already exists from a prior run, `mc mb rustfs/trustify` will exit non‑zero and fail this step. Please use a pattern that tolerates existing buckets (e.g., `mc mb --ignore-existing` or a pre-check) so the workflow remains idempotent and repeatable.
</issue_to_address>
### Comment 3
<location path=".github/workflows/e2e-minikube.yml" line_range="98" />
<code_context>
+ kubectl -n e2e-test wait --for=condition=available deployment/rustfs --timeout=120s
+ echo "RustFS is ready, creating bucket..."
+ kubectl -n e2e-test run create-bucket --rm -i --restart=Never \
+ --image=minio/mc:latest \
+ --command -- sh -c '
+ mc alias set rustfs http://rustfs.e2e-test.svc.cluster.local:9000 rustfsadmin rustfsadmin &&
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid using the `latest` tag for the `minio/mc` image to ensure reproducibility.
Using `minio/mc:latest` makes the workflow depend on whatever the current image version is, which can change behavior unexpectedly and introduce subtle bugs or security issues. Please pin a specific, known-good tag (or at least a major/minor version) to keep the E2E workflow stable and reproducible.
Suggested implementation:
```
kubectl -n e2e-test run create-bucket --rm -i --restart=Never \
--image=minio/mc:RELEASE.2024-05-28T16-45-23Z \
```
1. Verify that `minio/mc:RELEASE.2024-05-28T16-45-23Z` (or another specific tag you prefer) exists and works with your RustFS setup. If you already use a specific `minio/mc` tag elsewhere in the repo or infrastructure, reuse that tag here for consistency.
2. If you select a different tag, replace `RELEASE.2024-05-28T16-45-23Z` in the snippet above with that exact tag.
3. Optionally add a short comment near this step documenting why the tag is pinned (e.g. "Do not use :latest; pin mc version for reproducible E2E runs").
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| kubectl -n e2e-test wait --for=condition=available deployment/rustfs --timeout=120s | ||
| echo "RustFS is ready, creating bucket..." | ||
| kubectl -n e2e-test run create-bucket --rm -i --restart=Never \ | ||
| --image=minio/mc:latest \ |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid using the latest tag for the minio/mc image to ensure reproducibility.
Using minio/mc:latest makes the workflow depend on whatever the current image version is, which can change behavior unexpectedly and introduce subtle bugs or security issues. Please pin a specific, known-good tag (or at least a major/minor version) to keep the E2E workflow stable and reproducible.
Suggested implementation:
kubectl -n e2e-test run create-bucket --rm -i --restart=Never \
--image=minio/mc:RELEASE.2024-05-28T16-45-23Z \
- Verify that
minio/mc:RELEASE.2024-05-28T16-45-23Z(or another specific tag you prefer) exists and works with your RustFS setup. If you already use a specificminio/mctag elsewhere in the repo or infrastructure, reuse that tag here for consistency. - If you select a different tag, replace
RELEASE.2024-05-28T16-45-23Zin the snippet above with that exact tag. - Optionally add a short comment near this step documenting why the tag is pinned (e.g. "Do not use :latest; pin mc version for reproducible E2E runs").
Collaborator
Author
There was a problem hiding this comment.
Is the minio client used with rustfs until the rustfs project produce a client to remove this line
There was a problem hiding this comment.
Yes, in this workflow the MinIO client is just a convenient S3-compatible CLI used to create the bucket in RustFS until RustFS provides its own tooling or a different recommended way to initialize storage.
Concretely, RustFS exposes an S3-compatible API, and minio/mc is speaking that API to:
- connect to the RustFS endpoint
- create the test bucket
- apply any initial configuration you might need
So this step is an implementation detail of the E2E setup, not something that’s conceptually tied to MinIO itself. Once RustFS offers either:
- its own client CLI, or
- a Kubernetes init container / job or CRD to handle bucket creation,
we can replace this kubectl run ... --image=minio/mc line with the RustFS-native mechanism and drop the MinIO dependency entirely.
In the meantime, I’d still recommend pinning the minio/mc image tag so this temporary dependency stays stable and doesn’t introduce flaky runs.
If you’d like, we can add a short comment above the step to make the intent explicit, e.g.:
# TODO: Replace MinIO mc client with RustFS-native tooling once available
# Using mc only as a generic S3 client to create the RustFS test bucket
- name: Deploy RustFS
if: env.STORAGE_TYPE == 'rustfs'
run: |
kubectl -n e2e-test run create-bucket --rm -i --restart=Never \
--image=minio/mc:RELEASE.2024-05-28T16-45-23Z \
...
Add workflow_dispatch input to the e2e-minikube workflow allowing manual selection of RustFS as the storage backend instead of the default filesystem. Default push/PR behavior is unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new RustFS deployment manifests are duplicated between
devel/rustfs.yamlandtest/e2e/fixtures/rustfs.yaml; consider factoring them into a single reusable manifest or kustomize/helm overlay to keep configuration in one place. - Both the RustFS and MinIO mc images use the
:latesttag (rustfs/rustfs:latest,minio/mc:latest); pinning to explicit versions would make e2e and dev environments more deterministic and easier to debug when upstream images change. - Switching the default
STORAGE_TYPEin the e2e Minikube workflow torustfschanges the default CI behavior; consider keepingfilesystemas the default for pull_request runs and usingrustfsonly when explicitly requested viaworkflow_dispatchto avoid surprising changes in pipeline behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new RustFS deployment manifests are duplicated between `devel/rustfs.yaml` and `test/e2e/fixtures/rustfs.yaml`; consider factoring them into a single reusable manifest or kustomize/helm overlay to keep configuration in one place.
- Both the RustFS and MinIO mc images use the `:latest` tag (`rustfs/rustfs:latest`, `minio/mc:latest`); pinning to explicit versions would make e2e and dev environments more deterministic and easier to debug when upstream images change.
- Switching the default `STORAGE_TYPE` in the e2e Minikube workflow to `rustfs` changes the default CI behavior; consider keeping `filesystem` as the default for pull_request runs and using `rustfs` only when explicitly requested via `workflow_dispatch` to avoid surprising changes in pipeline behavior.
## Individual Comments
### Comment 1
<location path=".github/workflows/e2e-minikube.yml" line_range="99-101" />
<code_context>
+ # TODO: Replace MinIO mc client with RustFS-native tooling once available
+ # Using mc only as a generic S3 client to create the RustFS test bucket
+ - name: Deploy RustFS
+ if: env.STORAGE_TYPE == 'rustfs'
+ run: |
+ echo "Deploying RustFS S3-compatible storage..."
+ kubectl apply -f test/e2e/fixtures/rustfs.yaml
+ kubectl -n e2e-test wait --for=condition=available deployment/rustfs --timeout=120s
+ echo "RustFS is ready, creating bucket..."
+ kubectl -n e2e-test run create-bucket --rm -i --restart=Never \
+ --image=minio/mc:latest \
+ --command -- sh -c '
+ mc alias set rustfs http://rustfs.e2e-test.svc.cluster.local:9000 rustfsadmin rustfsadmin &&
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid using the `latest` tag for the MinIO mc image to keep e2e runs stable and reproducible.
Using `minio/mc:latest` makes the RustFS bootstrap step depend on upstream changes, which can introduce flaky or irreproducible e2e failures. Please pin to a specific, tested image tag (e.g. `minio/mc:<version>`) and update it intentionally when needed.
```suggestion
kubectl -n e2e-test run create-bucket --rm -i --restart=Never \
--image=minio/mc:RELEASE.2024-05-28T17-19-04Z \
--command -- sh -c '
```
</issue_to_address>
### Comment 2
<location path=".github/workflows/e2e-minikube.yml" line_range="318" />
<code_context>
+ else
+ echo "=== RustFS Status ==="
+ kubectl -n e2e-test get deployment rustfs || true
+ kubectl -n e2e-test logs deployment/rustfs --tail=50 || true
+ fi
</code_context>
<issue_to_address>
**issue (bug_risk):** The kubectl logs invocation targeting the Deployment is likely to fail; use pod selection instead.
`kubectl logs deployment/rustfs` will not work as written because `kubectl logs` expects a pod name or a label selector, not a bare Deployment. This means RustFS logs likely won’t be collected. Please switch to a pod-based invocation, e.g.:
```sh
echo "=== RustFS Status ==="
kubectl -n e2e-test get pods -l app=rustfs || true
kubectl -n e2e-test logs $(kubectl -n e2e-test get pods -l app=rustfs -o jsonpath='{.items[0].metadata.name}') --tail=50 || true
```
or, if supported in your kubectl version:
```sh
kubectl logs -l app=rustfs -n e2e-test --tail=50 || true
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+99
to
+101
| kubectl -n e2e-test run create-bucket --rm -i --restart=Never \ | ||
| --image=minio/mc:latest \ | ||
| --command -- sh -c ' |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid using the latest tag for the MinIO mc image to keep e2e runs stable and reproducible.
Using minio/mc:latest makes the RustFS bootstrap step depend on upstream changes, which can introduce flaky or irreproducible e2e failures. Please pin to a specific, tested image tag (e.g. minio/mc:<version>) and update it intentionally when needed.
Suggested change
| kubectl -n e2e-test run create-bucket --rm -i --restart=Never \ | |
| --image=minio/mc:latest \ | |
| --command -- sh -c ' | |
| kubectl -n e2e-test run create-bucket --rm -i --restart=Never \ | |
| --image=minio/mc:RELEASE.2024-05-28T17-19-04Z \ | |
| --command -- sh -c ' |
| else | ||
| echo "=== RustFS Status ===" | ||
| kubectl -n e2e-test get deployment rustfs || true | ||
| kubectl -n e2e-test logs deployment/rustfs --tail=50 || true |
There was a problem hiding this comment.
issue (bug_risk): The kubectl logs invocation targeting the Deployment is likely to fail; use pod selection instead.
kubectl logs deployment/rustfs will not work as written because kubectl logs expects a pod name or a label selector, not a bare Deployment. This means RustFS logs likely won’t be collected. Please switch to a pod-based invocation, e.g.:
echo "=== RustFS Status ==="
kubectl -n e2e-test get pods -l app=rustfs || true
kubectl -n e2e-test logs $(kubectl -n e2e-test get pods -l app=rustfs -o jsonpath='{.items[0].metadata.name}') --tail=50 || trueor, if supported in your kubectl version:
kubectl logs -l app=rustfs -n e2e-test --tail=50 || true
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add optional RustFS S3-compatible storage backend support to the E2E Minikube workflow and fixtures.
New Features:
Enhancements:
Assisted by Claude Opus 4.6
Summary by Sourcery
Introduce RustFS as an S3-compatible storage backend option and integrate it into development, Helm values, and E2E Minikube workflows, replacing filesystem PVC storage where appropriate.
New Features:
Enhancements:
Documentation: