@@ -297,7 +297,7 @@ func TestActionDispatcher(t *testing.T) {
297297
298298 action1 := & mockScheduledAction {}
299299 action1 .On ("StartTime" ).Return (time.Time {}, fleetapi .ErrNoStartTime )
300- action1 .On ("Expiration" ).Return (time .Now ().Add (time .Hour ), fleetapi .ErrNoStartTime )
300+ action1 .On ("Expiration" ).Return (time .Now ().Add (time .Hour ), fleetapi .ErrNoExpiration )
301301 action1 .On ("Type" ).Return (fleetapi .ActionTypeCancel )
302302 action1 .On ("ID" ).Return ("id" )
303303
@@ -360,8 +360,9 @@ func TestActionDispatcher(t *testing.T) {
360360 })
361361
362362 t .Run ("Dispatch of a retryable action returns an error" , func (t * testing.T ) {
363+ testError := errors .New ("test error" )
363364 def := & mockHandler {}
364- def .On ("Handle" , mock .Anything , mock .Anything , mock .Anything ).Return (errors . New ( "test error" ) ).Once ()
365+ def .On ("Handle" , mock .Anything , mock .Anything , mock .Anything ).Return (testError ).Once ()
365366
366367 saver := & mockSaver {}
367368 saver .On ("Save" ).Return (nil ).Times (2 )
@@ -382,6 +383,7 @@ func TestActionDispatcher(t *testing.T) {
382383 action .On ("RetryAttempt" ).Return (0 ).Once ()
383384 action .On ("SetRetryAttempt" , 1 ).Once ()
384385 action .On ("SetStartTime" , mock .Anything ).Once ()
386+ action .On ("GetError" ).Return (testError ).Once ()
385387
386388 dispatchCtx , cancelFn := context .WithCancel (context .Background ())
387389 defer cancelFn ()
@@ -685,6 +687,112 @@ func TestActionDispatcher(t *testing.T) {
685687 }
686688 require .NoError (t , shouldNotSetDetails , "set upgrade details should not be called" )
687689 })
690+
691+ t .Run ("check that failed upgrade actions do not block newer ones from dispatching" , func (t * testing.T ) {
692+ // we start by dispatching a scheduled upgrade action
693+ ctx := t .Context ()
694+ handlerErr := errors .New ("text handler error" )
695+
696+ def := & mockHandler {}
697+ def .On ("Handle" ,
698+ mock .Anything , mock .Anything , mock .Anything ).
699+ Return (handlerErr ).Once ()
700+
701+ saver := & mockSaver {}
702+ saver .On ("Save" ).Return (nil ).Times (4 )
703+ saver .On ("SetQueue" , mock .Anything ).Times (4 )
704+ actionQueue , err := queue .NewActionQueue ([]fleetapi.ScheduledAction {}, saver )
705+ require .NoError (t , err )
706+
707+ d , err := New (nil , t .TempDir (), def , actionQueue )
708+ require .NoError (t , err )
709+
710+ err = d .Register (& fleetapi.ActionUpgrade {}, def )
711+ require .NoError (t , err )
712+
713+ var gotDetails * details.Details
714+ detailsSetter := func (upgradeDetails * details.Details ) {
715+ gotDetails = upgradeDetails
716+ }
717+
718+ initialScheduledUpgradeAction := & fleetapi.ActionUpgrade {
719+ ActionID : "scheduled-action-id" ,
720+ ActionType : fleetapi .ActionTypeUpgrade ,
721+ ActionStartTime : time .Now ().Add (1 * time .Hour ).Format (time .RFC3339 ),
722+ Data : fleetapi.ActionUpgradeData {
723+ Version : "9.3.0" ,
724+ SourceURI : "https://test-uri.test.com" ,
725+ },
726+ }
727+
728+ dispatchDone := make (chan struct {})
729+ go func () {
730+ d .Dispatch (ctx , detailsSetter , ack , initialScheduledUpgradeAction )
731+ close (dispatchDone )
732+ }()
733+ select {
734+ case err := <- d .Errors ():
735+ t .Fatalf ("Unexpected error from Dispatch: %v" , err )
736+ case <- dispatchDone :
737+ }
738+
739+ // make sure that the upgrade details reported are matching our expectations
740+ require .NotNilf (t , gotDetails , "upgrade details should not be nil" )
741+ assert .Equal (t , initialScheduledUpgradeAction .ActionID , gotDetails .ActionID )
742+ assert .Equal (t , details .StateScheduled , gotDetails .State )
743+ assert .Equal (t , initialScheduledUpgradeAction .Data .Version , gotDetails .TargetVersion )
744+ assert .Empty (t , gotDetails .Metadata .ErrorMsg )
745+
746+ // affect directly the queue to get the dispatcher to actually dispatch our action
747+ removedItems := actionQueue .Cancel (initialScheduledUpgradeAction .ActionID )
748+ require .Equal (t , 1 , removedItems )
749+ actionNewStartTime := time .Now ().Add (- 5 * time .Minute ).UTC ()
750+ initialScheduledUpgradeAction .ActionStartTime = actionNewStartTime .Format (time .RFC3339 )
751+ actionQueue .Add (initialScheduledUpgradeAction , actionNewStartTime .Unix ())
752+
753+ go func () {
754+ d .Dispatch (ctx , detailsSetter , ack )
755+ }()
756+ if err := <- d .Errors (); err != nil {
757+ t .Fatalf ("Unexpected error from Dispatch: %v" , err )
758+ }
759+
760+ // make sure that upgrade details are still reported as scheduled but with a non-empty error
761+ require .NotNilf (t , gotDetails , "upgrade details should not be nil" )
762+ assert .Equal (t , initialScheduledUpgradeAction .ActionID , gotDetails .ActionID )
763+ assert .Equal (t , details .StateScheduled , gotDetails .State )
764+ assert .Equal (t , initialScheduledUpgradeAction .Data .Version , gotDetails .TargetVersion )
765+ assert .NotEmpty (t , gotDetails .Metadata .ErrorMsg )
766+
767+ // issue a brand-new upgrade action
768+ newUpgradeAction := & fleetapi.ActionUpgrade {
769+ ActionID : "upgrade-action-id" ,
770+ ActionType : fleetapi .ActionTypeUpgrade ,
771+ Data : fleetapi.ActionUpgradeData {
772+ Version : "9.3.0" ,
773+ SourceURI : "https://test-uri.test.com" ,
774+ },
775+ }
776+ def .On ("Handle" ,
777+ mock .Anything , mock .Anything , mock .Anything ).
778+ Return (nil ).Once ()
779+
780+ detailsSetter = func (upgradeDetails * details.Details ) {
781+ gotDetails = upgradeDetails
782+ }
783+ go func () {
784+ d .Dispatch (ctx , detailsSetter , ack , newUpgradeAction )
785+ }()
786+ if err := <- d .Errors (); err != nil {
787+ t .Fatalf ("Unexpected error from Dispatch: %v" , err )
788+ }
789+ require .Nil (t , gotDetails )
790+
791+ // make sure that the action queue doesn't have any actions
792+ assert .Empty (t , actionQueue .Actions ())
793+ def .AssertExpectations (t )
794+ saver .AssertExpectations (t )
795+ })
688796}
689797
690798func Test_ActionDispatcher_scheduleRetry (t * testing.T ) {
@@ -704,8 +812,13 @@ func Test_ActionDispatcher_scheduleRetry(t *testing.T) {
704812 action .On ("ID" ).Return ("id" )
705813 action .On ("RetryAttempt" ).Return (len (d .rt .steps )).Once ()
706814 action .On ("SetRetryAttempt" , mock .Anything ).Once ()
815+ action .On ("Type" ).Return (fleetapi .ActionTypeUpgrade ).Once ()
816+ action .On ("GetError" ).Return (nil ).Once ()
817+
818+ upgradeDetailsNeedUpdate := false
819+ d .scheduleRetry (context .Background (), action , ack , & upgradeDetailsNeedUpdate )
820+ assert .False (t , upgradeDetailsNeedUpdate )
707821
708- d .scheduleRetry (context .Background (), action , ack )
709822 saver .AssertExpectations (t )
710823 action .AssertExpectations (t )
711824 })
@@ -726,8 +839,13 @@ func Test_ActionDispatcher_scheduleRetry(t *testing.T) {
726839 action .On ("RetryAttempt" ).Return (0 ).Once ()
727840 action .On ("SetRetryAttempt" , 1 ).Once ()
728841 action .On ("SetStartTime" , mock .Anything ).Once ()
842+ action .On ("Type" ).Return (fleetapi .ActionTypeUpgrade ).Twice ()
843+ action .On ("GetError" ).Return (nil ).Once ()
844+
845+ upgradeDetailsNeedUpdate := false
846+ d .scheduleRetry (context .Background (), action , ack , & upgradeDetailsNeedUpdate )
847+ assert .True (t , upgradeDetailsNeedUpdate )
729848
730- d .scheduleRetry (context .Background (), action , ack )
731849 saver .AssertExpectations (t )
732850 action .AssertExpectations (t )
733851 })
0 commit comments