robustness: save etcd data before validation#21807
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: henrybear327 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted filessee 24 files with indirect coverage changes @@ Coverage Diff @@
## main #21807 +/- ##
==========================================
+ Coverage 70.21% 70.24% +0.03%
==========================================
Files 427 427
Lines 35304 35304
==========================================
+ Hits 24788 24801 +13
+ Misses 9125 9113 -12
+ Partials 1391 1390 -1 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| if err := r.Report(); err != nil { | ||
| t.Error(err) | ||
| } | ||
| if err := r.Finalize(t.Failed(), panicked); err != nil { |
There was a problem hiding this comment.
For future, maybe we could pass t into report and have it register a Cleanup and check t.Failed there. Not sure how to handle panic then.
There was a problem hiding this comment.
Let's leave this for discussion in a follow-up PR to keep this one concentrated :)
| // the visualization history if the test failed/panicked or if PERSIST_RESULTS is set. | ||
| func (r *TestReport) Finalize(tFailed bool, panicked bool) error { | ||
| if !r.dataSaved { | ||
| return fmt.Errorf("cannot finalize test report: data was not saved") |
There was a problem hiding this comment.
Why? Doesn't just mean that finelize should also save data?
There was a problem hiding this comment.
As we shift to preserving etcd data before the validation, the Finalize function is meant to first determine if the test report is needed to be saved (either the test failed or we set PERSIST_RESULTS), then save the porcupine visualization (because it doesn't exist before the validation).
Now looking at dataSaved, my original intention was to attempt to preserve as much as we can. So in Finalize, if we deem preserving report is necessary but we don't have one, let's make one final attempt to save the report.
Does this make sense to you?
henrybear327
force-pushed
the
robustness/save_before_validation_v2
branch
from
b5dbdb0 to
8681fc4
Compare
henrybear327
force-pushed
the
robustness/save_before_validation_v2
branch
from
8681fc4 to
fa00983
Compare
henrybear327
force-pushed
the
robustness/save_before_validation_v2
branch
from
fa00983 to
af56f51
Compare
Signed-off-by: Chun-Hung Tseng <henrytseng@google.com>
henrybear327
force-pushed
the
robustness/save_before_validation_v2
branch
from
af56f51 to
826d732
Compare
|
@henrybear327: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| if err = r.Report(); err != nil { | ||
| if err = r.SaveEtcdData(); err != nil { | ||
| lg.Error("Failed to save traffic generation report", zap.Error(err)) | ||
| } |
There was a problem hiding this comment.
What about saving visualization in antithesis?
3 participants
No description provided.