-
Notifications
You must be signed in to change notification settings - Fork 1
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
Convert ingress to Helm chart #18
Conversation
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.
LGTM! Had some thoughts but they're unrelated to this PR :)
@@ -2,5 +2,5 @@ apiVersion: v2 | |||
name: alerts-chart | |||
description: A Helm chart for Kubernetes | |||
type: application | |||
version: 0.1.0 | |||
version: 0.1.6 |
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.
Definitely not needed for this PR, but I wonder if it makes sense to have this version number in a central location for all charts so it can be updated once and read everywhere.
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.
Yes, I think that's a good idea!
port: | ||
number: 8080 | ||
- path: /alerts/* | ||
pathType: Prefix | ||
backend: | ||
service: | ||
name: phdi-{env}-alerts-service | ||
name: alerts-service |
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.
Also not needed for this PR, but I'm a little curious about the behavior of the ingress controller when not all the services are present. If we're not running record linkage for instance, what happens when traffic is routed to the /record-linkage
endpoint?
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.
Basically, it just throws a 502 Bad Gateway
, and will spit out an error in the event logs for the ingress in Azure
This removes the release/env name from the services so that we can reference them without interpolation (they are already inside a cluster with the environment name anyways)
Also attempting to fix the issue with the release workflow.
Also adds the TLS config needed for this PR in phdi-playground