make sure status code error code is propogated to devloopEndEvent #4468
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Testing on cloud code samples, Events: |
|
Readiness Healthcheck Test
|
Codecov Report
@@ Coverage Diff @@
## master #4468 +/- ##
==========================================
+ Coverage 72.26% 72.33% +0.06%
==========================================
Files 331 332 +1
Lines 12863 12929 +66
==========================================
+ Hits 9296 9352 +56
- Misses 2975 2981 +6
- Partials 592 596 +4
Continue to review full report at Codecov.
|
briandealwis
approved these changes
Jul 14, 2020
| return proto.StatusCode_STATUSCHECK_SUCCESS | ||
| } | ||
|
|
||
| func (d *Deployment) WithPodStatuses(scs []proto.StatusCode) *Deployment { |
There was a problem hiding this comment.
This seems like a test function that shouldn't be here?
| @@ -195,12 +206,15 @@ func statusCheckEventSucceeded() { | |||
| }) | |||
| } | |||
|
|
|||
| func statusCheckEventFailed(err error) { | |||
| aiErr := sErrors.ActionableErr(sErrors.StatusCheck, err) | |||
| func statusCheckEventFailed(err error, errCode proto.StatusCode) { | |||
There was a problem hiding this comment.
This ordering is inconsistent with StatusCheckEventEnded()
| @@ -299,8 +319,22 @@ func DevLoopFailedWithErrorCode(i int, statusCode proto.StatusCode, err error) { | |||
|
|
|||
| // DevLoopFailed notifies that a dev loop has failed in a given phase | |||
| func DevLoopFailedInPhase(iteration int, phase sErrors.Phase, err error) { | |||
There was a problem hiding this comment.
We really need to improve this. I think we should be aiming to generate the StatusCode at the error-raising time so that we're not guessing.
briandealwis
force-pushed
the
fix_devloop_end
branch
from
ba07b17
to
f1a9f52
Compare
|
Last commit removes spurious empty logs output that results from no log out or output with a trailing newline: |
briandealwis
added a commit
that referenced
this pull request
Jul 14, 2020
Also avoid spurious empty lines in log output Co-authored-by: Brian de Alwis <bdealwis@google.com>
briandealwis
added a commit
that referenced
this pull request
Jul 14, 2020
Also avoid spurious empty lines in log output Co-authored-by: Brian de Alwis <bdealwis@google.com>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discovered when testing, the
DevLoopEndEventconsumed by IDEs do not contain the failure seen in deploy phase.In this PR,
StatusCodetoBiuldState,DeployStateandStatusCheckStatedeploy.StatusCheckto report first deployment with a pod error asStatusCheck.ErrorCode.( in future, maybe we should create this as an array. One or more distinct error code can happen for multi-service deployments, e.g.
ImagePullErrfor "foo" deployment andPodUnhealthyfor "bar" deployment)DevLoopFailedInPhasenow consumes the newly addedStatusCode.Other small changes:
diag.validator.Podto only fetch events when unknown error code is read.