chatqna-ui helm chart - nginx reverse proxy #305
Conversation
|
@SeanCondon please fix the DCO failure. You may need to 'git commit -s' to add the sign-off-by line in your commit message |
|
I think we also need to disable the 'helm test' for this case(i.e. delete the testpod definition). Since we can't have a backward dependency from ui to backend in this repo layout. |
|
|
||
| location {{ .Values.BACKEND_SERVICE_ENDPOINT }} { | ||
| proxy_pass {{ .Values.backend.scheme }}://{{ .Release.Name }}-{{ .Values.backend.name }}:{{ .Values.backend.port }}/; | ||
| } |
There was a problem hiding this comment.
Would prefer to keep the meaning of BACKEND_SERVICE_ENDPOINT as the same, because docker-compose is also using it.
Also, the {{ .Release.Name }} is not necessary part of the ENDPOINT, since it's possible chatqna-ui and backend have different release name. We need to give user options to change that.
Hardcode the location shouldn't be a problem, if that is, let's define a new variable to do that.
Recommend this way:
location "/chatqna/" {
{{- if .Values.BACKEND_SERVICE_ENDPOINT }}
proxy_pass {{ .Values.BACKEND_SERVICE_ENDPOINT }}
{{- else }}
proxy_pass {{ .Values.backend.scheme }}://{{ .Release.Name }}-{{ .Values.backend.name }}:{{ .Values.backend.port }}/;
}
| proxy_pass {{ .Values.backend.scheme }}://{{ .Release.Name }}-{{ .Values.backend.name }}:{{ .Values.backend.port }}/; | ||
| } | ||
|
|
||
| location {{ .Values.DATAPREP_SERVICE_ENDPOINT }} { |
| # chatQnA Mega service URL, e.g. http://<service-name>:<port> | ||
| BACKEND_SERVICE_ENDPOINT: "" | ||
| BACKEND_SERVICE_ENDPOINT: "/chatqna/" |
There was a problem hiding this comment.
No change of this variable's meaning as said above.
|
|
||
| # data preparation service URL, http://<service-name>:<port> | ||
| DATAPREP_SERVICE_ENDPOINT: "" | ||
| DATAPREP_SERVICE_ENDPOINT: "/chatqna-data-prep/" |
|
Thanks @SeanCondon for creating this PR. Regarding this, I saw @jaswanth8888 has fixed the issue opea-project/GenAIExamples#493 on defining the urls during build. But it does not clean up the CORS issue. Maybe can @SeanCondon and @jaswanth8888 get aligned here? So that we would have same logic for the UI in both docker and kubernetes case? |
This is correct - it is a side effect of the strict checking of downstream dependencies by nginx pod. |
@Ruoyu-y I have created a PR in |
|
@SeanCondon @Ruoyu-y has merged a PR #377 to add the nginx reverse proxy at chatqna e2e level. So I'm wondering if this PR is still needed or not? |
Thanks for letting me know about #377 - I had not seen it. If the problem is solved then this change is not needed. Please feel free to reopen if needed. cc @Ruoyu-y. Please see opea-project/GenAIExamples#616 where I address this for |
Description
This PR adds an nginx reverse proxy to the ChatQnA UI helm chart which allows the path to the
chatqnapod and thedata-preppod to be configured at deploy time. This also has the benefit that it removes CORS errorsIssues
#276
Type of change
List the type of change like below. Please delete options that are not relevant.
Dependencies
List the newly introduced 3rd party dependency if exists.
Tests
Describe the tests that you ran to verify your changes.