veyron/services/mgmt/device/impl: implement per-instance updates
A suspended instance can now be updated to the latest version of its
installation.
This cl also include a couple minor unrelated cleanups that I noticed while
working on the update change:
- move the initialization of the instance and installation state to the very end
of their respective setup methods
- change setupPrincipal to take the blessing extension as an arg, instead of
letting it extract the title from the envelope and use that internally (it
seems less obscure that way)
- update the state of instances recovered by the reaper to 'started'
Change-Id: I36baca769dd66749e84f9e6fb583d8be3c209eae
diff --git a/services/mgmt/device/impl/app_service.go b/services/mgmt/device/impl/app_service.go
index 4614bab..8abcf82 100644
--- a/services/mgmt/device/impl/app_service.go
+++ b/services/mgmt/device/impl/app_service.go
@@ -25,14 +25,14 @@
// <pkg name>(700d)
// <pkg name>.__info(700d)
// ...
-// <version 1 timestamp>(700d)/ - timestamp of when the version was downloaded
+// <version 1 timestamp>(711d)/ - timestamp of when the version was downloaded
// bin(755d) - application binary
// previous - symbolic link to previous version directory
// envelope - application envelope (JSON-encoded)
// packages(755d)/ - installed packages (from envelope+installer)
// <pkg name>(755d)/
// ...
-// <version 2 timestamp>(700d)
+// <version 2 timestamp>(711d)
// ...
// current - symbolic link to the current version
// instances(711d)/
@@ -480,10 +480,6 @@
if _, err := newVersion(call.Context(), installationDir, envelope, ""); err != nil {
return "", err
}
- if err := initializeInstallation(installationDir, active); err != nil {
- return "", err
- }
-
// TODO(caprita,rjkroege): Should the installation ACLs really be
// seeded with the device ACL? Instead, might want to hide the deviceACL
// from the app?
@@ -491,7 +487,14 @@
if err := i.initializeSubACLs(call.LocalPrincipal(), installationDir, blessings, i.deviceACL.Copy()); err != nil {
return "", err
}
+ if err := initializeInstallation(installationDir, active); err != nil {
+ return "", err
+ }
deferrer = nil
+ // TODO(caprita): Using the title without cleaning out slashes
+ // introduces extra name components that mess up the device manager's
+ // apps object space. We should fix this either by santizing the title,
+ // or disallowing slashes in titles to begin with.
return naming.Join(envelope.Title, installationID), nil
}
@@ -555,7 +558,7 @@
}
// setupPrincipal sets up the instance's principal, with the right blessings.
-func setupPrincipal(ctx *context.T, instanceDir, versionDir string, call ipc.ServerContext, securityAgent *securityAgentState, info *instanceInfo) error {
+func setupPrincipal(ctx *context.T, instanceDir, blessingExtension string, call ipc.ServerContext, securityAgent *securityAgentState, info *instanceInfo) error {
var p security.Principal
if securityAgent != nil {
// TODO(caprita): Part of the cleanup upon destroying an
@@ -585,15 +588,6 @@
return verror.New(ErrOperationFailed, nil)
}
}
- // Read the app installation version's envelope to obtain the app title.
- //
- // NOTE: we could have gotten this from the suffix as well, but the
- // format of the object name suffix may change in the future: there's no
- // guarantee it will always include the title.
- envelope, err := loadEnvelope(versionDir)
- if err != nil {
- return err
- }
dmPrincipal := call.LocalPrincipal()
// Take the blessings conferred upon us by the Start-er, extend them
// with the app title.
@@ -602,7 +596,7 @@
return verror.New(ErrInvalidBlessing, nil)
}
// TODO(caprita): Revisit UnconstrainedUse.
- appBlessings, err := dmPrincipal.Bless(p.PublicKey(), grantedBlessings, envelope.Title, security.UnconstrainedUse())
+ appBlessings, err := dmPrincipal.Bless(p.PublicKey(), grantedBlessings, blessingExtension, security.UnconstrainedUse())
if err != nil {
vlog.Errorf("Bless() failed: %v", err)
return verror.New(ErrOperationFailed, nil)
@@ -732,6 +726,10 @@
if mkdirPerm(instanceDir, 0711) != nil {
return "", instanceID, verror.New(ErrOperationFailed, call.Context())
}
+ rootDir := filepath.Join(instanceDir, "root")
+ if err := mkdir(rootDir); err != nil {
+ return instanceDir, instanceID, verror.New(ErrOperationFailed, call.Context())
+ }
installationLink := filepath.Join(instanceDir, "installation")
if err := os.Symlink(installationDir, installationLink); err != nil {
vlog.Errorf("Symlink(%v, %v) failed: %v", installationDir, installationLink, err)
@@ -746,32 +744,38 @@
versionLink := filepath.Join(instanceDir, "version")
if err := os.Symlink(versionDir, versionLink); err != nil {
vlog.Errorf("Symlink(%v, %v) failed: %v", versionDir, versionLink, err)
- return instanceDir, instanceID, verror.New(ErrOperationFailed, call.Context())
}
- rootDir := filepath.Join(instanceDir, "root")
- if err := mkdir(rootDir); err != nil {
- return instanceDir, instanceID, verror.New(ErrOperationFailed, call.Context())
- }
- packagesDir, packagesLink := filepath.Join(versionDir, "packages"), filepath.Join(rootDir, "packages")
+ packagesDir, packagesLink := filepath.Join(versionLink, "packages"), filepath.Join(rootDir, "packages")
if err := os.Symlink(packagesDir, packagesLink); err != nil {
vlog.Errorf("Symlink(%v, %v) failed: %v", packagesDir, packagesLink, err)
- return instanceDir, instanceID, verror.New(ErrOperationFailed, call.Context())
}
+
instanceInfo := new(instanceInfo)
- if err := setupPrincipal(call.Context(), instanceDir, versionDir, call, i.securityAgent, instanceInfo); err != nil {
+ // Read the app installation version's envelope to obtain the app title,
+ // which we'll use as a blessing extension.
+ //
+ // NOTE: we could have gotten this from the suffix as well, but the
+ // format of the object name suffix may change in the future: there's no
+ // guarantee it will always include the title.
+ envelope, err := loadEnvelope(versionDir)
+ if err != nil {
+ return instanceDir, instanceID, err
+ }
+ // TODO(caprita): Using the app title as the blessing extension requires
+ // sanitizing to ensure no invalid blessing characters are used.
+ if err := setupPrincipal(call.Context(), instanceDir, envelope.Title, call, i.securityAgent, instanceInfo); err != nil {
return instanceDir, instanceID, err
}
if err := saveInstanceInfo(instanceDir, instanceInfo); err != nil {
return instanceDir, instanceID, err
}
- if err := initializeInstance(instanceDir, suspended); err != nil {
- return instanceDir, instanceID, err
- }
-
blessings, _ := call.RemoteBlessings().ForContext(call)
if err := i.initializeSubACLs(call.LocalPrincipal(), instanceDir, blessings, i.deviceACL.Copy()); err != nil {
return instanceDir, instanceID, err
}
+ if err := initializeInstance(instanceDir, suspended); err != nil {
+ return instanceDir, instanceID, err
+ }
return instanceDir, instanceID, nil
}
@@ -1096,9 +1100,41 @@
return transitionInstallation(installationDir, active, uninstalled)
}
-func updateInstance(instanceDir string, ctx *context.T) error {
- // TODO(caprita): Implement.
- return verror.New(ErrInvalidSuffix, nil)
+func updateInstance(instanceDir string, ctx *context.T) (err error) {
+ // Only suspended instances can be updated.
+ //
+ // TODO(caprita): Consider enabling updates for started instances as
+ // well, and handle the suspend/resume automatically.
+ if err := transitionInstance(instanceDir, suspended, updating); err != nil {
+ return err
+ }
+ defer func() {
+ terr := transitionInstance(instanceDir, updating, suspended)
+ if err == nil {
+ err = terr
+ }
+ }()
+
+ // Check if a newer version of the installation is available.
+ versionLink := filepath.Join(instanceDir, "version")
+ versionDir, err := filepath.EvalSymlinks(versionLink)
+ if err != nil {
+ vlog.Errorf("EvalSymlinks(%v) failed: %v", versionLink, err)
+ return verror.New(ErrOperationFailed, nil)
+ }
+ latestVersionLink := filepath.Join(instanceDir, "installation", "current")
+ latestVersionDir, err := filepath.EvalSymlinks(latestVersionLink)
+ if err != nil {
+ vlog.Errorf("EvalSymlinks(%v) failed: %v", latestVersionLink, err)
+ return verror.New(ErrOperationFailed, nil)
+ }
+ if versionDir == latestVersionDir {
+ return verror.New(ErrUpdateNoOp, ctx)
+ }
+ // Update to the newer version. Note, this is the only mutation
+ // performed to the instance, and, since it's atomic, the state of the
+ // instance is consistent at all times.
+ return updateLink(latestVersionDir, versionLink)
}
func updateInstallation(installationDir string, ctx *context.T) error {
diff --git a/services/mgmt/device/impl/app_state.go b/services/mgmt/device/impl/app_state.go
index 0a13676..ee07c86 100644
--- a/services/mgmt/device/impl/app_state.go
+++ b/services/mgmt/device/impl/app_state.go
@@ -57,6 +57,7 @@
suspended
stopping
stopped
+ updating
)
// String returns the name that will be used to encode the state as a file name
@@ -75,6 +76,8 @@
return "stopping"
case stopped:
return "stopped"
+ case updating:
+ return "updating"
default:
return "unknown"
}
diff --git a/services/mgmt/device/impl/impl_test.go b/services/mgmt/device/impl/impl_test.go
index 6cda65a..82b6e51 100644
--- a/services/mgmt/device/impl/impl_test.go
+++ b/services/mgmt/device/impl/impl_test.go
@@ -752,14 +752,26 @@
t.Fatalf("Second endpoint should have been v1EP2: %v, %v", endpoints, v1EP2)
}
- // Stop first instance.
- stopApp(t, ctx, appID, instance1ID)
- verifyAppWorkspace(t, root, appID, instance1ID)
-
- // Only second instance is still running.
+ // Trying to update first instance while it's running should fail.
+ updateInstanceExpectError(t, ctx, appID, instance1ID, impl.ErrInvalidOperation.ID)
+ // Suspend first instance and try again.
+ suspendApp(t, ctx, appID, instance1ID)
+ // Only the second instance should still be running and mounted.
if want, got := v1EP2, resolve(t, ctx, "appV1", 1)[0]; want != got {
t.Fatalf("Resolve(%v): want: %v, got %v", "appV1", want, got)
}
+ // Update succeeds now.
+ updateInstance(t, ctx, appID, instance1ID)
+ // Resume the first instance and verify it's running v2 now.
+ resumeApp(t, ctx, appID, instance1ID)
+ verifyPingArgs(t, pingCh, userName(t), "flag-val-install", "env-val-envelope")
+ resolve(t, ctx, "appV1", 1)
+ resolve(t, ctx, "appV2", 1)
+
+ // Stop first instance.
+ stopApp(t, ctx, appID, instance1ID)
+ verifyAppWorkspace(t, root, appID, instance1ID)
+ resolveExpectNotFound(t, ctx, "appV2")
// Start a third instance.
instance3ID := startApp(t, ctx, appID)
@@ -1235,6 +1247,8 @@
}
}
+// TODO(caprita): We need better test coverage for how updating/reverting apps
+// affects the package configured for the app.
func TestDeviceManagerPackages(t *testing.T) {
ctx, shutdown := initForTest()
defer shutdown()
diff --git a/services/mgmt/device/impl/instance_reaping.go b/services/mgmt/device/impl/instance_reaping.go
index 045193a..1de27ad 100644
--- a/services/mgmt/device/impl/instance_reaping.go
+++ b/services/mgmt/device/impl/instance_reaping.go
@@ -129,6 +129,14 @@
if instanceStateIs(instancePath, stopped) || instanceStateIs(instancePath, suspended) {
return
}
+ // If the app was updating, it means it was already suspended, so just
+ // update its state back to suspended.
+ if instanceStateIs(instancePath, updating) {
+ if err := initializeInstance(instancePath, suspended); err != nil {
+ vlog.Errorf("initializeInstance(%s,%s) failed: %v", instancePath, suspended, err)
+ }
+ return
+ }
vlog.VI(2).Infof("perInstance firing up on %s", instancePath)
nctx, _ := context.WithTimeout(ctx, appCycleTimeout*time.Second)
@@ -157,7 +165,7 @@
// No process is actually running for this instance.
vlog.VI(2).Infof("perinstance stats fetching failed: %v", err)
if err := initializeInstance(instancePath, suspended); err != nil {
- vlog.Errorf("initializeInstance failed: %v", err)
+ vlog.Errorf("initializeInstance(%s,%s) failed: %v", instancePath, suspended, err)
}
ptuple.err = err
c <- ptuple
@@ -178,6 +186,12 @@
info.Pid = pid
ptuple.err = saveInstanceInfo(instancePath, info)
}
+ // The instance was found to be running, so update its state accordingly
+ // (in case the device restarted while the instance was in one of the
+ // transitional states like starting, suspending, etc).
+ if err := initializeInstance(instancePath, started); err != nil {
+ vlog.Errorf("initializeInstance(%s,%s) failed: %v", instancePath, started, err)
+ }
vlog.VI(2).Infof("perInstance go routine for %v ending", instancePath)
c <- ptuple
diff --git a/services/mgmt/device/impl/util_test.go b/services/mgmt/device/impl/util_test.go
index fdb1b64..c5fcb77 100644
--- a/services/mgmt/device/impl/util_test.go
+++ b/services/mgmt/device/impl/util_test.go
@@ -258,6 +258,18 @@
}
}
+func updateInstance(t *testing.T, ctx *context.T, appID, instanceID string) {
+ if err := appStub(appID, instanceID).Update(ctx); err != nil {
+ t.Fatalf(testutil.FormatLogLine(2, "Update(%v/%v) failed: %v [%v]", appID, instanceID, verror.ErrorID(err), err))
+ }
+}
+
+func updateInstanceExpectError(t *testing.T, ctx *context.T, appID, instanceID string, expectedError verror.ID) {
+ if err := appStub(appID, instanceID).Update(ctx); err == nil || !verror.Is(err, expectedError) {
+ t.Fatalf(testutil.FormatLogLine(2, "Update(%v/%v) expected to fail with %v, got %v [%v]", appID, instanceID, expectedError, verror.ErrorID(err), err))
+ }
+}
+
func updateApp(t *testing.T, ctx *context.T, appID string) {
if err := appStub(appID).Update(ctx); err != nil {
t.Fatalf(testutil.FormatLogLine(2, "Update(%v) failed: %v [%v]", appID, verror.ErrorID(err), err))
