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

Allow setting customTimeoutAnnotation for headless workspace pods #4239

Merged
merged 2 commits into from
May 21, 2021

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented May 18, 2021

This changes two things:

  • In server: Don't set customTimeoutAnnotation when starting headless pods (currently ignored by ws-manager)
  • In ws-manager: Honor customTimeoutAnnotation for headless pods (when it is set)

See also Slack discussion (internal)

And also (optionally):

  • In ws-manager: When a workspace times out, log both how long it took and the expected timeout (new)

How to test

  1. Start a regular workspace (for any repo)
  2. The workspace pod should still have a gitpod/customTimeout label (e.g. 30m if you're on the Open Source / Free plan)
  3. Start a prebuild (e.g. using the #prebuild/ manual prefix)
  4. The prebuild pod should not have a gitpod/customTimeout label
  5. If you manually set a gitpod/customTimeout label on the prebuild pod (e.g. 1m), the prebuild should honor it instead of the default headless timeout (which is 60m)

@jankeromnes jankeromnes force-pushed the jx/headless-custom-timeouts branch 2 times, most recently from 84eec97 to 695a8d6 Compare May 18, 2021 15:56
@jankeromnes

This comment has been minimized.

@jankeromnes jankeromnes force-pushed the jx/headless-custom-timeouts branch from 695a8d6 to 2640e4c Compare May 18, 2021 16:33
@jankeromnes jankeromnes force-pushed the jx/headless-custom-timeouts branch 4 times, most recently from c52b49e to df3a2c4 Compare May 19, 2021 13:40
@jankeromnes jankeromnes marked this pull request as ready for review May 19, 2021 14:31
@jankeromnes
Copy link
Contributor Author

jankeromnes commented May 19, 2021

Seems to work as expected! 🙌 @csweichel please take a look when you have time

@jankeromnes jankeromnes requested a review from csweichel May 19, 2021 14:37
@jankeromnes jankeromnes force-pushed the jx/headless-custom-timeouts branch from df3a2c4 to f6731ea Compare May 20, 2021 02:14
@jankeromnes
Copy link
Contributor Author

jankeromnes commented May 20, 2021

(I also snuck in a new test 😁 basically a copy of the timing-out prebuild, but with a custom timeout of 2h so that it doesn't time out.)

Copy link
Contributor

@csweichel csweichel left a comment

Choose a reason for hiding this comment

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

LGTM

@jankeromnes jankeromnes merged commit 7f8e557 into main May 21, 2021
@jankeromnes jankeromnes deleted the jx/headless-custom-timeouts branch May 21, 2021 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants