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

Handle container lifecycle events through eventbridge #261

Merged
merged 15 commits into from
Aug 17, 2020
Merged

Handle container lifecycle events through eventbridge #261

merged 15 commits into from
Aug 17, 2020

Conversation

epicfaace
Copy link
Contributor

@epicfaace epicfaace commented Aug 14, 2020

Fixes #257.

Adds infrastructure for handling events (task start, task stop) through EventBridge. When an event is called, it calls the updateScanTaskStatus lambda function, which updates the scantask's status accordingly.

This approach is better than the earlier approach because we can be sure we catch all errors (previously, sometimes if the entire process crashed, the db wouldn't update that the task had stopped and it would be marked as "started" forever).

Local development

For local development, there is a handler that listens to Docker events (container start / stop) and then invokes updateScanTaskStatus with the right payload. This handler runs on the backend container during local development.

Logs

One change is that the output column no longer contains the error directly, as that is not returned from the EventBridge stopped event. Logs can now be accessed by directly going to a link on CloudWatch on the AWS console. Perhaps in the future we can show logs directly in Crossfeed by pulling data directly from CloudWatch.

image

Testing

I've deployed this on https://staging.crossfeed.cyber.dhs.gov/ and it seems to work well.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Aug 14, 2020

This pull request introduces 1 alert when merging 3699aae into 5bb9d69 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Aug 14, 2020

This pull request introduces 1 alert when merging dc0136c into 5bb9d69 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Aug 15, 2020

This pull request introduces 3 alerts and fixes 1 when merging ae39fa7 into 5bb9d69 - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class
  • 1 for Superfluous trailing arguments

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Aug 15, 2020

This pull request introduces 3 alerts and fixes 1 when merging 512c83d into 5bb9d69 - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class
  • 1 for Superfluous trailing arguments

fixed alerts:

  • 1 for Unused variable, import, function or class

@epicfaace epicfaace requested a review from cablej August 15, 2020 14:35
@epicfaace epicfaace marked this pull request as ready for review August 15, 2020 14:35
@epicfaace epicfaace changed the title add infra for handling events through eventbridge Handle container lifecycle events through eventbridge Aug 15, 2020
@epicfaace epicfaace requested a review from jhackshaw August 15, 2020 14:37
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Aug 15, 2020

This pull request introduces 1 alert and fixes 1 when merging 79719b1 into 5bb9d69 - view on LGTM.com

new alerts:

  • 1 for Superfluous trailing arguments

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Aug 15, 2020

This pull request introduces 1 alert and fixes 1 when merging ee55c2a into 5bb9d69 - view on LGTM.com

new alerts:

  • 1 for Superfluous trailing arguments

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Aug 16, 2020

This pull request introduces 1 alert and fixes 1 when merging 843867e into 5bb9d69 - view on LGTM.com

new alerts:

  • 1 for Superfluous trailing arguments

fixed alerts:

  • 1 for Unused variable, import, function or class

@epicfaace
Copy link
Contributor Author

@cablej can you take a look when you get a chance? I tested this on the old staging environment and everything seemed to work.

Copy link
Contributor

@cablej cablej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, we can consider fetching logs inline in the future

@cablej cablej merged commit bd9e939 into master Aug 17, 2020
@cablej cablej deleted the events branch August 17, 2020 21:25
@epicfaace epicfaace mentioned this pull request Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use ECS events + EventBridge to track task success/failure
2 participants