-
Notifications
You must be signed in to change notification settings - Fork 2
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
LNP-1112: Move to single deployment #740
LNP-1112: Move to single deployment #740
Conversation
@@ -43,7 +43,7 @@ spec: | |||
{{- end }} | |||
{{- end }} | |||
{{- with .Values.ingress.host }} | |||
- host: {{ tpl .pattern (dict "qualifier" $.Values.ingress.qualifier "Template" $.Template) }} | |||
- host: {{ tpl .pattern (dict "Template" $.Template) }} |
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 think places where you do tpl but only pass it (dict "Template" $.Template)
could be replaced with a direct reference to something in .Values
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.
These were the bits that blew up when I tried to directly replace as it was complaining about invalid values, maps etc.
host: | ||
pattern: "prisoner-content-hub-development-{{ .qualifier }}.apps.live.cloud-platform.service.justice.gov.uk" | ||
pattern: "prisoner-content-hub-development.apps.live.cloud-platform.service.justice.gov.uk" |
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 there's no other references to variables in this it no longer needs to be a pattern/ or use tpl
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.
Missed that one, will try and sort next PR.
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
Context
Yes
Intent
It removes the multiple deployments in dev so there is a single deployment for a PR/branch as per other environments and products.
No.
Considerations
No
No.
Checklist