RSDK-13906: Fix agent redownloading itself on fresh install on Windows

utils/urlparse_test.go

@@ -24,6 +24,7 @@ func TestDownloadFileURLForms(t *testing.T) {
 
 	// forward-slash spelling of the source path; on Windows "C:\x" -> "C:/x", on unix unchanged.
 	fwd := filepath.ToSlash(src)
+
 	vol := filepath.VolumeName(src)
 
 	type urlCase struct {

version_control.go

@@ -32,6 +32,9 @@ import (
 // This is to not to overwhelm servers, since we fetch config updates and call UpdateBinary every few seconds.
 const lastModifiedCheckFrequency = time.Minute * 2
 
+// osExecutable is swapped out in tests to simulate a particular running binary.
+var osExecutable = os.Executable
+
 func getCacheFilePath() string {
 	return filepath.Join(utils.ViamDirs.Cache, "version_cache.json")
 }
@@ -287,6 +290,14 @@ func (c *VersionCache) UpdateBinary(ctx context.Context, binary string) (bool, e
 		}
 	}
 
+	// if the current running binary matches the desired one, just adopt it
+	// and update the version cache (RSDK-13906)
+	if binary == SubsystemName && !goodBytes && data.CurrentVersion == "" && verData.Installed.IsZero() && len(verData.UnpackedSHA) > 1 {
+		if c.adoptRunningBinary(data, verData) {
+			return false, c.save()
+		}
+	}
+
 	// this is a new version
 	c.logger.Infof("new version (%s) found for %s", verData.Version, binary)
 
@@ -396,6 +407,49 @@ func (c *VersionCache) UpdateBinary(ctx context.Context, binary string) (bool, e
 	return needRestart, c.save()
 }
 
+// adoptRunningBinary records the currently running executable as the installed version.
+// Returns true on successful adoption, false otherwise.
+// Callers must hold c.mu.
+func (c *VersionCache) adoptRunningBinary(data *Versions, verData *VersionInfo) bool {
+	exePath, err := osExecutable()
+	if err != nil {
+		c.logger.Warnw("cannot determine running executable path, will download", "error", err)
+		return false
+	}
+	// the executable is normally started via the symlink in bin; resolve to the real file
+	exePath, err = filepath.EvalSymlinks(exePath)
+	if err != nil {
+		c.logger.Warnw("cannot resolve running executable path, will download", "path", exePath, "error", err)
+		return false
+	}
+	shasum, err := utils.GetFileSum(exePath)
+	if err != nil {
+		c.logger.Warnw("cannot checksum running executable, will download", "path", exePath, "error", err)
+		return false
+	}
+	if !bytes.Equal(shasum, verData.UnpackedSHA) {
+		c.logger.Warnw("checksum of running executable did not match symlink, will download", "path", exePath, "error", err)
+		return false
+	}
+
+	same, err := utils.CheckIfSame(exePath, verData.SymlinkPath)
+	if err != nil || !same {
+		if symErr := utils.ForceSymlink(exePath, verData.SymlinkPath); symErr != nil {
+			c.logger.Warnw("cannot symlink running binary, will download instead",
+				"path", exePath, "symlink", verData.SymlinkPath, "error", errors.Join(err, symErr))
+			return false
+		}
+	}
+
+	verData.DlPath = exePath
+	verData.UnpackedPath = exePath
+	verData.DlSHA = shasum
+	verData.Installed = time.Now()
+	data.CurrentVersion = data.TargetVersion
+	c.logger.Infof("running binary %s already matches version %s, adopting it without downloading", exePath, verData.Version)
+	return true
+}
+
 // files we will always refuse to delete.
 var baseProtectedFiles = []string{"config_cache.json", "version_cache.json", "viam-agent.pid"}
 

version_control_test.go

@@ -366,6 +366,115 @@ func TestUpdateBinary(t *testing.T) {
 	})
 }
 
+func TestUpdateBinaryAdoptRunningBinary(t *testing.T) {
+	utils.MockAndCreateViamDirs(t)
+	logger := logging.NewTestLogger(t)
+
+	// A fake "currently running binary" with known contents. Resolve the temp dir since
+	// adoption resolves symlinks (on darwin /var is a symlink to /private/var).
+	td, err := filepath.EvalSymlinks(t.TempDir())
+	test.That(t, err, test.ShouldBeNil)
+	fakeExe := filepath.Join(td, "viam-agent-from-installer.exe")
+	err = os.WriteFile(fakeExe, []byte("fake agent binary contents"), 0o755)
+	test.That(t, err, test.ShouldBeNil)
+	exeSHA, err := utils.GetFileSum(fakeExe)
+	test.That(t, err, test.ShouldBeNil)
+
+	oldOsExecutable := osExecutable
+	osExecutable = func() (string, error) { return fakeExe, nil }
+	t.Cleanup(func() { osExecutable = oldOsExecutable })
+
+	t.Run("fresh-install-adopts", func(t *testing.T) {
+		vi := &VersionInfo{
+			Version: "0.99.0",
+			// any attempted download would fail
+			URL:         "file:///nonexistent/viam-agent-v0.99.0",
+			UnpackedSHA: exeSHA,
+			SymlinkPath: filepath.Join(utils.ViamDirs.Bin, "viam-agent"),
+		}
+		vc := VersionCache{
+			logger:             logger,
+			cacheCleanupLogger: logger,
+			ViamAgent: &Versions{
+				TargetVersion: vi.Version,
+				Versions:      map[string]*VersionInfo{vi.Version: vi},
+			},
+		}
+
+		needsRestart, err := vc.UpdateBinary(t.Context(), SubsystemName)
+		test.That(t, err, test.ShouldBeNil)
+		test.That(t, needsRestart, test.ShouldBeFalse)
+		test.That(t, vc.ViamAgent.CurrentVersion, test.ShouldEqual, vi.Version)
+		test.That(t, vi.UnpackedPath, test.ShouldEqual, fakeExe)
+		test.That(t, vi.DlPath, test.ShouldEqual, fakeExe)
+		test.That(t, vi.Installed.IsZero(), test.ShouldBeFalse)
+		linkTarget, err := filepath.EvalSymlinks(vi.SymlinkPath)
+		test.That(t, err, test.ShouldBeNil)
+		test.That(t, linkTarget, test.ShouldEqual, fakeExe)
+
+		// steady state afterwards: no download, no restart
+		needsRestart, err = vc.UpdateBinary(t.Context(), SubsystemName)
+		test.That(t, err, test.ShouldBeNil)
+		test.That(t, needsRestart, test.ShouldBeFalse)
+	})
+
+	t.Run("checksum-mismatch-downloads", func(t *testing.T) {
+		sourceBinary := filepath.Join(td, "viam-agent-v0.99.1")
+		err := os.WriteFile(sourceBinary, []byte("different contents"), 0o755)
+		test.That(t, err, test.ShouldBeNil)
+		sourceSHA, err := utils.GetFileSum(sourceBinary)
+		test.That(t, err, test.ShouldBeNil)
+
+		vi := &VersionInfo{
+			Version:     "0.99.1",
+			URL:         "file://" + sourceBinary,
+			UnpackedSHA: sourceSHA,
+			SymlinkPath: filepath.Join(utils.ViamDirs.Bin, "viam-agent"),
+		}
+		vc := VersionCache{
+			logger:             logger,
+			cacheCleanupLogger: logger,
+			ViamAgent: &Versions{
+				TargetVersion: vi.Version,
+				Versions:      map[string]*VersionInfo{vi.Version: vi},
+			},
+		}
+
+		// the running binary's checksum does not match the target, so this must download
+		needsRestart, err := vc.UpdateBinary(t.Context(), SubsystemName)
+		test.That(t, err, test.ShouldBeNil)
+		test.That(t, needsRestart, test.ShouldBeTrue)
+		test.That(t, vc.ViamAgent.CurrentVersion, test.ShouldEqual, vi.Version)
+		test.That(t, vi.UnpackedPath, test.ShouldNotEqual, fakeExe)
+		testExists(t, filepath.Join(utils.ViamDirs.Cache, filepath.Base(vi.UnpackedPath)))
+	})
+
+	t.Run("viam-server-never-adopts", func(t *testing.T) {
+		vi := &VersionInfo{
+			Version:     "0.99.0",
+			URL:         "file:///nonexistent/viam-server-v0.99.0",
+			UnpackedSHA: exeSHA,
+			SymlinkPath: filepath.Join(utils.ViamDirs.Bin, "viam-server"),
+		}
+		vc := VersionCache{
+			logger:             logger,
+			cacheCleanupLogger: logger,
+			ViamServer: &Versions{
+				TargetVersion: vi.Version,
+				Versions:      map[string]*VersionInfo{vi.Version: vi},
+			},
+		}
+
+		// even though the checksum matches the running executable, viam-server must
+		// always be downloaded, so this errors on the unreachable URL
+		needsRestart, err := vc.UpdateBinary(t.Context(), viamserver.SubsysName)
+		test.That(t, err, test.ShouldNotBeNil)
+		test.That(t, err.Error(), test.ShouldContainSubstring, "downloading")
+		test.That(t, needsRestart, test.ShouldBeFalse)
+		test.That(t, vc.ViamServer.CurrentVersion, test.ShouldEqual, "")
+	})
+}
+
 // assert that a file exists.
 func testExists(t *testing.T, path string) {
 	t.Helper()