Skip to content

[supervisor] inflate all git repos #9961

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

Merged
merged 1 commit into from
May 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions components/content-service/pkg/initializer/initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,21 +517,24 @@ func PlaceWorkspaceReadyFile(ctx context.Context, wspath string, initsrc csapi.W
return nil
}

func GetCheckoutLocationFromInitializer(init *csapi.WorkspaceInitializer) string {
func GetCheckoutLocationsFromInitializer(init *csapi.WorkspaceInitializer) []string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this function replace GetCheckoutLocationFromInitializer as in

var (
    repoRoot string
    roots    = GetCheckoutLocationsFromInitializer(init)
)
if len(roots) > 0 {
    repoRoot = roots[0]
}

?

If so, I'd vote for removing GetCheckoutLocationFromInitializer in favour of this new function.

Copy link
Member Author

Choose a reason for hiding this comment

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

The single (main) repo_root is unfortunately used in a few other places, so I opted at adding a second variant. So we can slowly migrate based on need.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I understand now. 👍

switch {
case init.GetGit() != nil:
return init.GetGit().CheckoutLocation
return []string{init.GetGit().CheckoutLocation}
case init.GetPrebuild() != nil && len(init.GetPrebuild().Git) > 0:
return init.GetPrebuild().Git[0].CheckoutLocation
var result = make([]string, len(init.GetPrebuild().Git))
for i, c := range init.GetPrebuild().Git {
result[i] = c.CheckoutLocation
}
return result
case init.GetBackup() != nil:
return init.GetBackup().CheckoutLocation
return []string{init.GetBackup().CheckoutLocation}
case init.GetComposite() != nil:
var result []string
for _, c := range init.GetComposite().Initializer {
loc := GetCheckoutLocationFromInitializer(c)
if loc != "" {
return loc
}
result = append(result, GetCheckoutLocationsFromInitializer(c)...)
}
return result
}
return ""
return nil
}
77 changes: 77 additions & 0 deletions components/content-service/pkg/initializer/initializer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package initializer_test
import (
"context"
"fmt"
"strings"
"testing"

csapi "github.com/gitpod-io/gitpod/content-service/api"
Expand All @@ -29,6 +30,82 @@ func (f *RecordingInitializer) Run(ctx context.Context, mappings []archive.IDMap
return csapi.WorkspaceInitFromOther, nil
}

func TestGetCheckoutLocationsFromInitializer(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be a table driven test:

import "github.com/google/go-cmp/cmp"

type Expectation struct {
    Roots []string
}
tests := []struct{
    Name string
    Initializer *csapi.WorkspaceInitializer
}{
    {
        Name: "single git initializer",
        Initializer: ...
    }
}

for _, test := range tests {
    t.Run(test.Name, func(t *testing.T) {
        var act Expectation
        act.Roots = initializer.GetCheckoutLocationsFromInitializer(test.Initializer)

        if diff := cmp.Diff(test.Expectation, act); diff != "" {
            t.Errorf("unexpected result (-want +got):\n%s", diff)
        }
    })
}


var init []*csapi.WorkspaceInitializer
init = append(init, &csapi.WorkspaceInitializer{
Spec: &csapi.WorkspaceInitializer_Git{
Git: &csapi.GitInitializer{
CheckoutLocation: "/foo",
CloneTaget: "head",
Config: &csapi.GitConfig{
Authentication: csapi.GitAuthMethod_NO_AUTH,
},
RemoteUri: "somewhere-else",
TargetMode: csapi.CloneTargetMode_LOCAL_BRANCH,
},
},
})
init = append(init, &csapi.WorkspaceInitializer{
Spec: &csapi.WorkspaceInitializer_Git{
Git: &csapi.GitInitializer{
CheckoutLocation: "/bar",
CloneTaget: "head",
Config: &csapi.GitConfig{
Authentication: csapi.GitAuthMethod_NO_AUTH,
},
RemoteUri: "somewhere-else",
TargetMode: csapi.CloneTargetMode_LOCAL_BRANCH,
},
},
})

tests := []struct {
Name string
Initializer *csapi.WorkspaceInitializer
Expectation string
}{
{
Name: "single git initializer",
Initializer: &csapi.WorkspaceInitializer{
Spec: &csapi.WorkspaceInitializer_Git{
Git: &csapi.GitInitializer{
CheckoutLocation: "/foo",
CloneTaget: "head",
Config: &csapi.GitConfig{
Authentication: csapi.GitAuthMethod_NO_AUTH,
},
RemoteUri: "somewhere-else",
TargetMode: csapi.CloneTargetMode_LOCAL_BRANCH,
},
},
},
Expectation: "/foo",
},
{
Name: "multiple git initializer",
Initializer: &csapi.WorkspaceInitializer{
Spec: &csapi.WorkspaceInitializer_Composite{
Composite: &csapi.CompositeInitializer{
Initializer: init,
},
},
},
Expectation: "/foo,/bar",
},
}

for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
locations := strings.Join(initializer.GetCheckoutLocationsFromInitializer(test.Initializer), ",")
if locations != test.Expectation {
t.Errorf("expected %s, got %s", test.Expectation, locations)
}
})
}

}

func TestCompositeInitializer(t *testing.T) {
tests := []struct {
Name string
Expand Down
8 changes: 8 additions & 0 deletions components/supervisor/pkg/supervisor/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,10 @@ type WorkspaceConfig struct {
// is located. If there's no Git repo in this workspace, this will be empty.
RepoRoot string `env:"GITPOD_REPO_ROOT"`

// RepoRoots is the comma seprated list of locations in the filesystem where Git repositories
// are located. If there's no Git repo in this workspace, this will be empty.
RepoRoots string `env:"GITPOD_REPO_ROOTS"`

// PreventMetadataAccess exits supervisor/stops the workspace if we can access Google Cloud
// compute metadata from within the container.
PreventMetadataAccess bool `env:"GITPOD_PREVENT_METADATA_ACCESS"`
Expand Down Expand Up @@ -460,6 +464,10 @@ func loadWorkspaceConfigFromEnv() (*WorkspaceConfig, error) {
if err != nil {
return nil, xerrors.Errorf("cannot load workspace config: %w", err)
}
//TODO(sefftinge) remove me after deployment (backward compatibility)
if res.RepoRoots == "" {
res.RepoRoots = res.RepoRoot
}

return &res, nil
}
32 changes: 17 additions & 15 deletions components/supervisor/pkg/supervisor/supervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,21 +328,23 @@ func Run(options ...RunOption) {

if !cfg.isHeadless() {
go func() {
<-cstate.ContentReady()

start := time.Now()
defer func() {
log.Debugf("unshallow of local repository took %v", time.Since(start))
}()

cmd := runAsGitpodUser(exec.Command("git", "fetch", "--unshallow", "--tags"))
cmd.Env = childProcEnvvars
cmd.Dir = cfg.RepoRoot
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
err := cmd.Run()
if err != nil {
log.WithError(err).Error("git fetch error")
for _, repoRoot := range strings.Split(cfg.RepoRoots, ",") {
<-cstate.ContentReady()

start := time.Now()
defer func() {
log.Debugf("unshallow of local repository took %v", time.Since(start))
}()

cmd := runAsGitpodUser(exec.Command("git", "fetch", "--unshallow", "--tags"))
cmd.Env = childProcEnvvars
cmd.Dir = repoRoot
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
err := cmd.Run()
if err != nil {
log.WithError(err).Error("git fetch error")
}
}
}()
}
Expand Down
7 changes: 6 additions & 1 deletion components/ws-daemon/pkg/content/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,15 @@ func (s *WorkspaceService) InitWorkspace(ctx context.Context, req *api.InitWorks
}

func (s *WorkspaceService) creator(req *api.InitWorkspaceRequest) session.WorkspaceFactory {
var checkoutLocation string
allLocations := wsinit.GetCheckoutLocationsFromInitializer(req.Initializer)
if len(allLocations) > 0 {
checkoutLocation = allLocations[0]
}
return func(ctx context.Context, location string) (res *session.Workspace, err error) {
return &session.Workspace{
Location: location,
CheckoutLocation: wsinit.GetCheckoutLocationFromInitializer(req.Initializer),
CheckoutLocation: checkoutLocation,
CreatedAt: time.Now(),
Owner: req.Metadata.Owner,
WorkspaceID: req.Metadata.MetaId,
Expand Down
3 changes: 2 additions & 1 deletion components/ws-manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ Use the `-H` flag to connect to either the HTTP RPC interace (`http://localhost:

### Running tests
We use the standard Go `testing` package to run tests. To execute all `ws-manager` tests run `go test -v ./...`.
Some of our test-cases use _golden_ files. If you want to update one, delete that particular file and execute the tests with `-update`.
Some of our test-cases use _golden_ files. If you want to update one, delete that particular file and execute the tests with `-update` (i.e. `cd components/ws-manager/pkg/manager
go test -v -update -force .`).

Go has a load of handy flags for its testing abilities. For example the built-in race detector using `go test -race -v ./...`.

Expand Down
11 changes: 9 additions & 2 deletions components/ws-manager/pkg/manager/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -691,11 +691,18 @@ func (m *Manager) createWorkspaceEnvironment(startContext *startWorkspaceContext
return filepath.Join("/workspace", strings.TrimPrefix(segment, "/workspace"))
}

repoRoot := content.GetCheckoutLocationFromInitializer(spec.Initializer)
allRepoRoots := content.GetCheckoutLocationsFromInitializer(spec.Initializer)
if len(allRepoRoots) == 0 {
allRepoRoots = []string{""} // for backward compatibility, we are adding a single empty location (translates to /workspace/)
}
for i, root := range allRepoRoots {
allRepoRoots[i] = getWorkspaceRelativePath(root)
}

// Envs that start with GITPOD_ are appended to the Terminal environments
result := []corev1.EnvVar{}
result = append(result, corev1.EnvVar{Name: "GITPOD_REPO_ROOT", Value: getWorkspaceRelativePath(repoRoot)})
result = append(result, corev1.EnvVar{Name: "GITPOD_REPO_ROOT", Value: allRepoRoots[0]})
result = append(result, corev1.EnvVar{Name: "GITPOD_REPO_ROOTS", Value: strings.Join(allRepoRoots, ",")})
result = append(result, corev1.EnvVar{Name: "GITPOD_CLI_APITOKEN", Value: startContext.CLIAPIKey})
result = append(result, corev1.EnvVar{Name: "GITPOD_OWNER_ID", Value: startContext.Request.Metadata.Owner})
result = append(result, corev1.EnvVar{Name: "GITPOD_WORKSPACE_ID", Value: startContext.Request.Metadata.MetaId})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down Expand Up @@ -264,4 +268,4 @@
},
"status": {}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down Expand Up @@ -256,4 +260,4 @@
},
"status": {}
}
}
}
4 changes: 4 additions & 0 deletions components/ws-manager/pkg/manager/testdata/cdwp_class.golden
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down Expand Up @@ -278,4 +282,4 @@
},
"status": {}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down Expand Up @@ -254,4 +258,4 @@
},
"status": {}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down Expand Up @@ -293,4 +297,4 @@
},
"status": {}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down Expand Up @@ -261,4 +265,4 @@
},
"status": {}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down Expand Up @@ -250,4 +254,4 @@
},
"status": {}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@
"name": "GITPOD_REPO_ROOT",
"value": "/workspace"
},
{
"name": "GITPOD_REPO_ROOTS",
"value": "/workspace"
},
{
"name": "GITPOD_CLI_APITOKEN",
"value": "Ab=5=rRA*9:C'T{;RRB\u003e]vK2p6`fFfrS"
Expand Down Expand Up @@ -282,4 +286,4 @@
},
"status": {}
}
}
}
Loading