fix: improve restore stability #996
Conversation
tolusha
requested review from
AndrienkoAleksandr,
flacatus,
mmorhun and
nickboldt
as code owners
Codecov Report
@@ Coverage Diff @@
## main #996 +/- ##
==========================================
- Coverage 50.34% 50.33% -0.01%
==========================================
Files 56 56
Lines 6821 6822 +1
==========================================
Hits 3434 3434
- Misses 3018 3019 +1
Partials 369 369
Continue to review full report at Codecov.
|
tolusha
force-pushed
the
20159
branch
2 times, most recently
from
ecbed0a to
cf918e4
Compare
tolusha
changed the title
Signed-off-by: Anatolii Bazko <[email protected]>
Signed-off-by: Anatolii Bazko <[email protected]>
|
@tolusha: The following test failed, say
Full PR test history. Your PR dashboard. Instructions 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/test-infra repository. I understand the commands that are listed here. |
Signed-off-by: Anatolii Bazko <[email protected]>
Signed-off-by: Anatolii Bazko <[email protected]>
Signed-off-by: Anatolii Bazko <[email protected]>
Signed-off-by: Anatolii Bazko <[email protected]>
Signed-off-by: Anatolii Bazko <[email protected]>
Signed-off-by: Anatolii Bazko <[email protected]>
tolusha
changed the title
Signed-off-by: Anatolii Bazko <[email protected]>
Signed-off-by: Anatolii Bazko <[email protected]>
| @@ -316,11 +326,13 @@ func restoreCheCR(rctx *RestoreContext, dataDir string) (bool, error) { | |||
|
|
|||
| if err := rctx.r.client.Create(context.TODO(), cheCR); err != nil { | |||
| if errors.IsAlreadyExists(err) { | |||
| return false, rctx.r.client.Delete(context.TODO(), cheCR) | |||
| return true, nil | |||
There was a problem hiding this comment.
Add a few lines comment, please, that describes when such a situation possible and what this return mean.
| @@ -247,5 +246,6 @@ func (r *ReconcileCheClusterRestore) UpdateCRStatus(cr *chev1.CheClusterRestore) | |||
| logrus.Errorf("Failed to update %s CR status: %s", cr.Name, err.Error()) | |||
| return err | |||
| } | |||
| logrus.Infof("Status updated with %v: ", cr.Status) | |||
There was a problem hiding this comment.
Should we have this log at info level?
| return false, nil | ||
| } else if !errors.IsNotFound(err) { | ||
| return false, err | ||
| cr, _, _ := util.FindCheCRinNamespace(rctx.r.client, rctx.namespace) |
There was a problem hiding this comment.
I am afraid, it doesn't comply with the code above
Signed-off-by: Anatolii Bazko <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mmorhun, tolusha The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Anatolii Bazko [email protected]
What does this PR do?
Screenshot/screencast of this PR
N/A
What issues does this PR fix or reference?
eclipse-che/che#20159
How to test this PR?
N/A
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or referenceandHow to test this PRcompletedReviewers
Reviewers, please comment how you tested the PR when approving it.