@@ -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,118 @@ 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+ select {
757+ case err := <- d .Errors ():
758+ if err != nil {
759+ t .Fatalf ("Unexpected error from Dispatch: %v" , err )
760+ }
761+ }
762+
763+ // make sure that upgrade details are still reported as scheduled but with a non-empty error
764+ require .NotNilf (t , gotDetails , "upgrade details should not be nil" )
765+ assert .Equal (t , initialScheduledUpgradeAction .ActionID , gotDetails .ActionID )
766+ assert .Equal (t , details .StateScheduled , gotDetails .State )
767+ assert .Equal (t , initialScheduledUpgradeAction .Data .Version , gotDetails .TargetVersion )
768+ assert .NotEmpty (t , gotDetails .Metadata .ErrorMsg )
769+
770+ // issue a brand-new upgrade action
771+ newUpgradeAction := & fleetapi.ActionUpgrade {
772+ ActionID : "upgrade-action-id" ,
773+ ActionType : fleetapi .ActionTypeUpgrade ,
774+ Data : fleetapi.ActionUpgradeData {
775+ Version : "9.3.0" ,
776+ SourceURI : "https://test-uri.test.com" ,
777+ },
778+ }
779+ def .On ("Handle" ,
780+ mock .Anything , mock .Anything , mock .Anything ).
781+ Return (nil ).Once ()
782+
783+ detailsSetter = func (upgradeDetails * details.Details ) {
784+ gotDetails = upgradeDetails
785+ }
786+ go func () {
787+ d .Dispatch (ctx , detailsSetter , ack , newUpgradeAction )
788+ }()
789+ select {
790+ case err := <- d .Errors ():
791+ if err != nil {
792+ t .Fatalf ("Unexpected error from Dispatch: %v" , err )
793+ }
794+ }
795+ require .Nil (t , gotDetails )
796+
797+ // make sure that the action queue doesn't have any actions
798+ assert .Empty (t , actionQueue .Actions ())
799+ def .AssertExpectations (t )
800+ saver .AssertExpectations (t )
801+ })
688802}
689803
690804func Test_ActionDispatcher_scheduleRetry (t * testing.T ) {
@@ -704,8 +818,13 @@ func Test_ActionDispatcher_scheduleRetry(t *testing.T) {
704818 action .On ("ID" ).Return ("id" )
705819 action .On ("RetryAttempt" ).Return (len (d .rt .steps )).Once ()
706820 action .On ("SetRetryAttempt" , mock .Anything ).Once ()
821+ action .On ("Type" ).Return (fleetapi .ActionTypeUpgrade ).Once ()
822+ action .On ("GetError" ).Return (nil ).Once ()
823+
824+ upgradeDetailsNeedUpdate := false
825+ d .scheduleRetry (context .Background (), action , ack , & upgradeDetailsNeedUpdate )
826+ assert .False (t , upgradeDetailsNeedUpdate )
707827
708- d .scheduleRetry (context .Background (), action , ack )
709828 saver .AssertExpectations (t )
710829 action .AssertExpectations (t )
711830 })
@@ -726,8 +845,13 @@ func Test_ActionDispatcher_scheduleRetry(t *testing.T) {
726845 action .On ("RetryAttempt" ).Return (0 ).Once ()
727846 action .On ("SetRetryAttempt" , 1 ).Once ()
728847 action .On ("SetStartTime" , mock .Anything ).Once ()
848+ action .On ("Type" ).Return (fleetapi .ActionTypeUpgrade ).Twice ()
849+ action .On ("GetError" ).Return (nil ).Once ()
850+
851+ upgradeDetailsNeedUpdate := false
852+ d .scheduleRetry (context .Background (), action , ack , & upgradeDetailsNeedUpdate )
853+ assert .True (t , upgradeDetailsNeedUpdate )
729854
730- d .scheduleRetry (context .Background (), action , ack )
731855 saver .AssertExpectations (t )
732856 action .AssertExpectations (t )
733857 })
0 commit comments