[supervisor] Add unit tests for task composition

components/content-service/go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/gitpod-io/gitpod/common-go v0.0.0-00010101000000-000000000000
 	github.com/gitpod-io/gitpod/content-service/api v0.0.0-00010101000000-000000000000
 	github.com/go-ozzo/ozzo-validation v3.5.0+incompatible
-	github.com/golang/mock v1.5.0 // indirect
+	github.com/golang/mock v1.5.0
 	github.com/google/go-cmp v0.5.6
 	github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0

components/supervisor/go.mod

@@ -21,8 +21,8 @@ require (
 	github.com/improbable-eng/grpc-web v0.14.0
 	github.com/mailru/easygo v0.0.0-20190618140210-3c14a0dc985f
 	github.com/prometheus/procfs v0.6.0
-	github.com/sirupsen/logrus v1.8.1
 	github.com/rs/cors v1.7.0 // indirect
+	github.com/sirupsen/logrus v1.8.1
 	github.com/soheilhy/cmux v0.1.4
 	github.com/spf13/cobra v1.1.3
 	golang.org/x/crypto v0.0.0-20210506145944-38f3c27a63bf

components/supervisor/go.sum

@@ -265,6 +265,7 @@ github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDD
 github.com/docker/libtrust v0.0.0-20150114040149-fa567046d9b1/go.mod h1:cyGadeNEkKy96OOhEzfZl+yxihPEzKnqJwvfuSUqbZE=
 github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE=
 github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
+github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
 github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=
 github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
@@ -321,7 +322,6 @@ github.com/go-playground/universal-translator v0.17.0 h1:icxd5fm+REJzpZx7ZfpaD87
 github.com/go-playground/universal-translator v0.17.0/go.mod h1:UkSxE5sNxxRwHyU+Scu5vgOQjsIJAF8j9muTVoKLVtA=
 github.com/go-playground/validator/v10 v10.2.0 h1:KgJ0snyC2R9VXYN2rneOtQcw5aHQB1Vv0sFl1UcHBOY=
 github.com/go-playground/validator/v10 v10.2.0/go.mod h1:uOYAAleCW8F/7oMFd6aG0GOhaH6EGOAJShg8Id5JGkI=
-github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w=
 github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
 github.com/go-test/deep v1.0.5/go.mod h1:QV8Hv/iy04NyLBxAdO9njL0iVPN1S4d/A3NVv1V36o8=
 github.com/gobwas/httphead v0.0.0-20180130184737-2c6c146eadee h1:s+21KNqlpePfkah2I+gwHF8xmJWRjooY+5248k6m4A0=
@@ -484,9 +484,7 @@ github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22
 github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4=
 github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
 github.com/json-iterator/go v1.1.7/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
-github.com/json-iterator/go v1.1.8/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
 github.com/json-iterator/go v1.1.9/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
-github.com/json-iterator/go v1.1.10 h1:Kz6Cvnvv2wGdaG/V8yMvfkmNiXq9Ya2KUv4rouJJr68=
 github.com/json-iterator/go v1.1.10/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
 github.com/json-iterator/go v1.1.11 h1:uVUAXhF2To8cbw/3xN3pxj6kk7TYKs98NIrTqPlMWAQ=
 github.com/json-iterator/go v1.1.11/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
@@ -503,8 +501,8 @@ github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQL
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/klauspost/compress v1.10.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
-github.com/klauspost/compress v1.11.3 h1:dB4Bn0tN3wdCzQxnS8r06kV74qN/TAfaIS0bVE8h3jc=
 github.com/klauspost/compress v1.11.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
+github.com/klauspost/compress v1.11.13 h1:eSvu8Tmq6j2psUJqJrLcWH6K3w5Dwc+qipbaA6eVEN4=
 github.com/klauspost/compress v1.11.13/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/cpuid v1.2.3/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek=
 github.com/klauspost/cpuid v1.3.1 h1:5JNjFYYQrZeKRJ0734q51WCEEn2huer72Dc7K+R/b6s=
@@ -524,9 +522,6 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/leodido/go-urn v1.2.0 h1:hpXL4XnriNwQ/ABnpepYM/1vCLWNDfUNts8dX3xTG6Y=
 github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII=
-github.com/lightstep/lightstep-tracer-common/golang/gogo v0.0.0-20190605223551-bc2310a04743/go.mod h1:qklhhLq1aX+mtWk9cPHPzaBjWImj5ULL6C7HFJtXQMM=
-github.com/lightstep/lightstep-tracer-go v0.18.1/go.mod h1:jlF1pusYV4pidLvZ+XD0UBX0ZE6WURAspgAczcDHrL4=
-github.com/lyft/protoc-gen-validate v0.0.13/go.mod h1:XbGvPuh87YZc5TdIa2/I4pLk0QoUACkjt2znoq26NVQ=
 github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
 github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
 github.com/mailru/easygo v0.0.0-20190618140210-3c14a0dc985f h1:4+gHs0jJFJ06bfN8PshnM6cHcxGjRUVRLo5jndDiKRQ=
@@ -538,8 +533,6 @@ github.com/marstr/guid v1.1.0/go.mod h1:74gB1z2wpxxInTG6yaqA7KrtM0NZ+RbrcqDvYHef
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
 github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
 github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
-github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
-github.com/mattn/go-isatty v0.0.11/go.mod h1:PhnuNfih5lzO57/f3n+odYbM4JtupLOxQOAqxQCu2WE=
 github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY=
 github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU=
 github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
@@ -552,8 +545,7 @@ github.com/miekg/pkcs11 v1.0.3/go.mod h1:XsNlhZGX73bx86s2hdc/FuaLm2CPZJemRLMA+WT
 github.com/minio/md5-simd v1.1.0/go.mod h1:XpBqgZULrMYD3R+M28PcmP0CkI7PEMzB3U77ZrKZ0Gw=
 github.com/minio/md5-simd v1.1.1 h1:9ojcLbuZ4gXbB2sX53MKn8JUZ0sB/2wfwsEcRw+I08U=
 github.com/minio/md5-simd v1.1.1/go.mod h1:XpBqgZULrMYD3R+M28PcmP0CkI7PEMzB3U77ZrKZ0Gw=
-github.com/minio/minio-go/v7 v7.0.10 h1:1oUKe4EOPUEhw2qnPQaPsJ0lmVTYLFu03SiItauXs94=
-github.com/minio/minio-go/v7 v7.0.10/go.mod h1:td4gW1ldOsj1PbSNS+WYK43j+P1XVhX/8W8awaYlBFo=
+github.com/minio/minio-go/v7 v7.0.11 h1:7utSkCtMQPYYB1UB8FR3d0QSiOWE6F/JYXon29imYek=
 github.com/minio/minio-go/v7 v7.0.11/go.mod h1:WoyW+ySKAKjY98B9+7ZbI8z8S3jaxaisdcvj9TGlazA=
 github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKUJU=
 github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=

components/supervisor/pkg/supervisor/tasks.go

@@ -197,7 +197,7 @@ func (tm *tasksManager) init(ctx context.Context) {
 			successChan: make(chan bool, 1),
 			title:       title,
 		}
-		task.command = tm.getCommand(task)
+		task.command = getCommand(task, tm.config.isHeadless(), tm.contentSource, tm.storeLocation)
 		if tm.config.isHeadless() && task.command == "exit" {
 			task.State = api.TaskState_closed
 			task.successChan <- true
@@ -291,15 +291,15 @@ func (tm *tasksManager) Run(ctx context.Context, wg *sync.WaitGroup, successChan
 	successChan <- success
 }
 
-func (tm *tasksManager) getCommand(task *task) string {
-	commands := tm.getCommands(task)
+func getCommand(task *task, isHeadless bool, contentSource csapi.WorkspaceInitSource, storeLocation string) string {
+	commands := getCommands(task, isHeadless, contentSource, storeLocation)
 	command := composeCommand(composeCommandOptions{
 		commands: commands,
 		format:   "{\n%s\n}",
 		sep:      " && ",
 	})
 
-	if tm.config.isHeadless() {
+	if isHeadless {
 		// it's important that prebuild tasks exit eventually
 		// also, we need to save the log output in the workspace
 		if strings.TrimSpace(command) == "" {
@@ -308,7 +308,7 @@ func (tm *tasksManager) getCommand(task *task) string {
 		return command + "; exit"
 	}
 
-	histfileCommand := tm.getHistfileCommand(task, commands)
+	histfileCommand := getHistfileCommand(task, commands, contentSource, storeLocation)
 	if strings.TrimSpace(command) == "" {
 		return histfileCommand
 	}
@@ -318,9 +318,9 @@ func (tm *tasksManager) getCommand(task *task) string {
 	return histfileCommand + "; " + command
 }
 
-func (tm *tasksManager) getHistfileCommand(task *task, commands []*string) string {
+func getHistfileCommand(task *task, commands []*string, contentSource csapi.WorkspaceInitSource, storeLocation string) string {
 	histfileCommands := commands
-	if tm.contentSource == csapi.WorkspaceInitFromPrebuild {
+	if contentSource == csapi.WorkspaceInitFromPrebuild {
 		histfileCommands = []*string{task.config.Before, task.config.Init, task.config.Prebuild, task.config.Command}
 	}
 	histfileContent := composeCommand(composeCommandOptions{
@@ -331,7 +331,7 @@ func (tm *tasksManager) getHistfileCommand(task *task, commands []*string) strin
 		return ""
 	}
 
-	histfile := tm.storeLocation + "/cmd-" + task.Id
+	histfile := storeLocation + "/cmd-" + task.Id
 	err := os.WriteFile(histfile, []byte(histfileContent), 0644)
 	if err != nil {
 		log.WithField("histfile", histfile).WithError(err).Error("cannot write histfile")
@@ -343,29 +343,28 @@ func (tm *tasksManager) getHistfileCommand(task *task, commands []*string) strin
 	return " HISTFILE=" + histfile + " history -r"
 }
 
-func (tm *tasksManager) getCommands(task *task) []*string {
-	if tm.config.isHeadless() {
+func getCommands(task *task, isHeadless bool, contentSource csapi.WorkspaceInitSource, storeLocation string) []*string {
+	if isHeadless {
 		// prebuild
 		return []*string{task.config.Before, task.config.Init, task.config.Prebuild}
 	}
-	if tm.contentSource == csapi.WorkspaceInitFromPrebuild {
+	if contentSource == csapi.WorkspaceInitFromPrebuild {
 		// prebuilt
-		prebuildLogFileName := tm.prebuildLogFileName(task)
+		prebuildLogFileName := prebuildLogFileName(task, storeLocation)
 		legacyPrebuildLogFileName := logs.LegacyPrebuildLogFileName(task.Id)
 		printlogs := "[ -r " + legacyPrebuildLogFileName + " ] && cat " + legacyPrebuildLogFileName + "; [ -r " + prebuildLogFileName + " ] && cat " + prebuildLogFileName + "; true"
 		return []*string{task.config.Before, &printlogs, task.config.Command}
 	}
-	if tm.contentSource == csapi.WorkspaceInitFromBackup {
+	if contentSource == csapi.WorkspaceInitFromBackup {
 		// restart
 		return []*string{task.config.Before, task.config.Command}
 	}
 	// init
 	return []*string{task.config.Before, task.config.Init, task.config.Command}
-
 }
 
-func (tm *tasksManager) prebuildLogFileName(task *task) string {
-	return logs.PrebuildLogFileName(tm.storeLocation, task.Id)
+func prebuildLogFileName(task *task, storeLocation string) string {
+	return logs.PrebuildLogFileName(storeLocation, task.Id)
 }
 
 func (tm *tasksManager) watch(task *task, terminal *terminal.Term) {
@@ -382,7 +381,7 @@ func (tm *tasksManager) watch(task *task, terminal *terminal.Term) {
 		defer stdout.Close()
 
 		var (
-			fileName    = tm.prebuildLogFileName(task)
+			fileName    = prebuildLogFileName(task, tm.storeLocation)
 			oldFileName = fileName + "-old"
 		)
 		if _, err := os.Stat(fileName); err == nil {

components/supervisor/pkg/supervisor/tasks_test.go

@@ -16,7 +16,8 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"github.com/gitpod-io/gitpod/common-go/log"
-	"github.com/gitpod-io/gitpod/content-service/api"
+	csapi "github.com/gitpod-io/gitpod/content-service/api"
+	"github.com/gitpod-io/gitpod/supervisor/api"
 	"github.com/gitpod-io/gitpod/supervisor/pkg/terminal"
 )
 
@@ -28,15 +29,15 @@ func TestTaskManager(t *testing.T) {
 	tests := []struct {
 		Desc        string
 		Headless    bool
-		Source      api.WorkspaceInitSource
+		Source      csapi.WorkspaceInitSource
 		GitpodTasks *[]TaskConfig
 
 		ExpectedReporter testHeadlessTaskProgressReporter
 	}{
 		{
 			Desc:     "headless prebuild should finish without tasks",
 			Headless: true,
-			Source:   api.WorkspaceInitFromOther,
+			Source:   csapi.WorkspaceInitFromOther,
 
 			ExpectedReporter: testHeadlessTaskProgressReporter{
 				Done:    true,
@@ -46,7 +47,7 @@ func TestTaskManager(t *testing.T) {
 		{
 			Desc:        "headless prebuild should finish without init tasks",
 			Headless:    true,
-			Source:      api.WorkspaceInitFromOther,
+			Source:      csapi.WorkspaceInitFromOther,
 			GitpodTasks: &[]TaskConfig{{Command: &skipCommand}},
 
 			ExpectedReporter: testHeadlessTaskProgressReporter{
@@ -57,7 +58,7 @@ func TestTaskManager(t *testing.T) {
 		{
 			Desc:        "headless prebuild should finish with successful init tasks",
 			Headless:    true,
-			Source:      api.WorkspaceInitFromOther,
+			Source:      csapi.WorkspaceInitFromOther,
 			GitpodTasks: &[]TaskConfig{{Init: &skipCommand}, {Init: &skipCommand}},
 
 			ExpectedReporter: testHeadlessTaskProgressReporter{
@@ -68,7 +69,7 @@ func TestTaskManager(t *testing.T) {
 		{
 			Desc:        "headless prebuild should finish with failed init tasks",
 			Headless:    true,
-			Source:      api.WorkspaceInitFromOther,
+			Source:      csapi.WorkspaceInitFromOther,
 			GitpodTasks: &[]TaskConfig{{Init: &failCommand}, {Init: &failCommand}},
 
 			ExpectedReporter: testHeadlessTaskProgressReporter{
@@ -79,7 +80,7 @@ func TestTaskManager(t *testing.T) {
 		{
 			Desc:        "headless prebuild should finish with at least one failed init tasks (first)",
 			Headless:    true,
-			Source:      api.WorkspaceInitFromOther,
+			Source:      csapi.WorkspaceInitFromOther,
 			GitpodTasks: &[]TaskConfig{{Init: &failCommand}, {Init: &skipCommand}},
 
 			ExpectedReporter: testHeadlessTaskProgressReporter{
@@ -90,7 +91,7 @@ func TestTaskManager(t *testing.T) {
 		{
 			Desc:        "headless prebuild should finish with at least one failed init tasks (second)",
 			Headless:    true,
-			Source:      api.WorkspaceInitFromOther,
+			Source:      csapi.WorkspaceInitFromOther,
 			GitpodTasks: &[]TaskConfig{{Init: &skipCommand}, {Init: &failCommand}},
 
 			ExpectedReporter: testHeadlessTaskProgressReporter{
@@ -154,3 +155,56 @@ func (r *testHeadlessTaskProgressReporter) done(success bool) {
 	r.Done = true
 	r.Success = success
 }
+
+func TestGetTask(t *testing.T) {
+	p := func(v string) *string { return &v }
+	allTasks := TaskConfig{
+		Name:     p("hello world"),
+		Before:   p("before"),
+		Init:     p("init"),
+		Prebuild: p("prebuild"),
+		Command:  p("command"),
+	}
+	tests := []struct {
+		Name          string
+		Task          TaskConfig
+		IsHeadless    bool
+		ContentSource csapi.WorkspaceInitSource
+		Expectation   string
+	}{
+		{
+			Name:          "prebuild",
+			Task:          allTasks,
+			IsHeadless:    true,
+			ContentSource: csapi.WorkspaceInitFromOther,
+			Expectation:   "{\nbefore\n} && {\ninit\n} && {\nprebuild\n}; exit",
+		},
+		{
+			Name:          "from prebuild",
+			Task:          allTasks,
+			ContentSource: csapi.WorkspaceInitFromPrebuild,
+			Expectation:   "{\nbefore\n} && {\n[ -r /workspace/.prebuild-log-0 ] && cat /workspace/.prebuild-log-0; [ -r //prebuild-log-0 ] && cat //prebuild-log-0; true\n} && {\ncommand\n}",
+		},
+		{
+			Name:          "from other",
+			Task:          allTasks,
+			ContentSource: csapi.WorkspaceInitFromOther,
+			Expectation:   "{\nbefore\n} && {\ninit\n} && {\ncommand\n}",
+		},
+		{
+			Name:          "from backup",
+			Task:          allTasks,
+			ContentSource: csapi.WorkspaceInitFromOther,
+			Expectation:   "{\nbefore\n} && {\ninit\n} && {\ncommand\n}",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {
+			command := getCommand(&task{config: test.Task, TaskStatus: api.TaskStatus{Id: "0"}}, test.IsHeadless, test.ContentSource, "/")
+			if diff := cmp.Diff(test.Expectation, command); diff != "" {
+				t.Errorf("unexpected getCommand() (-want +got):\n%s", diff)
+			}
+		})
+	}
+}