Skip to content
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

[full-ci] Test Async Propagator #10655

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

kobergj
Copy link
Collaborator

@kobergj kobergj commented Nov 25, 2024

Tests the async propagator against ocis ci 🤞

cc @micbar @butonic

Copy link

update-docs bot commented Nov 25, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@kobergj kobergj marked this pull request as draft November 25, 2024 16:21
@micbar
Copy link
Contributor

micbar commented Nov 25, 2024

@ScharfViktor @individual-it @saw-jan

How much effort would it be, to adapt the testsuite?

Background: Async etag and size propagation can take 5 seconds and longer.

@saw-jan
Copy link
Member

saw-jan commented Nov 26, 2024

@ScharfViktor @individual-it @saw-jan

How much effort would it be, to adapt the testsuite?

Background: Async etag and size propagation can take 5 seconds and longer.

Tests must wait until propagation is complete and the implementation should not take long. But the problem would be how much time is enough to make tests reliably stable.
I see tests failing related to etag, size and quota.

we can try with 5 seconds and see how tests behave

@individual-it
Copy link
Member

or keep on retrying till the value is as expected (till some timeout)
that might save some time

@saw-jan
Copy link
Member

saw-jan commented Nov 26, 2024

or keep on retrying till the value is as expected (till some timeout) that might save some time

that is also another possibility

@saw-jan
Copy link
Member

saw-jan commented Nov 26, 2024

or keep on retrying till the value is as expected (till some timeout) that might save some time

that is also another possibility

I think, 5 seconds is the minimum. I could not get a test pass with <5 seconds
@micbar @kobergj is 5 seconds a minimum?

@micbar
Copy link
Contributor

micbar commented Nov 26, 2024

or keep on retrying till the value is as expected (till some timeout) that might save some time

that is also another possibility

I think, 5 seconds is the minimum. I could not get a test pass with <5 seconds @micbar @kobergj is 5 seconds a minimum?

It is controlled by STORAGE_USERS_ASYNC_PROPAGATOR_PROPAGATION_DELAY
This is the time period how long the storage waits before starting the treetime and treesize propagation. (which affects etag, folder sizes and quota)

The default is 5s (@mmattel although it shows up as 0s in the docs)

@mmattel
Copy link
Contributor

mmattel commented Nov 26, 2024

The default is 5s (@mmattel although it shows up as 0s in the docs)

At one point in time we need to think about how to handle this properly,
means real value == shown value...

@kobergj
Copy link
Collaborator Author

kobergj commented Nov 26, 2024

At one point in time we need to think about how to handle this properly,
means real value == shown value...

This is not so easy. There are 2 default values: The value we define in the defaults section of ocis and the backup default of the package itself. Keeping those in sync is always manual work - hence can be forgotten because it has no visible impact during implementation.

@butonic
Copy link
Member

butonic commented Nov 26, 2024

easiest fix would be to set it to 5 in the defaultconfig.gfo as well. keeping it in sync is manual, yes, but showing 0 in the docs is just wrong.

For CI we could set it to STORAGE_USERS_ASYNC_PROPAGATOR_PROPAGATION_DELAY=100ms in order to not drag out the length of the tests?

@saw-jan
Copy link
Member

saw-jan commented Nov 26, 2024

For CI we could set it to STORAGE_USERS_ASYNC_PROPAGATOR_PROPAGATION_DELAY=100ms in order to not drag out the length of the tests?

that would be great for the tests. I was just about to ask the same. THanks

@kobergj kobergj force-pushed the TestAsyncPropagator branch from 5e33cf2 to 997e1f7 Compare November 26, 2024 10:20
@kobergj
Copy link
Collaborator Author

kobergj commented Nov 26, 2024

easiest fix would be to set it to 5 in the defaultconfig.gfo as well. keeping it in sync is manual, yes, but showing 0 in the docs is just wrong.

Yes this is the way we are doing it. However we cannot automate/validate this because the package internal default could change at any time.

Note: I added the envvar for 100ms propagation delay.

@saw-jan
Copy link
Member

saw-jan commented Nov 26, 2024

Q: should we use async propagation for cs3 API test pipeline? tests fail
https://drone.owncloud.com/owncloud/ocis/41391/15/5

@micbar
Copy link
Contributor

micbar commented Nov 26, 2024

Q: should we use async propagation for cs3 API test pipeline? tests fail https://drone.owncloud.com/owncloud/ocis/41391/15/5

I will look into that.

@saw-jan
Copy link
Member

saw-jan commented Nov 26, 2024

some API tests still can fail. I will implement the wait in the tests.
https://drone.owncloud.com/owncloud/ocis/41391/37/5

@saw-jan
Copy link
Member

saw-jan commented Nov 26, 2024

  • Actions related to folder require a bit more wait.
  • Sharing also changes the etag of the resouce.

will push the fix

@saw-jan
Copy link
Member

saw-jan commented Nov 26, 2024

All API test pipelines are ✔️
https://drone.owncloud.com/owncloud/ocis/41401

@micbar micbar force-pushed the TestAsyncPropagator branch from a7e492a to ba21904 Compare November 27, 2024 12:45
@micbar
Copy link
Contributor

micbar commented Nov 27, 2024

I enabled the async propagator for the cs3apivalidator.

@micbar micbar force-pushed the TestAsyncPropagator branch from ba21904 to 28f0384 Compare November 27, 2024 12:58
Copy link

sonarcloud bot commented Nov 27, 2024

@micbar
Copy link
Contributor

micbar commented Nov 27, 2024

@aduffeck @kobergj we found a problem

  Scenario: Change etag of personal home and move subtree                                                                                             # /var/lib/cs3api-validator/etag-propagation.feature:33
    Given user "admin" has created a folder "a-folder/a-sub-folder-B" in the home directory with the alias "a-sub-folder-B"                           # <autogenerated>:1 -> *ResourcesFeatureContext
    And user "admin" has created a folder "a-folder/a-sub-folder/testFolder" in the home directory with the alias "testFolder"                        # <autogenerated>:1 -> *ResourcesFeatureContext
    And user "admin" has created a folder "a-folder/a-sub-folder/testFolder/test2" in the home directory with the alias "test2"                       # <autogenerated>:1 -> *ResourcesFeatureContext
    And user "admin" has uploaded a file "a-folder/a-sub-folder/testfile.txt" with content "text" in the home directory with the alias "testfile.txt" # <autogenerated>:1 -> *ResourcesFeatureContext
    And user "admin" remembers the fileinfo of the resource with the alias "Admin Home"                                                               # <autogenerated>:1 -> *ResourcesFeatureContext
    And user "admin" remembers the fileinfo of the resource with the alias "a-folder"                                                                 # <autogenerated>:1 -> *ResourcesFeatureContext
    And user "admin" remembers the fileinfo of the resource with the alias "a-sub-folder"                                                             # <autogenerated>:1 -> *ResourcesFeatureContext
    And user "admin" remembers the fileinfo of the resource with the alias "a-sub-folder-B"                                                           # <autogenerated>:1 -> *ResourcesFeatureContext
    And user "admin" remembers the fileinfo of the resource with the alias "testFolder"                                                               # <autogenerated>:1 -> *ResourcesFeatureContext
    And user "admin" remembers the fileinfo of the resource with the alias "test2"                                                                    # <autogenerated>:1 -> *ResourcesFeatureContext
    When user "admin" moves the resource with alias "a-sub-folder" inside a space to target "a-folder/a-sub-folder-B/a-sub-folder"                    # <autogenerated>:1 -> *ResourcesFeatureContext
    Then for user "admin" the etag of the resource with the alias "Admin Home" should have changed                                                    # <autogenerated>:1 -> *ResourcesFeatureContext
    And for user "admin" the etag of the resource with the alias "a-folder" should have changed                                                       # <autogenerated>:1 -> *ResourcesFeatureContext
    And for user "admin" the etag of the resource with the alias "a-sub-folder" should not have changed                                               # <autogenerated>:1 -> *ResourcesFeatureContext
    And for user "admin" the etag of the resource with the alias "a-sub-folder-B" should have changed                                                 # <autogenerated>:1 -> *ResourcesFeatureContext
    And for user "admin" the etag of the resource with the alias "testFolder" should not have changed                                                 # <autogenerated>:1 -> *ResourcesFeatureContext
    
	Error Trace:	/drone/src/helpers/assert.go:13
	            				/drone/src/steps/resources/steps.go:386
	            				/drone/src/steps/resources/steps.go:390
	            				/usr/local/go/src/reflect/value.go:596
	            				/usr/local/go/src/reflect/value.go:380
	            				/go/pkg/mod/github.com/cucumber/godog@v0.15.0/internal/models/stepdef.go:189
	            				/go/pkg/mod/github.com/cucumber/godog@v0.15.0/suite.go:287
	            				/go/pkg/mod/github.com/cucumber/godog@v0.15.0/suite.go:576
	            				/go/pkg/mod/github.com/cucumber/godog@v0.15.0/suite.go:638
	            				/go/pkg/mod/github.com/cucumber/godog@v0.15.0/run.go:124
	            				/go/pkg/mod/github.com/cucumber/godog@v0.15.0/run.go:135
	            				/go/pkg/mod/github.com/cucumber/godog@v0.15.0/run.go:284
	            				/go/pkg/mod/github.com/cucumber/godog@v0.15.0/run.go:348
	            				/drone/src/cs3apivalidator_test.go:38
	Error:      	Not equal: 
	            	expected: "\"3053fd257ebb5280d399606d6f91ccc0\""
	            	actual  : "\"96a0712470a9086f1f5782655e4caaae\""
	            	
	            	Diff:
	            	--- Expected
	            	+++ Actual
	            	@@ -1 +1 @@
	            	-"3053fd257ebb5280d399606d6f91ccc0"
	            	+"96a0712470a9086f1f5782655e4caaae"

@micbar
Copy link
Contributor

micbar commented Nov 29, 2024

I discovered a bug: After running owncloud/cs3api-validator#18 on my local ocis with async propagation, my space is empty but shows 4 bytes used.

I suspect we have an issue with deletion.

@butonic @aduffeck what happens if you delete a file A/B/C/testfile.txt and delete B before the propagation has been successful?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants