From 3499a3fe3c267397e99f18230fbefc93dd969aee Mon Sep 17 00:00:00 2001 From: ash Date: Thu, 8 Feb 2024 14:49:26 +0000 Subject: [PATCH 01/10] Refactor to make things more testable specifically: build.New() now takes more arguments. --- build/builder.go | 115 ++++++++++++++---------- build/builder_test.go | 203 +++++++----------------------------------- build/install.go | 9 +- build/install_test.go | 3 +- cmd/server.go | 2 +- internal/core/core.go | 102 +++++++++++++++++++++ internal/error.go | 28 ++++++ internal/tests.go | 79 ++++++++++++++++ remove/remove.go | 15 ++-- remove/remove_test.go | 9 +- server/server.go | 62 +++++++++---- server/server_test.go | 100 ++++++++++++++++++++- 12 files changed, 468 insertions(+), 259 deletions(-) create mode 100644 internal/core/core.go create mode 100644 internal/error.go diff --git a/build/builder.go b/build/builder.go index 7fc7992..051fc81 100644 --- a/build/builder.go +++ b/build/builder.go @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2023 Genome Research Ltd. + * Copyright (c) 2023, 2024 Genome Research Ltd. * * Permission is hereby granted, free of charge, to any person obtaining * a copy of this software and associated documentation files (the @@ -39,22 +39,17 @@ import ( "strings" "sync" "text/template" + "time" "github.com/wtsi-hgi/go-softpack-builder/config" "github.com/wtsi-hgi/go-softpack-builder/git" + "github.com/wtsi-hgi/go-softpack-builder/internal" + "github.com/wtsi-hgi/go-softpack-builder/internal/core" "github.com/wtsi-hgi/go-softpack-builder/s3" "github.com/wtsi-hgi/go-softpack-builder/wr" ) const ( - SingularityDefBasename = "singularity.def" - ExesBasename = "executables" - SoftpackYaml = "softpack.yml" - SpackLockFile = "spack.lock" - BuilderOut = "builder.out" - moduleForCoreBasename = "module" - UsageBasename = "README.md" - uploadEndpoint = "/upload" ) @@ -71,18 +66,14 @@ func init() { //nolint:gochecknoinits softpackTmpl = template.Must(template.New("").Parse(softpackTmplStr)) } -type Error string - -func (e Error) Error() string { return string(e) } - const ( - ErrInvalidJSON = Error("invalid spack lock JSON") - ErrEnvironmentBuilding = Error("build already running for environment") + ErrInvalidJSON = internal.Error("invalid spack lock JSON") + ErrEnvironmentBuilding = internal.Error("build already running for environment") - ErrInvalidEnvPath = Error("invalid environment path") - ErrInvalidVersion = Error("environment version required") - ErrNoPackages = Error("packages required") - ErrNoPackageName = Error("package names required") + ErrInvalidEnvPath = internal.Error("invalid environment path") + ErrInvalidVersion = internal.Error("environment version required") + ErrNoPackages = internal.Error("packages required") + ErrNoPackageName = internal.Error("package names required") ) // Package describes the name and optional version of a spack package. @@ -181,16 +172,20 @@ func (d *Definition) Validate() error { return d.Packages.Validate() } +type S3 interface { + UploadData(data io.Reader, dest string) error + OpenFile(source string) (io.ReadCloser, error) +} + +type Runner interface { + Run(deployment string) error +} + // Builder lets you do builds given config, S3 and a wr runner. type Builder struct { config *config.Config - s3 interface { - UploadData(data io.Reader, dest string) error - OpenFile(source string) (io.ReadCloser, error) - } - runner interface { - Run(deployment string) error - } + s3 S3 + runner Runner mu sync.Mutex runningEnvironments map[string]bool @@ -198,18 +193,37 @@ type Builder struct { postBuildMu sync.RWMutex } +// The status of an individual build – when it was requested, when it started +// actually being built, and when its build finished. +type Status struct { + Name string + Requested time.Time + BuildStart time.Time + BuildDone time.Time +} + // New takes the s3 build cache URL, the repo and checkout reference of your -// custom spack repo, and returns a Builder. -func New(config *config.Config) (*Builder, error) { - s3helper, err := s3.New(config.S3.BuildBase) - if err != nil { - return nil, err +// custom spack repo, and returns a Builder. Optionally, supply objects that +// satisfy the S3 and Runner interfaces; if nil, these default to using the s3 +// and wr packages. +func New(config *config.Config, s3helper S3, runner Runner) (*Builder, error) { + if s3helper == nil { + var err error + + s3helper, err = s3.New(config.S3.BuildBase) + if err != nil { + return nil, err + } + } + + if runner == nil { + runner = wr.New(config.WRDeployment) } return &Builder{ config: config, s3: s3helper, - runner: wr.New(config.WRDeployment), + runner: runner, runningEnvironments: make(map[string]bool), }, nil } @@ -233,6 +247,11 @@ func (b *Builder) SetPostBuildCallback(cb func()) { b.postBuild = cb } +// Status returns the status of all known builds. +func (b *Builder) Status() []Status { + return nil +} + // Build uploads a singularity.def generated by GenerateSingularityDef() to S3 // and adds a job to wr to build the image. You'll need a wr manager running // that can run jobs with root and access the S3, ie. a cloud deployment. @@ -300,7 +319,7 @@ func (b *Builder) generateAndUploadSingularityDef(def *Definition, s3Path string return "", err } - singDefUploadPath := filepath.Join(s3Path, SingularityDefBasename) + singDefUploadPath := filepath.Join(s3Path, core.SingularityDefBasename) err = b.s3.UploadData(strings.NewReader(singDef), singDefUploadPath) @@ -371,7 +390,7 @@ func (b *Builder) asyncBuild(def *Definition, wrInput, s3Path, singDef string) e } func (b *Builder) addLogToRepo(s3Path, environmentPath string) { - log, err := b.s3.OpenFile(filepath.Join(s3Path, BuilderOut)) + log, err := b.s3.OpenFile(filepath.Join(s3Path, core.BuilderOut)) if err != nil { slog.Error("error getting build log file", "err", err) @@ -379,14 +398,14 @@ func (b *Builder) addLogToRepo(s3Path, environmentPath string) { } if err := b.addArtifactsToRepo(map[string]io.Reader{ - BuilderOut: log, + core.BuilderOut: log, }, environmentPath); err != nil { slog.Error("error sending build log file to core", "err", err) } } func (b *Builder) getExes(s3Path string) ([]string, error) { - exeData, err := b.s3.OpenFile(filepath.Join(s3Path, ExesBasename)) + exeData, err := b.s3.OpenFile(filepath.Join(s3Path, core.ExesBasename)) if err != nil { return nil, err } @@ -401,7 +420,7 @@ func (b *Builder) getExes(s3Path string) ([]string, error) { func (b *Builder) prepareAndInstallArtifacts(def *Definition, s3Path, moduleFileData string, exes []string) error { - imageData, err := b.s3.OpenFile(filepath.Join(s3Path, ImageBasename)) + imageData, err := b.s3.OpenFile(filepath.Join(s3Path, core.ImageBasename)) if err != nil { return err } @@ -431,24 +450,24 @@ func (b *Builder) prepareArtifactsFromS3AndSendToCoreAndS3(def *Definition, s3Pa return b.addArtifactsToRepo( map[string]io.Reader{ - SpackLockFile: bytes.NewReader(lockData), - SoftpackYaml: strings.NewReader(concreteSpackYAMLFile), - SingularityDefBasename: strings.NewReader(singDef), - BuilderOut: logData, - moduleForCoreBasename: strings.NewReader(moduleFileData), - UsageBasename: strings.NewReader(readme), + core.SpackLockFile: bytes.NewReader(lockData), + core.SoftpackYaml: strings.NewReader(concreteSpackYAMLFile), + core.SingularityDefBasename: strings.NewReader(singDef), + core.BuilderOut: logData, + core.ModuleForCoreBasename: strings.NewReader(moduleFileData), + core.UsageBasename: strings.NewReader(readme), }, def.FullEnvironmentPath(), ) } func (b *Builder) getArtifactDataFromS3(s3Path string) (io.Reader, []byte, error) { - logData, err := b.s3.OpenFile(filepath.Join(s3Path, BuilderOut)) + logData, err := b.s3.OpenFile(filepath.Join(s3Path, core.BuilderOut)) if err != nil { return nil, nil, err } - lockFile, err := b.s3.OpenFile(filepath.Join(s3Path, SpackLockFile)) + lockFile, err := b.s3.OpenFile(filepath.Join(s3Path, core.SpackLockFile)) if err != nil { return nil, nil, err } @@ -469,7 +488,7 @@ func (b *Builder) generateAndUploadSpackYAML(lockData []byte, description string } if err = b.s3.UploadData(strings.NewReader(concreteSpackYAMLFile), - filepath.Join(s3Path, SoftpackYaml)); err != nil { + filepath.Join(s3Path, core.SoftpackYaml)); err != nil { return "", err } @@ -543,7 +562,7 @@ func SpackLockToSoftPackYML(spackLockData []byte, desc string, exes []string) (s func (b *Builder) generateAndUploadUsageFile(def *Definition, s3Path string) (string, error) { readme := def.ModuleUsage(b.config.Module.LoadPath) - if err := b.s3.UploadData(strings.NewReader(readme), filepath.Join(s3Path, UsageBasename)); err != nil { + if err := b.s3.UploadData(strings.NewReader(readme), filepath.Join(s3Path, core.UsageBasename)); err != nil { return "", err } @@ -585,7 +604,7 @@ func (b *Builder) addArtifactsToRepo(artifacts map[string]io.Reader, envPath str io.Copy(&sb, resp.Body) //nolint:errcheck - return Error(sb.String()) + return internal.Error(sb.String()) } return <-errCh diff --git a/build/builder_test.go b/build/builder_test.go index 9fc430c..a5e1069 100644 --- a/build/builder_test.go +++ b/build/builder_test.go @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2023 Genome Research Ltd. + * Copyright (c) 2023, 2024 Genome Research Ltd. * * Permission is hereby granted, free of charge, to any person obtaining * a copy of this software and associated documentation files (the @@ -25,18 +25,13 @@ package build import ( "crypto/sha256" - "errors" "fmt" "io" "log/slog" - "net/http" "net/http/httptest" - "net/url" "os" "os/exec" "path/filepath" - "strings" - "sync" "sync/atomic" "testing" "time" @@ -44,85 +39,13 @@ import ( . "github.com/smartystreets/goconvey/convey" "github.com/wtsi-hgi/go-softpack-builder/config" "github.com/wtsi-hgi/go-softpack-builder/internal" + "github.com/wtsi-hgi/go-softpack-builder/internal/core" "github.com/wtsi-hgi/go-softpack-builder/internal/gitmock" "github.com/wtsi-hgi/go-softpack-builder/wr" ) -const ErrMock = Error("Mock error") const moduleLoadPrefix = "HGI/softpack" -type mockS3 struct { - data string - def string - softpackYML string - readme string - fail bool - exes string -} - -func (m *mockS3) UploadData(data io.Reader, dest string) error { - if m.fail { - return ErrMock - } - - buff, err := io.ReadAll(data) - if err != nil { - return err - } - - switch filepath.Ext(dest) { - case ".def": - m.data = string(buff) - m.def = dest - case ".yml": - m.softpackYML = string(buff) - case ".md": - m.readme = string(buff) - } - - return nil -} - -func (m *mockS3) OpenFile(source string) (io.ReadCloser, error) { - if filepath.Base(source) == ExesBasename { - return io.NopCloser(strings.NewReader(m.exes)), nil - } - - if filepath.Base(source) == BuilderOut { - return io.NopCloser(strings.NewReader("output")), nil - } - - if filepath.Base(source) == SpackLockFile { - return io.NopCloser(strings.NewReader(`{"_meta":{"file-type":"spack-lockfile","lockfile-version":5,"specfile-version":4},"spack":{"version":"0.21.0.dev0","type":"git","commit":"dac3b453879439fd733b03d0106cc6fe070f71f6"},"roots":[{"hash":"oibd5a4hphfkgshqiav4fdkvw4hsq4ek","spec":"xxhash arch=None-None-x86_64_v3"}, {"hash":"1ibd5a4hphfkgshqiav4fdkvw4hsq4e1","spec":"py-anndata arch=None-None-x86_64_v3"}, {"hash":"2ibd5a4hphfkgshqiav4fdkvw4hsq4e2","spec":"r-seurat arch=None-None-x86_64_v3"}],"concrete_specs":{"oibd5a4hphfkgshqiav4fdkvw4hsq4ek":{"name":"xxhash","version":"0.8.1","arch":{"platform":"linux","platform_os":"ubuntu22.04","target":"x86_64_v3"},"compiler":{"name":"gcc","version":"11.4.0"},"namespace":"builtin","parameters":{"build_system":"makefile","cflags":[],"cppflags":[],"cxxflags":[],"fflags":[],"ldflags":[],"ldlibs":[]},"package_hash":"wuj5b2kjnmrzhtjszqovcvgc3q46m6hoehmiccimi5fs7nmsw22a====","hash":"oibd5a4hphfkgshqiav4fdkvw4hsq4ek"},"2ibd5a4hphfkgshqiav4fdkvw4hsq4e2":{"name":"r-seurat","version":"4","arch":{"platform":"linux","platform_os":"ubuntu22.04","target":"x86_64_v3"},"compiler":{"name":"gcc","version":"11.4.0"},"namespace":"builtin","parameters":{"build_system":"makefile","cflags":[],"cppflags":[],"cxxflags":[],"fflags":[],"ldflags":[],"ldlibs":[]},"package_hash":"2uj5b2kjnmrzhtjszqovcvgc3q46m6hoehmiccimi5fs7nmsw222====","hash":"2ibd5a4hphfkgshqiav4fdkvw4hsq4e2"}, "1ibd5a4hphfkgshqiav4fdkvw4hsq4e1":{"name":"py-anndata","version":"3.14","arch":{"platform":"linux","platform_os":"ubuntu22.04","target":"x86_64_v3"},"compiler":{"name":"gcc","version":"11.4.0"},"namespace":"builtin","parameters":{"build_system":"makefile","cflags":[],"cppflags":[],"cxxflags":[],"fflags":[],"ldflags":[],"ldlibs":[]},"package_hash":"2uj5b2kjnmrzhtjszqovcvgc3q46m6hoehmiccimi5fs7nmsw222====","hash":"1ibd5a4hphfkgshqiav4fdkvw4hsq4e1"}}}`)), nil //nolint:lll - } - - if filepath.Base(source) == ImageBasename { - return io.NopCloser(strings.NewReader("image")), nil - } - - return nil, io.ErrUnexpectedEOF -} - -type mockWR struct { - ch chan struct{} - cmd string - fail bool -} - -func (m *mockWR) Run(cmd string) error { - defer close(m.ch) - - if m.fail { - return ErrMock - } - - m.cmd = cmd - - <-time.After(10 * time.Millisecond) - - return nil -} - type modifyRunner struct { cmd string *wr.Runner @@ -136,69 +59,11 @@ func (m *modifyRunner) Run(_ string) error { return err } -type mockCore struct { - mu sync.RWMutex - err error - files map[string]string -} - -func (m *mockCore) setFile(filename, contents string) { - m.mu.Lock() - defer m.mu.Unlock() - - m.files[filename] = contents -} - -func (m *mockCore) getFile(filename string) (string, bool) { - m.mu.RLock() - defer m.mu.RUnlock() - - contents, ok := m.files[filename] - - return contents, ok -} - -func (m *mockCore) ServeHTTP(w http.ResponseWriter, r *http.Request) { - if m.err != nil { - http.Error(w, m.err.Error(), http.StatusInternalServerError) - - return - } - - mr, err := r.MultipartReader() - if err != nil { - return - } - - envPath, err := url.QueryUnescape(r.URL.RawQuery) - if err != nil { - return - } - - for { - p, err := mr.NextPart() - if errors.Is(err, io.EOF) { - break - } else if err != nil { - return - } - - name := p.FileName() - - buf, err := io.ReadAll(p) - if err != nil { - return - } - - m.setFile(filepath.Join(envPath, name), string(buf)) - } -} - func TestBuilder(t *testing.T) { Convey("Given binary cache and spack repo details and a Definition", t, func() { - ms3 := &mockS3{} - mwr := &mockWR{ch: make(chan struct{})} - mc := &mockCore{files: make(map[string]string)} + ms3 := &internal.MockS3{} + mwr := &internal.MockWR{Ch: make(chan struct{})} + mc := &core.MockCore{Files: make(map[string]string)} msc := httptest.NewServer(mc) gm, commitHash := gitmock.New() @@ -214,12 +79,8 @@ func TestBuilder(t *testing.T) { conf.Spack.FinalImage = "ubuntu:22.04" conf.Spack.ProcessorTarget = "x86_64_v4" - builder := &Builder{ - config: &conf, - s3: ms3, - runner: mwr, - runningEnvironments: make(map[string]bool), - } + builder, err := New(&conf, ms3, mwr) + So(err, ShouldBeNil) var bcbCount atomic.Uint64 bcb := func() { @@ -342,25 +203,25 @@ Stage: final conf.Module.WrapperScript = "/path/to/wrapper" conf.Module.LoadPath = moduleLoadPrefix conf.Spack.ProcessorTarget = "x86_64_v4" - ms3.exes = "xxhsum\nxxh32sum\nxxh64sum\nxxh128sum\nR\nRscript\npython\n" + ms3.Exes = "xxhsum\nxxh32sum\nxxh64sum\nxxh128sum\nR\nRscript\npython\n" err := builder.Build(def) So(err, ShouldBeNil) So(bcbCount.Load(), ShouldEqual, 0) - So(ms3.def, ShouldEqual, "groups/hgi/xxhash/0.8.1/singularity.def") - So(ms3.data, ShouldContainSubstring, "specs:\n - xxhash@0.8.1 arch=None-None-x86_64_v4\n"+ + So(ms3.Def, ShouldEqual, "groups/hgi/xxhash/0.8.1/singularity.def") + So(ms3.Data, ShouldContainSubstring, "specs:\n - xxhash@0.8.1 arch=None-None-x86_64_v4\n"+ " - r-seurat@4 arch=None-None-x86_64_v4\n - py-anndata@3.14 arch=None-None-x86_64_v4\n view") - <-mwr.ch - hash := fmt.Sprintf("%X", sha256.Sum256([]byte(ms3.data))) - So(mwr.cmd, ShouldContainSubstring, "echo doing build with hash "+hash+"; sudo singularity build") + <-mwr.Ch + hash := fmt.Sprintf("%X", sha256.Sum256([]byte(ms3.Data))) + So(mwr.Cmd, ShouldContainSubstring, "echo doing build with hash "+hash+"; sudo singularity build") modulePath := filepath.Join(conf.Module.ModuleInstallDir, def.EnvironmentPath, def.EnvironmentName, def.EnvironmentVersion) scriptsPath := filepath.Join(conf.Module.ScriptsInstallDir, def.EnvironmentPath, def.EnvironmentName, def.EnvironmentVersion+ScriptsDirSuffix) - imagePath := filepath.Join(scriptsPath, ImageBasename) + imagePath := filepath.Join(scriptsPath, core.ImageBasename) expectedExes := []string{"python", "R", "Rscript", "xxhsum", "xxh32sum", "xxh64sum", "xxh128sum"} expectedFiles := []string{modulePath, scriptsPath, imagePath} @@ -441,29 +302,29 @@ packages: expectedReadmeContent := "module load " + moduleLoadPrefix + "/groups/hgi/xxhash/0.8.1" for file, expectedData := range map[string]string{ - SoftpackYaml: expectedSoftpackYaml, - moduleForCoreBasename: "module-whatis", - SingularityDefBasename: "specs:\n - xxhash@0.8.1 arch=None-None-x86_64_v4", - SpackLockFile: `"concrete_specs":`, - BuilderOut: "output", - UsageBasename: expectedReadmeContent, + core.SoftpackYaml: expectedSoftpackYaml, + core.ModuleForCoreBasename: "module-whatis", + core.SingularityDefBasename: "specs:\n - xxhash@0.8.1 arch=None-None-x86_64_v4", + core.SpackLockFile: `"concrete_specs":`, + core.BuilderOut: "output", + core.UsageBasename: expectedReadmeContent, } { - data, okg := mc.getFile("groups/hgi/xxhash-0.8.1/" + file) + data, okg := mc.GetFile("groups/hgi/xxhash-0.8.1/" + file) So(okg, ShouldBeTrue) So(data, ShouldContainSubstring, expectedData) } - _, ok = mc.getFile("groups/hgi/xxhash-0.8.1/" + ImageBasename) + _, ok = mc.GetFile("groups/hgi/xxhash-0.8.1/" + core.ImageBasename) So(ok, ShouldBeFalse) - So(ms3.softpackYML, ShouldEqual, expectedSoftpackYaml) - So(ms3.readme, ShouldContainSubstring, expectedReadmeContent) + So(ms3.SoftpackYML, ShouldEqual, expectedSoftpackYaml) + So(ms3.Readme, ShouldContainSubstring, expectedReadmeContent) So(bcbCount.Load(), ShouldEqual, 1) }) Convey("Build returns an error if the upload fails", func() { - ms3.fail = true + ms3.Fail = true err := builder.Build(def) So(err, ShouldNotBeNil) @@ -471,11 +332,11 @@ packages: }) Convey("Build logs an error if the run fails", func() { - mwr.fail = true + mwr.Fail = true err := builder.Build(def) So(err, ShouldBeNil) - <-mwr.ch + <-mwr.Ch ok := waitFor(func() bool { return logWriter.String() != "" @@ -485,7 +346,7 @@ packages: So(logWriter.String(), ShouldContainSubstring, "msg=\"Async part of build failed\" err=\"Mock error\" s3Path=some_path/groups/hgi/xxhash/0.8.1") - data, ok := mc.getFile("groups/hgi/xxhash-0.8.1/" + BuilderOut) + data, ok := mc.GetFile("groups/hgi/xxhash-0.8.1/" + core.BuilderOut) So(ok, ShouldBeTrue) So(data, ShouldContainSubstring, "output") @@ -504,7 +365,7 @@ packages: conf.Module.ScriptsInstallDir = t.TempDir() conf.Module.WrapperScript = "/path/to/wrapper" conf.Module.LoadPath = moduleLoadPrefix - ms3.exes = "xxhsum\nxxh32sum\nxxh64sum\nxxh128sum\n" + ms3.Exes = "xxhsum\nxxh32sum\nxxh64sum\nxxh128sum\n" ch := make(chan bool, 1) mr := &modifyRunner{ @@ -531,7 +392,7 @@ packages: conf.Module.ScriptsInstallDir = t.TempDir() conf.Module.WrapperScript = "/path/to/wrapper" conf.Module.LoadPath = moduleLoadPrefix - ms3.exes = "xxhsum\nxxh32sum\nxxh64sum\nxxh128sum\n" + ms3.Exes = "xxhsum\nxxh32sum\nxxh64sum\nxxh128sum\n" err := builder.Build(def) So(err, ShouldBeNil) @@ -546,10 +407,10 @@ packages: So(logWriter.String(), ShouldContainSubstring, expectedLog) conf.CoreURL = msc.URL - mc.err = Error("an error") + mc.Err = internal.Error("an error") logWriter.Reset() - mwr.ch = make(chan struct{}) + mwr.Ch = make(chan struct{}) conf.Module.ModuleInstallDir = t.TempDir() conf.Module.ScriptsInstallDir = t.TempDir() diff --git a/build/install.go b/build/install.go index 117809a..3060ac7 100644 --- a/build/install.go +++ b/build/install.go @@ -27,14 +27,15 @@ import ( "io" "os" "path/filepath" + + "github.com/wtsi-hgi/go-softpack-builder/internal/core" ) const ( ScriptsDirSuffix = "-scripts" - ImageBasename = "singularity.sif" - perms = 0755 - flags = os.O_EXCL | os.O_CREATE | os.O_WRONLY + perms = 0755 + flags = os.O_EXCL | os.O_CREATE | os.O_WRONLY ) func installModule(scriptInstallBase, moduleInstallBase string, def *Definition, module, @@ -59,7 +60,7 @@ func installModule(scriptInstallBase, moduleInstallBase string, def *Definition, return err } - if err = installFile(image, filepath.Join(scriptsDir, ImageBasename)); err != nil { + if err = installFile(image, filepath.Join(scriptsDir, core.ImageBasename)); err != nil { return err } diff --git a/build/install_test.go b/build/install_test.go index 757e1b4..d8ba5e0 100644 --- a/build/install_test.go +++ b/build/install_test.go @@ -31,6 +31,7 @@ import ( "testing" . "github.com/smartystreets/goconvey/convey" + "github.com/wtsi-hgi/go-softpack-builder/internal/core" ) func TestInstall(t *testing.T) { @@ -55,7 +56,7 @@ func TestInstall(t *testing.T) { def.EnvironmentName, def.EnvironmentVersion)) scriptsDir := filepath.Join(tmpScriptsDir, def.EnvironmentPath, def.EnvironmentName, def.EnvironmentVersion+ScriptsDirSuffix) - createdImageFile := readFile(t, filepath.Join(scriptsDir, ImageBasename)) + createdImageFile := readFile(t, filepath.Join(scriptsDir, core.ImageBasename)) So(createdModuleFile, ShouldEqual, moduleFile) So(createdImageFile, ShouldEqual, imageFile) diff --git a/cmd/server.go b/cmd/server.go index b17d3f1..a3221a5 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -135,7 +135,7 @@ past reindexHours, and only if a reindex is not still ongoing. conf := getConfig() - b, err := build.New(conf) + b, err := build.New(conf, nil, nil) if err != nil { die("could not create a builder: %s", err) } diff --git a/internal/core/core.go b/internal/core/core.go new file mode 100644 index 0000000..32a61e0 --- /dev/null +++ b/internal/core/core.go @@ -0,0 +1,102 @@ +/******************************************************************************* + * Copyright (c) 2024 Genome Research Ltd. + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + ******************************************************************************/ + +package core + +import ( + "errors" + "io" + "net/http" + "net/url" + "path/filepath" + "sync" +) + +const ( + SingularityDefBasename = "singularity.def" + ExesBasename = "executables" + SoftpackYaml = "softpack.yml" + SpackLockFile = "spack.lock" + BuilderOut = "builder.out" + ModuleForCoreBasename = "module" + UsageBasename = "README.md" + ImageBasename = "singularity.sif" +) + +type MockCore struct { + mu sync.RWMutex + Err error + Files map[string]string +} + +func (m *MockCore) setFile(filename, contents string) { + m.mu.Lock() + defer m.mu.Unlock() + + m.Files[filename] = contents +} + +func (m *MockCore) GetFile(filename string) (string, bool) { + m.mu.RLock() + defer m.mu.RUnlock() + + contents, ok := m.Files[filename] + + return contents, ok +} + +func (m *MockCore) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if m.Err != nil { + http.Error(w, m.Err.Error(), http.StatusInternalServerError) + + return + } + + mr, err := r.MultipartReader() + if err != nil { + return + } + + envPath, err := url.QueryUnescape(r.URL.RawQuery) + if err != nil { + return + } + + for { + p, err := mr.NextPart() + if errors.Is(err, io.EOF) { + break + } else if err != nil { + return + } + + name := p.FileName() + + buf, err := io.ReadAll(p) + if err != nil { + return + } + + m.setFile(filepath.Join(envPath, name), string(buf)) + } +} diff --git a/internal/error.go b/internal/error.go new file mode 100644 index 0000000..e9273b8 --- /dev/null +++ b/internal/error.go @@ -0,0 +1,28 @@ +/******************************************************************************* + * Copyright (c) 2024 Genome Research Ltd. + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + ******************************************************************************/ + +package internal + +type Error string + +func (e Error) Error() string { return string(e) } diff --git a/internal/tests.go b/internal/tests.go index 07973c1..e030eac 100644 --- a/internal/tests.go +++ b/internal/tests.go @@ -25,8 +25,13 @@ package internal import ( "embed" + "io" + "path/filepath" "strings" "sync" + "time" + + "github.com/wtsi-hgi/go-softpack-builder/internal/core" ) //go:embed testdata @@ -64,3 +69,77 @@ func (c *ConcurrentStringBuilder) String() string { return c.Builder.String() } + +type MockS3 struct { + Data string + Def string + SoftpackYML string + Readme string + Fail bool + Exes string +} + +const ErrMock = Error("Mock error") + +func (m *MockS3) UploadData(data io.Reader, dest string) error { + if m.Fail { + return ErrMock + } + + buff, err := io.ReadAll(data) + if err != nil { + return err + } + + switch filepath.Ext(dest) { + case ".def": + m.Data = string(buff) + m.Def = dest + case ".yml": + m.SoftpackYML = string(buff) + case ".md": + m.Readme = string(buff) + } + + return nil +} + +func (m *MockS3) OpenFile(source string) (io.ReadCloser, error) { + if filepath.Base(source) == core.ExesBasename { + return io.NopCloser(strings.NewReader(m.Exes)), nil + } + + if filepath.Base(source) == core.BuilderOut { + return io.NopCloser(strings.NewReader("output")), nil + } + + if filepath.Base(source) == core.SpackLockFile { + return io.NopCloser(strings.NewReader(`{"_meta":{"file-type":"spack-lockfile","lockfile-version":5,"specfile-version":4},"spack":{"version":"0.21.0.dev0","type":"git","commit":"dac3b453879439fd733b03d0106cc6fe070f71f6"},"roots":[{"hash":"oibd5a4hphfkgshqiav4fdkvw4hsq4ek","spec":"xxhash arch=None-None-x86_64_v3"}, {"hash":"1ibd5a4hphfkgshqiav4fdkvw4hsq4e1","spec":"py-anndata arch=None-None-x86_64_v3"}, {"hash":"2ibd5a4hphfkgshqiav4fdkvw4hsq4e2","spec":"r-seurat arch=None-None-x86_64_v3"}],"concrete_specs":{"oibd5a4hphfkgshqiav4fdkvw4hsq4ek":{"name":"xxhash","version":"0.8.1","arch":{"platform":"linux","platform_os":"ubuntu22.04","target":"x86_64_v3"},"compiler":{"name":"gcc","version":"11.4.0"},"namespace":"builtin","parameters":{"build_system":"makefile","cflags":[],"cppflags":[],"cxxflags":[],"fflags":[],"ldflags":[],"ldlibs":[]},"package_hash":"wuj5b2kjnmrzhtjszqovcvgc3q46m6hoehmiccimi5fs7nmsw22a====","hash":"oibd5a4hphfkgshqiav4fdkvw4hsq4ek"},"2ibd5a4hphfkgshqiav4fdkvw4hsq4e2":{"name":"r-seurat","version":"4","arch":{"platform":"linux","platform_os":"ubuntu22.04","target":"x86_64_v3"},"compiler":{"name":"gcc","version":"11.4.0"},"namespace":"builtin","parameters":{"build_system":"makefile","cflags":[],"cppflags":[],"cxxflags":[],"fflags":[],"ldflags":[],"ldlibs":[]},"package_hash":"2uj5b2kjnmrzhtjszqovcvgc3q46m6hoehmiccimi5fs7nmsw222====","hash":"2ibd5a4hphfkgshqiav4fdkvw4hsq4e2"}, "1ibd5a4hphfkgshqiav4fdkvw4hsq4e1":{"name":"py-anndata","version":"3.14","arch":{"platform":"linux","platform_os":"ubuntu22.04","target":"x86_64_v3"},"compiler":{"name":"gcc","version":"11.4.0"},"namespace":"builtin","parameters":{"build_system":"makefile","cflags":[],"cppflags":[],"cxxflags":[],"fflags":[],"ldflags":[],"ldlibs":[]},"package_hash":"2uj5b2kjnmrzhtjszqovcvgc3q46m6hoehmiccimi5fs7nmsw222====","hash":"1ibd5a4hphfkgshqiav4fdkvw4hsq4e1"}}}`)), nil //nolint:lll + } + + if filepath.Base(source) == core.ImageBasename { + return io.NopCloser(strings.NewReader("image")), nil + } + + return nil, io.ErrUnexpectedEOF +} + +type MockWR struct { + Ch chan struct{} + Cmd string + Fail bool +} + +func (m *MockWR) Run(cmd string) error { + defer close(m.Ch) + + if m.Fail { + return ErrMock + } + + m.Cmd = cmd + + <-time.After(10 * time.Millisecond) + + return nil +} diff --git a/remove/remove.go b/remove/remove.go index 76957da..1ada6a1 100644 --- a/remove/remove.go +++ b/remove/remove.go @@ -15,17 +15,18 @@ import ( "github.com/wtsi-hgi/go-softpack-builder/build" "github.com/wtsi-hgi/go-softpack-builder/config" + "github.com/wtsi-hgi/go-softpack-builder/internal/core" "golang.org/x/sys/unix" ) var s3BasenamesForDeletion = [...]string{ //nolint:gochecknoglobals - build.SingularityDefBasename, - build.ExesBasename, - build.SoftpackYaml, - build.SpackLockFile, - build.BuilderOut, - build.UsageBasename, - build.ImageBasename, + core.SingularityDefBasename, + core.ExesBasename, + core.SoftpackYaml, + core.SpackLockFile, + core.BuilderOut, + core.UsageBasename, + core.ImageBasename, } type Error string diff --git a/remove/remove_test.go b/remove/remove_test.go index f83e9cd..37fe2af 100644 --- a/remove/remove_test.go +++ b/remove/remove_test.go @@ -15,6 +15,7 @@ import ( . "github.com/smartystreets/goconvey/convey" "github.com/wtsi-hgi/go-softpack-builder/build" "github.com/wtsi-hgi/go-softpack-builder/config" + "github.com/wtsi-hgi/go-softpack-builder/internal/core" ) const groupsDir = "groups" @@ -39,11 +40,11 @@ func TestRemove(t *testing.T) { var response coreResponse - core := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + mockCore := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { json.NewEncoder(w).Encode(response) //nolint:errcheck })) - conf.CoreURL = core.URL + conf.CoreURL = mockCore.URL s3Mock := new(mockS3) @@ -70,7 +71,7 @@ func TestRemove(t *testing.T) { So(err, ShouldBeNil) _, err = os.Stat(filepath.Join(conf.Module.ScriptsInstallDir, groupsDir, - group, env, version+build.ScriptsDirSuffix, build.ImageBasename)) + group, env, version+build.ScriptsDirSuffix, core.ImageBasename)) So(err, ShouldBeNil) removing := filepath.Join(conf.Module.ModuleInstallDir, groupsDir, group, env) @@ -147,7 +148,7 @@ func createTestEnv(t *testing.T) (*config.Config, string, string, string) { So(err, ShouldBeNil) So(f.Close(), ShouldBeNil) - f, err = os.Create(filepath.Join(scriptsPath, build.ImageBasename)) + f, err = os.Create(filepath.Join(scriptsPath, core.ImageBasename)) So(err, ShouldBeNil) _, err = io.WriteString(f, "An Image File") diff --git a/server/server.go b/server/server.go index cd91115..7df8d3e 100644 --- a/server/server.go +++ b/server/server.go @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2023 Genome Research Ltd. + * Copyright (c) 2023, 2024 Genome Research Ltd. * * Permission is hereby granted, free of charge, to any person obtaining * a copy of this software and associated documentation files (the @@ -32,6 +32,12 @@ import ( "github.com/wtsi-hgi/go-softpack-builder/build" ) +const ( + endpointEnvs = "/environments" + endpointEnvsBuild = endpointEnvs + "/build" + endpointEnvsStatus = endpointEnvs + "/status" +) + type Error string func (e Error) Error() string { @@ -42,6 +48,7 @@ func (e Error) Error() string { // given a build.Definition. type Builder interface { Build(*build.Definition) error + Status() []build.Status } // A Request object contains all of the information required to build an @@ -56,29 +63,46 @@ type Request struct { } // New takes a Builder that will be sent a Definition when the returned Handler -// receives request JSON. +// receives request JSON POSTed to /environments/build, and uses the Builder to +// get status information for builds when it receives a GET request to +// /environments/status. func New(b Builder) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - req := new(Request) + switch r.URL.Path { + case endpointEnvsBuild: + handleEnvBuild(b, w, r) + case endpointEnvsStatus: + handleEnvStatus(b, w, r) + default: + http.Error(w, "Not found", http.StatusNotFound) + } + }) +} - if err := json.NewDecoder(r.Body).Decode(req); err != nil { - http.Error(w, fmt.Sprintf("error parsing request: %s", err), http.StatusBadRequest) +func handleEnvBuild(b Builder, w http.ResponseWriter, r *http.Request) { + req := new(Request) - return - } + if err := json.NewDecoder(r.Body).Decode(req); err != nil { + http.Error(w, fmt.Sprintf("error parsing request: %s", err), http.StatusBadRequest) - def := new(build.Definition) - def.EnvironmentPath, def.EnvironmentName = path.Split(req.Name) - def.EnvironmentVersion = req.Version - def.Description = req.Model.Description - def.Packages = req.Model.Packages + return + } - if err := def.Validate(); err != nil { - http.Error(w, fmt.Sprintf("error validating request: %s", err), http.StatusBadRequest) - } + def := new(build.Definition) + def.EnvironmentPath, def.EnvironmentName = path.Split(req.Name) + def.EnvironmentVersion = req.Version + def.Description = req.Model.Description + def.Packages = req.Model.Packages - if err := b.Build(def); err != nil { - http.Error(w, fmt.Sprintf("error starting build: %s", err), http.StatusInternalServerError) - } - }) + if err := def.Validate(); err != nil { + http.Error(w, fmt.Sprintf("error validating request: %s", err), http.StatusBadRequest) + } + + if err := b.Build(def); err != nil { + http.Error(w, fmt.Sprintf("error starting build: %s", err), http.StatusInternalServerError) + } +} + +func handleEnvStatus(b Builder, w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(b.Status()) } diff --git a/server/server_test.go b/server/server_test.go index ed8f5a6..8fa7edd 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -24,27 +24,48 @@ package server import ( + "encoding/json" "io" "net/http" "net/http/httptest" + "path/filepath" "strings" "testing" + "time" . "github.com/smartystreets/goconvey/convey" "github.com/wtsi-hgi/go-softpack-builder/build" + "github.com/wtsi-hgi/go-softpack-builder/config" + "github.com/wtsi-hgi/go-softpack-builder/internal" + "github.com/wtsi-hgi/go-softpack-builder/internal/core" + "github.com/wtsi-hgi/go-softpack-builder/internal/gitmock" ) type mockBuilder struct { - received *build.Definition + received []*build.Definition + requested []time.Time } func (m *mockBuilder) Build(def *build.Definition) error { - m.received = def + m.received = append(m.received, def) return nil } -func TestServer(t *testing.T) { +func (m *mockBuilder) Status() []build.Status { + statuses := make([]build.Status, len(m.received)) + + for i, def := range m.received { + statuses[i] = build.Status{ + Name: filepath.Join(def.EnvironmentPath, def.EnvironmentName) + "-" + def.EnvironmentVersion, + Requested: m.requested[i], + } + } + + return statuses +} + +func TestServerMock(t *testing.T) { Convey("Posts to core result in a Definition being sent to Build()", t, func() { mb := new(mockBuilder) @@ -64,7 +85,7 @@ func TestServer(t *testing.T) { `)) So(err, ShouldBeNil) So(resp.StatusCode, ShouldEqual, http.StatusOK) - So(mb.received, ShouldResemble, &build.Definition{ + So(mb.received[0], ShouldResemble, &build.Definition{ EnvironmentPath: "users/user/", EnvironmentName: "myenv", EnvironmentVersion: "0.8.1", @@ -139,5 +160,76 @@ func TestServer(t *testing.T) { So(string(body), ShouldEqual, test.OutputError) } }) + + Convey("After which you can get the queued/building/built status for it", func() { + mb.requested = append(mb.requested, time.Now()) + resp, err := server.Client().Get(server.URL + "/environments/status") //nolint:noctx + So(err, ShouldBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusOK) + var statuses []build.Status + err = json.NewDecoder(resp.Body).Decode(&statuses) + So(err, ShouldBeNil) + So(len(statuses), ShouldEqual, 1) + So(statuses[0].Name, ShouldEqual, "users/user/myenv-0.8.1") + So(statuses[0].Requested, ShouldEqual, mb.requested[0]) + // Requested: , + // BuildStart: , + // BuildDone: , + + resp, err = server.Client().Post(server.URL+"/environments/build", "application/json", //nolint:noctx + strings.NewReader(` +{ + "name": "users/user/myotherenv", + "version": "1", + "model": { + "description": "help text", + "packages": [{"name": "xxhash", "version": "0.8.1"}] + } +} +`)) + So(err, ShouldBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusOK) + mb.requested = append(mb.requested, time.Now()) + + resp, err = server.Client().Get(server.URL + "/environments/status") //nolint:noctx + So(err, ShouldBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusOK) + statuses = []build.Status{} + err = json.NewDecoder(resp.Body).Decode(&statuses) + So(err, ShouldBeNil) + So(len(statuses), ShouldEqual, 2) + So(statuses[0].Name, ShouldEqual, "users/user/myenv-0.8.1") + So(statuses[1].Name, ShouldEqual, "users/user/myotherenv-1") + So(statuses[1].Requested, ShouldEqual, mb.requested[1]) + }) + }) +} + +func TestServerReal(t *testing.T) { + Convey("WIth a real builder", t, func() { + ms3 := &internal.MockS3{} + mwr := &internal.MockWR{Ch: make(chan struct{})} + mc := &core.MockCore{Files: make(map[string]string)} + msc := httptest.NewServer(mc) + + gm, _ := gitmock.New() + + gmhttp := httptest.NewServer(gm) + + var conf config.Config + conf.S3.BinaryCache = "s3://spack" + conf.S3.BuildBase = "some_path" + conf.CustomSpackRepo = gmhttp.URL + conf.CoreURL = msc.URL + conf.Spack.BuildImage = "spack/ubuntu-jammy:v0.20.1" + conf.Spack.FinalImage = "ubuntu:22.04" + conf.Spack.ProcessorTarget = "x86_64_v4" + + builder, err := build.New(&conf, ms3, mwr) + So(err, ShouldBeNil) + + handler := New(builder) + server := httptest.NewServer(handler) + So(server, ShouldNotBeNil) }) } From 969ae939727b45a136cbc2514cf683554f7a800f Mon Sep 17 00:00:00 2001 From: ash Date: Fri, 9 Feb 2024 13:21:28 +0000 Subject: [PATCH 02/10] Refactor wr Run() into Add(), Wait(), and Status() --- build/builder.go | 64 ++++++++++++++++---- build/builder_test.go | 4 +- internal/tests.go | 5 +- server/server_test.go | 75 ++++++++++++----------- wr/wr.go | 134 ++++++++++++++++++++++++++++++++++++++---- wr/wr_test.go | 22 +++++-- 6 files changed, 235 insertions(+), 69 deletions(-) diff --git a/build/builder.go b/build/builder.go index 051fc81..83ff073 100644 --- a/build/builder.go +++ b/build/builder.go @@ -178,7 +178,16 @@ type S3 interface { } type Runner interface { - Run(deployment string) error + Add(deployment string) error +} + +// The status of an individual build – when it was requested, when it started +// actually being built, and when its build finished. +type Status struct { + Name string + Requested time.Time + BuildStart time.Time + BuildDone time.Time } // Builder lets you do builds given config, S3 and a wr runner. @@ -189,17 +198,12 @@ type Builder struct { mu sync.Mutex runningEnvironments map[string]bool - postBuild func() - postBuildMu sync.RWMutex -} -// The status of an individual build – when it was requested, when it started -// actually being built, and when its build finished. -type Status struct { - Name string - Requested time.Time - BuildStart time.Time - BuildDone time.Time + postBuildMu sync.RWMutex + postBuild func() + + statusMu sync.RWMutex + statuses map[string]*Status } // New takes the s3 build cache URL, the repo and checkout reference of your @@ -225,6 +229,7 @@ func New(config *config.Config, s3helper S3, runner Runner) (*Builder, error) { s3: s3helper, runner: runner, runningEnvironments: make(map[string]bool), + statuses: make(map[string]*Status), }, nil } @@ -249,13 +254,24 @@ func (b *Builder) SetPostBuildCallback(cb func()) { // Status returns the status of all known builds. func (b *Builder) Status() []Status { - return nil + b.statusMu.RLock() + defer b.statusMu.RUnlock() + + statuses := make([]Status, 0, len(b.statuses)) + + for _, status := range b.statuses { + statuses = append(statuses, *status) + } + + return statuses } // Build uploads a singularity.def generated by GenerateSingularityDef() to S3 // and adds a job to wr to build the image. You'll need a wr manager running // that can run jobs with root and access the S3, ie. a cloud deployment. func (b *Builder) Build(def *Definition) (err error) { + b.buildStatus(def) + var fn func() fn, err = b.protectEnvironment(def.FullEnvironmentPath(), &err) @@ -287,6 +303,25 @@ func (b *Builder) Build(def *Definition) (err error) { return nil } +func (b *Builder) buildStatus(def *Definition) *Status { + b.statusMu.Lock() + defer b.statusMu.Unlock() + + name := filepath.Join(def.EnvironmentPath, def.EnvironmentName) + "-" + def.EnvironmentVersion + + status, exists := b.statuses[name] + if !exists { + status = &Status{ + Name: name, + Requested: time.Now(), + } + + b.statuses[name] = status + } + + return status +} + func (b *Builder) protectEnvironment(envPath string, err *error) (func(), error) { b.mu.Lock() @@ -359,7 +394,10 @@ func (b *Builder) startBuild(def *Definition, wrInput, s3Path, singDef, singDefP } func (b *Builder) asyncBuild(def *Definition, wrInput, s3Path, singDef string) error { - err := b.runner.Run(wrInput) + status := b.buildStatus(def) + status.BuildStart = time.Now() + + err := b.runner.Add(wrInput) b.postBuildMu.RLock() if b.postBuild != nil { diff --git a/build/builder_test.go b/build/builder_test.go index a5e1069..b15776f 100644 --- a/build/builder_test.go +++ b/build/builder_test.go @@ -52,8 +52,8 @@ type modifyRunner struct { ch chan bool } -func (m *modifyRunner) Run(_ string) error { - err := m.Runner.Run(m.cmd) +func (m *modifyRunner) Add(_ string) error { + err := m.Runner.Add(m.cmd) m.ch <- true return err diff --git a/internal/tests.go b/internal/tests.go index e030eac..f880b37 100644 --- a/internal/tests.go +++ b/internal/tests.go @@ -29,7 +29,6 @@ import ( "path/filepath" "strings" "sync" - "time" "github.com/wtsi-hgi/go-softpack-builder/internal/core" ) @@ -130,7 +129,7 @@ type MockWR struct { Fail bool } -func (m *MockWR) Run(cmd string) error { +func (m *MockWR) Add(cmd string) error { defer close(m.Ch) if m.Fail { @@ -139,7 +138,7 @@ func (m *MockWR) Run(cmd string) error { m.Cmd = cmd - <-time.After(10 * time.Millisecond) + // <-time.After(10 * time.Millisecond) return nil } diff --git a/server/server_test.go b/server/server_test.go index 8fa7edd..a4c1b65 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -72,19 +72,8 @@ func TestServerMock(t *testing.T) { handler := New(mb) server := httptest.NewServer(handler) - resp, err := server.Client().Post(server.URL+"/environments/build", "application/json", //nolint:noctx - strings.NewReader(` -{ - "name": "users/user/myenv", - "version": "0.8.1", - "model": { - "description": "help text", - "packages": [{"name": "xxhash", "version": "0.8.1"}] - } -} -`)) - So(err, ShouldBeNil) - So(resp.StatusCode, ShouldEqual, http.StatusOK) + postToBuildEndpoint(server, "users/user/myenv", "0.8.1") + So(mb.received[0], ShouldResemble, &build.Definition{ EnvironmentPath: "users/user/", EnvironmentName: "myenv", @@ -150,7 +139,7 @@ func TestServerMock(t *testing.T) { OutputError: "error validating request: package names required\n", }, } { - resp, err = server.Client().Post(server.URL+"/environments/build", "application/json", //nolint:noctx + resp, err := server.Client().Post(server.URL+endpointEnvsBuild, "application/json", //nolint:noctx strings.NewReader(test.InputJSON)) So(err, ShouldBeNil) @@ -163,7 +152,7 @@ func TestServerMock(t *testing.T) { Convey("After which you can get the queued/building/built status for it", func() { mb.requested = append(mb.requested, time.Now()) - resp, err := server.Client().Get(server.URL + "/environments/status") //nolint:noctx + resp, err := server.Client().Get(server.URL + endpointEnvsStatus) //nolint:noctx So(err, ShouldBeNil) So(resp.StatusCode, ShouldEqual, http.StatusOK) var statuses []build.Status @@ -172,26 +161,12 @@ func TestServerMock(t *testing.T) { So(len(statuses), ShouldEqual, 1) So(statuses[0].Name, ShouldEqual, "users/user/myenv-0.8.1") So(statuses[0].Requested, ShouldEqual, mb.requested[0]) - // Requested: , - // BuildStart: , - // BuildDone: , - resp, err = server.Client().Post(server.URL+"/environments/build", "application/json", //nolint:noctx - strings.NewReader(` -{ - "name": "users/user/myotherenv", - "version": "1", - "model": { - "description": "help text", - "packages": [{"name": "xxhash", "version": "0.8.1"}] - } -} -`)) - So(err, ShouldBeNil) - So(resp.StatusCode, ShouldEqual, http.StatusOK) + postToBuildEndpoint(server, "users/user/myotherenv", "1") + mb.requested = append(mb.requested, time.Now()) - resp, err = server.Client().Get(server.URL + "/environments/status") //nolint:noctx + resp, err = server.Client().Get(server.URL + endpointEnvsStatus) //nolint:noctx So(err, ShouldBeNil) So(resp.StatusCode, ShouldEqual, http.StatusOK) statuses = []build.Status{} @@ -206,7 +181,7 @@ func TestServerMock(t *testing.T) { } func TestServerReal(t *testing.T) { - Convey("WIth a real builder", t, func() { + Convey("With a real builder", t, func() { ms3 := &internal.MockS3{} mwr := &internal.MockWR{Ch: make(chan struct{})} mc := &core.MockCore{Files: make(map[string]string)} @@ -231,5 +206,39 @@ func TestServerReal(t *testing.T) { handler := New(builder) server := httptest.NewServer(handler) So(server, ShouldNotBeNil) + + buildSubmitted := time.Now() + postToBuildEndpoint(server, "users/user/myenv", "0.8.1") + + Convey("you get a real status", func() { + resp, err := server.Client().Get(server.URL + endpointEnvsStatus) //nolint:noctx + So(err, ShouldBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusOK) + var statuses []build.Status + err = json.NewDecoder(resp.Body).Decode(&statuses) + So(err, ShouldBeNil) + So(len(statuses), ShouldEqual, 1) + So(statuses[0].Name, ShouldEqual, "users/user/myenv-0.8.1") + So(statuses[0].Requested, ShouldHappenAfter, buildSubmitted) + So(statuses[0].BuildStart, ShouldHappenAfter, statuses[0].Requested) + // BuildDone: , + }) }) } + +func postToBuildEndpoint(server *httptest.Server, name, version string) { + resp, err := server.Client().Post(server.URL+endpointEnvsBuild, "application/json", //nolint:noctx + strings.NewReader(` +{ + "name": "`+name+`", + "version": "`+version+`", + "model": { + "description": "help text", + "packages": [{"name": "xxhash", "version": "0.8.1"}] + } +} +`)) + + So(err, ShouldBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusOK) +} diff --git a/wr/wr.go b/wr/wr.go index 49dfe07..bb32350 100644 --- a/wr/wr.go +++ b/wr/wr.go @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2023 Genome Research Ltd. + * Copyright (c) 2023, 2024 Genome Research Ltd. * * Permission is hereby granted, free of charge, to any person obtaining * a copy of this software and associated documentation files (the @@ -24,18 +24,38 @@ package wr import ( + "bufio" "bytes" _ "embed" "os/exec" "strings" "text/template" + "time" +) + +type WRJobStatus int + +const ( + WRJobStatusInvalid WRJobStatus = iota + WRJobStatusDelayed + WRJobStatusReady + WRJobStatusReserved + WRJobStatusRunning + WRJobStatusLost + WRJobStatusBuried + WRJobStatusComplete +) + +const ( + plainStatusCols = 2 + defaultPollDuration = 5 * time.Second ) type Error struct { msg string } -func (e Error) Error() string { return "wr add failed: " + e.msg } +func (e Error) Error() string { return "wr cmd failed: " + e.msg } //go:embed wr.tmpl var wrTmplStr string @@ -65,28 +85,40 @@ func SingularityBuildInS3WRInput(s3Path, hash string) (string, error) { // Runner lets you Run() a wr add command. type Runner struct { - deployment string - memory string + deployment string + memory string + pollDuration time.Duration } // New returns a Runner that will use the given wr deployment to wr add jobs // during Run(). func New(deployment string) *Runner { - return &Runner{deployment: deployment, memory: "43G"} + return &Runner{ + deployment: deployment, + memory: "43G", + pollDuration: defaultPollDuration, + } } // Run pipes the given wrInput (eg. as produced by -// SingularityBuildInS3WRInput()) to `wr add` and waits for the job to exit. The -// memory defaults to 8GB, time to 8hrs, and if the cmd in the input has +// SingularityBuildInS3WRInput()) to `wr add`, which adds a job to wr's queue +// and returns its ID. You should call Wait(ID) to actually wait for the job to +// finishing running. +// +// The memory defaults to 8GB, time to 8hrs, and if the cmd in the input has // previously been run, the cmd will be re-run. // // NB: if the cmd is a duplicate of a currently queued job, this will not -// generate an error, but just wait until that job completes. -func (r *Runner) Run(wrInput string) error { - cmd := exec.Command("wr", "add", "--deployment", r.deployment, "--sync", //nolint:gosec +// generate an error, but just return the id of the existing job. +func (r *Runner) Add(wrInput string) (string, error) { + cmd := exec.Command("wr", "add", "--deployment", r.deployment, "--simple", //nolint:gosec "--time", "8h", "--memory", r.memory, "-o", "2", "--rerun") cmd.Stdin = strings.NewReader(wrInput) + return r.runWRCmd(cmd) +} + +func (r *Runner) runWRCmd(cmd *exec.Cmd) (string, error) { var std bytes.Buffer cmd.Stdout = &std @@ -95,12 +127,90 @@ func (r *Runner) Run(wrInput string) error { err := cmd.Run() if err != nil { errStr := std.String() + if !strings.Contains(errStr, "EROR") { + return errStr, nil + } + if errStr == "" { errStr = err.Error() } - return Error{msg: errStr} + return "", Error{msg: errStr} + } + + return strings.TrimSpace(std.String()), nil +} + +// Wait waits for the given wr job to exit. +func (r *Runner) Wait(id string) (WRJobStatus, error) { + ticker := time.NewTicker(r.pollDuration) + defer ticker.Stop() + + for { + select { + case <-ticker.C: + status, err := r.Status(id) + if err != nil { + return status, err + } + + if status == WRJobStatusInvalid || status == WRJobStatusBuried || status == WRJobStatusComplete { + return status, err + } + } + } +} + +// Status returns the status of the wr job with the given internal ID. +// +// Returns WRJobStatusInvalid if the ID wasn't found. Returns WRJobStatusBuried +// if it failed. Only returns an error if there was a problem getting the +// status. +func (r *Runner) Status(id string) (WRJobStatus, error) { + cmd := exec.Command("wr", "status", "--deployment", r.deployment, "-o", //nolint:gosec + "plain", "-i", id, "-y") + + out, err := r.runWRCmd(cmd) + if err != nil { + return WRJobStatusInvalid, err } - return nil + status := WRJobStatusInvalid + + scanner := bufio.NewScanner(strings.NewReader(out)) + for scanner.Scan() { + cols := strings.Split(scanner.Text(), "\t") + if len(cols) != plainStatusCols { + continue + } + + if cols[0] != id { + continue + } + + status = statusStringToType(cols[1]) + } + + return status, scanner.Err() +} + +func statusStringToType(status string) WRJobStatus { + switch status { + case "delayed": + return WRJobStatusDelayed + case "ready": + return WRJobStatusReady + case "reserved": + return WRJobStatusReserved + case "running": + return WRJobStatusRunning + case "lost": + return WRJobStatusLost + case "buried": + return WRJobStatusBuried + case "complete": + return WRJobStatusComplete + default: + return WRJobStatusInvalid + } } diff --git a/wr/wr_test.go b/wr/wr_test.go index 1e14674..38f917d 100644 --- a/wr/wr_test.go +++ b/wr/wr_test.go @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2023 Genome Research Ltd. + * Copyright (c) 2023, 2024 Genome Research Ltd. * * Permission is hereby granted, free of charge, to any person obtaining * a copy of this software and associated documentation files (the @@ -75,10 +75,14 @@ func TestWR(t *testing.T) { runner := New("development") runner.memory = "100M" + runner.pollDuration = 10 * time.Millisecond runArgs, repGrp := uniqueRunArgs("sleep 2s") - err := runner.Run(runArgs) + jobID, err := runner.Add(runArgs) So(err, ShouldBeNil) + status, err := runner.Wait(jobID) + So(err, ShouldBeNil) + So(status, ShouldEqual, WRJobStatusComplete) So(time.Since(now), ShouldBeGreaterThan, 2*time.Second) cmd := exec.Command("wr", "status", "--deployment", runner.deployment, "-i", repGrp, "-o", "json") //nolint:gosec @@ -92,13 +96,19 @@ func TestWR(t *testing.T) { So(stderr.String(), ShouldBeBlank) So(stdout.String(), ShouldContainSubstring, `"State":"complete"`) - err = runner.Run(runArgs) + jobID2, err := runner.Add(runArgs) + So(err, ShouldBeNil) + So(jobID2, ShouldEqual, jobID) + status, err = runner.Wait(jobID) So(err, ShouldBeNil) + So(status, ShouldEqual, WRJobStatusComplete) runArgs, _ = uniqueRunArgs("false") - err = runner.Run(runArgs) - So(err, ShouldNotBeNil) - So(err.Error(), ShouldEqual, "wr add failed: exit status 1") + jobID, err = runner.Add(runArgs) + So(err, ShouldBeNil) + status, err = runner.Wait(jobID) + So(err, ShouldBeNil) + So(status, ShouldEqual, WRJobStatusBuried) }) } From 86c617d7e2e985cd95fd77c57658436ab8f9814f Mon Sep 17 00:00:00 2001 From: ash Date: Fri, 9 Feb 2024 13:52:10 +0000 Subject: [PATCH 03/10] Refactor: implementing Runner interface --- build/builder.go | 11 +++++++++-- build/builder_test.go | 16 +++++++++++++--- internal/tests.go | 29 ++++++++++++++++++++--------- wr/wr.go | 21 ++++++++++----------- wr/wr_test.go | 2 +- 5 files changed, 53 insertions(+), 26 deletions(-) diff --git a/build/builder.go b/build/builder.go index 83ff073..37fad28 100644 --- a/build/builder.go +++ b/build/builder.go @@ -178,7 +178,9 @@ type S3 interface { } type Runner interface { - Add(deployment string) error + Add(deployment string) (string, error) + Wait(id string) (wr.WRJobStatus, error) + Status(id string) (wr.WRJobStatus, error) } // The status of an individual build – when it was requested, when it started @@ -397,7 +399,12 @@ func (b *Builder) asyncBuild(def *Definition, wrInput, s3Path, singDef string) e status := b.buildStatus(def) status.BuildStart = time.Now() - err := b.runner.Add(wrInput) + jobID, err := b.runner.Add(wrInput) + if err != nil { + return err + } + + _, err = b.runner.Wait(jobID) b.postBuildMu.RLock() if b.postBuild != nil { diff --git a/build/builder_test.go b/build/builder_test.go index b15776f..b34060f 100644 --- a/build/builder_test.go +++ b/build/builder_test.go @@ -52,11 +52,21 @@ type modifyRunner struct { ch chan bool } -func (m *modifyRunner) Add(_ string) error { - err := m.Runner.Add(m.cmd) +func (m *modifyRunner) Add(_ string) (string, error) { + jobID, err := m.Runner.Add(m.cmd) + + return jobID, err +} + +func (m *modifyRunner) Wait(id string) (wr.WRJobStatus, error) { + status, err := m.Runner.Wait(id) m.ch <- true - return err + return status, err +} + +func (m *modifyRunner) Status(id string) (wr.WRJobStatus, error) { + return m.Runner.Status(id) } func TestBuilder(t *testing.T) { diff --git a/internal/tests.go b/internal/tests.go index f880b37..711a927 100644 --- a/internal/tests.go +++ b/internal/tests.go @@ -29,8 +29,10 @@ import ( "path/filepath" "strings" "sync" + "time" "github.com/wtsi-hgi/go-softpack-builder/internal/core" + "github.com/wtsi-hgi/go-softpack-builder/wr" ) //go:embed testdata @@ -124,21 +126,30 @@ func (m *MockS3) OpenFile(source string) (io.ReadCloser, error) { } type MockWR struct { - Ch chan struct{} - Cmd string - Fail bool + Ch chan struct{} + Cmd string + Fail bool + ReturnStatus wr.WRJobStatus } -func (m *MockWR) Add(cmd string) error { +func (m *MockWR) Add(cmd string) (string, error) { defer close(m.Ch) + m.Cmd = cmd + + return "abc123", nil +} + +func (m *MockWR) Wait(id string) (wr.WRJobStatus, error) { + <-time.After(10 * time.Millisecond) + if m.Fail { - return ErrMock + return wr.WRJobStatusBuried, ErrMock } - m.Cmd = cmd - - // <-time.After(10 * time.Millisecond) + return wr.WRJobStatusComplete, nil +} - return nil +func (m *MockWR) Status(id string) (wr.WRJobStatus, error) { + return m.ReturnStatus, nil } diff --git a/wr/wr.go b/wr/wr.go index bb32350..356547a 100644 --- a/wr/wr.go +++ b/wr/wr.go @@ -146,19 +146,18 @@ func (r *Runner) Wait(id string) (WRJobStatus, error) { ticker := time.NewTicker(r.pollDuration) defer ticker.Stop() - for { - select { - case <-ticker.C: - status, err := r.Status(id) - if err != nil { - return status, err - } - - if status == WRJobStatusInvalid || status == WRJobStatusBuried || status == WRJobStatusComplete { - return status, err - } + for range ticker.C { + status, err := r.Status(id) + if err != nil { + return status, err + } + + if status == WRJobStatusInvalid || status == WRJobStatusBuried || status == WRJobStatusComplete { + return status, err } } + + return WRJobStatusInvalid, nil } // Status returns the status of the wr job with the given internal ID. diff --git a/wr/wr_test.go b/wr/wr_test.go index 38f917d..e26f9d8 100644 --- a/wr/wr_test.go +++ b/wr/wr_test.go @@ -69,7 +69,7 @@ func TestWR(t *testing.T) { return } - Convey("You can run a WR command", t, func() { + Convey("You can run a cmd via wr", t, func() { now := time.Now() runner := New("development") From c21594c259c5e0c1ce30ee2c2000186cd43602f8 Mon Sep 17 00:00:00 2001 From: ash Date: Fri, 9 Feb 2024 13:55:02 +0000 Subject: [PATCH 04/10] Document test mocks --- internal/tests.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/tests.go b/internal/tests.go index 711a927..278c59b 100644 --- a/internal/tests.go +++ b/internal/tests.go @@ -71,6 +71,8 @@ func (c *ConcurrentStringBuilder) String() string { return c.Builder.String() } +// MockS3 can be used to test a build.Builder by implementing the build.S3 +// interface. type MockS3 struct { Data string Def string @@ -82,6 +84,7 @@ type MockS3 struct { const ErrMock = Error("Mock error") +// UploadData implements the build.S3 interface. func (m *MockS3) UploadData(data io.Reader, dest string) error { if m.Fail { return ErrMock @@ -105,6 +108,7 @@ func (m *MockS3) UploadData(data io.Reader, dest string) error { return nil } +// OpenFile implements the build.S3 interface. func (m *MockS3) OpenFile(source string) (io.ReadCloser, error) { if filepath.Base(source) == core.ExesBasename { return io.NopCloser(strings.NewReader(m.Exes)), nil @@ -125,6 +129,7 @@ func (m *MockS3) OpenFile(source string) (io.ReadCloser, error) { return nil, io.ErrUnexpectedEOF } +// MockWR can be used to test a build.Builder without having real wr running. type MockWR struct { Ch chan struct{} Cmd string @@ -132,6 +137,7 @@ type MockWR struct { ReturnStatus wr.WRJobStatus } +// Add implements build.Runner interface. func (m *MockWR) Add(cmd string) (string, error) { defer close(m.Ch) @@ -140,6 +146,7 @@ func (m *MockWR) Add(cmd string) (string, error) { return "abc123", nil } +// Wait implements build.Runner interface. func (m *MockWR) Wait(id string) (wr.WRJobStatus, error) { <-time.After(10 * time.Millisecond) @@ -150,6 +157,7 @@ func (m *MockWR) Wait(id string) (wr.WRJobStatus, error) { return wr.WRJobStatusComplete, nil } +// Status implements build.Runner interface. func (m *MockWR) Status(id string) (wr.WRJobStatus, error) { return m.ReturnStatus, nil } From 7b26aa845731f08686fae0a0983ce617aad45102 Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 12 Feb 2024 10:07:18 +0000 Subject: [PATCH 05/10] Implement WaitForRunning() --- wr/wr.go | 58 +++++++++++++++++++++++++++++++++++++++++--------- wr/wr_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 97 insertions(+), 20 deletions(-) diff --git a/wr/wr.go b/wr/wr.go index 356547a..32ccebf 100644 --- a/wr/wr.go +++ b/wr/wr.go @@ -141,23 +141,61 @@ func (r *Runner) runWRCmd(cmd *exec.Cmd) (string, error) { return strings.TrimSpace(std.String()), nil } -// Wait waits for the given wr job to exit. -func (r *Runner) Wait(id string) (WRJobStatus, error) { +// WaitForRunning waits until the given wr job either starts running, or exits. +func (r *Runner) WaitForRunning(id string) error { + var err error + + cb := func(status WRJobStatus, cbErr error) bool { + err = cbErr + + return err != nil || statusIsStarted(status) || statusIsExited(status) + } + + r.pollStatus(id, cb) + + return err +} + +func statusIsStarted(status WRJobStatus) bool { + return status == WRJobStatusRunning || status == WRJobStatusLost +} + +func statusIsExited(status WRJobStatus) bool { + return status == WRJobStatusInvalid || status == WRJobStatusBuried || status == WRJobStatusComplete +} + +// pollStatusCallback receives a WRJobStatus and error, and should return true +// if you want to stop polling now. +type pollStatusCallback = func(WRJobStatus, error) bool + +func (r *Runner) pollStatus(id string, cb pollStatusCallback) { ticker := time.NewTicker(r.pollDuration) defer ticker.Stop() for range ticker.C { - status, err := r.Status(id) - if err != nil { - return status, err + if cb(r.Status(id)) { + return } + } +} - if status == WRJobStatusInvalid || status == WRJobStatusBuried || status == WRJobStatusComplete { - return status, err - } +// Wait waits for the given wr job to exit. +func (r *Runner) Wait(id string) (WRJobStatus, error) { + var ( + status WRJobStatus + err error + ) + + cb := func(cbStatus WRJobStatus, cbErr error) bool { + status = cbStatus + err = cbErr + + return err != nil || statusIsExited(status) } - return WRJobStatusInvalid, nil + r.pollStatus(id, cb) + + return status, err } // Status returns the status of the wr job with the given internal ID. @@ -193,7 +231,7 @@ func (r *Runner) Status(id string) (WRJobStatus, error) { return status, scanner.Err() } -func statusStringToType(status string) WRJobStatus { +func statusStringToType(status string) WRJobStatus { //nolint:gocyclo switch status { case "delayed": return WRJobStatusDelayed diff --git a/wr/wr_test.go b/wr/wr_test.go index e26f9d8..eae3e3c 100644 --- a/wr/wr_test.go +++ b/wr/wr_test.go @@ -69,17 +69,17 @@ func TestWR(t *testing.T) { return } + runner := New("development") + runner.memory = "100M" + runner.pollDuration = 10 * time.Millisecond + Convey("You can run a cmd via wr", t, func() { now := time.Now() - - runner := New("development") - - runner.memory = "100M" - runner.pollDuration = 10 * time.Millisecond - - runArgs, repGrp := uniqueRunArgs("sleep 2s") + runArgs, repGrp := uniqueRunArgs("sleep 2s", "") jobID, err := runner.Add(runArgs) So(err, ShouldBeNil) + err = runner.WaitForRunning(jobID) + So(err, ShouldBeNil) status, err := runner.Wait(jobID) So(err, ShouldBeNil) So(status, ShouldEqual, WRJobStatusComplete) @@ -99,20 +99,58 @@ func TestWR(t *testing.T) { jobID2, err := runner.Add(runArgs) So(err, ShouldBeNil) So(jobID2, ShouldEqual, jobID) + err = runner.WaitForRunning(jobID) + So(err, ShouldBeNil) status, err = runner.Wait(jobID) So(err, ShouldBeNil) So(status, ShouldEqual, WRJobStatusComplete) - runArgs, _ = uniqueRunArgs("false") + runArgs, _ = uniqueRunArgs("false", "") jobID, err = runner.Add(runArgs) So(err, ShouldBeNil) + err = runner.WaitForRunning(jobID) + So(err, ShouldBeNil) status, err = runner.Wait(jobID) So(err, ShouldBeNil) So(status, ShouldEqual, WRJobStatusBuried) }) + + Convey("WaitForRunning returns when the build starts running", t, func() { + cmd := exec.Command("wr", "limit", "--deployment", runner.deployment, "-g", "limited:0") //nolint:gosec + err := cmd.Run() + So(err, ShouldBeNil) + + runArgs, _ := uniqueRunArgs("sleep 2s", "limited") + jobID, err := runner.Add(runArgs) + So(err, ShouldBeNil) + + runningCh := make(chan time.Time) + errCh := make(chan error, 1) + go func() { + errCh <- runner.WaitForRunning(jobID) + runningCh <- time.Now() + }() + + <-time.After(100 * time.Millisecond) + + startTime := time.Now() + cmd = exec.Command("wr", "limit", "--deployment", runner.deployment, "-g", "limited:1") //nolint:gosec + err = cmd.Run() + So(err, ShouldBeNil) + + status, err := runner.Wait(jobID) + So(err, ShouldBeNil) + endTime := time.Now() + So(status, ShouldEqual, WRJobStatusComplete) + + So(<-errCh, ShouldBeNil) + runningTime := <-runningCh + So(runningTime, ShouldHappenAfter, startTime) + So(runningTime, ShouldHappenBefore, endTime.Add(-1*time.Second)) + }) } -func uniqueRunArgs(cmd string) (string, string) { +func uniqueRunArgs(cmd, limitGrp string) (string, string) { b := make([]byte, 16) n, err := rand.Read(b) So(n, ShouldEqual, 16) @@ -120,5 +158,6 @@ func uniqueRunArgs(cmd string) (string, string) { repGrp := fmt.Sprintf("%x", b) - return `{"cmd":"` + cmd + ` && echo ` + repGrp + `", "rep_grp": "` + repGrp + `", "retries": 0}`, repGrp + return `{"cmd":"` + cmd + ` && echo ` + repGrp + `", "rep_grp": "` + repGrp + + `", "limit_grps": ["` + limitGrp + `"], "retries": 0}`, repGrp } From e8f5d7edabe7b75be4d06e9d7bd6a4859a268025 Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 12 Feb 2024 11:01:26 +0000 Subject: [PATCH 06/10] Finish implementing build statuses --- build/builder.go | 18 ++++++++++++++- build/builder_test.go | 6 ++++- internal/tests.go | 54 ++++++++++++++++++++++++++++++++++++------- server/server_test.go | 41 ++++++++++++++++++++++++-------- 4 files changed, 99 insertions(+), 20 deletions(-) diff --git a/build/builder.go b/build/builder.go index 37fad28..e428552 100644 --- a/build/builder.go +++ b/build/builder.go @@ -179,6 +179,7 @@ type S3 interface { type Runner interface { Add(deployment string) (string, error) + WaitForRunning(id string) error Wait(id string) (wr.WRJobStatus, error) Status(id string) (wr.WRJobStatus, error) } @@ -206,6 +207,8 @@ type Builder struct { statusMu sync.RWMutex statuses map[string]*Status + + runnerPollInterval time.Duration } // New takes the s3 build cache URL, the repo and checkout reference of your @@ -232,6 +235,7 @@ func New(config *config.Config, s3helper S3, runner Runner) (*Builder, error) { runner: runner, runningEnvironments: make(map[string]bool), statuses: make(map[string]*Status), + runnerPollInterval: 1 * time.Second, }, nil } @@ -397,15 +401,27 @@ func (b *Builder) startBuild(def *Definition, wrInput, s3Path, singDef, singDefP func (b *Builder) asyncBuild(def *Definition, wrInput, s3Path, singDef string) error { status := b.buildStatus(def) - status.BuildStart = time.Now() jobID, err := b.runner.Add(wrInput) if err != nil { return err } + err = b.runner.WaitForRunning(jobID) + if err != nil { + return err + } + + b.statusMu.Lock() + status.BuildStart = time.Now() + b.statusMu.Unlock() + _, err = b.runner.Wait(jobID) + b.statusMu.Lock() + status.BuildDone = time.Now() + b.statusMu.Unlock() + b.postBuildMu.RLock() if b.postBuild != nil { // if spack ran at all, it might've pushed things to the cache, even if diff --git a/build/builder_test.go b/build/builder_test.go index b34060f..d857a22 100644 --- a/build/builder_test.go +++ b/build/builder_test.go @@ -72,7 +72,7 @@ func (m *modifyRunner) Status(id string) (wr.WRJobStatus, error) { func TestBuilder(t *testing.T) { Convey("Given binary cache and spack repo details and a Definition", t, func() { ms3 := &internal.MockS3{} - mwr := &internal.MockWR{Ch: make(chan struct{})} + mwr := internal.NewMockWR(1*time.Millisecond, 10*time.Millisecond) mc := &core.MockCore{Files: make(map[string]string)} msc := httptest.NewServer(mc) @@ -223,6 +223,7 @@ Stage: final So(ms3.Data, ShouldContainSubstring, "specs:\n - xxhash@0.8.1 arch=None-None-x86_64_v4\n"+ " - r-seurat@4 arch=None-None-x86_64_v4\n - py-anndata@3.14 arch=None-None-x86_64_v4\n view") + mwr.SetRunning() <-mwr.Ch hash := fmt.Sprintf("%X", sha256.Sum256([]byte(ms3.Data))) So(mwr.Cmd, ShouldContainSubstring, "echo doing build with hash "+hash+"; sudo singularity build") @@ -346,6 +347,7 @@ packages: err := builder.Build(def) So(err, ShouldBeNil) + mwr.SetRunning() <-mwr.Ch ok := waitFor(func() bool { @@ -407,6 +409,8 @@ packages: err := builder.Build(def) So(err, ShouldBeNil) + mwr.SetRunning() + ok := waitFor(func() bool { return logWriter.String() != "" }) diff --git a/internal/tests.go b/internal/tests.go index 278c59b..18fa8a5 100644 --- a/internal/tests.go +++ b/internal/tests.go @@ -131,24 +131,59 @@ func (m *MockS3) OpenFile(source string) (io.ReadCloser, error) { // MockWR can be used to test a build.Builder without having real wr running. type MockWR struct { - Ch chan struct{} - Cmd string - Fail bool + Ch chan struct{} + Cmd string + Fail bool + PollForStatusInterval time.Duration + JobDuration time.Duration + + sync.RWMutex ReturnStatus wr.WRJobStatus } +func NewMockWR(pollForStatusInterval, jobDuration time.Duration) *MockWR { + return &MockWR{ + Ch: make(chan struct{}), + PollForStatusInterval: pollForStatusInterval, + JobDuration: jobDuration, + } +} + // Add implements build.Runner interface. func (m *MockWR) Add(cmd string) (string, error) { - defer close(m.Ch) - m.Cmd = cmd return "abc123", nil } +// SetRunning should be used before waiting on Ch and if you need to test +// BuildStart time in Status. +func (m *MockWR) SetRunning() { + m.Lock() + defer m.Unlock() + + m.ReturnStatus = wr.WRJobStatusRunning +} + +// WaitForRunning implements build.Runner interface. +func (m *MockWR) WaitForRunning(string) error { + for { + m.RLock() + rs := m.ReturnStatus + m.RUnlock() + + if rs == wr.WRJobStatusRunning || rs == wr.WRJobStatusBuried || rs == wr.WRJobStatusComplete { + return nil + } + + <-time.After(m.PollForStatusInterval) + } +} + // Wait implements build.Runner interface. -func (m *MockWR) Wait(id string) (wr.WRJobStatus, error) { - <-time.After(10 * time.Millisecond) +func (m *MockWR) Wait(string) (wr.WRJobStatus, error) { + defer close(m.Ch) + <-time.After(m.JobDuration) if m.Fail { return wr.WRJobStatusBuried, ErrMock @@ -158,6 +193,9 @@ func (m *MockWR) Wait(id string) (wr.WRJobStatus, error) { } // Status implements build.Runner interface. -func (m *MockWR) Status(id string) (wr.WRJobStatus, error) { +func (m *MockWR) Status(string) (wr.WRJobStatus, error) { + m.RLock() + defer m.RUnlock() + return m.ReturnStatus, nil } diff --git a/server/server_test.go b/server/server_test.go index a4c1b65..4b8809a 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -183,7 +183,8 @@ func TestServerMock(t *testing.T) { func TestServerReal(t *testing.T) { Convey("With a real builder", t, func() { ms3 := &internal.MockS3{} - mwr := &internal.MockWR{Ch: make(chan struct{})} + mockStatusPollInterval := 1 * time.Millisecond + mwr := internal.NewMockWR(mockStatusPollInterval, 10*time.Millisecond) mc := &core.MockCore{Files: make(map[string]string)} msc := httptest.NewServer(mc) @@ -205,23 +206,32 @@ func TestServerReal(t *testing.T) { handler := New(builder) server := httptest.NewServer(handler) - So(server, ShouldNotBeNil) + So(server != nil, ShouldBeTrue) buildSubmitted := time.Now() postToBuildEndpoint(server, "users/user/myenv", "0.8.1") Convey("you get a real status", func() { - resp, err := server.Client().Get(server.URL + endpointEnvsStatus) //nolint:noctx - So(err, ShouldBeNil) - So(resp.StatusCode, ShouldEqual, http.StatusOK) - var statuses []build.Status - err = json.NewDecoder(resp.Body).Decode(&statuses) - So(err, ShouldBeNil) + statuses := getTestStatuses(server) So(len(statuses), ShouldEqual, 1) So(statuses[0].Name, ShouldEqual, "users/user/myenv-0.8.1") So(statuses[0].Requested, ShouldHappenAfter, buildSubmitted) - So(statuses[0].BuildStart, ShouldHappenAfter, statuses[0].Requested) - // BuildDone: , + So(statuses[0].BuildStart.IsZero(), ShouldBeTrue) + So(statuses[0].BuildDone.IsZero(), ShouldBeTrue) + + runT := time.Now() + mwr.SetRunning() + <-time.After(2 * mockStatusPollInterval) + statuses = getTestStatuses(server) + So(len(statuses), ShouldEqual, 1) + So(statuses[0].Requested, ShouldHappenAfter, buildSubmitted) + buildStart := statuses[0].BuildStart + So(buildStart, ShouldHappenAfter, runT) + So(statuses[0].BuildDone.IsZero(), ShouldBeTrue) + + <-time.After(mwr.JobDuration) + statuses = getTestStatuses(server) + So(statuses[0].BuildDone, ShouldHappenAfter, buildStart) }) }) } @@ -242,3 +252,14 @@ func postToBuildEndpoint(server *httptest.Server, name, version string) { So(err, ShouldBeNil) So(resp.StatusCode, ShouldEqual, http.StatusOK) } + +func getTestStatuses(server *httptest.Server) []build.Status { + resp, err := server.Client().Get(server.URL + endpointEnvsStatus) //nolint:noctx + So(err, ShouldBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusOK) + var statuses []build.Status + err = json.NewDecoder(resp.Body).Decode(&statuses) + So(err, ShouldBeNil) + + return statuses +} From c6a39915568b9984ca6788ad86905d0848aa198a Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 12 Feb 2024 11:32:39 +0000 Subject: [PATCH 07/10] Fix lint errors --- build/builder_test.go | 20 ++++++++++++++------ internal/core/core.go | 5 +++++ internal/tests.go | 6 +++--- server/server.go | 9 ++++++--- server/server_test.go | 2 ++ 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/build/builder_test.go b/build/builder_test.go index d857a22..f751245 100644 --- a/build/builder_test.go +++ b/build/builder_test.go @@ -219,7 +219,7 @@ Stage: final So(bcbCount.Load(), ShouldEqual, 0) - So(ms3.Def, ShouldEqual, "groups/hgi/xxhash/0.8.1/singularity.def") + So(ms3.Def, ShouldEqual, filepath.Join(def.getS3Path(), "singularity.def")) So(ms3.Data, ShouldContainSubstring, "specs:\n - xxhash@0.8.1 arch=None-None-x86_64_v4\n"+ " - r-seurat@4 arch=None-None-x86_64_v4\n - py-anndata@3.14 arch=None-None-x86_64_v4\n view") @@ -310,7 +310,7 @@ packages: - r-seurat@4 ` - expectedReadmeContent := "module load " + moduleLoadPrefix + "/groups/hgi/xxhash/0.8.1" + expectedReadmeContent := "module load " + filepath.Join(moduleLoadPrefix, def.getS3Path()) for file, expectedData := range map[string]string{ core.SoftpackYaml: expectedSoftpackYaml, @@ -320,12 +320,12 @@ packages: core.BuilderOut: "output", core.UsageBasename: expectedReadmeContent, } { - data, okg := mc.GetFile("groups/hgi/xxhash-0.8.1/" + file) + data, okg := mc.GetFile(filepath.Join(def.getRepoPath(), file)) So(okg, ShouldBeTrue) So(data, ShouldContainSubstring, expectedData) } - _, ok = mc.GetFile("groups/hgi/xxhash-0.8.1/" + core.ImageBasename) + _, ok = mc.GetFile(filepath.Join(def.getRepoPath(), core.ImageBasename)) So(ok, ShouldBeFalse) So(ms3.SoftpackYML, ShouldEqual, expectedSoftpackYaml) @@ -356,9 +356,9 @@ packages: So(ok, ShouldBeTrue) So(logWriter.String(), ShouldContainSubstring, - "msg=\"Async part of build failed\" err=\"Mock error\" s3Path=some_path/groups/hgi/xxhash/0.8.1") + "msg=\"Async part of build failed\" err=\"Mock error\" s3Path=some_path/"+def.getS3Path()) - data, ok := mc.GetFile("groups/hgi/xxhash-0.8.1/" + core.BuilderOut) + data, ok := mc.GetFile(filepath.Join(def.getRepoPath(), core.BuilderOut)) So(ok, ShouldBeTrue) So(data, ShouldContainSubstring, "output") @@ -468,6 +468,14 @@ func getExampleDefinition() *Definition { } } +func (d *Definition) getS3Path() string { + return filepath.Join(d.EnvironmentPath, d.EnvironmentName, d.EnvironmentVersion) +} + +func (d *Definition) getRepoPath() string { + return filepath.Join(d.EnvironmentPath, d.EnvironmentName) + "-" + d.EnvironmentVersion +} + func waitFor(toRun func() bool) bool { timeout := time.NewTimer(5 * time.Second) ticker := time.NewTicker(10 * time.Millisecond) diff --git a/internal/core/core.go b/internal/core/core.go index 32a61e0..ffbaeec 100644 --- a/internal/core/core.go +++ b/internal/core/core.go @@ -26,6 +26,7 @@ package core import ( "errors" "io" + "mime/multipart" "net/http" "net/url" "path/filepath" @@ -82,6 +83,10 @@ func (m *MockCore) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + m.readFileFromQuery(mr, envPath) +} + +func (m *MockCore) readFileFromQuery(mr *multipart.Reader, envPath string) { for { p, err := mr.NextPart() if errors.Is(err, io.EOF) { diff --git a/internal/tests.go b/internal/tests.go index 18fa8a5..b8c1837 100644 --- a/internal/tests.go +++ b/internal/tests.go @@ -150,7 +150,7 @@ func NewMockWR(pollForStatusInterval, jobDuration time.Duration) *MockWR { } // Add implements build.Runner interface. -func (m *MockWR) Add(cmd string) (string, error) { +func (m *MockWR) Add(cmd string) (string, error) { //nolint:unparam m.Cmd = cmd return "abc123", nil @@ -166,7 +166,7 @@ func (m *MockWR) SetRunning() { } // WaitForRunning implements build.Runner interface. -func (m *MockWR) WaitForRunning(string) error { +func (m *MockWR) WaitForRunning(string) error { //nolint:unparam for { m.RLock() rs := m.ReturnStatus @@ -193,7 +193,7 @@ func (m *MockWR) Wait(string) (wr.WRJobStatus, error) { } // Status implements build.Runner interface. -func (m *MockWR) Status(string) (wr.WRJobStatus, error) { +func (m *MockWR) Status(string) (wr.WRJobStatus, error) { //nolint:unparam m.RLock() defer m.RUnlock() diff --git a/server/server.go b/server/server.go index 7df8d3e..7b7ced3 100644 --- a/server/server.go +++ b/server/server.go @@ -72,7 +72,7 @@ func New(b Builder) http.Handler { case endpointEnvsBuild: handleEnvBuild(b, w, r) case endpointEnvsStatus: - handleEnvStatus(b, w, r) + handleEnvStatus(b, w) default: http.Error(w, "Not found", http.StatusNotFound) } @@ -103,6 +103,9 @@ func handleEnvBuild(b Builder, w http.ResponseWriter, r *http.Request) { } } -func handleEnvStatus(b Builder, w http.ResponseWriter, r *http.Request) { - json.NewEncoder(w).Encode(b.Status()) +func handleEnvStatus(b Builder, w http.ResponseWriter) { + err := json.NewEncoder(w).Encode(b.Status()) + if err != nil { + http.Error(w, fmt.Sprintf("error serialising status: %s", err), http.StatusInternalServerError) + } } diff --git a/server/server_test.go b/server/server_test.go index 4b8809a..4c9ae40 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -257,7 +257,9 @@ func getTestStatuses(server *httptest.Server) []build.Status { resp, err := server.Client().Get(server.URL + endpointEnvsStatus) //nolint:noctx So(err, ShouldBeNil) So(resp.StatusCode, ShouldEqual, http.StatusOK) + var statuses []build.Status + err = json.NewDecoder(resp.Body).Decode(&statuses) So(err, ShouldBeNil) From aca4e728da579cec97537a8d9f965b6671809881 Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 12 Feb 2024 13:18:26 +0000 Subject: [PATCH 08/10] Only parse wr's stdout when getting job ID it can print errors on stderr... --- wr/wr.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/wr/wr.go b/wr/wr.go index 32ccebf..1174ab5 100644 --- a/wr/wr.go +++ b/wr/wr.go @@ -119,16 +119,16 @@ func (r *Runner) Add(wrInput string) (string, error) { } func (r *Runner) runWRCmd(cmd *exec.Cmd) (string, error) { - var std bytes.Buffer + var stdout, stderr bytes.Buffer - cmd.Stdout = &std - cmd.Stderr = &std + cmd.Stdout = &stdout + cmd.Stderr = &stderr err := cmd.Run() if err != nil { - errStr := std.String() + errStr := stderr.String() if !strings.Contains(errStr, "EROR") { - return errStr, nil + return strings.TrimSpace(stdout.String()), nil } if errStr == "" { @@ -138,7 +138,7 @@ func (r *Runner) runWRCmd(cmd *exec.Cmd) (string, error) { return "", Error{msg: errStr} } - return strings.TrimSpace(std.String()), nil + return strings.TrimSpace(stdout.String()), nil } // WaitForRunning waits until the given wr job either starts running, or exits. From f24b126747a34751bed07ca1356e231d25b638fe Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 12 Feb 2024 13:26:12 +0000 Subject: [PATCH 09/10] Log errors from `wr status` --- wr/wr.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/wr/wr.go b/wr/wr.go index 1174ab5..f9e764a 100644 --- a/wr/wr.go +++ b/wr/wr.go @@ -27,6 +27,7 @@ import ( "bufio" "bytes" _ "embed" + "log/slog" "os/exec" "strings" "text/template" @@ -209,12 +210,16 @@ func (r *Runner) Status(id string) (WRJobStatus, error) { out, err := r.runWRCmd(cmd) if err != nil { + slog.Error("wr status command failed", "err", err) + return WRJobStatusInvalid, err } - status := WRJobStatusInvalid + return parseWRStatus(out, id) +} - scanner := bufio.NewScanner(strings.NewReader(out)) +func parseWRStatus(wrStatusOutput, id string) (WRJobStatus, error) { + scanner := bufio.NewScanner(strings.NewReader(wrStatusOutput)) for scanner.Scan() { cols := strings.Split(scanner.Text(), "\t") if len(cols) != plainStatusCols { @@ -225,10 +230,12 @@ func (r *Runner) Status(id string) (WRJobStatus, error) { continue } - status = statusStringToType(cols[1]) + return statusStringToType(cols[1]), nil } - return status, scanner.Err() + slog.Error("wr status parsing to find a job failed", "id", id, "err", scanner.Err()) + + return WRJobStatusInvalid, scanner.Err() } func statusStringToType(status string) WRJobStatus { //nolint:gocyclo From 889fef7eb755670b18dc442c41b33edf337cf682 Mon Sep 17 00:00:00 2001 From: ash Date: Mon, 12 Feb 2024 13:40:43 +0000 Subject: [PATCH 10/10] Render null/zero times as null in JSON output --- README.md | 21 +++++++++++++++++++-- build/builder.go | 15 +++++++++------ server/server_test.go | 20 ++++++++++---------- 3 files changed, 38 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 81e32db..66ddf62 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # go-softpack-builder (gsb) Go implementation of a softpack builder service. -After receiving a POST (see Testing section below) with desired environment -details, this service does the following: +After receiving a POST to `/environments/build` (see Testing section below) with +desired environment details, this service does the following: 1. A singularity definition file, singularity.def, is created and uploaded to an environment-specific subdirectory of your S3 build location. @@ -43,6 +43,23 @@ details, this service does the following: It can be reproduced exactly at any time using the singularity.def, assuming you configure specific images (ie. not :latest) to use. +After receiving a GET to `/environments/status`, this service returns a JSON +response with the following structure: + +```json +[ + { + "Name": "users/foo/bar", + "Requested": "2024-02-12T11:58:49.808672303Z", + "BuildStart": "2024-02-12T11:58:55.430080969Z", + "BuildDone": "2024-02-12T11:59:00.532174828Z" + } +] +``` + +The times are quoted strings in the RFC 3339 format with sub-second precision, +or null. + ## Initial setup You'll need an S3 bucket to be a binary cache, which needs GPG keys. Here's one diff --git a/build/builder.go b/build/builder.go index e428552..9c27016 100644 --- a/build/builder.go +++ b/build/builder.go @@ -188,9 +188,9 @@ type Runner interface { // actually being built, and when its build finished. type Status struct { Name string - Requested time.Time - BuildStart time.Time - BuildDone time.Time + Requested *time.Time + BuildStart *time.Time + BuildDone *time.Time } // Builder lets you do builds given config, S3 and a wr runner. @@ -317,9 +317,10 @@ func (b *Builder) buildStatus(def *Definition) *Status { status, exists := b.statuses[name] if !exists { + now := time.Now() status = &Status{ Name: name, - Requested: time.Now(), + Requested: &now, } b.statuses[name] = status @@ -413,13 +414,15 @@ func (b *Builder) asyncBuild(def *Definition, wrInput, s3Path, singDef string) e } b.statusMu.Lock() - status.BuildStart = time.Now() + buildStart := time.Now() + status.BuildStart = &buildStart b.statusMu.Unlock() _, err = b.runner.Wait(jobID) b.statusMu.Lock() - status.BuildDone = time.Now() + buildDone := time.Now() + status.BuildDone = &buildDone b.statusMu.Unlock() b.postBuildMu.RLock() diff --git a/server/server_test.go b/server/server_test.go index 4c9ae40..fe26380 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -58,7 +58,7 @@ func (m *mockBuilder) Status() []build.Status { for i, def := range m.received { statuses[i] = build.Status{ Name: filepath.Join(def.EnvironmentPath, def.EnvironmentName) + "-" + def.EnvironmentVersion, - Requested: m.requested[i], + Requested: &m.requested[i], } } @@ -160,7 +160,7 @@ func TestServerMock(t *testing.T) { So(err, ShouldBeNil) So(len(statuses), ShouldEqual, 1) So(statuses[0].Name, ShouldEqual, "users/user/myenv-0.8.1") - So(statuses[0].Requested, ShouldEqual, mb.requested[0]) + So(*statuses[0].Requested, ShouldHappenWithin, 0*time.Microsecond, mb.requested[0]) postToBuildEndpoint(server, "users/user/myotherenv", "1") @@ -175,7 +175,7 @@ func TestServerMock(t *testing.T) { So(len(statuses), ShouldEqual, 2) So(statuses[0].Name, ShouldEqual, "users/user/myenv-0.8.1") So(statuses[1].Name, ShouldEqual, "users/user/myotherenv-1") - So(statuses[1].Requested, ShouldEqual, mb.requested[1]) + So(*statuses[1].Requested, ShouldHappenWithin, 0*time.Microsecond, mb.requested[1]) }) }) } @@ -215,23 +215,23 @@ func TestServerReal(t *testing.T) { statuses := getTestStatuses(server) So(len(statuses), ShouldEqual, 1) So(statuses[0].Name, ShouldEqual, "users/user/myenv-0.8.1") - So(statuses[0].Requested, ShouldHappenAfter, buildSubmitted) - So(statuses[0].BuildStart.IsZero(), ShouldBeTrue) - So(statuses[0].BuildDone.IsZero(), ShouldBeTrue) + So(*statuses[0].Requested, ShouldHappenAfter, buildSubmitted) + So(statuses[0].BuildStart, ShouldBeNil) + So(statuses[0].BuildDone, ShouldBeNil) runT := time.Now() mwr.SetRunning() <-time.After(2 * mockStatusPollInterval) statuses = getTestStatuses(server) So(len(statuses), ShouldEqual, 1) - So(statuses[0].Requested, ShouldHappenAfter, buildSubmitted) - buildStart := statuses[0].BuildStart + So(*statuses[0].Requested, ShouldHappenAfter, buildSubmitted) + buildStart := *statuses[0].BuildStart So(buildStart, ShouldHappenAfter, runT) - So(statuses[0].BuildDone.IsZero(), ShouldBeTrue) + So(statuses[0].BuildDone, ShouldBeNil) <-time.After(mwr.JobDuration) statuses = getTestStatuses(server) - So(statuses[0].BuildDone, ShouldHappenAfter, buildStart) + So(*statuses[0].BuildDone, ShouldHappenAfter, buildStart) }) }) }