From b56cc217106d2dfb36cec8d2233ff4762e28dd91 Mon Sep 17 00:00:00 2001 From: Bryan White Date: Thu, 14 Nov 2019 15:03:49 +0100 Subject: [PATCH] cmd/storagenode-updater: make updater test windows compatible (#3542) --- cmd/storagenode-updater/main.go | 39 ++++-------------- cmd/storagenode-updater/main_test.go | 54 ++++++++++++------------- cmd/storagenode-updater/restart.go | 45 +++++++++++++++++++++ cmd/storagenode-updater/restart_mock.go | 8 ++++ cmd/storagenode-updater/windows.go | 2 +- internal/testcontext/compile.go | 4 +- 6 files changed, 90 insertions(+), 62 deletions(-) create mode 100644 cmd/storagenode-updater/restart.go create mode 100644 cmd/storagenode-updater/restart_mock.go diff --git a/cmd/storagenode-updater/main.go b/cmd/storagenode-updater/main.go index f047985aa..f8bda2883 100644 --- a/cmd/storagenode-updater/main.go +++ b/cmd/storagenode-updater/main.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "errors" - "fmt" "io" "io/ioutil" "log" @@ -187,11 +186,16 @@ func update(ctx context.Context, binPath, serviceName string) (err error) { return nil } - tempArchive, err := ioutil.TempFile(os.TempDir(), serviceName) + tempArchive, err := ioutil.TempFile("", serviceName) if err != nil { return errs.New("cannot create temporary archive: %v", err) } - defer func() { err = errs.Combine(err, os.Remove(tempArchive.Name())) }() + defer func() { + err = errs.Combine(err, + tempArchive.Close(), + os.Remove(tempArchive.Name()), + ) + }() downloadURL := parseDownloadURL(processVersion.Suggested.URL) log.Println("start downloading", downloadURL, "to", tempArchive.Name()) @@ -327,35 +331,6 @@ func unpackBinary(ctx context.Context, archive, target string) (err error) { return nil } -func restartService(name string) error { - switch runtime.GOOS { - case "windows": - // TODO: combine stdout with err if err - restartSvcBatPath := filepath.Join(os.TempDir(), "restartservice.bat") - restartSvcBat, err := os.Create(restartSvcBatPath) - if err != nil { - return err - } - - restartStr := fmt.Sprintf("net stop %s && net start %s", name, name) - _, err = restartSvcBat.WriteString(restartStr) - if err != nil { - return err - } - if err := restartSvcBat.Close(); err != nil { - return err - } - - _, err = exec.Command(restartSvcBat.Name()).CombinedOutput() - if err != nil { - return err - } - default: - return nil - } - return nil -} - func fileExists(filename string) bool { info, err := os.Stat(filename) if os.IsNotExist(err) { diff --git a/cmd/storagenode-updater/main_test.go b/cmd/storagenode-updater/main_test.go index b88dcc786..e700b23b4 100644 --- a/cmd/storagenode-updater/main_test.go +++ b/cmd/storagenode-updater/main_test.go @@ -13,7 +13,6 @@ import ( "os" "os/exec" "path/filepath" - "runtime" "testing" "time" @@ -35,13 +34,7 @@ const ( newVersion = "v0.19.5" ) -func TestAutoUpdater_unix(t *testing.T) { - t.Skip("TODO: the version server must listen on random port") - - if runtime.GOOS == "windows" { - t.Skip("requires storagenode and storagenode-updater to be installed as windows services") - } - +func TestAutoUpdater(t *testing.T) { // TODO cleanup `.exe` extension for different OS ctx := testcontext.New(t) @@ -61,9 +54,9 @@ func TestAutoUpdater_unix(t *testing.T) { } // build real bin with old version, will be used for both storagenode and updater - oldBin := ctx.CompileWithVersion("", oldInfo) + oldBin := ctx.CompileWithVersion("storj.io/storj/cmd/storagenode-updater", oldInfo) storagenodePath := ctx.File("fake", "storagenode.exe") - copy(ctx, t, oldBin, storagenodePath) + copyBin(ctx, t, oldBin, storagenodePath) updaterPath := ctx.File("fake", "storagenode-updater.exe") move(t, oldBin, updaterPath) @@ -75,7 +68,8 @@ func TestAutoUpdater_unix(t *testing.T) { Version: newSemVer, Release: false, } - newBin := ctx.CompileWithVersion("", newInfo) + newBin := ctx.CompileWithVersion("storj.io/storj/cmd/storagenode-updater", newInfo) + updateBins := map[string]string{ "storagenode": newBin, "storagenode-updater": newBin, @@ -91,14 +85,15 @@ func TestAutoUpdater_unix(t *testing.T) { identConfig := testIdentityFiles(ctx, t) // run updater (update) - args := []string{"run"} - args = append(args, "--config-dir", ctx.Dir()) - args = append(args, "--server-address", "http://"+versionControlPeer.Addr()) - args = append(args, "--binary-location", storagenodePath) - args = append(args, "--check-interval", "0s") - args = append(args, "--identity.cert-path", identConfig.CertPath) - args = append(args, "--identity.key-path", identConfig.KeyPath) - args = append(args, "--log", logPath) + args := []string{"run", + "--config-dir", ctx.Dir(), + "--server-address", "http://" + versionControlPeer.Addr(), + "--binary-location", storagenodePath, + "--check-interval", "0s", + "--identity.cert-path", identConfig.CertPath, + "--identity.key-path", identConfig.KeyPath, + "--log", logPath, + } // NB: updater currently uses `log.SetOutput` so all output after that call // only goes to the log file. @@ -110,9 +105,10 @@ func TestAutoUpdater_unix(t *testing.T) { if !assert.Contains(t, logStr, "storagenode restarted successfully") { t.Log(logStr) } - if !assert.Contains(t, logStr, "storagenode-updater restarted successfully") { - t.Log(logStr) - } + // TODO: re-enable when updater is self-updating + //if !assert.Contains(t, logStr, "storagenode-updater restarted successfully") { + // t.Log(logStr) + //} } else { t.Log(string(out)) } @@ -126,11 +122,12 @@ func TestAutoUpdater_unix(t *testing.T) { require.NotNil(t, oldStoragenodeInfo) require.NotZero(t, oldStoragenodeInfo.Size()) - backupUpdater := ctx.File("fake", "storagenode-updater.old.exe") - backupUpdaterInfo, err := os.Stat(backupUpdater) - require.NoError(t, err) - require.NotNil(t, backupUpdaterInfo) - require.NotZero(t, backupUpdaterInfo.Size()) + // TODO: re-enable when updater is self-updating + //backupUpdater := ctx.File("fake", "storagenode-updater.old.exe") + //backupUpdaterInfo, err := os.Stat(backupUpdater) + //require.NoError(t, err) + //require.NotNil(t, backupUpdaterInfo) + //require.NotZero(t, backupUpdaterInfo.Size()) } func move(t *testing.T, src, dst string) { @@ -138,7 +135,7 @@ func move(t *testing.T, src, dst string) { require.NoError(t, err) } -func copy(ctx *testcontext.Context, t *testing.T, src, dst string) { +func copyBin(ctx *testcontext.Context, t *testing.T, src, dst string) { s, err := os.Open(src) require.NoError(t, err) defer ctx.Check(s.Close) @@ -249,6 +246,7 @@ func zipBin(ctx *testcontext.Context, t *testing.T, dst, src string) { zipFile, err := os.Create(dst) require.NoError(t, err) + defer ctx.Check(zipFile.Close) base := filepath.Base(dst) base = base[:len(base)-len(".zip")] diff --git a/cmd/storagenode-updater/restart.go b/cmd/storagenode-updater/restart.go new file mode 100644 index 000000000..2ef263757 --- /dev/null +++ b/cmd/storagenode-updater/restart.go @@ -0,0 +1,45 @@ +// Copyright (C) 2019 Storj Labs, Inc. +// See LICENSE for copying information. + +// +build !unittest + +package main + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + + "github.com/zeebo/errs" +) + +func restartService(name string) error { + switch runtime.GOOS { + case "windows": + // TODO: cleanup temp .bat file + restartSvcBatPath := filepath.Join(os.TempDir(), "restartservice.bat") + restartSvcBat, err := os.Create(restartSvcBatPath) + if err != nil { + return err + } + + restartStr := fmt.Sprintf("net stop %s && net start %s", name, name) + _, err = restartSvcBat.WriteString(restartStr) + if err != nil { + return err + } + if err := restartSvcBat.Close(); err != nil { + return err + } + + out, err := exec.Command(restartSvcBat.Name()).CombinedOutput() + if err != nil { + return errs.New("%s", string(out)) + } + default: + return nil + } + return nil +} diff --git a/cmd/storagenode-updater/restart_mock.go b/cmd/storagenode-updater/restart_mock.go new file mode 100644 index 000000000..fe9f914fc --- /dev/null +++ b/cmd/storagenode-updater/restart_mock.go @@ -0,0 +1,8 @@ +// Copyright (C) 2019 Storj Labs, Inc. +// See LICENSE for copying information. + +// +build unittest + +package main + +func restartService(name string) error { return nil } diff --git a/cmd/storagenode-updater/windows.go b/cmd/storagenode-updater/windows.go index f372ee8c1..775c92119 100644 --- a/cmd/storagenode-updater/windows.go +++ b/cmd/storagenode-updater/windows.go @@ -7,7 +7,7 @@ // // sc.exe create storagenode-updater binpath= "C:\Users\MyUser\storagenode-updater.exe run ..." -// +build windows +// +build windows,!unittest package main diff --git a/internal/testcontext/compile.go b/internal/testcontext/compile.go index 0916c055b..95d9e9f0f 100644 --- a/internal/testcontext/compile.go +++ b/internal/testcontext/compile.go @@ -45,7 +45,9 @@ func (ctx *Context) Compile(pkg string, preArgs ...string) string { args = append(args, "-race") } if drpcEnabled { - args = append(args, "-tags=drpc") + args = append(args, "-tags=drpc,unittest") + } else { + args = append(args, "-tag=unittest") } args = append(args, "-o", exe, pkg)