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

Git commit + push on workspace shutdown to backup unsaved workspace project changes #22308

Open
AObuchow opened this issue Jun 20, 2023 · 9 comments
Assignees
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/devworkspace-operator kind/enhancement A feature request - must adhere to the feature request template.

Comments

@AObuchow
Copy link

Is your enhancement related to a problem? Please describe

Using ephemeral storage can often provide faster workspace startup time (as storage does not have to be provisioned). However, there is the inherent disadvantage that any code changes to the project that the user is working on will not be persisted upon workspace shutdown. The user needs to commit and push their changes with git in order to retrieve them later on.

Describe the solution you'd like

It'd be nice to have a spec.devEnvironments.gitBackup: <bool> configuration option in the Che Cluster CR, that when enabled, will automatically commit all changes to the project in use, and push them to a unique branch, e.g. che-backup-<checked-out-branch-name>-YYYY-MM-DD-HH-MM-SS.

This could be implemented as a pre-stop event for workspace pods at the DevWorkspace Operator level.

There are some considerations that need to be made for this feature. Some that come to mind:

  • A project can have multiple remotes, such as origin and fork. Since these remote names may be different on a per-devfile-basis, setting the default remote to "backup" to may be tricky. We may need something at the devfile or devworkspace level to specify which remote should be used for backups.
  • Using this feature should not prevent the use of other devfile preStop events (though these are currently unsupported by DWO/Che)
  • Enabling this feature should probably not be allowed when persistent workspace storage is used, as it will create an unnecessary commit of the user's work (which can be frustrating for keeping an organized git log) without providing any advantages (their changes would persist regardless of this feature).
  • This feature should have no effect if the project being used is not a git project

Describe alternatives you've considered

As preStop events are not currently supported by DWO, there's no way to manually configure a git commit and push on workspace shutdown from a devfile. Users using ephemeral storage must remember to always manually commit and push their workspaces changes before their workspace shuts down.

Additional context

No response

@AObuchow AObuchow added kind/enhancement A feature request - must adhere to the feature request template. area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/devworkspace-operator labels Jun 20, 2023
@AObuchow AObuchow self-assigned this Jun 20, 2023
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Jun 20, 2023
@AObuchow AObuchow removed the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Jun 20, 2023
@AObuchow
Copy link
Author

@ibuziuk @l0rd @amisevsk CC'ing you all in case of any thoughts/input on this feature.

@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Jun 20, 2023
@AObuchow AObuchow changed the title Git commit + push on workspace shutdown to back up unsaved workspace project changes Git commit + push on workspace shutdown to backup unsaved workspace project changes Jun 21, 2023
@l0rd
Copy link
Contributor

l0rd commented Jun 21, 2023

Looks good to me. I couple of comments:

  • This is something that developer should be able to configure at single workspace level (as the ephemeral storage config).
  • I would start with using origin as the remote (no origin or no write privileges on origin then no backup is done)
  • The prefix of the backup branch name should not have hardcoded "che" in it, ideally it should be configurable
  • You are not describing what happens when the developer restart the workspace: we should either automatically clone the last backup branch or ask the user (I prefer the first option as a first iteration as it looks the simplest to implement)

@amisevsk
Copy link
Contributor

If we want to restore from a backup branch when possible, we should probably choose a deterministic name for the branch (e.g. <workspaceID>-<suffix> for some reasonable suffix). Otherwise, I don't know how we would automatically clone from that branch.

For determining the remote to check, we already determine which remote to clone from, so we could check if that branch exists and clone that instead of what's selected in the devworkspace, though this might cause confusion ("I've been pushing to branch X but now branch Y is checked out")

From a first pass, it looks like the configurable options we'd need to consider would be

  • Enable/disable feature (default: disabled)
  • Enable/disable restore from branch on restart (default: enabled?)
  • (Maybe) configure branch name format.
  • (Maybe) override remote to push to.

In addition, we could consider whether we want to allow configuration at a workspace level or a per-project level (or both). Attributes would be a good candidate for this since they allow structured data rather than just strings.

Implementation wise, we could potentially do everything required within DWO/project-clone itself, though if we implement it as a preStop hook we might have to consider collisions with preStop events, if we ever decide to implement those. Currently DWO will provision a preStop event handler when async storage is used, but this might be extended.

@AObuchow AObuchow removed the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Jun 21, 2023
@l0rd
Copy link
Contributor

l0rd commented Jun 26, 2023

A project that may be interesting for this https://github.com/tkellogg/dura

@AObuchow
Copy link
Author

@l0rd @amisevsk Thank you both for the thoughtful input.

In addition, we could consider whether we want to allow configuration at a workspace level or a per-project level (or both). Attributes would be a good candidate for this since they allow structured data rather than just strings.

Ideally, I'd like to support configuring this at the workspace and project level. If we only support this on a per-project basis, then users who want to use this feature for all workspaces will have to copy/paste the attribute for all projects. If we only support it on a workspace basis, then users don't have the granularity to only use it for the project they are actively working on.
However, for a first-iteration, I think configuration on a per-project basis would be fine.

Implementation wise, we could potentially do everything required within DWO/project-clone itself, though if we implement it as a preStop hook we might have to consider collisions with preStop events, if we ever decide to implement those. Currently DWO will provision a preStop event handler when async storage is used, but this might be extended.

I'm considering trying to implement support for multiple preStop events as a precursor to this feature. This would lay a foundation for this feature to build upon without colliding with the async storage preStop event handler (or normal preStop events). Additionally, supporting preStop events would probably allow for rough testing of this feature by configuring a devfile/devworkspace accordingly.

A project that may be interesting for this https://github.com/tkellogg/dura

This seems really cool & relevant to this feature. I think I should spend some time experimenting with dura to make sure it'll work nicely for our use case. My concern is ensuring the main workspace container image has the dura binary installed. However, the same concern exists for git: users could use a workspace container image that doesn't have git installed - so this concern might be outside our control.

@che-bot
Copy link
Contributor

che-bot commented Dec 24, 2023

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 24, 2023
@che-bot che-bot closed this as completed Dec 31, 2023
@AObuchow
Copy link
Author

AObuchow commented Jan 2, 2024

/remove-lifecycle stale

@AObuchow AObuchow removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 2, 2024
@AObuchow AObuchow reopened this Jan 2, 2024
@tolusha
Copy link
Contributor

tolusha commented Apr 3, 2024

@AObuchow
Can we close the issue?

@AObuchow
Copy link
Author

@tolusha I'm actually (slowly) still working on this issue, though it's low priority since no one has actively requested for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/devworkspace-operator kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

No branches or pull requests

5 participants