ActionQueue retry on failure #1944
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ee/control/control_test.go
Outdated
| // Verify error consumer was called | ||
| assert.Equal(t, 1, errConsumer.updates) | ||
|
|
||
| // Verify hash was still recorded despite error |
There was a problem hiding this comment.
Hm, maybe I'm missing something -- is this the desired behavior? I thought we didn't want to store/record the hash, so that we don't have to wait for a change in configuration (i.e. a new hash) to perform the retry. (I think that'd look like changing the behavior around here: https://github.com/kolide/launcher/blob/main/ee/control/control.go#L338-L346.)
There was a problem hiding this comment.
I wasn't fully understanding that part but I think I get it now, the hash not being saved would be what will retrigger the fetch and update. Let me adjust that
Co-authored-by: James Pickett <James-Pickett@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #1708
We are implementing a retry pattern by returning an error from func (aq *ActionQueue) Update(data io.Reader) error if any action fails to process. Then the updated data for that subsystem shouldn't be stored, so Update would be called again in 1 minute by the control system, effectively handling the retry for us.
This pull request introduces error handling improvements and additional test cases for the
ActionQueueandControlServicecomponents. The main changes include adding error handling for action processing failures, updating test cases to handle these errors, and adding new test cases for retry logic in theControlService.Error handling improvements:
ee/control/actionqueue/actionqueue.go: IntroducedprocessErrorto capture errors during action processing and return it at the end of theUpdatemethod. [1] [2] [3]Updates to existing tests:
ee/control/actionqueue/actionqueue_test.go: Modified theTestActionQueue_HandlesDuplicatesWhenFirstActionCouldNotBeSenttest to check for errors on the first attempt and ensure success on the second attempt. [1] [2]New test cases:
ee/control/control_test.go: AddedTestControlServiceUpdateErrto verify that theControlServicehandles update errors correctly and records the hash despite the error.ee/control/control_test.go: AddedTestControlServiceRetryAfterUpdateErrto test the retry logic after an update error, ensuring that subsequent updates succeed and the final hash is recorded.Mock updates:
ee/control/control_test.go: Modified themockConsumerto support returning an error in theUpdatemethod. [1] [2]