Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add better error handling #40

Closed
WilboMo opened this issue Apr 15, 2021 · 3 comments
Closed

Add better error handling #40

WilboMo opened this issue Apr 15, 2021 · 3 comments
Assignees
Labels
priority/p0 Must have
Milestone

Comments

@WilboMo
Copy link
Contributor

WilboMo commented Apr 15, 2021

What I'd like:
Implement a mechanism to audit errors returned by Updater's functions and methods. The errors needs to be scanned to determine if the arisen error is fatal to Updater's operations and thus the instance should be aborted, or if it is a minor problem which can be passed and re-tried the next time the instance is caught by Updater for processing.

This issue originates from the following PR comment:

... continue updating next instance and worry about updating this instance in next program iteration. 
Similar things for lot many errors, I think we need to scan all the errors and proceed for non fatal error.

_Originally posted by @srgothi92 in 
https://github.com/bottlerocket-os/bottlerocket-ecs-updater/pull/38#discussion_r614438390_

@WilboMo
Copy link
Contributor Author

WilboMo commented Apr 16, 2021

#35 (comment)

@srgothi92
Copy link
Contributor

Some refactoring is done in PR-56 and PR-51 which partially addresses this issue, however we should scan the complete code base and make sure all the errors are handled properly. For each error it is important to decide on 3 actions:

  1. Is error fatal; if yes, make sure updater stops.
  2. Do we need to reset any cluster state on error
  3. Can we just log the error and continue

Additionally, we should try to run set of tests (manual or automated) which touches all the error and make sure they are handled correctly.

@jhaynes jhaynes added the priority/p0 Must have label May 21, 2021
@srgothi92 srgothi92 self-assigned this Jun 1, 2021
@srgothi92
Copy link
Contributor

Re-verified all the cases and added log fixes wherever requires as part of PR-77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/p0 Must have
Projects
None yet
Development

No branches or pull requests

3 participants