fix: add some missing workspace updates #7790
Conversation
| if jobType.WorkspaceBuild.State == nil { | ||
| break | ||
| } |
There was a problem hiding this comment.
I am not sure what exactly this was guarding against (is there a danger to updating with empty state, maybe it would remove any previous state but that might be desired?) so maybe removing it has some unintended consequences...it fixes the unit test I added though. Seems not all workspace build failures include state.
There was a problem hiding this comment.
I added it back but only gated the update behind it since that appears specifically for updating the state and I suppose we might not want to do that when no state was included (still not sure though) but we still want the publish.
provisionerd/runner/runner.go
Outdated
| @@ -1116,6 +1116,7 @@ func (r *Runner) failedJobf(format string, args ...interface{}) *proto.FailedJob | |||
| JobId: r.job.JobId, | |||
| Error: message, | |||
| ErrorCode: code, | |||
| Type: &proto.FailedJob_WorkspaceBuild_{}, | |||
There was a problem hiding this comment.
I was able to trigger this by using the Docker template, creating a workspace, deleting my Docker volumes, then trying to start the workspace again.
It does not seem to trigger through the unit test. Not yet sure how to make it take this code path.
36c2959 to
6fab6a2
Compare
There are some failure cases where we do not set the type as a workspace build which causes the workspace update to never be published.
Otherwise the associated test fails due to the logger fataling on error messages.
0e5e497 to
2675eea
Compare
Fixes #7186. Probably. I was not able to reproduce with a template that had invalid credentials (I used the Heroku template) but I was able to reproduce with the Docker template by deleting my volumes (well I restarted my dev workspace which killed the volumes).
There were two issues: