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 scheduled jobs #745

Merged
merged 2 commits into from
Sep 3, 2024
Merged

Add scheduled jobs #745

merged 2 commits into from
Sep 3, 2024

Conversation

coilysiren
Copy link
Contributor

Ticket

Resolves #525

Changes

  • Adds cron task that calls that database
  • Adds an AWS Step Functions scheduled job that calls the cron task every hour
  • Updates the documentation for the above

Testing

Tested via navapbc/platform-test#130, and triggering a step functions job manually.

image

Copy link
Contributor

@lorenyu lorenyu left a comment

Choose a reason for hiding this comment

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

After this is merged don't forget to:

  1. Check that template-only-cd.yml ran ok, if not you'll need to manually push changes to the platform-test* repos for the ones that failed. (Don't forget to update .template-version in the platform-test* repos if you need to do a manual update)
  2. Uncomment the cron job in platform-test (feel free to make the change directly on main, no need to open a PR)

Comment on lines 8 to 13
scheduled_jobs = {
cron = {
task_command = ["python", "-m", "flask", "--app", "app.py", "cron"]
schedule_expression = "cron(0 * ? * * *)"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

For the template we want this commented out since it's just an example, similar to what we did in file_upload_jobs.tf, then after this is merged in template-infra, we want to uncomment this config in platform-test so that we can exercise the functionality.

@lorenyu
Copy link
Contributor

lorenyu commented Sep 1, 2024

Another way to update platform-test if the template CD doesn't work is to:

  1. Reopen the platform-test PR
  2. Update .template-version and push
  3. Merge that PR

@lorenyu lorenyu changed the title Adds scheduled jobs Add scheduled jobs Sep 3, 2024
@coilysiren coilysiren merged commit 9616fcf into main Sep 3, 2024
12 of 14 checks passed
@coilysiren coilysiren deleted the kai/sfn branch September 3, 2024 16:48
coilysiren added a commit to navapbc/platform-test that referenced this pull request Sep 27, 2024
coilysiren added a commit to HHS/simpler-grants-gov that referenced this pull request Oct 17, 2024
## Summary

Relates to: #2351, #2472

### Time to review: __5 mins__

## Changes proposed

There are two goals to this PR:

1. Make a material change that gets us closer in line with the platform.
Specifically we are pulling in
navapbc/template-infra#745
2. Updates the step functions in advance of adding another step function
- for the terraform in ECS stuff

## Context for reviewers

The majority of the diff is security related, specifically: 

- via guidance from @lorenyu, the permissions for managing the step
function and workflow triggers were moved off of the task exec role.
that's the diff on `infra/modules/service/access-control.tf`
- those permissions were moved to
`infra/modules/service/scheduler_role.tf` and
`infra/modules/service/workflow_orchestrator_role.tf`

The rest of the diff is around easy of configuration:

- `infra/modules/service/scheduled_jobs.tf` represents the new, highly
configurable, way to create scheduled jobs
- `infra/api/app-config/env-config/scheduled_jobs.tf` shows how two
scheduled jobs are configured

## Additional information

<img width="1238" alt="image"
src="https://github.com/user-attachments/assets/24e6e50e-74b0-4cf8-b61b-bad34af90118">

<img width="1237" alt="image"
src="https://github.com/user-attachments/assets/aa297341-8d7d-4ac7-bcc9-ef102df25e05">
doug-s-nava pushed a commit to HHS/simpler-grants-gov that referenced this pull request Oct 18, 2024
## Summary

Relates to: #2351, #2472

### Time to review: __5 mins__

## Changes proposed

There are two goals to this PR:

1. Make a material change that gets us closer in line with the platform.
Specifically we are pulling in
navapbc/template-infra#745
2. Updates the step functions in advance of adding another step function
- for the terraform in ECS stuff

## Context for reviewers

The majority of the diff is security related, specifically: 

- via guidance from @lorenyu, the permissions for managing the step
function and workflow triggers were moved off of the task exec role.
that's the diff on `infra/modules/service/access-control.tf`
- those permissions were moved to
`infra/modules/service/scheduler_role.tf` and
`infra/modules/service/workflow_orchestrator_role.tf`

The rest of the diff is around easy of configuration:

- `infra/modules/service/scheduled_jobs.tf` represents the new, highly
configurable, way to create scheduled jobs
- `infra/api/app-config/env-config/scheduled_jobs.tf` shows how two
scheduled jobs are configured

## Additional information

<img width="1238" alt="image"
src="https://github.com/user-attachments/assets/24e6e50e-74b0-4cf8-b61b-bad34af90118">

<img width="1237" alt="image"
src="https://github.com/user-attachments/assets/aa297341-8d7d-4ac7-bcc9-ef102df25e05">
coilysiren added a commit that referenced this pull request Oct 18, 2024
## Ticket

Resolves #744

## Changes

- Changes the ETL job from invoking ECS directly, to instead invoking
Step Functions which then invokes ECS
- Makes some changes to the scheduled changes to increase their
distinction from the events jobs, and the readability of their IAM roles

## Context for reviewers

This is a similar PR to
#745. In-fact they use
many of the same resources. To goal of this PR is to run ETL events via
Step Functions. Step Functions create a tracking layer that's not
available when invoking ECS directly. This tracking layer allows you to
see the success and failure status of your jobs.

## Testing

navapbc/platform-test#136
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.

Jobs that trigger on a schedule
2 participants