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

Parwinder/bcda-8627-archive cleanup job migration #1042

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

bhagatparwinder
Copy link
Contributor

🎫 Ticket

https://jira.cms.gov/browse/BCDA-8627

🛠 Changes

Updated River Client to set archive cleanup periodic job.

ℹ️ Context

  • We are migrating away from Jenkins and moving to GHA.
  • Archive/Cleanup Job has been moved to the bcda-app.
  • Job is scheduled using River Go

bcda/bcdacli/cli.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@carlpartridge carlpartridge left a comment

Choose a reason for hiding this comment

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

Looking good!

bcda/bcdacli/cli.go Outdated Show resolved Hide resolved
bcdaworker/queueing/river.go Show resolved Hide resolved
bcdaworker/queueing/river.go Outdated Show resolved Hide resolved
bcdaworker/queueing/river.go Outdated Show resolved Hide resolved
@carlpartridge
Copy link
Collaborator

What is our testing strategy going to be for making sure these files are being cleaned up properly? Can we stick a bunch of files into EFS to test? They will need to match up with expired jobs right?

@bhagatparwinder
Copy link
Contributor Author

What is our testing strategy going to be for making sure these files are being cleaned up properly? Can we stick a bunch of files into EFS to test? They will need to match up with expired jobs right?

To test it in DEV/TEST environment, I believe that is the route we will have to take. We will have to find out:

  • How to get files in EFS
  • What are the correct files (for expired, canceled, and failed jobs)

@bhagatparwinder bhagatparwinder marked this pull request as ready for review February 7, 2025 21:28
@bhagatparwinder bhagatparwinder enabled auto-merge (squash) February 7, 2025 21:28
@carlpartridge
Copy link
Collaborator

What is our testing strategy going to be for making sure these files are being cleaned up properly? Can we stick a bunch of files into EFS to test? They will need to match up with expired jobs right?

To test it in DEV/TEST environment, I believe that is the route we will have to take. We will have to find out:

  • How to get files in EFS
  • What are the correct files (for expired, canceled, and failed jobs)

Thinking about this a bit. One way we could test is that we generate some jobs at 10:30 and 11:30. Then the next day (24 hrs later) at ~11:01 the 10:30 jobs should be cleaned up and marked appropriately right? And the 11:30 jobs should not be touched? The issue with this approach is that it requires a 24hr testing window, which means that it will be slow to get a deploy out (not the end of the world or anything).

It also occurs to me that we cant disable the current jenkins script as it is doing all environments. At least not without deploying bcda-ops changes. This could also make it difficult to test? Also the jenkins script will start failing as the CLI commands have been removed.

I think with all these difficulties to test it might be safer/easier to bring back the CLI commands for now? @laurenkrugen-navapbc @austincanada How do you feel about all this?

@bhagatparwinder
Copy link
Contributor Author

What is our testing strategy going to be for making sure these files are being cleaned up properly? Can we stick a bunch of files into EFS to test? They will need to match up with expired jobs right?

To test it in DEV/TEST environment, I believe that is the route we will have to take. We will have to find out:

  • How to get files in EFS
  • What are the correct files (for expired, canceled, and failed jobs)

Thinking about this a bit. One way we could test is that we generate some jobs at 10:30 and 11:30. Then the next day (24 hrs later) at ~11:01 the 10:30 jobs should be cleaned up and marked appropriately right? And the 11:30 jobs should not be touched? The issue with this approach is that it requires a 24hr testing window, which means that it will be slow to get a deploy out (not the end of the world or anything).

It also occurs to me that we cant disable the current jenkins script as it is doing all environments. At least not without deploying bcda-ops changes. This could also make it difficult to test? Also the jenkins script will start failing as the CLI commands have been removed.

I think with all these difficulties to test it might be safer/easier to bring back the CLI commands for now? @laurenkrugen-navapbc @austincanada How do you feel about all this?

I like it. Providing an alternative:

  1. The AC on my tickets requests the removal of those Jenkins jobs.
  2. We need a bcda-ops deployment to create the MDTCoC ACO. So I can make additional changes to BCDA ops if needed.
  3. We have not changed what the jobs do but how we kick them off.
  4. If we go this route, we do need a task to clean up Jenkins.

@laurenkrugen-navapbc
Copy link
Contributor

What is our testing strategy going to be for making sure these files are being cleaned up properly? Can we stick a bunch of files into EFS to test? They will need to match up with expired jobs right?

To test it in DEV/TEST environment, I believe that is the route we will have to take. We will have to find out:

  • How to get files in EFS
  • What are the correct files (for expired, canceled, and failed jobs)

Thinking about this a bit. One way we could test is that we generate some jobs at 10:30 and 11:30. Then the next day (24 hrs later) at ~11:01 the 10:30 jobs should be cleaned up and marked appropriately right? And the 11:30 jobs should not be touched? The issue with this approach is that it requires a 24hr testing window, which means that it will be slow to get a deploy out (not the end of the world or anything).

It also occurs to me that we cant disable the current jenkins script as it is doing all environments. At least not without deploying bcda-ops changes. This could also make it difficult to test? Also the jenkins script will start failing as the CLI commands have been removed.

I think with all these difficulties to test it might be safer/easier to bring back the CLI commands for now? @laurenkrugen-navapbc @austincanada How do you feel about all this?

I think having a written strategy + expectations of the cleanup would be good -- I also think one (or more) of us can help QA this. We could temporarily update the jenkins script to not clean up files in our dev env (remove dev as default value); that way we can still manually cleanup with the old script if needed, but it won't automatically cleanup files before we can test the new script.

@bhagatparwinder bhagatparwinder enabled auto-merge (squash) February 11, 2025 01:51
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.

4 participants