[ws-manager-mk2] Fix git status (#17097)

* [ws-manager-mk2] Fix git status

* Remove unused var

Author: WVerlaek

components/ws-daemon/pkg/controller/workspace_operations.go

@@ -17,29 +17,17 @@ import (
 	"github.com/gitpod-io/gitpod/common-go/tracing"
 	csapi "github.com/gitpod-io/gitpod/content-service/api"
 	"github.com/gitpod-io/gitpod/content-service/pkg/archive"
-	"github.com/gitpod-io/gitpod/content-service/pkg/git"
 	wsinit "github.com/gitpod-io/gitpod/content-service/pkg/initializer"
 	"github.com/gitpod-io/gitpod/content-service/pkg/logs"
 	"github.com/gitpod-io/gitpod/content-service/pkg/storage"
 	"github.com/gitpod-io/gitpod/ws-daemon/pkg/content"
 	"github.com/gitpod-io/gitpod/ws-daemon/pkg/internal/session"
-	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
 	"github.com/opentracing/opentracing-go"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/xerrors"
 )
 
-const (
-	// maxPendingChanges is the limit beyond which we no longer report pending changes.
-	// For example, if a workspace has then 150 untracked files, we'll report the first
-	// 100 followed by "... and 50 more".
-	//
-	// We do this to keep the load on our infrastructure light and because beyond this number
-	// the changes are irrelevant anyways.
-	maxPendingChanges = 100
-)
-
 type WorkspaceOperations interface {
 	// InitWorkspace initializes the workspace content
 	InitWorkspace(ctx context.Context, options InitOptions) (string, error)
@@ -478,68 +466,6 @@ func (wso *DefaultWorkspaceOperations) uploadWorkspaceContent(ctx context.Contex
 	return nil
 }
 
-// UpdateGitStatus attempts to update the LastGitStatus from the workspace's local working copy.
-func (wsc *WorkspaceController) UpdateGitStatus(ctx context.Context, ws *workspacev1.Workspace, s *session.Workspace) (res *csapi.GitStatus, err error) {
-	var loc string
-	loc = s.Location
-	if loc == "" {
-		// FWB workspaces don't have `Location` set, but rather ServiceLocDaemon and ServiceLocNode.
-		// We'd can't easily produce the Git status, because in this context `mark` isn't mounted, and `upper`
-		// only contains the full git working copy if the content was just initialised.
-		// Something like
-		//   loc = filepath.Join(s.ServiceLocDaemon, "mark", "workspace")
-		// does not work.
-		//
-		// TODO(cw): figure out a way to get ahold of the Git status.
-		glog.WithField("loc", loc).WithFields(s.OWI()).Debug("not updating Git status of FWB workspace")
-		return
-	}
-
-	loc = filepath.Join(loc, s.CheckoutLocation)
-	if !git.IsWorkingCopy(loc) {
-		glog.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
-	}
-
-	c := git.Client{Location: loc}
-
-	stat, err := c.Status(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	lastGitStatus := toGitStatus(stat)
-
-	err = s.Persist()
-	if err != nil {
-		glog.WithError(err).Warn("cannot persist latest Git status")
-		err = nil
-	}
-
-	return lastGitStatus, err
-}
-
-func toGitStatus(s *git.Status) *csapi.GitStatus {
-	limit := func(entries []string) []string {
-		if len(entries) > maxPendingChanges {
-			return append(entries[0:maxPendingChanges], fmt.Sprintf("... and %d more", len(entries)-maxPendingChanges))
-		}
-
-		return entries
-	}
-
-	return &csapi.GitStatus{
-		Branch:               s.BranchHead,
-		LatestCommit:         s.LatestCommit,
-		UncommitedFiles:      limit(s.UncommitedFiles),
-		TotalUncommitedFiles: int64(len(s.UncommitedFiles)),
-		UntrackedFiles:       limit(s.UntrackedFiles),
-		TotalUntrackedFiles:  int64(len(s.UntrackedFiles)),
-		UnpushedCommits:      limit(s.UnpushedCommits),
-		TotalUnpushedCommits: int64(len(s.UnpushedCommits)),
-	}
-}
-
 func retryIfErr(ctx context.Context, attempts int, log *logrus.Entry, op func(ctx context.Context) error) (err error) {
 	//nolint:ineffassign
 	span, ctx := opentracing.StartSpanFromContext(ctx, "retryIfErr")

components/ws-manager-mk2/service/manager.go

@@ -1024,6 +1024,7 @@ func (wsm *WorkspaceManagerServer) extractWorkspaceStatus(ws *workspacev1.Worksp
 			Admission:  admissionLevel,
 			OwnerToken: ws.Status.OwnerToken,
 		},
+		Repo: convertGitStatus(ws.Status.GitStatus),
 	}
 
 	return res
@@ -1038,6 +1039,22 @@ func getConditionMessageIfTrue(conds []metav1.Condition, tpe string) string {
 	return ""
 }
 
+func convertGitStatus(gs *workspacev1.GitStatus) *csapi.GitStatus {
+	if gs == nil {
+		return nil
+	}
+	return &csapi.GitStatus{
+		Branch:               gs.Branch,
+		LatestCommit:         gs.LatestCommit,
+		UncommitedFiles:      gs.UncommitedFiles,
+		TotalUncommitedFiles: gs.TotalUncommitedFiles,
+		UntrackedFiles:       gs.UntrackedFiles,
+		TotalUntrackedFiles:  gs.TotalUntrackedFiles,
+		UnpushedCommits:      gs.UnpushedCommits,
+		TotalUnpushedCommits: gs.TotalUnpushedCommits,
+	}
+}
+
 func convertCondition(conds []metav1.Condition, tpe string) wsmanapi.WorkspaceConditionBool {
 	res := wsk8s.GetCondition(conds, tpe)
 	if res == nil {

test/README.md

@@ -67,6 +67,8 @@ If you want to run an entire test suite, the easiest is to use `./test/run.sh`:
 If you're iterating on a single test, the easiest is to use `go test` directly.
 If your integration tests depends on having having a user token available, then you'll have to set `USER_TOKEN` manually (see `test/run.sh` on how to fetch the credentials that are used during our build)
 
+If you want to run the workspace tests against `ws-manager-mk2`, set the `WS_MANAGER_MK2=true` env var when running the tests.
+
 ```console
 cd test
 go test -v ./... \

test/tests/components/ws-manager/git_status_test.go

@@ -0,0 +1,133 @@
+// Copyright (c) 2020 Gitpod GmbH. All rights reserved.
+// Licensed under the GNU Affero General Public License (AGPL).
+// See License.AGPL.txt in the project root for license information.
+
+package wsmanager
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"google.golang.org/protobuf/proto"
+	"sigs.k8s.io/e2e-framework/pkg/envconf"
+	"sigs.k8s.io/e2e-framework/pkg/features"
+
+	csapi "github.com/gitpod-io/gitpod/content-service/api"
+	agent "github.com/gitpod-io/gitpod/test/pkg/agent/workspace/api"
+	"github.com/gitpod-io/gitpod/test/pkg/integration"
+	wsmanapi "github.com/gitpod-io/gitpod/ws-manager/api"
+)
+
+// TestGitStatus tests that the git status is reported after a workspace is stopped.
+func TestGitStatus(t *testing.T) {
+	f := features.New("git-status").
+		WithLabel("component", "ws-manager").
+		Assess("it should report the git status of a workspace when it stops", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
+			tests := []struct {
+				Name             string
+				ContextURL       string
+				WorkspaceRoot    string
+				CheckoutLocation string
+			}{
+				{
+					Name:             "classic",
+					ContextURL:       "https://github.com/gitpod-io/empty",
+					WorkspaceRoot:    "/workspace/empty",
+					CheckoutLocation: "empty",
+				},
+			}
+			for _, test := range tests {
+				test := test
+				t.Run(test.Name, func(t *testing.T) {
+					t.Parallel()
+
+					ctx, cancel := context.WithTimeout(context.Background(), time.Duration(10*len(tests))*time.Minute)
+					defer cancel()
+
+					api := integration.NewComponentAPI(ctx, cfg.Namespace(), kubeconfig, cfg.Client())
+					t.Cleanup(func() {
+						api.Done(t)
+					})
+
+					// TODO: change to use server API to launch the workspace, so we could run the integration test as the user code flow
+					//       which is client -> server -> ws-manager rather than client -> ws-manager directly
+					ws1, stopWs, err := integration.LaunchWorkspaceDirectly(t, ctx, api, integration.WithRequestModifier(func(w *wsmanapi.StartWorkspaceRequest) error {
+						w.Spec.Initializer = &csapi.WorkspaceInitializer{
+							Spec: &csapi.WorkspaceInitializer_Git{
+								Git: &csapi.GitInitializer{
+									RemoteUri:        test.ContextURL,
+									CheckoutLocation: test.CheckoutLocation,
+									Config:           &csapi.GitConfig{},
+								},
+							},
+						}
+						w.Spec.WorkspaceLocation = test.CheckoutLocation
+						return nil
+					}))
+					if err != nil {
+						t.Fatal(err)
+					}
+					t.Cleanup(func() {
+						if err != nil {
+							sctx, scancel := context.WithTimeout(context.Background(), 5*time.Minute)
+							defer scancel()
+
+							sapi := integration.NewComponentAPI(sctx, cfg.Namespace(), kubeconfig, cfg.Client())
+							defer sapi.Done(t)
+
+							_, err = stopWs(true, sapi)
+							if err != nil {
+								t.Fatal(err)
+							}
+						}
+					})
+
+					rsa, closer, err := integration.Instrument(integration.ComponentWorkspace, "workspace", cfg.Namespace(), kubeconfig, cfg.Client(),
+						integration.WithInstanceID(ws1.Req.Id),
+						integration.WithContainer("workspace"),
+						integration.WithWorkspacekitLift(true),
+					)
+					if err != nil {
+						t.Fatal(err)
+					}
+					integration.DeferCloser(t, closer)
+
+					var resp agent.WriteFileResponse
+					err = rsa.Call("WorkspaceAgent.WriteFile", &agent.WriteFileRequest{
+						Path:    fmt.Sprintf("%s/foobar.txt", test.WorkspaceRoot),
+						Content: []byte("hello world"),
+						Mode:    0644,
+					}, &resp)
+					rsa.Close()
+					if err != nil {
+						if _, serr := stopWs(true, api); serr != nil {
+							t.Errorf("cannot stop workspace: %q", serr)
+						}
+						t.Fatal(err)
+					}
+
+					lastStatus, err := stopWs(true, api)
+					if err != nil {
+						t.Fatal(err)
+					}
+
+					t.Logf("last status: %v", lastStatus)
+					expected := &csapi.GitStatus{
+						Branch:              "main",
+						UntrackedFiles:      []string{"foobar.txt"},
+						TotalUntrackedFiles: 1,
+					}
+					if !proto.Equal(lastStatus.Repo, expected) {
+						t.Fatalf("unexpected git status: expected \"%+q\", got \"%+q\"", expected, lastStatus.Repo)
+					}
+				})
+			}
+			return ctx
+		}).
+		Feature()
+
+	testEnv.Test(t, f)
+
+}