Fix actor reminder e2e tests + fixes to reminder partition by artursouza · Pull Request #4021 · dapr/dapr

artursouza · 2021-12-13T01:19:57Z

Description

Fixes the reminder E2E tests.
Fixes to the reminder storage solution with partitioning.

Issue reference

Please reference the issue this PR will close: #3984

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Unit tests passing
End-to-end tests passing
Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
Specification has been updated / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
Provided sample for the feature / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

RELEASE NOTE: Fix logging of conflict when loading actor state store and fixes to actor reminders partitioning.

artursouza · 2021-12-13T01:20:42Z

/ok-to-test

dapr-bot · 2021-12-13T01:21:19Z

Found the available test cluster - dapr-aks-e2e-07 for linux. Please check the build status.

dapr-bot · 2021-12-13T01:21:58Z

Found the available test cluster - dapr-aks-e2e-06 for windows. Please check the build status.

codecov · 2021-12-13T01:25:10Z

Codecov Report

Merging #4021 (a9592e2) into master (f2d77e7) will decrease coverage by 0.21%.
The diff coverage is 64.33%.

❗ Current head a9592e2 differs from pull request most recent head 853021e. Consider uploading reports for the commit 853021e to get more accurate results

@@            Coverage Diff             @@
##           master    #4021      +/-   ##
==========================================
- Coverage   63.40%   63.18%   -0.22%     
==========================================
  Files         101      101              
  Lines        9581     9652      +71     
==========================================
+ Hits         6075     6099      +24     
- Misses       3032     3074      +42     
- Partials      474      479       +5

Impacted Files	Coverage Δ
pkg/actors/actors.go	`64.83% <61.36%> (-2.01%)`	⬇️
pkg/runtime/runtime.go	`63.61% <100.00%> (-0.27%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2d77e7...853021e. Read the comment docs.

dapr-bot · 2021-12-13T02:08:35Z

Congrats! All end-to-end tests have passed on linux. Thanks for your contribution!

dapr-bot · 2021-12-13T02:35:44Z

Congrats! All end-to-end tests have passed on windows. Thanks for your contribution!

artursouza · 2021-12-13T18:14:37Z

/ok-to-test

dapr-bot · 2021-12-13T18:16:03Z

Found the available test cluster - dapr-aks-e2e-07 for linux. Please check the build status.

dapr-bot · 2021-12-13T18:17:16Z

Found the available test cluster - dapr-aks-e2e-05 for windows. Please check the build status.

dapr-bot · 2021-12-13T18:38:24Z

End-to-end tests failed on windows. Please check the build logs

dapr-bot · 2021-12-13T19:07:16Z

End-to-end tests failed on linux. Please check the build logs

artursouza · 2021-12-13T20:55:36Z

/ok-to-test

dapr-bot · 2021-12-13T20:56:42Z

Found the available test cluster - dapr-aks-e2e-07 for linux. Please check the build status.

dapr-bot · 2021-12-13T20:58:41Z

Found the available test cluster - dapr-aks-e2e-05 for windows. Please check the build status.

dapr-bot · 2021-12-13T21:21:42Z

End-to-end tests failed on windows. Please check the build logs

dapr-bot · 2021-12-13T21:49:28Z

End-to-end tests failed on linux. Please check the build logs

artursouza · 2021-12-13T22:19:13Z

/ok-to-test

dapr-bot · 2021-12-13T22:20:03Z

Found the available test cluster - dapr-aks-e2e-07 for linux. Please check the build status.

dapr-bot · 2021-12-13T22:20:26Z

Found the available test cluster - dapr-aks-e2e-05 for windows. Please check the build status.

dapr-bot · 2021-12-13T22:39:13Z

End-to-end tests failed on windows. Please check the build logs

dapr-bot · 2021-12-13T23:09:25Z

Congrats! All end-to-end tests have passed on linux. Thanks for your contribution!

artursouza · 2021-12-14T07:00:01Z

/ok-to-test

dapr-bot · 2021-12-14T07:00:49Z

Found the available test cluster - dapr-aks-e2e-07 for linux. Please check the build status.

dapr-bot · 2021-12-14T07:01:50Z

Found the available test cluster - dapr-aks-e2e-05 for windows. Please check the build status.

dapr-bot · 2021-12-14T07:23:12Z

End-to-end tests failed on windows. Please check the build logs

dapr-bot · 2021-12-14T07:48:26Z

End-to-end tests failed on linux. Please check the build logs

artursouza · 2022-01-01T01:14:03Z

 		diag.DefaultMonitoring.ComponentInitialized(s.Spec.Type)
 	}

-	if a.hostingActors() && (a.actorStateStoreName == "" || a.actorStateStoreCount != 1) {


This code is executed before sidecar invokes /dapr/config, which means that a.hostingActors() would return false. This was not always like that but we had many changes in the order of loading things in Dapr sidecar and this was a side effect of that.

yaron2

lgtm

wcs1only · 2022-01-03T18:35:27Z

-	_, err := appManager.WaitUntilDeploymentState(appManager.IsDeploymentDone)
+	// 120s is the observed time to spawn a new test app POD and terminate the old one.
+	// This sleep time is important for the stream log to work.
+	time.Sleep(120 * time.Second)


This seems extremely high, and we're already cutting close the 10 minutes limit for some of our test suites. The line below this one should gate this appropriately. If deployment state is running, your logs should stream properly. If you are seeing errors, it might be because the StreamContainerLogs() attempts to run against all pods in a deployment, and might try against a terminated pod, I think? The new pod should still work, even in the presence of this error.

This one is needed because it is the time I observed for the new POD to be created and the old one to be terminated. Otherwise, tests can fail because the requests can go to any of the PODs and add flakiness.

So WaitUntilDeploymentState doesn't sufficiently gate on the deployment being completed? Could we just update WaitUntilDeploymentState to also gate on the number of replicas being what we expect?

I checked the code. It does that already. Let me remove this. Nice catch.

artursouza · 2022-01-03T19:10:03Z

/ok-to-test

dapr-bot · 2022-01-03T19:10:48Z

Found the available test cluster - dapr-aks-e2e-08 for linux. Please check the build status.

dapr-bot · 2022-01-03T19:12:36Z

Found the available test cluster - dapr-aks-e2e-06 for windows. Please check the build status.

dapr-bot · 2022-01-03T20:00:42Z

End-to-end tests failed on linux. Please check the build logs

Signed-off-by: Artur Souza <[email protected]>

artursouza · 2022-01-03T20:14:41Z

/ok-to-test

dapr-bot · 2022-01-03T20:15:29Z

Found the available test cluster - dapr-aks-e2e-08 for linux. Please check the build status.

dapr-bot · 2022-01-03T20:30:25Z

End-to-end tests failed on windows. Please check the build logs

dapr-bot · 2022-01-03T21:05:27Z

Congrats! All end-to-end tests have passed on linux. Thanks for your contribution!

wcs1only · 2022-01-03T22:52:19Z

/ok-to-test

dapr-bot · 2022-01-03T22:52:55Z

Found the available test cluster - dapr-aks-e2e-08 for linux. Please check the build status.

wcs1only

Thanks for removing the sleeps. LGTM

dapr-bot · 2022-01-03T22:53:39Z

Found the available test cluster - dapr-aks-e2e-05 for windows. Please check the build status.

dapr-bot · 2022-01-03T23:12:47Z

End-to-end tests failed on windows. Please check the build logs

dapr-bot · 2022-01-03T23:42:31Z

End-to-end tests failed on linux. Please check the build logs

* Fix actor metadata concurrency and reminder E2E tests. Fix error handling in create/delete reminders with partitioning. Add more logging to actor reminders. Fix race condition in e2e app actorfeatures. Signed-off-by: Artur Souza <[email protected]> * Fix use of pointer for reminders. Signed-off-by: Artur Souza <[email protected]> * handle empty data on reading reminder. Signed-off-by: Artur Souza <[email protected]> * Uses strong consistency in all state operations for actors. Signed-off-by: Artur Souza <[email protected]> * Use Set instead of Multi for reminder partition. Signed-off-by: Artur Souza <[email protected]> * Fix logic on migrating actor type. Signed-off-by: Artur Souza <[email protected]> * Use SetAppEnv() to change reminder partition in e2e test. Signed-off-by: Artur Souza <[email protected]> * Make actor logs more verbose. Signed-off-by: Artur Souza <[email protected]> * Use exponential backoff for reminder actor features e2e test. Signed-off-by: Artur Souza <[email protected]> * Sleep on update env for stream log to work. Signed-off-by: Artur Souza <[email protected]> * Use k8s Restart on actor reminder restart e2e. Signed-off-by: Artur Souza <[email protected]> * Add extra sleep to reminder restart e2 test. Signed-off-by: Artur Souza <[email protected]> * More logging, improve read of reminders. Signed-off-by: Artur Souza <[email protected]> * Fix root cause of flaky tests for actors. Signed-off-by: Artur Souza <[email protected]> * Address PR coments. Signed-off-by: Artur Souza <[email protected]> Signed-off-by: x-shadow-man <[email protected]>

* master: Saves 'describe' output of K8s resources for E2E tests. (dapr#4082) Fix flakiness on get metadata e2e. (dapr#4080) Return error instead of crashing when receiving grpc_proxy request on uninitialized daprd Fix actor reminder e2e tests + fixes to reminder partition (dapr#4021) Upgrading fasthttp to fix shutdown issue Fixed create timer duetime remove max graceful shutdown duration limit fix lint add more test case fix lint Make the grace period configurable Signed-off-by: Taction <[email protected]>

* Fix actor metadata concurrency and reminder E2E tests. Fix error handling in create/delete reminders with partitioning. Add more logging to actor reminders. Fix race condition in e2e app actorfeatures. Signed-off-by: Artur Souza <[email protected]> * Fix use of pointer for reminders. Signed-off-by: Artur Souza <[email protected]> * handle empty data on reading reminder. Signed-off-by: Artur Souza <[email protected]> * Uses strong consistency in all state operations for actors. Signed-off-by: Artur Souza <[email protected]> * Use Set instead of Multi for reminder partition. Signed-off-by: Artur Souza <[email protected]> * Fix logic on migrating actor type. Signed-off-by: Artur Souza <[email protected]> * Use SetAppEnv() to change reminder partition in e2e test. Signed-off-by: Artur Souza <[email protected]> * Make actor logs more verbose. Signed-off-by: Artur Souza <[email protected]> * Use exponential backoff for reminder actor features e2e test. Signed-off-by: Artur Souza <[email protected]> * Sleep on update env for stream log to work. Signed-off-by: Artur Souza <[email protected]> * Use k8s Restart on actor reminder restart e2e. Signed-off-by: Artur Souza <[email protected]> * Add extra sleep to reminder restart e2 test. Signed-off-by: Artur Souza <[email protected]> * More logging, improve read of reminders. Signed-off-by: Artur Souza <[email protected]> * Fix root cause of flaky tests for actors. Signed-off-by: Artur Souza <[email protected]> * Address PR coments. Signed-off-by: Artur Souza <[email protected]> Signed-off-by: x-shadow-man <[email protected]>

* Fix actor metadata concurrency and reminder E2E tests. Fix error handling in create/delete reminders with partitioning. Add more logging to actor reminders. Fix race condition in e2e app actorfeatures. Signed-off-by: Artur Souza <[email protected]> * Fix use of pointer for reminders. Signed-off-by: Artur Souza <[email protected]> * handle empty data on reading reminder. Signed-off-by: Artur Souza <[email protected]> * Uses strong consistency in all state operations for actors. Signed-off-by: Artur Souza <[email protected]> * Use Set instead of Multi for reminder partition. Signed-off-by: Artur Souza <[email protected]> * Fix logic on migrating actor type. Signed-off-by: Artur Souza <[email protected]> * Use SetAppEnv() to change reminder partition in e2e test. Signed-off-by: Artur Souza <[email protected]> * Make actor logs more verbose. Signed-off-by: Artur Souza <[email protected]> * Use exponential backoff for reminder actor features e2e test. Signed-off-by: Artur Souza <[email protected]> * Sleep on update env for stream log to work. Signed-off-by: Artur Souza <[email protected]> * Use k8s Restart on actor reminder restart e2e. Signed-off-by: Artur Souza <[email protected]> * Add extra sleep to reminder restart e2 test. Signed-off-by: Artur Souza <[email protected]> * More logging, improve read of reminders. Signed-off-by: Artur Souza <[email protected]> * Fix root cause of flaky tests for actors. Signed-off-by: Artur Souza <[email protected]> * Address PR coments. Signed-off-by: Artur Souza <[email protected]>

artursouza requested review from a team as code owners December 13, 2021 01:19

artursouza marked this pull request as draft December 13, 2021 01:20

artursouza changed the title ~~Add retry to fetch logs for actor reminder restart e2e.~~ Add retry to for actor reminder e2e tests. Dec 14, 2021

artursouza commented Jan 1, 2022

View reviewed changes

yaron2 previously approved these changes Jan 1, 2022

View reviewed changes

Merge branch 'master' into retry_actor_reminder_restart_e2e

d5faca5

wcs1only reviewed Jan 3, 2022

View reviewed changes

Comment thread tests/e2e/actor_features/actor_features_test.go Outdated

wcs1only reviewed Jan 3, 2022

View reviewed changes

Comment thread tests/runner/kube_testplatform.go Outdated

artursouza dismissed yaron2’s stale review via 9d3dbf4 January 3, 2022 19:05

Address PR coments.

853021e

Signed-off-by: Artur Souza <[email protected]>

wcs1only approved these changes Jan 3, 2022

View reviewed changes

artursouza merged commit 1c47694 into dapr:master Jan 3, 2022

artursouza added this to the v1.6 milestone Jan 3, 2022

Uh oh!

Conversation

artursouza commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue reference

Checklist

Uh oh!

artursouza commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

codecov Bot commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

artursouza commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

artursouza commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

artursouza commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

dapr-bot commented Dec 13, 2021

Uh oh!

artursouza commented Dec 14, 2021

Uh oh!

dapr-bot commented Dec 14, 2021

Uh oh!

dapr-bot commented Dec 14, 2021

Uh oh!

dapr-bot commented Dec 14, 2021

Uh oh!

dapr-bot commented Dec 14, 2021

Uh oh!

artursouza Jan 1, 2022

Choose a reason for hiding this comment

Uh oh!

yaron2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wcs1only Jan 3, 2022

Choose a reason for hiding this comment

Uh oh!

artursouza Jan 3, 2022

Choose a reason for hiding this comment

Uh oh!

wcs1only Jan 3, 2022

Choose a reason for hiding this comment

Uh oh!

artursouza Jan 3, 2022

Choose a reason for hiding this comment

artursouza commented Dec 13, 2021 •

edited

Loading

codecov Bot commented Dec 13, 2021 •

edited

Loading