veyron/services/mgmt/impl/node: implement Update and add test logic for Update.
The change refactors some code so we can share functionality between Install and
Update.
Other minor changes:
- move the origin link at the level of the installation rather than version
- (unit test) remove the shutdown ping from the app since it doesn't really help
synchronize anything
- (unit test) change signatures of resolve and resolveExpectError to be more
useful
- (unit test) factor out common code to create an Application stub
Change-Id: I86daa5f26c50d7eb4ae5d772d4d74995a3a75df6
diff --git a/services/mgmt/node/impl/app_invoker.go b/services/mgmt/node/impl/app_invoker.go
index 871a78e..c7d6a3a 100644
--- a/services/mgmt/node/impl/app_invoker.go
+++ b/services/mgmt/node/impl/app_invoker.go
@@ -10,10 +10,10 @@
// app-<hash 1>/ - the application dir is named using a hash of the application title
// installation-<id 1>/ - installations are labelled with ids
// <status> - one of the values for installationState enum
+// origin - object name for application envelope
// <version 1 timestamp>/ - timestamp of when the version was downloaded
// bin - application binary
// previous - symbolic link to previous version directory (TODO)
-// origin - object name for application envelope
// envelope - application envelope (JSON-encoded)
// <version 2 timestamp>
// ...
@@ -90,6 +90,7 @@
"os"
"os/exec"
"path/filepath"
+ "reflect"
"strings"
"time"
@@ -97,6 +98,7 @@
vexec "veyron/services/mgmt/lib/exec"
iconfig "veyron/services/mgmt/node/config"
+ "veyron2/context"
"veyron2/ipc"
"veyron2/mgmt"
"veyron2/naming"
@@ -155,19 +157,19 @@
vlog.Errorf("Marshal(%v) failed: %v", envelope, err)
return errOperationFailed
}
- envelopePath := filepath.Join(dir, "envelope")
- if err := ioutil.WriteFile(envelopePath, jsonEnvelope, 0600); err != nil {
- vlog.Errorf("WriteFile(%v) failed: %v", envelopePath, err)
+ path := filepath.Join(dir, "envelope")
+ if err := ioutil.WriteFile(path, jsonEnvelope, 0600); err != nil {
+ vlog.Errorf("WriteFile(%v) failed: %v", path, err)
return errOperationFailed
}
return nil
}
func loadEnvelope(dir string) (*application.Envelope, error) {
- envelopePath := filepath.Join(dir, "envelope")
+ path := filepath.Join(dir, "envelope")
envelope := new(application.Envelope)
- if envelopeBytes, err := ioutil.ReadFile(envelopePath); err != nil {
- vlog.Errorf("ReadFile(%v) failed: %v", envelopePath, err)
+ if envelopeBytes, err := ioutil.ReadFile(path); err != nil {
+ vlog.Errorf("ReadFile(%v) failed: %v", path, err)
return nil, errOperationFailed
} else if err := json.Unmarshal(envelopeBytes, envelope); err != nil {
vlog.Errorf("Unmarshal(%v) failed: %v", envelopeBytes, err)
@@ -185,6 +187,16 @@
return nil
}
+func loadOrigin(dir string) (string, error) {
+ path := filepath.Join(dir, "origin")
+ if originBytes, err := ioutil.ReadFile(path); err != nil {
+ vlog.Errorf("ReadFile(%v) failed: %v", path, err)
+ return "", errOperationFailed
+ } else {
+ return string(originBytes), nil
+ }
+}
+
// generateID returns a new unique id string. The uniqueness is based on the
// current timestamp. Not cryptographically secure.
func generateID() string {
@@ -227,30 +239,53 @@
return nil
}
-func (i *appInvoker) Install(call ipc.ServerContext, applicationVON string) (string, error) {
+func fetchAppEnvelope(ctx context.T, origin string) (*application.Envelope, error) {
+ envelope, err := fetchEnvelope(ctx, origin)
+ if err != nil {
+ return nil, err
+ }
+ if envelope.Title == application.NodeManagerTitle {
+ // Disallow node manager apps from being installed like a
+ // regular app.
+ return nil, errInvalidOperation
+ }
+ return envelope, nil
+}
+
+// newVersion sets up the directory for a new application version.
+func newVersion(installationDir string, envelope *application.Envelope) (string, error) {
+ versionDir := filepath.Join(installationDir, generateVersionDirName())
+ if err := mkdir(versionDir); err != nil {
+ return "", errOperationFailed
+ }
+ // TODO(caprita): Share binaries if already existing locally.
+ if err := generateBinary(versionDir, "bin", envelope, true); err != nil {
+ return versionDir, err
+ }
+ if err := saveEnvelope(versionDir, envelope); err != nil {
+ return versionDir, err
+ }
+ // updateLink should be the last thing we do, after we've ensured the
+ // new version is viable (currently, that just means it installs
+ // properly).
+ return versionDir, updateLink(versionDir, filepath.Join(installationDir, "current"))
+}
+
+func (i *appInvoker) Install(_ ipc.ServerContext, applicationVON string) (string, error) {
if len(i.suffix) > 0 {
return "", errInvalidSuffix
}
ctx, cancel := rt.R().NewContext().WithTimeout(time.Minute)
defer cancel()
- envelope, err := fetchEnvelope(ctx, applicationVON)
+ envelope, err := fetchAppEnvelope(ctx, applicationVON)
if err != nil {
return "", err
}
- if envelope.Title == application.NodeManagerTitle {
- // Disallow node manager apps from being installed like a
- // regular app.
- return "", errInvalidOperation
- }
installationID := generateID()
installationDir := filepath.Join(i.config.Root, applicationDirName(envelope.Title), installationDirName(installationID))
- versionDir := filepath.Join(installationDir, generateVersionDirName())
- if err := mkdir(versionDir); err != nil {
- return "", errOperationFailed
- }
deferrer := func() {
if err := os.RemoveAll(installationDir); err != nil {
- vlog.Errorf("RemoveAll(%v) failed: %v", versionDir, err)
+ vlog.Errorf("RemoveAll(%v) failed: %v", installationDir, err)
}
}
defer func() {
@@ -258,21 +293,12 @@
deferrer()
}
}()
- // TODO(caprita): Share binaries if already existing locally.
- if err := generateBinary(versionDir, "bin", envelope, true); err != nil {
+ if _, err := newVersion(installationDir, envelope); err != nil {
return "", err
}
- if err := saveEnvelope(versionDir, envelope); err != nil {
+ if err := saveOrigin(installationDir, applicationVON); err != nil {
return "", err
}
- if err := saveOrigin(versionDir, applicationVON); err != nil {
- return "", err
- }
- link := filepath.Join(installationDir, "current")
- if err := os.Symlink(versionDir, link); err != nil {
- vlog.Errorf("Symlink(%v, %v) failed: %v", versionDir, link, err)
- return "", errOperationFailed
- }
if err := initializeInstallation(installationDir, active); err != nil {
return "", err
}
@@ -466,10 +492,8 @@
err = i.run(instanceDir)
}
if err != nil {
- if instanceDir != "" {
- if err := os.RemoveAll(instanceDir); err != nil {
- vlog.Errorf("RemoveAll(%v) failed: %v", instanceDir, err)
- }
+ if err := os.RemoveAll(instanceDir); err != nil {
+ vlog.Errorf("RemoveAll(%v) failed: %v", instanceDir, err)
}
return nil, err
}
@@ -577,8 +601,50 @@
return transitionInstallation(installationDir, active, uninstalled)
}
-func (*appInvoker) Update(ipc.ServerContext) error {
- // TODO(jsimsa): Implement.
+func (i *appInvoker) Update(ipc.ServerContext) error {
+ installationDir, err := i.installationDir()
+ if err != nil {
+ return err
+ }
+ if !installationStateIs(installationDir, active) {
+ return errInvalidOperation
+ }
+ originVON, err := loadOrigin(installationDir)
+ if err != nil {
+ return err
+ }
+ ctx, cancel := rt.R().NewContext().WithTimeout(time.Minute)
+ defer cancel()
+ newEnvelope, err := fetchAppEnvelope(ctx, originVON)
+ if err != nil {
+ return err
+ }
+ currLink := filepath.Join(installationDir, "current")
+ // NOTE(caprita): A race can occur between two competing updates, where
+ // both use the old version as their baseline. This can result in both
+ // updates succeeding even if they are updating the app installation to
+ // the same new envelope. This will result in one of the updates
+ // becoming the new 'current'. Both versions will point their
+ // 'previous' link to the old version. This doesn't appear to be of
+ // practical concern, so we avoid the complexity of synchronizing
+ // updates.
+ oldEnvelope, err := loadEnvelope(currLink)
+ if err != nil {
+ return err
+ }
+ if oldEnvelope.Title != newEnvelope.Title {
+ return errIncompatibleUpdate
+ }
+ if reflect.DeepEqual(oldEnvelope, newEnvelope) {
+ return errUpdateNoOp
+ }
+ versionDir, err := newVersion(installationDir, newEnvelope)
+ if err != nil {
+ if err := os.RemoveAll(versionDir); err != nil {
+ vlog.Errorf("RemoveAll(%v) failed: %v", versionDir, err)
+ }
+ return err
+ }
return nil
}
diff --git a/services/mgmt/node/impl/impl_test.go b/services/mgmt/node/impl/impl_test.go
index d6856f2..216178e 100644
--- a/services/mgmt/node/impl/impl_test.go
+++ b/services/mgmt/node/impl/impl_test.go
@@ -163,7 +163,6 @@
if err := ioutil.WriteFile("testfile", []byte("goodbye world"), 0600); err != nil {
vlog.Fatalf("Failed to write testfile: %v", err)
}
- ping()
}
// generateScript is very similar in behavior to its namesake in invoker.go.
@@ -277,7 +276,7 @@
// attempting an update while another one is ongoing will fail.
nm.Cmd.Env = exec.Setenv(nm.Cmd.Env, "PAUSE_BEFORE_STOP", "1")
- resolveExpectError(t, "factoryNM", verror.NotFound) // Ensure a clean slate.
+ resolveExpectNotFound(t, "factoryNM") // Ensure a clean slate.
// Start the node manager -- we use the blackbox-generated command to
// start it. We could have also used the scriptPathFactory to start it, but
@@ -294,7 +293,7 @@
}
}()
readPID(t, nm)
- resolve(t, "factoryNM") // Verify the node manager has published itself.
+ resolve(t, "factoryNM", 1) // Verify the node manager has published itself.
// Simulate an invalid envelope in the application repository.
*envelope = *nodeEnvelopeFromCmd(nm.Cmd)
@@ -341,13 +340,13 @@
// A successful update means the node manager has stopped itself. We
// relaunch it from the current link.
runNM := blackbox.HelperCommand(t, "execScript", currLink)
- resolveExpectError(t, "v2NM", verror.NotFound) // Ensure a clean slate.
+ resolveExpectNotFound(t, "v2NM") // Ensure a clean slate.
if err := runNM.Cmd.Start(); err != nil {
t.Fatalf("Start() failed: %v", err)
}
deferrer = runNM.Cleanup
readPID(t, runNM)
- resolve(t, "v2NM") // Current link should have been launching v2.
+ resolve(t, "v2NM", 1) // Current link should have been launching v2.
// Try issuing an update without changing the envelope in the application
// repository: this should fail, and current link should be unchanged.
@@ -383,13 +382,13 @@
// that we can verify that a second revert fails while a revert is in
// progress.
runNM.Cmd.Env = exec.Setenv(nm.Cmd.Env, "PAUSE_BEFORE_STOP", "1")
- resolveExpectError(t, "v3NM", verror.NotFound) // Ensure a clean slate.
+ resolveExpectNotFound(t, "v3NM") // Ensure a clean slate.
if err := runNM.Cmd.Start(); err != nil {
t.Fatalf("Start() failed: %v", err)
}
deferrer = runNM.Cleanup
readPID(t, runNM)
- resolve(t, "v3NM") // Current link should have been launching v3.
+ resolve(t, "v3NM", 1) // Current link should have been launching v3.
// Revert the node manager to its previous version (v2).
revert(t, "v3NM")
@@ -404,13 +403,13 @@
// Re-launch the node manager from current link.
runNM = blackbox.HelperCommand(t, "execScript", currLink)
- resolveExpectError(t, "v2NM", verror.NotFound) // Ensure a clean slate.
+ resolveExpectNotFound(t, "v2NM") // Ensure a clean slate.
if err := runNM.Cmd.Start(); err != nil {
t.Fatalf("Start() failed: %v", err)
}
deferrer = runNM.Cleanup
readPID(t, runNM)
- resolve(t, "v2NM") // Current link should have been launching v2.
+ resolve(t, "v2NM", 1) // Current link should have been launching v2.
// Revert the node manager to its previous version (factory).
revert(t, "v2NM")
@@ -423,13 +422,13 @@
// Re-launch the node manager from current link.
runNM = blackbox.HelperCommand(t, "execScript", currLink)
- resolveExpectError(t, "factoryNM", verror.NotFound) // Ensure a clean slate.
+ resolveExpectNotFound(t, "factoryNM") // Ensure a clean slate.
if err := runNM.Cmd.Start(); err != nil {
t.Fatalf("Start() failed: %v", err)
}
deferrer = runNM.Cleanup
pid := readPID(t, runNM)
- resolve(t, "factoryNM") // Current link should have been launching factory version.
+ resolve(t, "factoryNM", 1) // Current link should have been launching factory version.
syscall.Kill(pid, syscall.SIGINT)
runNM.Expect("factoryNM terminating")
runNM.ExpectEOFAndWait()
@@ -439,13 +438,18 @@
func (p pingServerDisp) Ping(ipc.ServerCall) { p <- struct{}{} }
-func installApp(t *testing.T) string {
+func appStub(t *testing.T, nameComponents ...string) node.Application {
appsName := "nm//apps"
- stub, err := node.BindApplication(appsName)
+ appName := naming.Join(append([]string{appsName}, nameComponents...)...)
+ stub, err := node.BindApplication(appName)
if err != nil {
- t.Fatalf("BindApplication(%v) failed: %v", appsName, err)
+ t.Fatalf("BindApplication(%v) failed: %v", appName, err)
}
- appID, err := stub.Install(rt.R().NewContext(), "ar")
+ return stub
+}
+
+func installApp(t *testing.T) string {
+ appID, err := appStub(t).Install(rt.R().NewContext(), "ar")
if err != nil {
t.Fatalf("Install failed: %v", err)
}
@@ -453,17 +457,11 @@
}
func startAppImpl(t *testing.T, appID string) (string, error) {
- appsName := "nm//apps"
- appName := naming.Join(appsName, appID)
- stub, err := node.BindApplication(appName)
- if err != nil {
- t.Fatalf("BindApplication(%v) failed: %v", appName, err)
- }
- if instanceIDs, err := stub.Start(rt.R().NewContext()); err != nil {
+ if instanceIDs, err := appStub(t, appID).Start(rt.R().NewContext()); err != nil {
return "", err
} else {
if want, got := 1, len(instanceIDs); want != got {
- t.Fatalf("Expected %v instance ids, got %v instead", want, got)
+ t.Fatalf("Start(%v): expected %v instance ids, got %v instead", appID, want, got)
}
return instanceIDs[0], nil
}
@@ -472,7 +470,7 @@
func startApp(t *testing.T, appID string) string {
instanceID, err := startAppImpl(t, appID)
if err != nil {
- t.Fatalf("Start failed: %v", err)
+ t.Fatalf("Start(%v) failed: %v", appID, err)
}
return instanceID
}
@@ -484,53 +482,38 @@
}
func stopApp(t *testing.T, appID, instanceID string) {
- appsName := "nm//apps"
- appName := naming.Join(appsName, appID)
- instanceName := naming.Join(appName, instanceID)
- stub, err := node.BindApplication(instanceName)
- if err != nil {
- t.Fatalf("BindApplication(%v) failed: %v", instanceName, err)
- }
- if err := stub.Stop(rt.R().NewContext(), 5); err != nil {
- t.Fatalf("Stop failed: %v", err)
+ if err := appStub(t, appID, instanceID).Stop(rt.R().NewContext(), 5); err != nil {
+ t.Fatalf("Stop(%v/%v) failed: %v", appID, instanceID, err)
}
}
func suspendApp(t *testing.T, appID, instanceID string) {
- appsName := "nm//apps"
- appName := naming.Join(appsName, appID)
- instanceName := naming.Join(appName, instanceID)
- stub, err := node.BindApplication(instanceName)
- if err != nil {
- t.Fatalf("BindApplication(%v) failed: %v", instanceName, err)
- }
- if err := stub.Suspend(rt.R().NewContext()); err != nil {
- t.Fatalf("Suspend failed: %v", err)
+ if err := appStub(t, appID, instanceID).Suspend(rt.R().NewContext()); err != nil {
+ t.Fatalf("Suspend(%v/%v) failed: %v", appID, instanceID, err)
}
}
func resumeApp(t *testing.T, appID, instanceID string) {
- appsName := "nm//apps"
- appName := naming.Join(appsName, appID)
- instanceName := naming.Join(appName, instanceID)
- stub, err := node.BindApplication(instanceName)
- if err != nil {
- t.Fatalf("BindApplication(%v) failed: %v", instanceName, err)
+ if err := appStub(t, appID, instanceID).Resume(rt.R().NewContext()); err != nil {
+ t.Fatalf("Resume(%v/%v) failed: %v", appID, instanceID, err)
}
- if err := stub.Resume(rt.R().NewContext()); err != nil {
- t.Fatalf("Resume failed: %v", err)
+}
+
+func updateApp(t *testing.T, appID string) {
+ if err := appStub(t, appID).Update(rt.R().NewContext()); err != nil {
+ t.Fatalf("Update(%v) failed: %v", appID, err)
+ }
+}
+
+func updateAppShouldFail(t *testing.T, appID string, expectedError verror.ID) {
+ if err := appStub(t, appID).Update(rt.R().NewContext()); err == nil || !verror.Is(err, expectedError) {
+ t.Fatalf("Update(%v) expected to fail with %v, got %v instead", appID, expectedError, err)
}
}
func uninstallApp(t *testing.T, appID string) {
- appsName := "nm//apps"
- appName := naming.Join(appsName, appID)
- stub, err := node.BindApplication(appName)
- if err != nil {
- t.Fatalf("BindApplication(%v) failed: %v", appName, err)
- }
- if err := stub.Uninstall(rt.R().NewContext()); err != nil {
- t.Fatalf("Uninstall failed: %v", err)
+ if err := appStub(t, appID).Uninstall(rt.R().NewContext()); err != nil {
+ t.Fatalf("Uninstall(%v) failed: %v", appID, err)
}
}
@@ -590,8 +573,8 @@
t.Fatalf("Serve(%q, <dispatcher>) failed: %v", "pingserver", err)
}
- // Create an envelope for an app.
- app := blackbox.HelperCommand(t, "app", "app1")
+ // Create an envelope for a first version of the app.
+ app := blackbox.HelperCommand(t, "app", "appV1")
defer setupChildCommand(app)()
appTitle := "google naps"
*envelope = *envelopeFromCmd(appTitle, app.Cmd)
@@ -599,30 +582,111 @@
// Install the app.
appID := installApp(t)
- // Start the app.
- instanceID := startApp(t, appID)
+ // Start an instance of the app.
+ instance1ID := startApp(t, appID)
+ <-pingCh // Wait until the app pings us that it's ready.
+ v1EP1 := resolve(t, "appV1", 1)[0]
+
+ // Suspend the app instance.
+ suspendApp(t, appID, instance1ID)
+ resolveExpectNotFound(t, "appV1")
+
+ resumeApp(t, appID, instance1ID)
+ <-pingCh
+ oldV1EP1 := v1EP1
+ if v1EP1 = resolve(t, "appV1", 1)[0]; v1EP1 == oldV1EP1 {
+ t.Fatalf("Expected a new endpoint for the app after suspend/resume")
+ }
+
+ // Start a second instance.
+ instance2ID := startApp(t, appID)
<-pingCh // Wait until the app pings us that it's ready.
- // Suspend the app.
- suspendApp(t, appID, instanceID)
- <-pingCh // App should have pinged us before it terminated.
-
- resumeApp(t, appID, instanceID)
- <-pingCh
+ // There should be two endpoints mounted as "appV1", one for each
+ // instance of the app.
+ endpoints := resolve(t, "appV1", 2)
+ v1EP2 := endpoints[0]
+ if endpoints[0] == v1EP1 {
+ v1EP2 = endpoints[1]
+ if v1EP2 == v1EP1 {
+ t.Fatalf("Both endpoints are the same")
+ }
+ } else if endpoints[1] != v1EP1 {
+ t.Fatalf("Second endpoint should have been v1EP1: %v, %v", endpoints, v1EP1)
+ }
// TODO(caprita): test Suspend and Resume, and verify various
// non-standard combinations (suspend when stopped; resume while still
// running; stop while suspended).
- // Stop the app.
- stopApp(t, appID, instanceID)
- <-pingCh // App should have pinged us before it terminated.
+ // Suspend the first instance.
+ suspendApp(t, appID, instance1ID)
+ // Only the second instance should still be running and mounted.
+ if want, got := v1EP2, resolve(t, "appV1", 1)[0]; want != got {
+ t.Fatalf("Resolve(%v): want: %v, got %v", "appV1", want, got)
+ }
- verifyAppWorkspace(t, root, appID, instanceID)
+ // Updating the installation to itself is a no-op.
+ updateAppShouldFail(t, appID, verror.NotFound)
+
+ // Updating the installation should not work with a mismatched title.
+ *envelope = *envelopeFromCmd("bogus", app.Cmd)
+ updateAppShouldFail(t, appID, verror.BadArg)
+
+ // Create a second version of the app and update the app to it.
+ app = blackbox.HelperCommand(t, "app", "appV2")
+ defer setupChildCommand(app)()
+ *envelope = *envelopeFromCmd(appTitle, app.Cmd)
+ updateApp(t, appID)
+
+ // Second instance should still be running.
+ if want, got := v1EP2, resolve(t, "appV1", 1)[0]; want != got {
+ t.Fatalf("Resolve(%v): want: %v, got %v", "appV1", want, got)
+ }
+
+ // Resume first instance.
+ resumeApp(t, appID, instance1ID)
+ <-pingCh
+ // Both instances should still be running the first version of the app.
+ // Check that the mounttable contains two endpoints, one of which is
+ // v1EP2.
+ endpoints = resolve(t, "appV1", 2)
+ if endpoints[0] == v1EP2 {
+ if endpoints[1] == v1EP2 {
+ t.Fatalf("Both endpoints are the same")
+ }
+ } else if endpoints[1] != v1EP2 {
+ t.Fatalf("Second endpoint should have been v1EP2: %v, %v", endpoints, v1EP2)
+ }
+
+ // Stop first instance.
+ stopApp(t, appID, instance1ID)
+ verifyAppWorkspace(t, root, appID, instance1ID)
+
+ // Only second instance is still running.
+ if want, got := v1EP2, resolve(t, "appV1", 1)[0]; want != got {
+ t.Fatalf("Resolve(%v): want: %v, got %v", "appV1", want, got)
+ }
+
+ // Start a third instance.
+ instance3ID := startApp(t, appID)
+ <-pingCh // Wait until the app pings us that it's ready.
+ resolve(t, "appV2", 1)
+
+ // Stop second instance.
+ stopApp(t, appID, instance2ID)
+ resolveExpectNotFound(t, "appV1")
+
+ // Stop third instance.
+ stopApp(t, appID, instance3ID)
+ resolveExpectNotFound(t, "appV2")
// Uninstall the app.
uninstallApp(t, appID)
+ // Updating the installation should no longer be allowed.
+ updateAppShouldFail(t, appID, verror.BadArg)
+
// Starting new instances should no longer be allowed.
startAppShouldFail(t, appID, verror.BadArg)
@@ -688,7 +752,7 @@
readPID(t, nm)
// Create an envelope for an app.
- app := blackbox.HelperCommand(t, "app", "app1")
+ app := blackbox.HelperCommand(t, "app", "")
defer setupChildCommand(app)()
appTitle := "google naps"
*envelope = *envelopeFromCmd(appTitle, app.Cmd)
diff --git a/services/mgmt/node/impl/node_invoker.go b/services/mgmt/node/impl/node_invoker.go
index 0557d7e..19e78ae 100644
--- a/services/mgmt/node/impl/node_invoker.go
+++ b/services/mgmt/node/impl/node_invoker.go
@@ -149,8 +149,8 @@
return link, scriptPath, nil
}
-func (i *nodeInvoker) updateLink(newScript string) error {
- link := i.config.CurrentLink
+// TODO(caprita): Move updateLink to util.go now that app_invoker also uses it.
+func updateLink(newScript, link string) error {
newLink := link + ".new"
fi, err := os.Lstat(newLink)
if err == nil {
@@ -171,7 +171,7 @@
}
func (i *nodeInvoker) revertNodeManager() error {
- if err := i.updateLink(i.config.Previous); err != nil {
+ if err := updateLink(i.config.Previous, i.config.CurrentLink); err != nil {
return err
}
rt.R().Stop()
@@ -239,7 +239,7 @@
}
// Ensure that the current symbolic link points to the same script.
if pathNew != pathOld {
- i.updateLink(pathOld)
+ updateLink(pathOld, i.config.CurrentLink)
vlog.Errorf("new node manager test failed")
return errOperationFailed
}
@@ -314,7 +314,7 @@
return err
}
// If the binary has changed, update the node manager symlink.
- if err := i.updateLink(filepath.Join(workspace, "noded.sh")); err != nil {
+ if err := updateLink(filepath.Join(workspace, "noded.sh"), i.config.CurrentLink); err != nil {
return err
}
rt.R().Stop()
diff --git a/services/mgmt/node/impl/util_test.go b/services/mgmt/node/impl/util_test.go
index d2fed3d..5b7d4c9 100644
--- a/services/mgmt/node/impl/util_test.go
+++ b/services/mgmt/node/impl/util_test.go
@@ -89,24 +89,24 @@
}
// resolveExpectError verifies that the given name is not in the mounttable.
-func resolveExpectError(t *testing.T, name string, errID verror.ID) {
+func resolveExpectNotFound(t *testing.T, name string) {
if results, err := rt.R().Namespace().Resolve(rt.R().NewContext(), name); err == nil {
t.Fatalf("Resolve(%v) succeeded with results %v when it was expected to fail", name, results)
- } else if !verror.Is(err, errID) {
- t.Fatalf("Resolve(%v) failed with error %v, expected error ID %v", err, errID)
+ } else if expectErr := verror.NotFound; !verror.Is(err, expectErr) {
+ t.Fatalf("Resolve(%v) failed with error %v, expected error ID %v", err, expectErr)
}
}
// resolve looks up the given name in the mounttable.
-func resolve(t *testing.T, name string) string {
+func resolve(t *testing.T, name string, replicas int) []string {
results, err := rt.R().Namespace().Resolve(rt.R().NewContext(), name)
if err != nil {
t.Fatalf("Resolve(%v) failed: %v", name, err)
}
- if want, got := 1, len(results); want != got {
+ if want, got := replicas, len(results); want != got {
t.Fatalf("Resolve(%v) expected %d result(s), got %d instead", name, want, got)
}
- return results[0]
+ return results
}
// The following set of functions are convenience wrappers around Update and
