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.

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

Don't mark prebuilds with failing tasks unavailable #4975

Merged
merged 2 commits into from
Jul 29, 2021

Conversation

csweichel
Copy link
Contributor

@csweichel csweichel commented Jul 27, 2021

This PR correctly marks prebuilds with a failed headless task as available, and updates ws-manager to not spuriously add the failed condition if the headless task fails.

fixes #4964

How to test

  1. Run a prebuild with a failing headless task, e.g. https://csweichel-don-t-abort-prebuild-4964.staging.gitpod-dev.com#https://github.com/csweichel/test-repo/tree/task-failure
  2. Check the database and find the prebuild marked as available

image

@csweichel
Copy link
Contributor Author

/auto-cc

@roboquat roboquat requested a review from rl-gitpod July 27, 2021 18:59
@csweichel csweichel removed the request for review from rl-gitpod July 27, 2021 18:59
@codecov
Copy link

codecov bot commented Jul 27, 2021

Codecov Report

Merging #4975 (ed9bcd7) into main (698a17b) will increase coverage by 13.32%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #4975       +/-   ##
===========================================
+ Coverage   23.02%   36.34%   +13.32%     
===========================================
  Files          11       13        +2     
  Lines        1915     3747     +1832     
===========================================
+ Hits          441     1362      +921     
- Misses       1415     2270      +855     
- Partials       59      115       +56     
Flag Coverage Δ
components-ws-daemon-app ?
components-ws-manager-app 36.34% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/ws-manager/pkg/manager/status.go 72.21% <100.00%> (ø)
components/ws-daemon/pkg/content/config.go
...onents/ws-daemon/pkg/internal/session/workspace.go
components/ws-daemon/pkg/resources/dispatch.go
components/ws-daemon/pkg/resources/controller.go
components/ws-daemon/pkg/resources/limiter.go
components/ws-daemon/pkg/content/archive.go
components/ws-daemon/pkg/content/tar.go
components/ws-daemon/pkg/internal/session/store.go
components/ws-daemon/pkg/quota/size.go
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 698a17b...ed9bcd7. Read the comment docs.

@csweichel csweichel marked this pull request as ready for review July 28, 2021 06:31
@csweichel csweichel force-pushed the csweichel/don-t-abort-prebuild-4964 branch from f100a7c to 5a2cd98 Compare July 28, 2021 17:26
@csweichel
Copy link
Contributor Author

@geropl debug leftover is removed. Please have another look

@geropl
Copy link
Member

geropl commented Jul 29, 2021

/werft run

👍 started the job as gitpod-build-csweichel-don-t-abort-prebuild-4964.4

@geropl geropl force-pushed the csweichel/don-t-abort-prebuild-4964 branch from 5a2cd98 to ed9bcd7 Compare July 29, 2021 07:55
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.

Code LGTM, tested and worked 👍

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 633a975dcfd6b697616f5278363f392b12b1f86c

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csweichel, geropl

Associated issue: #4964

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 479abb7 into main Jul 29, 2021
@roboquat roboquat deleted the csweichel/don-t-abort-prebuild-4964 branch July 29, 2021 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't abort prebuild if task failed
3 participants