services/device/device: updateall: kill instance only if running; recover if err
Changing updateall to only attempt to kill an instance if the instance is found
to be running. If we then try Kill and it returns an error, we still check if
it's running or not (since the device manager force-kills the app) and recover.
Before, we used to leave the app not running and exit, which not only left the
updates in a partial state, but also left previously running instances dead.
Also, update the kill deadline to 10 seconds, and use it consistently between
the 'kill' and 'updateall' commands.
Change-Id: Ib129b94536739b385401d68f509e860a786d7650
diff --git a/services/device/device/doc.go b/services/device/device/doc.go
index 53c9a17..3fe77dd 100644
--- a/services/device/device/doc.go
+++ b/services/device/device/doc.go
@@ -295,7 +295,7 @@
<object name> is the vanadium object name to update, as follows:
<devicename>/apps/apptitle/installationid/instanceid: updates the given
-instance, suspending/resuming it if running
+instance, killing/restarting it if running
<devicename>/apps/apptitle/installationid: updates the given installation and
then all its instances
diff --git a/services/device/device/instance_impl.go b/services/device/device/instance_impl.go
index 8f20c8d..7a3febd 100644
--- a/services/device/device/instance_impl.go
+++ b/services/device/device/instance_impl.go
@@ -37,6 +37,8 @@
return nil
}
+const killDeadline = 10 * time.Second
+
var cmdKill = &cmdline.Command{
Run: runKill,
Name: "kill",
@@ -53,7 +55,7 @@
}
appName := args[0]
- if err := device.ApplicationClient(appName).Kill(gctx, 5*time.Second); err != nil {
+ if err := device.ApplicationClient(appName).Kill(gctx, killDeadline); err != nil {
return fmt.Errorf("Kill failed: %v", err)
}
fmt.Fprintf(cmd.Stdout(), "Kill succeeded\n")
diff --git a/services/device/device/instance_impl_test.go b/services/device/device/instance_impl_test.go
index e58d1e1..fdbe781 100644
--- a/services/device/device/instance_impl_test.go
+++ b/services/device/device/instance_impl_test.go
@@ -68,7 +68,7 @@
t.Fatalf("Unexpected output from list. Got %q, expected %q", got, expected)
}
expected := []interface{}{
- KillStimulus{"Kill", 5 * time.Second},
+ KillStimulus{"Kill", 10 * time.Second},
}
if got := appTape.Play(); !reflect.DeepEqual(expected, got) {
t.Errorf("invalid call sequence. Got %v, want %v", got, expected)
diff --git a/services/device/device/updateall.go b/services/device/device/updateall.go
index 4a4e57f..c660863 100644
--- a/services/device/device/updateall.go
+++ b/services/device/device/updateall.go
@@ -33,7 +33,7 @@
ArgsLong: `
<object name> is the vanadium object name to update, as follows:
-<devicename>/apps/apptitle/installationid/instanceid: updates the given instance, suspending/resuming it if running
+<devicename>/apps/apptitle/installationid/instanceid: updates the given instance, killing/restarting it if running
<devicename>/apps/apptitle/installationid: updates the given installation and then all its instances
@@ -83,6 +83,18 @@
return nil
}
+func instanceIsRunning(von string) (bool, error) {
+ status, err := device.ApplicationClient(von).Status(gctx)
+ if err != nil {
+ return false, fmt.Errorf("Failed to get status for instance %q: %v", von, err)
+ }
+ s, ok := status.(device.StatusInstance)
+ if !ok {
+ return false, fmt.Errorf("Status for instance %q of wrong type (%T)", von, status)
+ }
+ return s.Value.State == device.InstanceStateRunning, nil
+}
+
func updateInstance(cmd *cmdline.Command, von string) (retErr error) {
defer func() {
if retErr == nil {
@@ -92,9 +104,27 @@
fmt.Fprintf(cmd.Stderr(), "ERROR: %v.\n", retErr)
}
}()
- // Try killing the app in case it was running.
- switch err := device.ApplicationClient(von).Kill(gctx, 5*time.Second); {
- case err == nil:
+ running, err := instanceIsRunning(von)
+ if err != nil {
+ return err
+ }
+ if running {
+ // Try killing the app.
+ if err := device.ApplicationClient(von).Kill(gctx, killDeadline); err != nil {
+ // Check the app's state again in case we killed it,
+ // nevermind any errors. The sleep is because Kill
+ // currently (4/29/15) returns asynchronously with the
+ // device manager shooting the app down.
+ time.Sleep(time.Second)
+ running, rerr := instanceIsRunning(von)
+ if rerr != nil {
+ return rerr
+ }
+ if running {
+ return fmt.Errorf("failed to kill instance %q: %v", von, err)
+ }
+ fmt.Fprintf(cmd.Stderr(), "Kill(%s) returned an error (%s) but app is now not running.\n", von, err)
+ }
// App was running, and we killed it.
defer func() {
// Re-start the instance.
@@ -107,21 +137,14 @@
}
}
}()
- case verror.ErrorID(err) == deviceimpl.ErrInvalidOperation.ID:
- // App was likely not running.
- //
- // TODO(caprita): change returned error to distinguish no-op
- // app sttate transitions from failed state transitions.
- default:
- return fmt.Errorf("failed to suspend instance %q: %v.\n", von, err)
}
// Update the instance.
switch err := device.ApplicationClient(von).Update(gctx); {
case err == nil:
return nil
case verror.ErrorID(err) == deviceimpl.ErrUpdateNoOp.ID:
- // TODO(caprita): Ideally, we wouldn't even attempt a suspend /
- // resume if there's no newer version of the application.
+ // TODO(caprita): Ideally, we wouldn't even attempt a kill /
+ // restart if there's no newer version of the application.
fmt.Fprintf(cmd.Stdout(), "Instance %q already up to date.\n", von)
return nil
default:
