Add rustfs#65
Open
kwonkwonn wants to merge 3 commits into
Open
Conversation
kwonkwonn
force-pushed
the
object_storage_intergration
branch
3 times, most recently
from
a664e76 to
9b1ec88
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
kwonkwonn
force-pushed
the
object_storage_intergration
branch
from
9b1ec88 to
2923f85
Compare
kwonkwonn
force-pushed
the
object_storage_intergration
branch
from
2923f85 to
1198a35
Compare
kwonkwonn
commented
May 25, 2026
kwonkwonn
left a comment
kwonkwonn
left a comment
Contributor
Author
There was a problem hiding this comment.
공식 문서상 aws-s3-sdk 와 완전히 compatible 하다고 합니다.
그래서 aws-s3-sdk 를 가져다 쓸 예정이고, list 및 bucket 생성은 컨트롤에서
업로드 및 다운로드는 preasigned link 를 코어에 넘기는 방식으로 할 예정입니다
| module github.com/easy-cloud-Knet/KWS_Control | ||
|
|
||
| go 1.23.0 | ||
| go 1.24 |
Contributor
Author
There was a problem hiding this comment.
1.23.0 <-> aws sdk 와 호환성 문제가 있어서 업데이트 했는데 혹시 관련해서 문제 있을까요
| endpoint := os.Getenv("RUSTFS_ENDPOINT") | ||
| if endpoint == "" { | ||
| endpoint = "http://localhost:9001" | ||
| } |
Contributor
Author
There was a problem hiding this comment.
minioadmin<-- docker-compose.test.yaml 파일의 인자를 일단 덮어쓰도록 했습니다.
control 단에서 accesskey 넘어가는건 마지막 단계에서 할 거 같아요
Contributor
Author
|
feat: integrate RustFS as external snapshot storage Summary
Changes
Notes
|
📝 WalkthroughWalkthroughAdds a RustFSClient (S3 wrapper) with presign and object APIs, snapshot service functions that use the client, HTTP snapshot endpoints and models, test Docker Compose wiring for RustFS, end-to-end snapshot tests, and Go/tooling dependency updates. ChangesRustFS Client and Snapshot Flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/snapshot.go`:
- Around line 36-40: The handler is returning raw err.Error() to clients (e.g.,
after service.TakeSnapshot in the snapshot handler) which leaks internal
details; instead keep the existing log.Error call to record full details but
change the HTTP response body passed to util.RespondError to a generic message
(e.g., "internal server error" or "failed to take snapshot") that does not
include error internals; apply the same change for the other occurrences
referenced (the error branches around lines 54-59 and 78-82) so all calls to
util.RespondError use non-sensitive, user-friendly messages while preserving the
detailed server-side logs.
In `@client/rustfs.go`:
- Around line 132-147: The ListObjects implementation in RustFSClient currently
issues a single ListObjectsV2 call and truncates results beyond 1000 keys;
replace that single-call logic with the AWS SDK v2 paginator
(s3.NewListObjectsV2Paginator) to iterate all pages, accumulating
aws.ToString(obj.Key) for each out.Contents until the paginator is done,
propagate any page error as before, and ensure you still return an empty slice
(not nil) when no objects are found; update the ListObjects function to use the
paginator and preserve the existing error message formatting.
- Around line 40-42: The default RUSTFS endpoint fallback currently sets
endpoint = "http://localhost:9001" which points to the web console; change the
fallback to the S3 API port by setting the default to "http://localhost:9000" in
client/rustfs.go where the variable endpoint is assigned so S3-compatible calls
go to the correct port.
In `@service/external_snap.go`:
- Around line 14-24: ListSnapshots currently returns an error when
rustfs.ListObjects fails for a VM that has no per-UUID bucket; change
ListSnapshots (and the ListObjects error handling) to treat a NoSuchBucket (or
equivalent "bucket not found") error as "no snapshots" by returning an empty
[]string and nil error instead of propagating a 500; use the same rustfs client
from client.GetRustFSClient and detect the missing-bucket condition via the
rustfs/minio/S3 error sentinel or by matching the NoSuchBucket error string,
then return []string{} , nil.
- Around line 39-53: TakeSnapshot currently presigns a PUT against string(uuid)
without ensuring the per-VM bucket exists, causing NoSuchBucket on upload;
modify TakeSnapshot (before calling rustfs.PresignPutObject) to ensure the
bucket exists by calling rustfs.CreateBucket (or a new helper EnsureBucket) for
bucket string(uuid) and treat BucketAlreadyOwnedByYou/BucketAlreadyExists as
non-fatal (swallow those specific errors), returning other errors; keep the
existing rustfs.PresignPutObject call after the bucket existence check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f119b54-f2ff-44ca-94ff-d730db692a7a
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
.gitignoreDockerfileapi/handlers.goapi/snapshot.goclient/model/snapshot.goclient/model/vm.goclient/rustfs.gogo.modservice/external_snap.gotests/docker-compose.test.yml
✅ Files skipped from review due to trivial changes (1)
- Dockerfile
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/docker-compose.test.yml
kwonkwonn
force-pushed
the
object_storage_intergration
branch
from
78662e2 to
43238cf
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/blackbox_io_test.sh (1)
455-461: ⚡ Quick win
set -emakes a silentmc_execfailure abort the suite without a reported assertion.
mc_execon Line 455 runs as a bare command underset -euo pipefail. Ifmc mb/mc cpreturns non-zero (e.g., alias setup failed, bucket name collision, RRustFS not yet ready), the script exits immediately—before the Line 457/461 assertions can surface aFAIL. Because all output is routed to/dev/null, the run terminates with no diagnostic about why. Consider gating the seed step on success (or asserting it) so the failure is reported through the harness.♻️ Proposed guard for the seed step
-mc_exec "mc mb r/${SNAP_UUID} && echo seed > /tmp/f && mc cp /tmp/f r/${SNAP_UUID}/snap-seed" +if mc_exec "mc mb r/${SNAP_UUID} && echo seed > /tmp/f && mc cp /tmp/f r/${SNAP_UUID}/snap-seed"; then + pass "Seeded snapshot bucket/key via mc" +else + fail "Failed to seed snapshot bucket/key via mc" "mc seed ok" "mc seed failed" +fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/blackbox_io_test.sh` around lines 455 - 461, The seed step uses mc_exec as a bare command under set -euo pipefail so any silent failure aborts the suite without a test assertion; change the seed step (the mc_exec invocation that runs "mc mb ... && mc cp ...") to be guarded and asserted: run mc_exec and capture its exit status and output, or wrap it in an assertion helper (e.g., use assert_status/assert_exit or a custom assert_command) so failures produce a controlled test failure and include the mc_exec stdout/stderr (don’t redirect all output to /dev/null) before proceeding to the GET /vm/snapshot checks (references: mc_exec invocation, assert_status, assert_body_contains in the test).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/blackbox_io_test.sh`:
- Around line 455-461: The seed step uses mc_exec as a bare command under set
-euo pipefail so any silent failure aborts the suite without a test assertion;
change the seed step (the mc_exec invocation that runs "mc mb ... && mc cp ...")
to be guarded and asserted: run mc_exec and capture its exit status and output,
or wrap it in an assertion helper (e.g., use assert_status/assert_exit or a
custom assert_command) so failures produce a controlled test failure and include
the mc_exec stdout/stderr (don’t redirect all output to /dev/null) before
proceeding to the GET /vm/snapshot checks (references: mc_exec invocation,
assert_status, assert_body_contains in the test).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0129e5f0-099e-447d-9fcd-2d9900a8d1d0
📒 Files selected for processing (8)
.gitignoreapi/handlers.goapi/snapshot.goclient/model/snapshot.goclient/model/vm.goclient/rustfs.goservice/external_snap.gotests/blackbox_io_test.sh
✅ Files skipped from review due to trivial changes (2)
- client/model/vm.go
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (5)
- client/model/snapshot.go
- api/handlers.go
- service/external_snap.go
- client/rustfs.go
- api/snapshot.go
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
Closes #64 — Story 1: rustfsClient 구현 및 테스트 환경 구성
client/rustfs.go: aws-sdk-go-v2 기반 RustFS 클라이언트 구현
Test plan
docker compose -f docker-compose.test.yml up --build 실행 후 kws-test-rustfs healthy 확인
NewRustFSClient() 호출 시 버킷 생성 및 ListBuckets() 반환값 로그 확인
Presigned URL로 Core에서 추가 인증 없이 업로드/다운로드 가능 여부 확인
Out of scope (후속 태스크)
Service 레이어 연동 (Story 2)
Repository / DB 스키마 (별도 태스크)
동기/비동기 처리 방식 확정 (Story 3)
notes
go.mod 및 dockerfile 에 변화가 있었습니다.
aws-sdk import 중에 발생했는데 특정 버전에 sdk 를 맞추는것도 좋지 않아보여서 업데이트했습니다.
혹시 관련하여 우려사항 있다면 알려주세용
Type of Change
Testing
Checklist
Summary by CodeRabbit
New Features
Tests
Chores