-
Notifications
You must be signed in to change notification settings - Fork 17
add example for custom ingress #146
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
Conversation
|
@sreya I have to add a bunch of this stuff to platform one's big bang deployment since it only has 1 LB which is shared by all of their services. The ingress stuff is required for them even with serviceNext. |
jawnsy
left a comment
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. I'll be more careful to check the preview this time before merging 😅
| {{- end }} | ||
| spec: | ||
| {{- include "coder.ingress.tls" . }} | ||
| rules: |
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.
suggest we add a defaultBackend too: https://kubernetes.io/docs/concepts/services-networking/ingress/#default-backend
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.
Default backend is supposed to be on the controller rather than the ingress resources (by convention)... this choice was deliberate.
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.
@mterhar TIL! Do you have a link to learn about this convention?
| name: coderd-ingress | ||
| namespace: {{ .Release.Namespace | quote }} | ||
| annotations: | ||
| nginx.ingress.kubernetes.io/proxy-body-size: "0" |
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 this can be put in the default values file, then we can make all of the annotations configurable, similar to how we do it with securityContext: https://github.com/cdr/enterprise-helm/blob/e158a751b9a1c3198adff2c08b4a5f50c1e33fdf/templates/coderd.yaml#L36-L42
right now there's no way to clear the nginx.ingress.kubernetes.io/proxy-body-size annotation here
FWIW, I think this resolves: https://app.clubhouse.io/coder/story/14644/route-traffic-to-coderd-using-ingress -- thank you!
| # useDefault set to false disables creation of the ingress controller | ||
| useDefault: false | ||
| # add annotations for TLS issuers and such | ||
| annotations: {} No newline at end of file |
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.
not mandatory but would be nice to modify the OpenShift one to add an example that creates an OpenShift Route (it needs some different annotations)
|
[ch14644] |
|
This pull request has been linked to Clubhouse Story #14644: Route traffic to coderd using Ingress. |
I'm using the following command to ensure the right objects are being created:
helm template coder . -f examples/ingress/ingress.values.yaml --set postgres.default.enable=falseThe goal of this is to:
ingress.enable=true