[ws-daemon] do not fail workspace if git status failed during dispose

[sagor999]

Description

(This PR is stacked on top of #11330, please review commit: 8d2d1c6)

Some workspaces fail with cannot get git status, which is not a critical error.
Observed in some cases it failed due to potentially corrupt\misconfigured .git folder (potentially due to user's actions).

This PR would make it so that we don't fail workspace when this happens and instead continue disposing it.
For example, we already do that if workspace folder does not contain .git folder in it:

gitpod/components/ws-daemon/pkg/internal/session/workspace.go

Lines 309 to 312 in 905be0a

	if !git.IsWorkingCopy(loc) {
	log.WithField("loc", loc).WithField("checkout location", s.CheckoutLocation).WithFields(s.OWI()).Debug("did not find a Git working copy - not updating Git status")
	return nil, nil
	}

Main reasoning is that even if we cannot get git status, all that means is that we not going to show the user in dashboard uncommitted changes, which should not be a reason for failing whole workspace.

Related Issue(s)

Fixes #

How to test

Open workspace and corrupt your .git folder
Observe that workspace still finishes correctly, albeit without showing any uncommitted changes in dashboard.

Release Notes

none

Documentation

Werft options:

• /werft with-preview

[sagor999]

/hold
would like to get input from @csweichel on this one, so holding to make sure he has a chance to review

[aledbf]

@sagor999 I think we need to update the configuration and configure /mnt/workingarea as safe using
git config --global --add safe.directory * instead of individual directories https://github.blog/2022-04-18-highlights-from-git-2-36/
(file /home/gitpod/.gitconfig in ws-daemon)

[sagor999]

@aledbf I was referring to this particular case:
https://cloudlogging.app.goo.gl/8GkTztnkJ5MdpwAz8
(check ws-daemon error log there)

I think what you are referring to is a frequent similar error that seems to be coming from prebuilds (unsafe directory error).
I will try to take a look at it as well, but it is out of scope for this particular PR (even though it is kind of related)

[jenting]

I like this approach 👍
I encountered a scenario with the error git status failed, but the volume snapshot is ready to use.

In the scenario, we enable the PVC feature flag and manually make the node into a NotReady state (by disabling the k3s-agent on the host).
Then, the workspace pod becomes a Terminating state—finally, the VolumeSnapshot object is created by ws-manager and taken the snapshot of the PVC.
After relaunching the workspace pod, the workspace pod launches at another Ready node, and the user's content is recovered from the VolumeSnapshot.

Without this PR change, the dashboard shows the error cannot get git status.

The pro is we keep the user's content by PVC w/o data loss, and the con is we can't see the git difference in the dashboard.

[csweichel]

LGTM

(minor nit, hence the hold)

/hold

[csweichel]

/hold

[sagor999]

/unhold

[aledbf]

/hold

(due to the build error)

[sagor999]

/werft run with-clean-slate-deployment=true

👍 started the job as gitpod-build-pavel-git-status-fix.3
(with .werft/ from main)

[sagor999]

/werft run with-clean-slate-deployment=true

👍 started the job as gitpod-build-pavel-git-status-fix.4
(with .werft/ from main)

[sagor999]

something funky with unit tests in this build. each time it is a different test that fails, but opened it and did a leeway build and everything built fine...
going to try to rebase.

[mads-hartmann]

/werft run

👍 started the job as gitpod-build-pavel-git-status-fix.6
(with .werft/ from main)

[mads-hartmann]

The test succeeded this time. The set of of components that Leeway decided it needed to build is very different between gitpod-build-pavel-git-status-fix.5 and gitpod-build-pavel-git-status-fix.6 - I'm not quite sure why.

But given the failing tests were sporadic and unrelated to this PR I'll remove the hold so it can be merged and you can at least wake up to a merged PR ☺️ I'll following up to understand why Leeway would decide to build different packages when executed from the same branch at 10 hours apart.

/unhold

Update: Here is the internal Slack - I think that we're using a shared Leeway cache for all non-main branches which might hide test flakiness as you only need one successful build across all non-main branches. So you experienced the flakiness because you happened to be the first to build these components off main (if my theory is correct)