[#222] Add support for aio v1.7.0 in integration tests #239

Open
KurlesHS wants to merge 1 commit from KurlesHS/frostfs-http-gw:feature/add-aio-1.7.0-integration-tests into master
Member

close #222

close #222
KurlesHS self-assigned this 2025-05-05 08:56:15 +00:00
KurlesHS added 1 commit 2025-05-05 08:56:16 +00:00
[#222] Add support for aio v1.7.0 in integration tests
All checks were successful
/ DCO (pull_request) Successful in 36s
/ Vulncheck (pull_request) Successful in 1m8s
/ OCI image (pull_request) Successful in 1m20s
/ Builds (pull_request) Successful in 58s
/ Lint (pull_request) Successful in 3m19s
/ Tests (pull_request) Successful in 1m25s
/ Integration tests (pull_request) Successful in 6m56s
6969be9ac1
Signed-off-by: Aleksey Kravchenko <al.kravchenko@yadro.com>
KurlesHS changed title from WIP: [#222] Add support for aio v1.7.0 in integration tests to [#222] Add support for aio v1.7.0 in integration tests 2025-05-05 13:57:49 +00:00
requested reviews from storage-services-committers, storage-services-developers 2025-05-05 13:57:50 +00:00
dkirillov reviewed 2025-05-07 11:38:42 +00:00
@ -53,1 +58,4 @@
func versionToUint(t *testing.T, v string) uint64 {
parts := strings.Split(v, ".")
require.Len(t, parts, 3)
Member

What about versions like v0.33.0-rc.6?

What about versions like `v0.33.0-rc.6`?
Author
Member

This function is used to convert the version numbers of aio-containers (which we test and explicitly list at the beginning of the TestIntegration function) into a numeric format. The versions follow the "MAJOR.MINOR.PATCH" format. If in the future there's a need to support versions with an additional suffix (e.g., "1.7.2-nightly1"), this function can be extended accordingly—but for now, that would be premature.

This function is used to convert the version numbers of aio-containers (which we test and explicitly list at the beginning of the TestIntegration function) into a numeric format. The versions follow the "MAJOR.MINOR.PATCH" format. If in the future there's a need to support versions with an additional suffix (e.g., "1.7.2-nightly1"), this function can be extended accordingly—but for now, that would be premature.
@ -54,0 +72,4 @@
func publicReadWriteRules() []chain.Rule {
return []chain.Rule{
{
Status: chain.Allow, Actions: chain.Actions{
Member

Can we fix formatting (run make fmt at least)?
For example:

diff --git a/cmd/http-gw/integration_test.go b/cmd/http-gw/integration_test.go
index 8c22931..d1a61f4 100644
--- a/cmd/http-gw/integration_test.go
+++ b/cmd/http-gw/integration_test.go
@@ -72,22 +72,24 @@ func versionToUint(t *testing.T, v string) uint64 {
 func publicReadWriteRules() []chain.Rule {
        return []chain.Rule{
                {
-                       Status: chain.Allow, Actions: chain.Actions{
-                       Inverted: false,
-                       Names: []string{
-                               native.MethodPutObject,
-                               native.MethodGetObject,
-                               native.MethodHeadObject,
-                               native.MethodDeleteObject,
-                               native.MethodSearchObject,
-                               native.MethodRangeObject,
-                               native.MethodHashObject,
-                               native.MethodPatchObject,
+                       Status: chain.Allow,
+                       Actions: chain.Actions{
+                               Names: []string{
+                                       native.MethodPutObject,
+                                       native.MethodGetObject,
+                                       native.MethodHeadObject,
+                                       native.MethodDeleteObject,
+                                       native.MethodSearchObject,
+                                       native.MethodRangeObject,
+                                       native.MethodHashObject,
+                                       native.MethodPatchObject,
+                               },
                        },
-               }, Resources: chain.Resources{
-                       Inverted: false,
-                       Names:    []string{native.ResourceFormatRootObjects},
-               }, Any: false},
+                       Resources: chain.Resources{
+                               Names: []string{native.ResourceFormatRootObjects},
+                       },
+                       Any: false,
+               },
        }
 }

Can we fix formatting (run `make fmt` at least)? For example: ```diff diff --git a/cmd/http-gw/integration_test.go b/cmd/http-gw/integration_test.go index 8c22931..d1a61f4 100644 --- a/cmd/http-gw/integration_test.go +++ b/cmd/http-gw/integration_test.go @@ -72,22 +72,24 @@ func versionToUint(t *testing.T, v string) uint64 { func publicReadWriteRules() []chain.Rule { return []chain.Rule{ { - Status: chain.Allow, Actions: chain.Actions{ - Inverted: false, - Names: []string{ - native.MethodPutObject, - native.MethodGetObject, - native.MethodHeadObject, - native.MethodDeleteObject, - native.MethodSearchObject, - native.MethodRangeObject, - native.MethodHashObject, - native.MethodPatchObject, + Status: chain.Allow, + Actions: chain.Actions{ + Names: []string{ + native.MethodPutObject, + native.MethodGetObject, + native.MethodHeadObject, + native.MethodDeleteObject, + native.MethodSearchObject, + native.MethodRangeObject, + native.MethodHashObject, + native.MethodPatchObject, + }, }, - }, Resources: chain.Resources{ - Inverted: false, - Names: []string{native.ResourceFormatRootObjects}, - }, Any: false}, + Resources: chain.Resources{ + Names: []string{native.ResourceFormatRootObjects}, + }, + Any: false, + }, } } ```
Member
Please align brackets https://git.frostfs.info/KurlesHS/frostfs-http-gw/src/commit/101d6370b8deaed19ba1ff1bed93ae34759a0ad5/cmd/http-gw/integration_test.go#L73 and https://git.frostfs.info/KurlesHS/frostfs-http-gw/src/commit/101d6370b8deaed19ba1ff1bed93ae34759a0ad5/cmd/http-gw/integration_test.go#L92
Author
Member

For some reason, gofmt is totally fine with this on my machine. =(

For some reason, gofmt is totally fine with this on my machine. =(
@ -54,0 +99,4 @@
Op: chain.CondStringEquals,
Kind: chain.KindRequest,
Key: native.PropertyKeyActorRole,
Value: native.PropertyValueContainerRoleOwner,
Member

I don't know how exactly storage node determines role when bearer token is applied, but I think using public key condition is more robust
cc @alexvanin

I don't know how exactly storage node determines role when bearer token is applied, but I think using public key condition is more robust cc @alexvanin
@ -566,2 +630,3 @@
func createContainerBase(ctx context.Context, t *testing.T, clientPool *pool.Pool, ownerID user.ID, basicACL acl.Basic, name string) (cid.ID, error) {
func createContainerWithAPE(ctx context.Context, t *testing.T, clientPool *pool.Pool, ownerID user.ID, name string) (cid.ID, error) {
return createContainerBase(ctx, t, clientPool, ownerID, acl.PublicRWExtended, publicReadWriteRules(), name)
Member

acl here must be 0 instead of acl.PublicRWExtended

acl here must be `0` instead of `acl.PublicRWExtended`
dkirillov marked this conversation as resolved
@ -568,0 +632,4 @@
return createContainerBase(ctx, t, clientPool, ownerID, acl.PublicRWExtended, publicReadWriteRules(), name)
}
func waitForAPECacheInvalidated(ctx context.Context, clientPool *pool.Pool, expectedCh chain.Chain, cnrID cid.ID) error {
Member

Maybe the better name is waitForAPEBeApplied or something like this?

Maybe the better name is `waitForAPEBeApplied` or something like this?
dkirillov marked this conversation as resolved
@ -568,0 +640,4 @@
},
}
checkCtxDone := func(ctx context.Context) error {
Member

By the way, why do need this function?

Can we just get error from clientPool.ListAPEChains (we pass context there) or by using ctx.Err() when case <- ctx.Done(): happens?

By the way, why do need this function? Can we just get error from `clientPool.ListAPEChains` (we pass context there) or by using `ctx.Err()` when `case <- ctx.Done():` happens?
dkirillov marked this conversation as resolved
@ -568,0 +664,4 @@
// At the moment, according to the core team, there is no way through the API
// to check whether the APE chain stored in the contact has been applied to the container.
// So after we make sure that the APE chain is stored, we just wait for a certain period of time
// (8 seconds by default, the time until the next block and cache invalidation)
Member

Maybe we should use AIO images where this cache is disables at all?

cc @alexvanin

Maybe we should use AIO images where this cache is disables at all? cc @alexvanin
@ -568,0 +697,4 @@
}
err = clientPool.AddAPEChain(ctx, prmAddAPEChain)
require.NoError(t, err)
err = waitForAPECacheInvalidated(ctx, clientPool, ch, cnrID)
Member

Should we check err here too?

Should we check `err` here too?
dkirillov marked this conversation as resolved
@ -602,2 +747,4 @@
fmt.Println(CID.String())
if len(apeRules) != 0 {
chainID := randomString()
Member

Why not just

chainID := "http-aio-" + CID.String()

?

Why not just ```golang chainID := "http-aio-" + CID.String() ``` ?
dkirillov marked this conversation as resolved
KurlesHS force-pushed feature/add-aio-1.7.0-integration-tests from 6969be9ac1 to 101d6370b8 2025-05-07 13:34:35 +00:00 Compare
KurlesHS changed title from [#222] Add support for aio v1.7.0 in integration tests to WIP: [#222] Add support for aio v1.7.0 in integration tests 2025-05-07 13:42:13 +00:00
KurlesHS force-pushed feature/add-aio-1.7.0-integration-tests from 101d6370b8 to 91c2aeead8 2025-05-07 13:47:26 +00:00 Compare
KurlesHS force-pushed feature/add-aio-1.7.0-integration-tests from 91c2aeead8 to 3b0883086f 2025-05-07 13:56:03 +00:00 Compare
KurlesHS changed title from WIP: [#222] Add support for aio v1.7.0 in integration tests to [#222] Add support for aio v1.7.0 in integration tests 2025-05-07 14:16:35 +00:00
All checks were successful
/ DCO (pull_request) Successful in 50s
Required
Details
/ Builds (pull_request) Successful in 1m3s
Required
Details
/ Vulncheck (pull_request) Successful in 1m0s
Required
Details
/ OCI image (pull_request) Successful in 1m25s
Required
Details
/ Lint (pull_request) Successful in 2m20s
Required
Details
/ Tests (pull_request) Successful in 1m10s
Required
Details
/ Integration tests (pull_request) Successful in 7m12s
Required
Details
This pull request doesn't have enough approvals yet. 0 of 2 approvals granted.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u feature/add-aio-1.7.0-integration-tests:KurlesHS-feature/add-aio-1.7.0-integration-tests
git checkout KurlesHS-feature/add-aio-1.7.0-integration-tests
Sign in to join this conversation.
No reviewers
TrueCloudLab/storage-services-committers
TrueCloudLab/storage-services-developers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-http-gw#239
No description provided.