Skip to content

Conversation

@snf2ye
Copy link
Contributor

@snf2ye snf2ye commented Sep 25, 2024

Jira ticket: https://broadworkbench.atlassian.net/browse/DC-1227

Addresses

The TDR dev instance is more expensive than our staging and production instances. This is most likely due to our "personal" deployments. We also know that logging is one of our biggest costs of our infrastructure. We can see that our personal deployments are creating a lot of logs. So, by shutting them down, we expect the cost of the dev environment to decrease.

@fboulnois pointed out that we are seeing hundreds of thousands of logs per day on our personal environments:
image

Summary of changes

Testing Strategy

I followed these instructions to start up and shut down my own personal deployment.

Next Steps

Once this PR is approved, I will run helmfile destroy on all of our remaining personal deployments. They can be recreated by running helmfile apply from the datarepo-helm-definitions repo as indicated in the instructions.

@snf2ye snf2ye requested review from a team as code owners September 25, 2024 19:02
@snf2ye snf2ye requested review from fboulnois and rushtong and removed request for a team September 25, 2024 19:02
Comment on lines 376 to 378
However, there are still some use cases for personal dev environments. So, you should feel free to
use one of the existing personal dev environments for your work (but check with the data repo team to
see if anyone else is using a particular environment).
Copy link
Contributor Author

@snf2ye snf2ye Sep 25, 2024

Choose a reason for hiding this comment

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

Note to reviewers: The terraform code to create personal deployments is in the terraform-jade repository (that is now archived). My opinion is that we can just use existing namespace configurations - it doesn't really matter whose name was originally assigned to them. We shouldn't set up new personal deployments for new members of the team.

Copy link
Contributor

@fboulnois fboulnois left a comment

Choose a reason for hiding this comment

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

🎉

Copy link
Member

@pshapiro4broad pshapiro4broad left a comment

Choose a reason for hiding this comment

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

Looks OK. Not directly related to your change, but I believe the invocation of db-connect.sh in this doc is no longer valid.

Comment on lines 414 to 415
cd jade-data-repo/ops
DB=datarepo-ZZ SUFFIX=ZZ ENVIRONMENT=dev ./db-connect.sh
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this wasn't updated as part of #1731

Actually looking at this more closely I'm not sure how to invoke the script for this case. @fboulnois can you provide the new command line for this operation?

Copy link
Contributor

Choose a reason for hiding this comment

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

That functionality is no longer supported in the db-connect.sh script. This decision was based on feedback as a part of #1731 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After looking more into this, given that personal deployments will mainly be used for testing helm charts, I don't expect for there to be extensive postgres querying. Users can access the database in the Gcloud UI for some one-off queries.

@pshapiro4broad
Copy link
Member

Also I see that this PR causes test to run. Since it's a documentation only change, can we update our test triggers so they aren't run on this change? I think this is a behavior regression vs the previous test configuration.

@sonarqubecloud
Copy link

@snf2ye
Copy link
Contributor Author

snf2ye commented Sep 30, 2024

Also I see that this PR causes test to run. Since it's a documentation only change, can we update our test triggers so they aren't run on this change? I think this is a behavior regression vs the previous test configuration.

@pshapiro4broad - This has been the case for a while - We're in a tricky spot b/c we require tests to run in order for us to be able to merge the PR. So, if we skip the test run, then we're forced to override checks in order to merge the PR.

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.

5 participants