remove vllm-check/tgi-check init-container

[robinliubin]

add wget vllm into h2ogpt container

fixing: #1603

[joby-h20]

Tested by injecting the changes directly to deployment manifest.

https://h2oai.slack.com/archives/C06R0QAP54N/p1715188293152829?thread_ts=1715178088.028339&cid=C06R0QAP54N

[joby-h20]

lgtm

[achraf-mer]

added one comment about using a list or arguments, but if tested and works fine then LGTM, thanks.

[EshamAaqib]

LGTM

[ozahavi]

This change may cause issues if we ever want to introduce liveness\readiness probes to h2oGPT.

[EshamAaqib]

^ +1, If the inference service is vLLM and if Model Lock is used the check will not be needed, it can be disabled completely

[robinliubin]

This change may cause issues if we ever want to introduce liveness\readiness probes to h2oGPT.

thanks @ozahavi, would you elaborate more about moving wget into h2ogpt container would be an issue for liveness/readiness?

[robinliubin]

^ +1, If the inference service is vLLM and if Model Lock is used the check will not be needed, it can be disabled completely

like this:

{{- if and .Values.vllm.enabled (not .Values.h2ogpt.externalLLM.enabled) }}

[EshamAaqib]

^ +1, If the inference service is vLLM and if Model Lock is used the check will not be needed, it can be disabled completely

like this:

{{- if and .Values.vllm.enabled (not .Values.h2ogpt.externalLLM.enabled) }}

Yes, and also you can stick with Model Lock itself for locally hosted LLMs and if the inference is vLLM the wget check is not needed (if you use Model Lock)

Ex: https://github.com/h2oai/public-cloud-infrastructure/blob/develop/hamc/hadc-customer-account/terraform/environment/applications/main-h2ogpt.tf#L5

[robinliubin]

^ +1, If the inference service is vLLM and if Model Lock is used the check will not be needed, it can be disabled completely

like this:

{{- if and .Values.vllm.enabled (not .Values.h2ogpt.externalLLM.enabled) }}

Yes, and also you can stick with Model Lock itself for locally hosted LLMs and if the inference is vLLM the wget check is not needed (if you use Model Lock)

Ex: https://github.com/h2oai/public-cloud-infrastructure/blob/develop/hamc/hadc-customer-account/terraform/environment/applications/main-h2ogpt.tf#L5

like this:

{{- if and .Values.vllm.enabled (not .Values.h2ogpt.externalLLM.modelLock) }}

[robinliubin]

tested with: helm template h2oai helm/h2ogpt-chart --set vllm.enabled=true --set h2ogpt.externalLLM.modelLocak=null
it rendered to:

       args:
          - >
            until wget -O- http://h2oai-h2ogpt-vllm-inference:5000/v1/models >/dev/null 2>&1;
              do
                echo "Waiting for inference service to become ready...";
                sleep 5;
              done
              
            python3 /workspace/generate.py

[EshamAaqib]

^ +1, If the inference service is vLLM and if Model Lock is used the check will not be needed, it can be disabled completely

like this:

{{- if and .Values.vllm.enabled (not .Values.h2ogpt.externalLLM.enabled) }}

Yes, and also you can stick with Model Lock itself for locally hosted LLMs and if the inference is vLLM the wget check is not needed (if you use Model Lock)

Ex: https://github.com/h2oai/public-cloud-infrastructure/blob/develop/hamc/hadc-customer-account/terraform/environment/applications/main-h2ogpt.tf#L5

like this:

{{- if and .Values.vllm.enabled (not .Values.h2ogpt.externalLLM.modelLock) }}

Yup lgtm