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

[ws-manager] Backup on pod eviction #4405

Merged
merged 4 commits into from
Jun 8, 2021
Merged

Conversation

csweichel
Copy link
Contributor

This PR stores the hostIP as annotation s.t. in case of eviction we know which ws-daemon to talk to. When pods get evicted they loose their association with the node they ran on, which until now broke backups.

@aledbf aledbf self-requested a review June 7, 2021 11:02
Copy link
Member

@aledbf aledbf left a comment

Choose a reason for hiding this comment

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

LGTM

CHANGELOG.md Outdated
@@ -2,6 +2,10 @@

All notable changes to this project will be documented in this file.

## July 2021

- Improve backup stability when pods get evicted ([#4405](https://github.com/gitpod-io/gitpod/pull/4405))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think we're still in the June cycle. 😇

@jankeromnes
Copy link
Contributor

jankeromnes commented Jun 7, 2021

Many thanks for the quick fix!

If possible, it would be cool to have quick "how to test" instructions, in order to confirm that backups still work in both cases:

  • normal timeout

  • pod eviction

Copy link
Contributor

@JanKoehnlein JanKoehnlein left a comment

Choose a reason for hiding this comment

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

LGTM

@JanKoehnlein
Copy link
Contributor

Test instructions:

  • consume all of disk space via
dd if=/dev/zero of=/workspace/hd-fillup.zeros bs=1G
  • watch your ws being evicted
  • look at workspace-manager log files to see the message on successful backup.

See also #4415

Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

One small NIT (misleading log message).

Everything else looks fine! 👍

components/ws-manager/pkg/manager/monitor.go Outdated Show resolved Hide resolved
JanKoehnlein and others added 2 commits June 8, 2021 10:37
@JanKoehnlein JanKoehnlein merged commit 715ef34 into main Jun 8, 2021
@JanKoehnlein JanKoehnlein deleted the cw/backup-on-eviction branch June 8, 2021 09:19
JanKoehnlein added a commit that referenced this pull request Jun 9, 2021
* [ws-manager] Backup on pod eviction

* Fixed changelog

* Update components/ws-manager/pkg/manager/monitor.go

Co-authored-by: Cornelius A. Ludmann <cornelius.ludmann@typefox.io>
Co-authored-by: Jan Koehnlein <jan@gitpod.io>
Co-authored-by: Cornelius A. Ludmann <cornelius.ludmann@typefox.io>
csweichel added a commit that referenced this pull request Jun 23, 2021
csweichel added a commit that referenced this pull request Jun 25, 2021
MatthewFagan pushed a commit to trilogy-group/gitpod that referenced this pull request Nov 23, 2021
* [ws-manager] Backup on pod eviction

* Fixed changelog

* Update components/ws-manager/pkg/manager/monitor.go

Co-authored-by: Cornelius A. Ludmann <cornelius.ludmann@typefox.io>
Co-authored-by: Jan Koehnlein <jan@gitpod.io>
Co-authored-by: Cornelius A. Ludmann <cornelius.ludmann@typefox.io>
MatthewFagan pushed a commit to trilogy-group/gitpod that referenced this pull request Nov 29, 2021
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.

None yet

5 participants