Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ws-manager] make sure to fail prebuild if we cannot store snapshot from it #10053

Merged
merged 1 commit into from
May 17, 2022
Merged

[ws-manager] make sure to fail prebuild if we cannot store snapshot from it #10053

merged 1 commit into from
May 17, 2022

Conversation

sagor999
Copy link
Contributor

@sagor999 sagor999 commented May 16, 2022 •
edited
Loading

Description

Related Issue(s)

Fixes #9710

How to test

Easiest way to test is to modify this line:

gitpod/components/ws-manager/pkg/manager/monitor.go

Line 933 in c9be66c

res, err = snc.TakeSnapshot(ctx, &wsdaemon.TakeSnapshotRequest{Id: workspaceID})

with

//res, err = snc.TakeSnapshot(ctx, &wsdaemon.TakeSnapshotRequest{Id: workspaceID})
res = nil
err = xerrors.Error("fake error")

and run prebuild.
Now it will report proper error that webapp team needs:
Pod phase: STOPPED, condition error: cannot take snapshot: I am a fake error, condition snapshot:

Release Notes

[prebuild] fix incorrect handling of failed prebuilds

Documentation

@sagor999 sagor999 changed the title wip [ws-manager] make sure to fail prebuild if we cannot store snapshot from it May 17, 2022
@sagor999 sagor999 marked this pull request as ready for review May 17, 2022 00:44
@sagor999 sagor999 requested a review from a team May 17, 2022 00:44
@sagor999 sagor999 requested a review from geropl May 17, 2022 00:44
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label May 17, 2022
@sagor999
Copy link
Contributor Author

@geropl this should now report proper error when snapshot fails for prebuild. Let me know if this is what you need.

@geropl
Copy link
Member

geropl commented May 17, 2022

Awesome, thank you @sagor999 . Will look into #10054 now.

geropl
geropl approved these changes May 17, 2022
View reviewed changes
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected! ✔️

@geropl geropl mentioned this pull request May 17, 2022
Closed
jenting
jenting approved these changes May 17, 2022
View reviewed changes
Copy link
Contributor

@jenting jenting left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@roboquat roboquat merged commit 30e8f7d into main May 17, 2022
@roboquat roboquat deleted the pavel/9710 branch May 17, 2022 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geropl geropl geropl approved these changes

@jenting jenting jenting approved these changes

Assignees
No one assigned
Labels
release-note size/XS team: workspace Issue belongs to the Workspace team
Projects
None yet
Milestone
No milestone
4 participants
@sagor999 @geropl @jenting @roboquat