[#1] Add frostfs backend #1

Merged
alexvanin merged 3 commits from KurlesHS/rclone:feature/add-frostfs-support into tcl/master 2025-01-16 11:01:23 +00:00
Member

To run the FrostFS backend integration tests, we need to prepare a configuration file with the "TestFrostFS" section, for example:

/rclone.conf

[TestFrostFS]
type = frostfs
frostfs_endpoint = s01.frostfs.devenv:8080
wallet = /wallets/wallet.json
password = strong-pass
placement_policy = REP 1
basic_acl = 0x1fffffff

and then start tests

$ RCLONE_CONFIG=/rclone.conf go test -run TestIntegration ./backend/frostfs

Signed-off-by: Aleksey Kravchenko al.kravchenko@yadro.com

To run the FrostFS backend integration tests, we need to prepare a configuration file with the "TestFrostFS" section, for example: `/rclone.conf` ``` [TestFrostFS] type = frostfs frostfs_endpoint = s01.frostfs.devenv:8080 wallet = /wallets/wallet.json password = strong-pass placement_policy = REP 1 basic_acl = 0x1fffffff ``` and then start tests ```bash $ RCLONE_CONFIG=/rclone.conf go test -run TestIntegration ./backend/frostfs ``` Signed-off-by: Aleksey Kravchenko <al.kravchenko@yadro.com>
KurlesHS added 1 commit 2024-12-21 13:35:11 +00:00
[#1] Add frostfs backend
All checks were successful
/ Builds (pull_request) Successful in 2m28s
54ad3809d2
Signed-off-by: Aleksey Kravchenko <al.kravchenko@yadro.com>
KurlesHS force-pushed feature/add-frostfs-support from 54ad3809d2 to 538ac9c9df 2024-12-21 14:48:02 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 538ac9c9df to 387f80f636 2024-12-21 14:51:17 +00:00 Compare
KurlesHS added 1 commit 2024-12-21 14:55:36 +00:00
[#1] Add frostfs backend
All checks were successful
/ DCO (pull_request) Successful in 1m38s
/ Builds (pull_request) Successful in 2m48s
/ Lint (pull_request) Successful in 6m12s
ca9abdb92e
Signed-off-by: Aleksey Kravchenko <al.kravchenko@yadro.com>
KurlesHS force-pushed feature/add-frostfs-support from ca9abdb92e to 4a647a40c7 2024-12-21 15:01:00 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 4a647a40c7 to d9fbd56598 2024-12-21 15:13:49 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from d9fbd56598 to 54c191978d 2024-12-21 15:59:37 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 54c191978d to a370ad60f9 2024-12-21 16:03:42 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from a370ad60f9 to 0e26cadc30 2024-12-21 16:13:23 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 0e26cadc30 to 2d25df59c5 2024-12-21 16:19:57 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 2d25df59c5 to aacfcd8a28 2024-12-21 16:24:46 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from aacfcd8a28 to 7f403ca2db 2024-12-21 16:31:03 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 7f403ca2db to 7453b0430f 2024-12-21 16:52:38 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 7453b0430f to bfaad53f9a 2024-12-21 16:56:28 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from bfaad53f9a to f47ef1e0d3 2024-12-21 17:02:51 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from f47ef1e0d3 to 6053fb8cce 2024-12-24 07:30:53 +00:00 Compare
requested reviews from storage-services-committers, storage-services-developers 2024-12-24 07:58:29 +00:00
KurlesHS self-assigned this 2024-12-24 07:58:52 +00:00
KurlesHS changed title from WIP: [#1] Add frostfs backend to [#1] Add frostfs backend 2024-12-24 07:58:55 +00:00
dkirillov reviewed 2024-12-25 12:07:34 +00:00
@ -18,6 +18,7 @@ import (
_ "github.com/rclone/rclone/backend/fichier"
_ "github.com/rclone/rclone/backend/filefabric"
_ "github.com/rclone/rclone/backend/filescom"
_ "github.com/rclone/rclone/backend/frostfs"
Member

I suppose we also should add some docs to readme and other places (see contributing guide https://git.frostfs.info/TrueCloudLab/rclone/src/branch/tcl/master/CONTRIBUTING.md)

I suppose we also should add some docs to readme and other places (see contributing guide https://git.frostfs.info/TrueCloudLab/rclone/src/branch/tcl/master/CONTRIBUTING.md)
Author
Member

Done.

Done.
@ -0,0 +18,4 @@
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl"
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap"
Member

Unnecessary empty line

Unnecessary empty line
KurlesHS marked this conversation as resolved
@ -0,0 +19,4 @@
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap"
// containerApi "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/container"
Member

Debug?

Debug?
KurlesHS marked this conversation as resolved
@ -0,0 +135,4 @@
}
const (
attrFilePath = "FilePath"
Member

Instead of this const we can use object.AttributeFilePath

Instead of this const we can use `object.AttributeFilePath`
KurlesHS marked this conversation as resolved
@ -0,0 +151,4 @@
Password string `config:"password"`
PlacementPolicy string `config:"placement_policy"`
BasicACLStr string `config:"basic_acl"`
BasicACL acl.Basic `config:"-"`
Member

We must not use basic acl. Since new versions of frostfs-node doesn't support it.
We should either:

  • parse current basic_acl flag and form appropriate APE rules and set if for container (after creation)
  • replace basic_acl flag to ape_rules and set these rules for container
We must not use basic acl. Since new versions of frostfs-node doesn't support it. We should either: * parse current `basic_acl` flag and form appropriate APE rules and set if for container (after creation) * replace `basic_acl` flag to `ape_rules` and set these rules for container
Author
Member

I have renamed this parameter as "container creation policy" and implemented its parsing in the corresponding APE rules.

I have renamed this parameter as "container creation policy" and implemented its parsing in the corresponding APE rules.
@ -0,0 +285,4 @@
if attr.Key() == object.AttributeFileName {
objInfo.name = attr.Value()
}
if attr.Key() == attrFilePath {
Member

We can use object.AttributeFilePath

We can use `object.AttributeFilePath`
KurlesHS marked this conversation as resolved
@ -0,0 +469,4 @@
// Hashes returns the supported hash sets.
func (f *Fs) Hashes() hash.Set {
return hash.Set(hash.SHA256)
Member

Can be just

return hash.SHA256
Can be just ```golang return hash.SHA256 ```
Author
Member

Unfortunately, no, because the hash.SHA256 variable is of type hash.Type.

Unfortunately, no, because the `hash.SHA256` variable is of type `hash.Type`.
@ -0,0 +616,4 @@
func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ...fs.OpenOption) error {
// When updating an object, the path to it should not change.
src = robject.NewStaticObjectInfo(o.Remote(), src.ModTime(ctx), src.Size(), src.Storable(), nil, src.Fs())
obj, err := o.fs.Put(ctx, in, src, options...)
Member

It seems we can use Patch now instead of Put and Delete

It seems we can use `Patch` now instead of `Put` and `Delete`
Author
Member

Done. However, it was not possible to avoid using the "Delete" method as the "Patch" method does not remove the original object.

Done. However, it was not possible to avoid using the "Delete" method as the "Patch" method does not remove the original object.
Owner

Patch may retain object ID, is it possible that we "update" file with the same payload?
Do we have any randomization in header?

`Patch` may retain object ID, is it possible that we "update" file with the same payload? Do we have any randomization in header?
Author
Member

Patch may retain object ID, is it possible that we "update" file with the same payload?

Yes, it's posible

Do we have any randomization in header?

No, we don't. But I added a check for matching object IDs before and after executing the "Patch" method. If they match, the update process ends.

> `Patch` may retain object ID, is it possible that we "update" file with the same payload? Yes, it's posible > Do we have any randomization in header? No, we don't. But I added a check for matching object IDs before and after executing the "Patch" method. If they match, the update process ends.
@ -0,0 +46,4 @@
}
seen[address] = struct{}{}
endpointInfo := endpointInfo{
Member

nitpick: Can we use name that don't collide with type name
For example:

eInfo := endpointInfo{
nitpick: Can we use name that don't collide with type name For example: ```golang eInfo := endpointInfo{ ```
KurlesHS marked this conversation as resolved
go.mod Outdated
@ -3,1 +3,3 @@
go 1.21
go 1.22
toolchain go1.22.10
Member

Do we really need to fix toolchain?

Do we really need to fix toolchain?
KurlesHS marked this conversation as resolved
KurlesHS force-pushed feature/add-frostfs-support from 6053fb8cce to ef2d650f44 2024-12-25 13:32:53 +00:00 Compare
KurlesHS changed title from [#1] Add frostfs backend to WIP: [#1] Add frostfs backend 2024-12-27 05:32:07 +00:00
KurlesHS force-pushed feature/add-frostfs-support from ef2d650f44 to 52adedbb76 2024-12-27 06:17:48 +00:00 Compare
nzinkevich reviewed 2024-12-27 06:54:39 +00:00
@ -0,0 +110,4 @@
},
},
},
{
Member

question: shall we support basic-acl considering it was dropped from frostfs-cli? Or it's for back-compatibility only?

question: shall we support `basic-acl` considering it was dropped from `frostfs-cli`? Or it's for back-compatibility only?
Author
Member
see https://git.frostfs.info/TrueCloudLab/rclone/pulls/1#issuecomment-64261
@ -0,0 +119,4 @@
Value: "public-read-write",
Help: "Public container, anyone can read and write",
},
{
Member

Is it a correct value? I haven't found it here

Is it a correct value? I haven't found it [here](https://developers.neo.org/docs/n3/Advances/neofs/topics/acl-permissions#basic-acl-permissions)
Author
Member

To avoid ambiguity, I renamed this parameter to "container creation policy". #1 (comment)

To avoid ambiguity, I renamed this parameter to "container creation policy". https://git.frostfs.info/TrueCloudLab/rclone/pulls/1#issuecomment-64261
KurlesHS force-pushed feature/add-frostfs-support from 52adedbb76 to 7884f2e622 2024-12-27 16:19:29 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 7884f2e622 to 25cf424937 2024-12-27 17:40:06 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 25cf424937 to bb44b4a20c 2025-01-10 16:10:28 +00:00 Compare
KurlesHS changed title from WIP: [#1] Add frostfs backend to [#1] Add frostfs backend 2025-01-10 16:31:23 +00:00
KurlesHS changed title from [#1] Add frostfs backend to WIP: [#1] Add frostfs backend 2025-01-14 08:50:43 +00:00
KurlesHS force-pushed feature/add-frostfs-support from bb44b4a20c to 111801b51a 2025-01-14 09:17:52 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 111801b51a to 18a193ef37 2025-01-14 09:19:09 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 18a193ef37 to 888a1bb7a1 2025-01-14 09:20:22 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 888a1bb7a1 to 4c8b84c93a 2025-01-14 09:31:12 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 4c8b84c93a to 814e1a0d18 2025-01-14 09:32:35 +00:00 Compare
KurlesHS added 1 commit 2025-01-14 10:39:07 +00:00
[#1] Add forgejo actions
Some checks failed
/ DCO (pull_request) Successful in 39s
/ Builds (pull_request) Successful in 1m26s
/ Lint (pull_request) Successful in 3m42s
/ Test (pull_request) Failing after 1m10s
83e11d38e3
Signed-off-by: Aleksey Kravchenko <al.kravchenko@yadro.com>
KurlesHS force-pushed feature/add-frostfs-support from 83e11d38e3 to d7751246e4 2025-01-14 10:44:30 +00:00 Compare
KurlesHS added 1 commit 2025-01-14 10:45:46 +00:00
[#1] Add forgejo actions
Some checks failed
/ DCO (pull_request) Successful in 39s
/ Builds (pull_request) Successful in 1m21s
/ Lint (pull_request) Successful in 3m43s
/ Test (pull_request) Failing after 14s
7ad3b9e285
Signed-off-by: Aleksey Kravchenko <al.kravchenko@yadro.com>
KurlesHS force-pushed feature/add-frostfs-support from 7ad3b9e285 to eafb429201 2025-01-14 10:55:32 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from eafb429201 to bf17a132bb 2025-01-14 10:58:42 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from bf17a132bb to d2e5429f82 2025-01-14 11:02:04 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from d2e5429f82 to 10fdf60eaa 2025-01-14 11:11:47 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 10fdf60eaa to 1dc6754c6d 2025-01-14 19:28:01 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 1dc6754c6d to 436d0fcd6e 2025-01-14 19:33:30 +00:00 Compare
KurlesHS changed title from WIP: [#1] Add frostfs backend to [#1] Add frostfs backend 2025-01-15 08:04:26 +00:00
KurlesHS force-pushed feature/add-frostfs-support from 436d0fcd6e to 802952078e 2025-01-15 09:34:12 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 802952078e to aca96c9681 2025-01-15 09:44:53 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from aca96c9681 to 79813b16f9 2025-01-15 11:16:58 +00:00 Compare
alexvanin approved these changes 2025-01-15 11:22:22 +00:00
Dismissed
alexvanin left a comment
Owner

Overall looks good to me

Overall looks good to me
@ -0,0 +959,4 @@
if idEr, ok := dirEntry.(fs.IDer); ok {
if err = cnrID.DecodeString(idEr.ID()); err == nil {
f.containerIDCache[containerName] = cnrID.String()
}
Owner

That's a really wide code ladder 😃

That's a really wide code ladder 😃
Author
Member

I did a little refactoring of this section of the code.

I did a little refactoring of this section of the code.
@ -0,0 +1123,4 @@
addr.SetObject(id)
prmDelete.SetAddress(addr)
if err = f.pool.DeleteObject(ctx, prmDelete); err != nil {
Owner

@dkirillov @mbiryukova is there a room for future optimization by creating tombstone objects manually?

@dkirillov @mbiryukova is there a room for future optimization by creating tombstone objects manually?
@ -116,3 +116,3 @@
--tpslimit-burst int Max burst of transactions for --tpslimit (default 1)
--use-cookies Enable session cookiejar
--user-agent string Set the user-agent to a specified string (default "rclone/v1.68.2")
--user-agent string Set the user-agent to a specified string (default "rclone/v1.68.2-beta.8331.25cf42493.feature/add-frostfs-support")
Owner

Is this change autogenerated, by the way?

Is this change autogenerated, by the way?
Author
Member

Yes, the makefile has a separate step for generating documentation, which takes a fairly large piece of information directly from the code.

Yes, the makefile has a separate step for generating documentation, which takes a fairly large piece of information directly from the code.
requested reviews from storage-services-committers, storage-services-developers 2025-01-15 12:15:09 +00:00
mbiryukova approved these changes 2025-01-16 05:54:05 +00:00
Dismissed
@ -0,0 +174,4 @@
Option request_timeout.
FrostFS request timeout
Enter a value of type Duration. Press Enter for the default (4s).
Member

Should this default value match default value from properties description below?

Should this default value match default value from properties description below?
Author
Member

Fixed.

Fixed.
KurlesHS force-pushed feature/add-frostfs-support from 79813b16f9 to 3a783d209e 2025-01-16 07:58:00 +00:00 Compare
KurlesHS dismissed alexvanin's review 2025-01-16 07:58:00 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

KurlesHS dismissed mbiryukova's review 2025-01-16 07:58:00 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

KurlesHS force-pushed feature/add-frostfs-support from 3a783d209e to 1652352544 2025-01-16 08:06:49 +00:00 Compare
KurlesHS force-pushed feature/add-frostfs-support from 1652352544 to b93e134b5b 2025-01-16 08:11:47 +00:00 Compare
alexvanin approved these changes 2025-01-16 09:28:45 +00:00
alexvanin merged commit b93e134b5b into tcl/master 2025-01-16 11:01:23 +00:00
alexvanin referenced this pull request from a commit 2025-01-16 11:01:24 +00:00
alexvanin referenced this pull request from a commit 2025-01-16 11:01:24 +00:00
alexvanin referenced this pull request from a commit 2025-01-16 11:01:24 +00:00
alexvanin deleted branch feature/add-frostfs-support 2025-01-16 11:01:25 +00:00
KurlesHS referenced this pull request from a commit 2025-03-11 07:34:05 +00:00
KurlesHS referenced this pull request from a commit 2025-03-11 07:34:05 +00:00
Sign in to join this conversation.
No description provided.