diff --git a/robot/impl/local_robot.go b/robot/impl/local_robot.go index f0cfe981a34..91b6661f7a8 100644 --- a/robot/impl/local_robot.go +++ b/robot/impl/local_robot.go @@ -56,10 +56,9 @@ type localRobot struct { logger golog.Logger activeBackgroundWorkers sync.WaitGroup cancelBackgroundWorkers func() - remotesChanged chan string closeContext context.Context - triggerConfig chan bool - configTimer *time.Ticker + triggerConfig chan struct{} + configTicker *time.Ticker revealSensitiveConfigDiffs bool lastWeakDependentsRound int64 @@ -134,11 +133,10 @@ func (r *localRobot) Close(ctx context.Context) error { r.webSvc.Stop() } if r.cancelBackgroundWorkers != nil { - close(r.remotesChanged) r.cancelBackgroundWorkers() r.cancelBackgroundWorkers = nil - if r.configTimer != nil { - r.configTimer.Stop() + if r.configTicker != nil { + r.configTicker.Stop() } close(r.triggerConfig) } @@ -362,11 +360,10 @@ func newWithResources( ), operations: operation.NewManager(logger), logger: logger, - remotesChanged: make(chan string), closeContext: closeCtx, cancelBackgroundWorkers: cancel, - triggerConfig: make(chan bool), - configTimer: nil, + triggerConfig: make(chan struct{}), + configTicker: nil, revealSensitiveConfigDiffs: rOpts.revealSensitiveConfigDiffs, cloudConnSvc: cloud.NewCloudConnectionService(cfg.Cloud, logger), } @@ -453,41 +450,18 @@ func newWithResources( } r.activeBackgroundWorkers.Add(1) - // this goroutine listen for changes in connection status of a remote + r.configTicker = time.NewTicker(5 * time.Second) + // This goroutine tries to complete the config and update weak dependencies + // if any resources are not configured. It executes every 5 seconds or when + // manually triggered. Manual triggers are sent when changes in remotes are + // detected and in testing. goutils.ManagedGo(func() { for { - if closeCtx.Err() != nil { - return - } - select { - case <-closeCtx.Done(): - return - case n, ok := <-r.remotesChanged: - if !ok { - return - } - if rr, ok := r.manager.RemoteByName(n); ok { - rn := fromRemoteNameToRemoteNodeName(n) - r.manager.updateRemoteResourceNames(closeCtx, rn, rr) - r.updateWeakDependents(ctx) - } - } - } - }, r.activeBackgroundWorkers.Done) - - r.activeBackgroundWorkers.Add(1) - r.configTimer = time.NewTicker(25 * time.Second) - // this goroutine tries to complete the config if any resources are still unconfigured, it execute on a timer or via a channel - goutils.ManagedGo(func() { - for { - if closeCtx.Err() != nil { - return - } select { case <-closeCtx.Done(): return + case <-r.configTicker.C: case <-r.triggerConfig: - case <-r.configTimer.C: } anyChanges := r.manager.updateRemotesResourceNames(closeCtx) if r.manager.anyResourcesNotConfigured() { diff --git a/robot/impl/resource_manager.go b/robot/impl/resource_manager.go index 1f94fb14f17..1cac83e6667 100644 --- a/robot/impl/resource_manager.go +++ b/robot/impl/resource_manager.go @@ -480,14 +480,12 @@ func (manager *resourceManager) completeConfig( } manager.addRemote(ctx, rr, gNode, *remConf) rr.SetParentNotifier(func() { - rName := remConf.Name - if robot.closeContext.Err() != nil { - return - } + // Trigger completeConfig goroutine execution when a change in remote + // is detected. select { case <-robot.closeContext.Done(): return - case robot.remotesChanged <- rName: + case robot.triggerConfig <- struct{}{}: } }) default: diff --git a/robot/impl/robot_reconfigure_test.go b/robot/impl/robot_reconfigure_test.go index 9c68b898ccf..c0cf82fb408 100644 --- a/robot/impl/robot_reconfigure_test.go +++ b/robot/impl/robot_reconfigure_test.go @@ -317,7 +317,7 @@ func TestRobotReconfigure(t *testing.T) { rr, ok := robot.(*localRobot) test.That(t, ok, test.ShouldBeTrue) - rr.triggerConfig <- true + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(context.Background(), 200*time.Millisecond) @@ -1978,7 +1978,7 @@ func TestRobotReconfigure(t *testing.T) { rr, ok := robot.(*localRobot) test.That(t, ok, test.ShouldBeTrue) - rr.triggerConfig <- true + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(context.Background(), 200*time.Millisecond) mock6, err = robot.ResourceByName(mockNamed("mock6")) @@ -2672,7 +2672,7 @@ func TestRemoteRobotsGold(t *testing.T) { rr, ok := r.(*localRobot) test.That(t, ok, test.ShouldBeTrue) - rr.triggerConfig <- true + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(ctx, 2*time.Second) @@ -2745,12 +2745,12 @@ func TestRemoteRobotsGold(t *testing.T) { err = remote3.StartWeb(ctx, options) test.That(t, err, test.ShouldBeNil) - utils.SelectContextOrWait(ctx, 26*time.Second) + utils.SelectContextOrWait(ctx, 5*time.Second) rr, ok = r.(*localRobot) test.That(t, ok, test.ShouldBeTrue) - rr.triggerConfig <- true + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(ctx, 2*time.Second) @@ -2825,7 +2825,7 @@ func TestInferRemoteRobotDependencyConnectAtStartup(t *testing.T) { rr, ok := r.(*localRobot) test.That(t, ok, test.ShouldBeTrue) - rr.triggerConfig <- true + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(ctx, 2*time.Second) expectedSet := rdktestutils.NewResourceNameSet( @@ -2844,7 +2844,7 @@ func TestInferRemoteRobotDependencyConnectAtStartup(t *testing.T) { test.That(t, foo.Close(context.Background()), test.ShouldBeNil) // wait for local_robot to detect that the remote is now offline utils.SelectContextOrWait(ctx, 15*time.Second) - rr.triggerConfig <- true + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(ctx, 2*time.Second) test.That( @@ -2873,12 +2873,12 @@ func TestInferRemoteRobotDependencyConnectAtStartup(t *testing.T) { err = foo2.StartWeb(ctx, options) test.That(t, err, test.ShouldBeNil) - utils.SelectContextOrWait(ctx, 26*time.Second) + utils.SelectContextOrWait(ctx, 5*time.Second) rr, ok = r.(*localRobot) test.That(t, ok, test.ShouldBeTrue) - rr.triggerConfig <- true + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(ctx, 2*time.Second) @@ -2948,8 +2948,8 @@ func TestInferRemoteRobotDependencyConnectAfterStartup(t *testing.T) { rr, ok := r.(*localRobot) test.That(t, ok, test.ShouldBeTrue) - utils.SelectContextOrWait(ctx, 15*time.Second) - rr.triggerConfig <- true + utils.SelectContextOrWait(ctx, 5*time.Second) + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(ctx, 2*time.Second) expectedSet := rdktestutils.NewResourceNameSet( @@ -2967,8 +2967,8 @@ func TestInferRemoteRobotDependencyConnectAfterStartup(t *testing.T) { test.That(t, foo.Close(context.Background()), test.ShouldBeNil) // wait for local_robot to detect that the remote is now offline - utils.SelectContextOrWait(ctx, 15*time.Second) - rr.triggerConfig <- true + utils.SelectContextOrWait(ctx, 5*time.Second) + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(ctx, 2*time.Second) test.That( @@ -3069,7 +3069,7 @@ func TestInferRemoteRobotDependencyAmbiguous(t *testing.T) { rr, ok := r.(*localRobot) test.That(t, ok, test.ShouldBeTrue) - rr.triggerConfig <- true + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(ctx, 2*time.Second) // we expect the robot to correctly detect the ambiguous dependency and not build the resource @@ -3100,8 +3100,8 @@ func TestInferRemoteRobotDependencyAmbiguous(t *testing.T) { }, } r.Reconfigure(ctx, reConfig) - utils.SelectContextOrWait(ctx, 15*time.Second) - rr.triggerConfig <- true + utils.SelectContextOrWait(ctx, 5*time.Second) + rr.triggerConfig <- struct{}{} utils.SelectContextOrWait(ctx, 2*time.Second) finalSet := rdktestutils.NewResourceNameSet(