veyron/services/mgmt/node/impl: knock off a couple easy TODOs by shuffling some
code around and renaming some methods.
Change-Id: I63d272e8cdd93010fee92a2b470af0533f56554d
diff --git a/services/mgmt/node/impl/impl_test.go b/services/mgmt/node/impl/impl_test.go
index 7764dec..21f4d94 100644
--- a/services/mgmt/node/impl/impl_test.go
+++ b/services/mgmt/node/impl/impl_test.go
@@ -298,8 +298,8 @@
// Simulate an invalid envelope in the application repository.
*envelope = *nodeEnvelopeFromCmd(nm.Cmd)
envelope.Title = "bogus"
- updateExpectError(t, "factoryNM", verror.BadArg) // Incorrect title.
- revertExpectError(t, "factoryNM", verror.NotFound) // No previous version available.
+ updateNodeExpectError(t, "factoryNM", verror.BadArg) // Incorrect title.
+ revertNodeExpectError(t, "factoryNM", verror.NotFound) // No previous version available.
// Set up a second version of the node manager. We use the blackbox
// command solely to collect the args and env we need to provide the
@@ -310,7 +310,7 @@
nmV2 := blackbox.HelperCommand(t, "nodeManager", "v2NM")
defer setupChildCommand(nmV2)()
*envelope = *nodeEnvelopeFromCmd(nmV2.Cmd)
- update(t, "factoryNM")
+ updateNode(t, "factoryNM")
// Current link should have been updated to point to v2.
evalLink := func() string {
@@ -330,7 +330,7 @@
readPID(t, nm)
nm.Expect("v2NM terminating")
- updateExpectError(t, "factoryNM", verror.Exists) // Update already in progress.
+ updateNodeExpectError(t, "factoryNM", verror.Exists) // Update already in progress.
nm.CloseStdin()
nm.Expect("factoryNM terminating")
@@ -350,7 +350,7 @@
// Try issuing an update without changing the envelope in the application
// repository: this should fail, and current link should be unchanged.
- updateExpectError(t, "v2NM", verror.NotFound)
+ updateNodeExpectError(t, "v2NM", verror.NotFound)
if evalLink() != scriptPathV2 {
t.Fatalf("script changed")
}
@@ -359,7 +359,7 @@
nmV3 := blackbox.HelperCommand(t, "nodeManager", "v3NM")
defer setupChildCommand(nmV3)()
*envelope = *nodeEnvelopeFromCmd(nmV3.Cmd)
- update(t, "v2NM")
+ updateNode(t, "v2NM")
scriptPathV3 := evalLink()
if scriptPathV3 == scriptPathV2 {
@@ -391,8 +391,8 @@
resolve(t, "v3NM", 1) // Current link should have been launching v3.
// Revert the node manager to its previous version (v2).
- revert(t, "v3NM")
- revertExpectError(t, "v3NM", verror.Exists) // Revert already in progress.
+ revertNode(t, "v3NM")
+ revertNodeExpectError(t, "v3NM", verror.Exists) // Revert already in progress.
runNM.CloseStdin()
runNM.Expect("v3NM terminating")
if evalLink() != scriptPathV2 {
@@ -412,7 +412,7 @@
resolve(t, "v2NM", 1) // Current link should have been launching v2.
// Revert the node manager to its previous version (factory).
- revert(t, "v2NM")
+ revertNode(t, "v2NM")
runNM.Expect("v2NM terminating")
if evalLink() != scriptPathFactory {
t.Fatalf("current link was not reverted correctly")
@@ -438,99 +438,6 @@
func (p pingServerDisp) Ping(ipc.ServerCall) { p <- struct{}{} }
-func appStub(t *testing.T, nameComponents ...string) node.Application {
- appsName := "nm//apps"
- appName := naming.Join(append([]string{appsName}, nameComponents...)...)
- stub, err := node.BindApplication(appName)
- if err != nil {
- t.Fatalf("BindApplication(%v) failed: %v", appName, err)
- }
- 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)
- }
- return appID
-}
-
-func startAppImpl(t *testing.T, appID string) (string, error) {
- 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("Start(%v): expected %v instance ids, got %v instead", appID, want, got)
- }
- return instanceIDs[0], nil
- }
-}
-
-func startApp(t *testing.T, appID string) string {
- instanceID, err := startAppImpl(t, appID)
- if err != nil {
- t.Fatalf("Start(%v) failed: %v", appID, err)
- }
- return instanceID
-}
-
-// TODO(caprita): Rename the *ShouldFail methods to *ExpectError (to match the
-// similar methods on node). Also move to util_test.go.
-func startAppShouldFail(t *testing.T, appID string, expectedError verror.ID) {
- if _, err := startAppImpl(t, appID); err == nil || !verror.Is(err, expectedError) {
- t.Fatalf("Start(%v) expected to fail with %v, got %v instead", appID, expectedError, err)
- }
-}
-
-func stopApp(t *testing.T, appID, instanceID string) {
- 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) {
- 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) {
- if err := appStub(t, appID, instanceID).Resume(rt.R().NewContext()); err != nil {
- t.Fatalf("Resume(%v/%v) failed: %v", appID, instanceID, 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 revertApp(t *testing.T, appID string) {
- if err := appStub(t, appID).Revert(rt.R().NewContext()); err != nil {
- t.Fatalf("Revert(%v) failed: %v", appID, err)
- }
-}
-
-func revertAppShouldFail(t *testing.T, appID string, expectedError verror.ID) {
- if err := appStub(t, appID).Revert(rt.R().NewContext()); err == nil || !verror.Is(err, expectedError) {
- t.Fatalf("Revert(%v) expected to fail with %v, got %v instead", appID, expectedError, err)
- }
-}
-
-func uninstallApp(t *testing.T, appID string) {
- if err := appStub(t, appID).Uninstall(rt.R().NewContext()); err != nil {
- t.Fatalf("Uninstall(%v) failed: %v", appID, err)
- }
-}
-
func verifyAppWorkspace(t *testing.T, root, appID, instanceID string) {
// HACK ALERT: for now, we peek inside the node manager's directory
// structure (which ought to be opaque) to check for what the app has
@@ -641,11 +548,11 @@
}
// Updating the installation to itself is a no-op.
- updateAppShouldFail(t, appID, verror.NotFound)
+ updateAppExpectError(t, appID, verror.NotFound)
// Updating the installation should not work with a mismatched title.
*envelope = *envelopeFromCmd("bogus", app.Cmd)
- updateAppShouldFail(t, appID, verror.BadArg)
+ updateAppExpectError(t, appID, verror.BadArg)
// Create a second version of the app and update the app to it.
app = blackbox.HelperCommand(t, "app", "appV2")
@@ -706,19 +613,19 @@
resolveExpectNotFound(t, "appV1")
// We are already on the first version, no further revert possible.
- revertAppShouldFail(t, appID, verror.NotFound)
+ revertAppExpectError(t, appID, verror.NotFound)
// Uninstall the app.
uninstallApp(t, appID)
// Updating the installation should no longer be allowed.
- updateAppShouldFail(t, appID, verror.BadArg)
+ updateAppExpectError(t, appID, verror.BadArg)
// Reverting the installation should no longer be allowed.
- revertAppShouldFail(t, appID, verror.BadArg)
+ revertAppExpectError(t, appID, verror.BadArg)
// Starting new instances should no longer be allowed.
- startAppShouldFail(t, appID, verror.BadArg)
+ startAppExpectError(t, appID, verror.BadArg)
// Cleanly shut down the node manager.
syscall.Kill(nm.Cmd.Process.Pid, syscall.SIGINT)
diff --git a/services/mgmt/node/impl/node_invoker.go b/services/mgmt/node/impl/node_invoker.go
index 701d50e..837caf3 100644
--- a/services/mgmt/node/impl/node_invoker.go
+++ b/services/mgmt/node/impl/node_invoker.go
@@ -149,27 +149,6 @@
return link, scriptPath, nil
}
-// TODO(caprita): Move updateLink to util.go now that app_invoker also uses it.
-func updateLink(target, link string) error {
- newLink := link + ".new"
- fi, err := os.Lstat(newLink)
- if err == nil {
- if err := os.Remove(fi.Name()); err != nil {
- vlog.Errorf("Remove(%v) failed: %v", fi.Name(), err)
- return errOperationFailed
- }
- }
- if err := os.Symlink(target, newLink); err != nil {
- vlog.Errorf("Symlink(%v, %v) failed: %v", target, newLink, err)
- return errOperationFailed
- }
- if err := os.Rename(newLink, link); err != nil {
- vlog.Errorf("Rename(%v, %v) failed: %v", newLink, link, err)
- return errOperationFailed
- }
- return nil
-}
-
func (i *nodeInvoker) revertNodeManager() error {
if err := updateLink(i.config.Previous, i.config.CurrentLink); err != nil {
return err
diff --git a/services/mgmt/node/impl/util.go b/services/mgmt/node/impl/util.go
index 3524911..44a2c8e 100644
--- a/services/mgmt/node/impl/util.go
+++ b/services/mgmt/node/impl/util.go
@@ -62,3 +62,23 @@
func generateVersionDirName() string {
return time.Now().Format(time.RFC3339Nano)
}
+
+func updateLink(target, link string) error {
+ newLink := link + ".new"
+ fi, err := os.Lstat(newLink)
+ if err == nil {
+ if err := os.Remove(fi.Name()); err != nil {
+ vlog.Errorf("Remove(%v) failed: %v", fi.Name(), err)
+ return errOperationFailed
+ }
+ }
+ if err := os.Symlink(target, newLink); err != nil {
+ vlog.Errorf("Symlink(%v, %v) failed: %v", target, newLink, err)
+ return errOperationFailed
+ }
+ if err := os.Rename(newLink, link); err != nil {
+ vlog.Errorf("Rename(%v, %v) failed: %v", newLink, link, err)
+ return errOperationFailed
+ }
+ return nil
+}
diff --git a/services/mgmt/node/impl/util_test.go b/services/mgmt/node/impl/util_test.go
index 5b7d4c9..ecb6f79 100644
--- a/services/mgmt/node/impl/util_test.go
+++ b/services/mgmt/node/impl/util_test.go
@@ -110,46 +110,131 @@
}
// The following set of functions are convenience wrappers around Update and
-// Revert.
+// Revert for node manager.
-func updateExpectError(t *testing.T, name string, errID verror.ID) {
- if err := invokeUpdate(t, name); !verror.Is(err, errID) {
- t.Fatalf("Unexpected update on %s error %v, expected error ID %v", name, err, errID)
- }
-}
-
-func update(t *testing.T, name string) {
- if err := invokeUpdate(t, name); err != nil {
- t.Fatalf("Update() on %s failed: %v", name, err)
- }
-}
-
-func invokeUpdate(t *testing.T, name string) error {
- address := naming.Join(name, "nm")
- stub, err := node.BindNode(address)
+func nodeStub(t *testing.T, name string) node.Node {
+ nodeName := naming.Join(name, "nm")
+ stub, err := node.BindNode(nodeName)
if err != nil {
- t.Fatalf("BindNode(%v) failed: %v", address, err)
+ t.Fatalf("BindNode(%v) failed: %v", nodeName, err)
}
- return stub.Update(rt.R().NewContext())
+ return stub
}
-func revertExpectError(t *testing.T, name string, errID verror.ID) {
- if err := invokeRevert(t, name); !verror.Is(err, errID) {
- t.Fatalf("Unexpected revert error %v, expected error ID %v", err, errID)
+func updateNodeExpectError(t *testing.T, name string, errID verror.ID) {
+ if err := nodeStub(t, name).Update(rt.R().NewContext()); !verror.Is(err, errID) {
+ t.Fatalf("Update(%v) expected to fail with %v, got %v instead", name, errID, err)
}
}
-func revert(t *testing.T, name string) {
- if err := invokeRevert(t, name); err != nil {
- t.Fatalf("Revert() failed: %v", err)
+func updateNode(t *testing.T, name string) {
+ if err := nodeStub(t, name).Update(rt.R().NewContext()); err != nil {
+ t.Fatalf("Update(%v) failed: %v", name, err)
}
}
-func invokeRevert(t *testing.T, name string) error {
- address := naming.Join(name, "nm")
- stub, err := node.BindNode(address)
+func revertNodeExpectError(t *testing.T, name string, errID verror.ID) {
+ if err := nodeStub(t, name).Revert(rt.R().NewContext()); !verror.Is(err, errID) {
+ t.Fatalf("Revert(%v) expected to fail with %v, got %v instead", name, errID, err)
+ }
+}
+
+func revertNode(t *testing.T, name string) {
+ if err := nodeStub(t, name).Revert(rt.R().NewContext()); err != nil {
+ t.Fatalf("Revert(%v) failed: %v", name, err)
+ }
+}
+
+// The following set of functions are convenience wrappers around various app
+// management methods.
+
+func appStub(t *testing.T, nameComponents ...string) node.Application {
+ appsName := "nm//apps"
+ appName := naming.Join(append([]string{appsName}, nameComponents...)...)
+ stub, err := node.BindApplication(appName)
if err != nil {
- t.Fatalf("BindNode(%v) failed: %v", address, err)
+ t.Fatalf("BindApplication(%v) failed: %v", appName, err)
}
- return stub.Revert(rt.R().NewContext())
+ 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)
+ }
+ return appID
+}
+
+func startAppImpl(t *testing.T, appID string) (string, error) {
+ 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("Start(%v): expected %v instance ids, got %v instead", appID, want, got)
+ }
+ return instanceIDs[0], nil
+ }
+}
+
+func startApp(t *testing.T, appID string) string {
+ instanceID, err := startAppImpl(t, appID)
+ if err != nil {
+ t.Fatalf("Start(%v) failed: %v", appID, err)
+ }
+ return instanceID
+}
+
+func startAppExpectError(t *testing.T, appID string, expectedError verror.ID) {
+ if _, err := startAppImpl(t, appID); err == nil || !verror.Is(err, expectedError) {
+ t.Fatalf("Start(%v) expected to fail with %v, got %v instead", appID, expectedError, err)
+ }
+}
+
+func stopApp(t *testing.T, appID, instanceID string) {
+ 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) {
+ 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) {
+ if err := appStub(t, appID, instanceID).Resume(rt.R().NewContext()); err != nil {
+ t.Fatalf("Resume(%v/%v) failed: %v", appID, instanceID, 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 updateAppExpectError(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 revertApp(t *testing.T, appID string) {
+ if err := appStub(t, appID).Revert(rt.R().NewContext()); err != nil {
+ t.Fatalf("Revert(%v) failed: %v", appID, err)
+ }
+}
+
+func revertAppExpectError(t *testing.T, appID string, expectedError verror.ID) {
+ if err := appStub(t, appID).Revert(rt.R().NewContext()); err == nil || !verror.Is(err, expectedError) {
+ t.Fatalf("Revert(%v) expected to fail with %v, got %v instead", appID, expectedError, err)
+ }
+}
+
+func uninstallApp(t *testing.T, appID string) {
+ if err := appStub(t, appID).Uninstall(rt.R().NewContext()); err != nil {
+ t.Fatalf("Uninstall(%v) failed: %v", appID, err)
+ }
}
