add istio ingress options #278
Conversation
|
We are using the virtual service and gateway at $JOB, works well for us. |
|
#274 is related in the same vein, a third ingress option. Depending on what we do in this PR, will make fixing that issue easier. |
| @@ -969,8 +973,26 @@ spec: | |||
| .. | |||
| ``` | |||
|
|
|||
| #### 3.3. Redeploy or upgrade your ReporPortal installation with Helm | |||
There was a problem hiding this comment.
This line is essential, since it's necessary to redeploy the application after actions above. I agree that it's not clear in such view. Could you please just add some text under the heading?
| @@ -0,0 +1,35 @@ | |||
| {{- if .Values.ingress.istio.gateway.create }} | |||
There was a problem hiding this comment.
Should not also be created if .Values.ingress.istio.enable set on false I presume.
| # set ingress.enable to false | ||
| # set ingress.istio.enable true | ||
| # configure hostnames with ingress.hosts[] | ||
| istio: |
There was a problem hiding this comment.
Please move it outside ingress values and name it something like istioIngress. Since putting it inside 'Ingress' section makes these two options logically connected. I would expect chart automatically disable nginx in case of istio enabled, but this not the case.
|
@Jmainguy Please check out my comments. |
|
Thank you for feedback, will work on fixing up |
|
Hei question, What is the reason this PR got closed without mergin? |
|
@GianniGiglio hi. You didn't answer on the comments a long time. |
|
@raikbitters I wasn't involved in this ticket just bumped on it by accident and I was wondering if you guy's where planning to support istio ingress since I have a simular use case. :) You have me confussed with someone else :) |
|
@GianniGiglio sorry for the misunderstanding. We don't consider supporting Istio right now. |
Taking @nabbott2008 virtualservice from #185, and adding it as an option to the chart.
Fix some typos in the README.md
move around some information to make it clear ingress isn't the only option.
Would love more feedback on the README.md changes and where to place the values, I figured under ingress would make sense, could argue for moving it to its own block if you wanted to.