From c72c42cbf93782b93f4cabae4b1bd7d0954263d5 Mon Sep 17 00:00:00 2001 From: Bruno Mercier Costa <94469565+emrcbrn@users.noreply.github.com> Date: Thu, 29 Oct 2026 13:24:08 +0100 Subject: [PATCH] [client] Start the update manager when management forces an update (#8232) --- client/internal/updater/manager.go | 22 +++++-- client/internal/updater/manager_linux_test.go | 35 +++++++++++- client/internal/updater/manager_mode_test.go | 57 ++++++++++++++++--- client/internal/updater/manager_test.go | 8 +-- client/server/server.go | 2 +- 5 files changed, 106 insertions(+), 18 deletions(-) diff --git a/client/internal/updater/manager.go b/client/internal/updater/manager.go index c730713d3..d78bc147b 100644 --- a/client/internal/updater/manager.go +++ b/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() { diff --git a/client/internal/updater/manager_linux_test.go b/client/internal/updater/manager_linux_test.go index 4501ddde7..502b1ba3b 100644 --- a/client/internal/updater/manager_linux_test.go +++ b/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) + } +} diff --git a/client/internal/updater/manager_mode_test.go b/client/internal/updater/manager_mode_test.go index aa238f94c..8daca70f5 100644 --- a/client/internal/updater/manager_mode_test.go +++ b/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") + } +} diff --git a/client/internal/updater/manager_test.go b/client/internal/updater/manager_test.go index 939c09814..8f5a8fe5f 100644 --- a/client/internal/updater/manager_test.go +++ b/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 } diff --git a/client/server/server.go b/client/server/server.go index 94469c683..a04720763 100644 --- a/client/server/server.go +++ b/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) }