Skip to content

[supervisor] Add unit tests for task composition #4903

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.

Sign up for GitHub

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
Jul 23, 2021
Merged

[supervisor] Add unit tests for task composition #4903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion components/content-service/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion components/supervisor/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 3 additions & 11 deletions components/supervisor/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand All @@ -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=
Expand All @@ -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=
Expand All @@ -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=
Expand All @@ -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=
Expand Down
33 changes: 16 additions & 17 deletions components/supervisor/pkg/supervisor/tasks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) == "" {
Expand All @@ -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
}
Expand All @@ -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{
Expand All @@ -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")
Expand All @@ -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) {
Expand All @@ -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 {
Expand Down
70 changes: 62 additions & 8 deletions components/supervisor/pkg/supervisor/tasks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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,
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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)
}
})
}
}