[client] Start the update manager when management forces an update (#8232)

client/internal/updater/manager.go

@@ -41,6 +41,8 @@ func (u UpdateState) Name() string {
 }
 
 type Manager struct {
+	// ctx is the daemon lifetime context a forced directive starts the loop with.
+	ctx            context.Context
 	statusRecorder *peer.Status
 	stateManager   *statemanager.Manager
 
@@ -77,8 +79,9 @@ type Manager struct {
 }
 
 // NewManager creates a new update manager. The manager is single-use: once Stop() is called, it cannot be restarted.
-func NewManager(statusRecorder *peer.Status, stateManager *statemanager.Manager) *Manager {
+func NewManager(ctx context.Context, statusRecorder *peer.Status, stateManager *statemanager.Manager) *Manager {
 	manager := &Manager{
+		ctx:                 ctx,
 		statusRecorder:      statusRecorder,
 		stateManager:        stateManager,
 		mgmUpdateChan:       make(chan struct{}, 1),
@@ -177,13 +180,23 @@ func (m *Manager) SetVersion(expectedVersion string, forceUpdate bool) {
 		return
 	}
 
+	if m.applyDirective(expectedVersion, forceUpdate) && forceUpdate {
+		// A forced update installs without user interaction, so it must not wait
+		// for a UI to subscribe to daemon events, the only other caller of Start.
+		m.Start(m.ctx)
+	}
+}
+
+// applyDirective records a management directive and reports whether it put the
+// manager into a new managed state.
+func (m *Manager) applyDirective(expectedVersion string, forceUpdate bool) bool {
 	m.updateMutex.Lock()
 	defer m.updateMutex.Unlock()
 
 	if expectedVersion == "" {
 		log.Errorf("empty expected version provided")
 		m.setModeLocked(modeDownloadOnly)
-		return
+		return false
 	}
 
 	var expectedSemVer *v.Version
@@ -196,13 +209,13 @@ func (m *Manager) SetVersion(expectedVersion string, forceUpdate bool) {
 			case m.mgmUpdateChan <- struct{}{}:
 			default:
 			}
-			return
+			return false
 		}
 		expectedSemVer = parsed
 	}
 
 	if m.sameDirectiveLocked(expectedSemVer, forceUpdate) {
-		return
+		return false
 	}
 
 	m.setModeLocked(modeManaged)
@@ -214,6 +227,7 @@ func (m *Manager) SetVersion(expectedVersion string, forceUpdate bool) {
 	case m.mgmUpdateChan <- struct{}{}:
 	default:
 	}
+	return true
 }
 
 func (m *Manager) ResetMode() {

client/internal/updater/manager_linux_test.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"fmt"
 	"path"
+	"strings"
 	"testing"
 	"time"
 
@@ -52,7 +53,7 @@ func Test_LatestVersion_Linux(t *testing.T) {
 		sub := recorder.SubscribeToEvents()
 		defer recorder.UnsubscribeFromEvents(sub)
 
-		m := NewManager(recorder, statemanager.New(tmpFile))
+		m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 		m.update = mockUpdate
 		m.currentVersion = c.daemonVersion
 		m.Start(context.Background())
@@ -95,7 +96,7 @@ func Test_SetVersion_FallsBackToDownloadOnly_Linux(t *testing.T) {
 	sub := recorder.SubscribeToEvents()
 	defer recorder.UnsubscribeFromEvents(sub)
 
-	m := NewManager(recorder, statemanager.New(tmpFile))
+	m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 	m.update = &versionUpdateMock{latestVersion: v.Must(v.NewSemver("1.0.5"))}
 	m.currentVersion = "1.0.0"
 	m.Start(context.Background())
@@ -111,3 +112,33 @@ func Test_SetVersion_FallsBackToDownloadOnly_Linux(t *testing.T) {
 
 	m.Stop()
 }
+
+// The installer here is a stub that fails at once, so the forced install path
+// can run end to end in a test without installing anything.
+func Test_SetVersion_ForceInstallsWithoutSubscriber_Linux(t *testing.T) {
+	recorder := peer.NewRecorder("")
+	sub := recorder.SubscribeToEvents()
+	defer recorder.UnsubscribeFromEvents(sub)
+
+	m := NewManager(context.Background(), recorder, statemanager.New(path.Join(t.TempDir(), "update-test-force-install.json")))
+	m.update = &versionUpdateMock{}
+	m.currentVersion = "1.0.0"
+	m.autoUpdateSupported = func() bool { return true }
+	defer m.Stop()
+
+	// No UI has subscribed to daemon events, and the test never calls Start.
+	m.SetVersion("1.0.1", true)
+
+	event := waitForAnyEvent(sub, 2*time.Second)
+	if event == nil || event.GetMessage() != "Updating client" {
+		t.Fatalf("a forced directive newer than the current version must start an install without a UI subscriber, got %v", event)
+	}
+
+	// The stub installer's error proves RunInstallation itself was called.
+	for event != nil && event.GetMessage() != "Auto-update failed" {
+		event = waitForAnyEvent(sub, 2*time.Second)
+	}
+	if event == nil || !strings.Contains(event.GetUserMessage(), "unsupported platform") {
+		t.Fatalf("the forced install must reach the installer, got %v", event)
+	}
+}

client/internal/updater/manager_mode_test.go

@@ -19,7 +19,7 @@ func Test_UndecidedMode_SuppressesNotification(t *testing.T) {
 	defer recorder.UnsubscribeFromEvents(sub)
 
 	mockUpdate := &versionUpdateMock{latestVersion: v.Must(v.NewSemver("1.0.1"))}
-	m := NewManager(recorder, statemanager.New(tmpFile))
+	m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 	m.update = mockUpdate
 	m.currentVersion = "1.0.0"
 	m.Start(context.Background())
@@ -52,7 +52,7 @@ func Test_ResetMode_ReturnsToUndecided(t *testing.T) {
 	defer recorder.UnsubscribeFromEvents(sub)
 
 	mockUpdate := &versionUpdateMock{latestVersion: v.Must(v.NewSemver("1.0.1"))}
-	m := NewManager(recorder, statemanager.New(tmpFile))
+	m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 	m.update = mockUpdate
 	m.currentVersion = "1.0.0"
 	m.autoUpdateSupported = func() bool { return true }
@@ -94,7 +94,7 @@ func Test_SetDownloadOnly_ClearsPendingVersion(t *testing.T) {
 	sub := recorder.SubscribeToEvents()
 	defer recorder.UnsubscribeFromEvents(sub)
 
-	m := NewManager(recorder, statemanager.New(tmpFile))
+	m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 	m.update = &versionUpdateMock{latestVersion: v.Must(v.NewSemver("1.0.1"))}
 	m.currentVersion = "1.0.0"
 	m.autoUpdateSupported = func() bool { return true }
@@ -123,7 +123,7 @@ func Test_ResetMode_SilencesStaleForceDirective(t *testing.T) {
 	defer recorder.UnsubscribeFromEvents(sub)
 
 	mockUpdate := &versionUpdateMock{}
-	m := NewManager(recorder, statemanager.New(tmpFile))
+	m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 	m.update = mockUpdate
 	m.currentVersion = "1.0.0"
 	m.autoUpdateSupported = func() bool { return true }
@@ -159,7 +159,7 @@ func Test_SetVersion_MalformedFallsBackToDownloadOnly(t *testing.T) {
 	sub := recorder.SubscribeToEvents()
 	defer recorder.UnsubscribeFromEvents(sub)
 
-	m := NewManager(recorder, statemanager.New(tmpFile))
+	m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 	m.update = &versionUpdateMock{latestVersion: v.Must(v.NewSemver("1.0.1"))}
 	m.currentVersion = "1.0.0"
 	m.autoUpdateSupported = func() bool { return true }
@@ -177,7 +177,7 @@ func Test_SetVersion_MalformedFallsBackToDownloadOnly(t *testing.T) {
 }
 
 func Test_SetVersion_ForceChangeAppliesWithSameVersion(t *testing.T) {
-	m := NewManager(peer.NewRecorder(""), statemanager.New(path.Join(t.TempDir(), "update-test-force-change.json")))
+	m := NewManager(context.Background(), peer.NewRecorder(""), statemanager.New(path.Join(t.TempDir(), "update-test-force-change.json")))
 	m.update = &versionUpdateMock{}
 	m.autoUpdateSupported = func() bool { return true }
 
@@ -195,7 +195,7 @@ func Test_SetVersion_ForceChangeAppliesWithSameVersion(t *testing.T) {
 }
 
 func Test_SetVersion_RepeatedDirectiveKeepsMode(t *testing.T) {
-	m := NewManager(peer.NewRecorder(""), statemanager.New(path.Join(t.TempDir(), "update-test-repeat.json")))
+	m := NewManager(context.Background(), peer.NewRecorder(""), statemanager.New(path.Join(t.TempDir(), "update-test-repeat.json")))
 	m.update = &versionUpdateMock{}
 	m.autoUpdateSupported = func() bool { return true }
 
@@ -215,3 +215,46 @@ func Test_SetVersion_RepeatedDirectiveKeepsMode(t *testing.T) {
 		}
 	}
 }
+
+func managerStarted(m *Manager) bool {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	return m.cancel != nil
+}
+
+func Test_SetVersion_ForceStartsLoopWithoutSubscriber(t *testing.T) {
+	recorder := peer.NewRecorder("")
+	sub := recorder.SubscribeToEvents()
+	defer recorder.UnsubscribeFromEvents(sub)
+
+	m := NewManager(context.Background(), recorder, statemanager.New(path.Join(t.TempDir(), "update-test-force-start.json")))
+	m.update = &versionUpdateMock{}
+	// Already on the forced version, so the started loop has nothing to install.
+	m.currentVersion = "1.0.1"
+	m.autoUpdateSupported = func() bool { return true }
+	defer m.Stop()
+
+	// No UI has subscribed to daemon events, so nothing else calls Start.
+	m.SetVersion("1.0.1", true)
+
+	if !managerStarted(m) {
+		t.Fatal("a forced directive must start the update loop without a UI subscriber")
+	}
+	if event := waitForAnyEvent(sub, 300*time.Millisecond); event != nil {
+		t.Fatalf("no install expected when already on the forced version, got %v", event)
+	}
+}
+
+func Test_SetVersion_EnforcedWithoutForceWaitsForSubscriber(t *testing.T) {
+	m := NewManager(context.Background(), peer.NewRecorder(""), statemanager.New(path.Join(t.TempDir(), "update-test-enforced-wait.json")))
+	m.update = &versionUpdateMock{}
+	m.currentVersion = "1.0.0"
+	m.autoUpdateSupported = func() bool { return true }
+	defer m.Stop()
+
+	m.SetVersion("1.0.1", false)
+
+	if managerStarted(m) {
+		t.Fatal("an enforced directive without force needs the UI prompt, so it must not start the loop")
+	}
+}

client/internal/updater/manager_test.go

@@ -50,7 +50,7 @@ func Test_LatestVersion(t *testing.T) {
 		sub := recorder.SubscribeToEvents()
 		defer recorder.UnsubscribeFromEvents(sub)
 
-		m := NewManager(recorder, statemanager.New(tmpFile))
+		m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 		m.update = mockUpdate
 		m.currentVersion = c.daemonVersion
 		m.autoUpdateSupported = func() bool { return true }
@@ -153,7 +153,7 @@ func Test_HandleUpdate(t *testing.T) {
 		sub := recorder.SubscribeToEvents()
 		defer recorder.UnsubscribeFromEvents(sub)
 
-		m := NewManager(recorder, statemanager.New(tmpFile))
+		m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 		m.update = &versionUpdateMock{latestVersion: c.latestVersion}
 		m.currentVersion = c.daemonVersion
 		m.autoUpdateSupported = func() bool { return true }
@@ -185,7 +185,7 @@ func Test_EnforcedMetadata(t *testing.T) {
 	sub := recorder.SubscribeToEvents()
 	defer recorder.UnsubscribeFromEvents(sub)
 
-	m := NewManager(recorder, statemanager.New(tmpFile))
+	m := NewManager(context.Background(), recorder, statemanager.New(tmpFile))
 	m.update = &versionUpdateMock{latestVersion: v.Must(v.NewSemver("1.0.1"))}
 	m.currentVersion = "1.0.0"
 	m.Start(context.Background())
@@ -206,7 +206,7 @@ func Test_EnforcedMetadata(t *testing.T) {
 	sub2 := recorder2.SubscribeToEvents()
 	defer recorder2.UnsubscribeFromEvents(sub2)
 
-	m2 := NewManager(recorder2, statemanager.New(tmpFile2))
+	m2 := NewManager(context.Background(), recorder2, statemanager.New(tmpFile2))
 	m2.update = &versionUpdateMock{latestVersion: nil}
 	m2.currentVersion = "1.0.0"
 	m2.autoUpdateSupported = func() bool { return true }

client/server/server.go

@@ -268,7 +268,7 @@ func (s *Server) Start() error {
 
 	if s.updateManager == nil {
 		stateMgr := statemanager.New(s.profileManager.GetStatePath())
-		s.updateManager = updater.NewManager(s.statusRecorder, stateMgr)
+		s.updateManager = updater.NewManager(s.rootCtx, s.statusRecorder, stateMgr)
 		s.updateManager.CheckUpdateSuccess(s.rootCtx)
 	}