-
Notifications
You must be signed in to change notification settings - Fork 114
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 CFGOV Helm Charts for Local Deployment to K8s #8683
base: main
Are you sure you want to change the base?
Conversation
The dj_database_url.config method uses the DATABASE_URL variable by default so it's not necessary to explicitly specify it.
8599055
to
d305d01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a question about best-practices, should the helm/charts/postgresql-16.2.3.tgz and helm/charts/opensearch-1.31.3.tgz files be committed to the repo?
Co-authored-by: Will Barton <[email protected]>
Co-authored-by: Will Barton <[email protected]>
@willbarton, I made your changes. @chosak, do you also want to review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to get the tests running and passing, and it looks like they're failing on OpenSearch.
@@ -360,6 +360,7 @@ | |||
os.getenv("ES_USER", "admin"), | |||
os.getenv("ES_PASS", "admin"), | |||
), | |||
"use_ssl": True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this setting is causing a problem running the unit tests and functional tests in GHA.
I think we'll need to add a OPENSEARCH_DSL["default"]["use_ssl"] = False
override to cfgov/settings/test.py
for unit tests to work.
For the functional tests... the GitHub Action just calls manage.py runserver
, which defaults to cfgov/settings/local.py
, which... I don't think it is right. But changing that is out of scope here, so we'd need to add the same override to that file as well.
Alternatively, find a different way to solve the problem running in Kubernetes that this setting fixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disabled use_ssl
in both local.py
and and test.py
. Appears that both tests are passing now.
@cooldragontattoo yes, please, I'd still like to review. |
This PR adds a CFGOV Helm chart that allow for deployment to a local K8s cluster.
Additions
index.sh
andtest.sql.gz
to the CFGOV docker image.helm-init.sh
script to deploy the helm chart effortlessly.helm-uninstall.sh
script that tears down everything related to the CFGOV Helm deployment.How to test this PR
docker build . -t cfgov
helm-init.sh
localhost:<local_port>
localhost:<local_port>/about_us/blog
to make sure the Open Search index is working.Notes and todos
Checklist
/docs
folder) – for basic, close-to-the-code docs on working with this repoFront-end testing
Browser testing
Check the current browser support cutoff list](https://cfpb.github.io/consumerfinance.gov/browser-support#current-browser-support-metrics) for browsers that are advisable
to prioritize for testing.
Accessibility
Other