DESENG-811: Use Helm Charts in MET #2633
Conversation
...that coexist in the e903c2-tools namespace, to avoid collisions
since we use the common SSO keycloak instance
Personal space is nice :)
For readability, you understand Co-authored-by: Baelx <[email protected]>
Add Helm chart and templates for MET API deployment
Also add OIDC configuration endpoint and environment variable handling
DESENG-811: Implement Helm chart for MET Web
DESENG-811: Use helm charts for Redash/MET-Analytics
DESENG-811: Add helm charts for analytics API
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## omega-project #2633 +/- ##
=================================================
+ Coverage 74.94% 75.45% +0.50%
=================================================
Files 611 653 +42
Lines 22375 23305 +930
Branches 1932 1927 -5
=================================================
+ Hits 16769 17584 +815
- Misses 5334 5449 +115
Partials 272 272
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
jareth-whitney
left a comment
jareth-whitney
left a comment
There was a problem hiding this comment.
Hi Nat, this looks like a collection of great new environment definition that will modernize our platform. PR approved! Please just take a look at my comments, and especially address the following:
- Small typo
- Review usage of nullish coalescing operator
- prod values email.environment string
- deployment-scheduler vault.hashicorp.com/auth-path - is this static or should it be dynamic?
| test -f venv/bin/activate || python3.12 -m venv $(CURRENT_ABS_DIR)/venv ;\ | ||
| . venv/bin/activate ;\ | ||
| pip install --upgrade pip ;\ | ||
| pip install -Ur requirements/prod.txt ;\ |
There was a problem hiding this comment.
Is this script performing unwanted upgrades as is?
| if __name__ == "__main__": | ||
| application.run(debug=True, host='0.0.0.0', port=5001) | ||
| # Never set debug=True in production | ||
| application.run(debug=False, host='0.0.0.0', port=5001) |
|
|
||
| class ClosingSoonEmailService: # pylint: disable=too-few-public-methods | ||
| """Mail for newly published engagements""" | ||
| """Service to send emails for engagements that are closing soon.""" |
There was a problem hiding this comment.
Thank you for implementing a better description here.
| // This file is a stub for the environment variables. | ||
| // In production, this is generated and served by Nginx. | ||
| // In development, values are provided by process.env. | ||
| // It is left here so as to avoid 404 errors in development. |
There was a problem hiding this comment.
Thank you for the detailed explanations
| protocol: TCP | ||
| resources: | ||
| limits: | ||
| cpu: 150m |
There was a problem hiding this comment.
The CPU usage limitations are real :````)
| baseUrl: 'loginproxy.gov.bc.ca' | ||
|
|
||
| email: | ||
| environment: 'Sent from the Production environment' |
There was a problem hiding this comment.
Should we give the environment string a consistent structure to match dev and test? Totally optional.
There was a problem hiding this comment.
Since this will be sending to public users, I decided to make the string a bit more plain-language since "You are using the PROD environment" wouldn't make much sense to them. I can update the others to match, but I get the feeling we'll be reworking this template to remove the environment banner eventually
| @@ -10,7 +9,7 @@ parameters: | |||
| - name: SOURCE_REPOSITORY_URL | |||
| displayName: Git Repository URL | |||
| description: The URL of the repository with your application source code. | |||
| value: https://github.com/VineetBala-AOT/redash | |||
There was a problem hiding this comment.
oh my gosh, good catch
| vault.hashicorp.com/agent-pre-populate-only: 'true' | ||
| vault.hashicorp.com/namespace: platform-services | ||
| vault.hashicorp.com/role: {{ .Values.vault.engine }} | ||
| vault.hashicorp.com/auth-path: auth/k8s-gold |
There was a problem hiding this comment.
Will this value be consistent in another use context for this platform or should it be captured as an env variable?
| @@ -0,0 +1,70 @@ | |||
| apiVersion: apps.openshift.io/v1 | |||
| kind: DeploymentConfig | |||
There was a problem hiding this comment.
Aren't these being deprecated?
There was a problem hiding this comment.
Yes; DeploymentConfigs are deprecated. Tickets DESENG-802 through -810 were created to convert these DeploymentConfigs to Deployments, which is out of scope for this ticket.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Issue #: 🎟️ DESENG-811
Description of changes:
This PR consists of the following approved PRs:
User Guide update ticket (if applicable):