make the HOST variable configurable

[menardorama]

make the HOST variable configurable

Kubernetes clusters can be single stack or dual stack. This change address the recent change where dual stack cluster is set to be the default.

Setting can now be configurable in the chart

Relevant issues

Fixes #5517

Type

🐛 Bug Fix

Changes

Add a value for the HOST env variable in order to make the helm chart less restrictive on the environment where it is deployed

[REQUIRED] Testing - Attach a screenshot of any new tests passing locall

helm template . --set listen="0.0.0.0"

      containers:
        - name: litellm
          securityContext:
            {}
          image: "ghcr.io/berriai/litellm-database:main-v1.43.18"
          imagePullPolicy: IfNotPresent
          env:
            - name: HOST
              value: "0.0.0.0"
            - name: PORT
              value: "4000"

helm template .

      containers:
        - name: litellm
          securityContext:
            {}
          image: "ghcr.io/berriai/litellm-database:main-v1.43.18"
          imagePullPolicy: IfNotPresent
          env:
            - name: HOST
              value: ":"
            - name: PORT
              value: "4000"

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
litellm	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 5, 2024 7:48am

[ishaan-jaff]

had a follow up question

[ishaan-jaff]

would this work for ipv4 addresses ?

[menardorama]

you just have to set the value to 0.0.0.0 for ipv4

[laurb9]

I think the default should be the original 0.0.0.0, as this PR addresses a regression introduced when it was replaced with ::

[qdrddr]

are you sure the listen: "" in the values.yaml file should be under ingress and not the image? @menardorama & @ishaan-jaff

bcdeb63#diff-2105d8e1ded03b7b26c7c794db81f24dd466f6f793fd07a37120863950c2429dR59-R80

[krrishdholakia]

@menardorama @qdrddr @laurb9 is this PR ready for merge? Seems like a few open comments. Ideally we can have this as part of tonight's release train

[menardorama]

are you sure the listen: "" in the values.yaml file should be under ingress and not the image? @menardorama & @ishaan-jaff

bcdeb63#diff-2105d8e1ded03b7b26c7c794db81f24dd466f6f793fd07a37120863950c2429dR59-R80

I don't understand ....

You mean the place in the values ? Listen address has nothing to do with ingress but whatever as soon as it fix the issue.....
Actually my ci is patching the deployment after helm install I. Order to address the issue.

[qdrddr]

are you sure the listen: "" in the values.yaml file should be under ingress and not the image? @menardorama & @ishaan-jaff
bcdeb63#diff-2105d8e1ded03b7b26c7c794db81f24dd466f6f793fd07a37120863950c2429dR59-R80

I don't understand ....

You mean the place in the values ? Listen address has nothing to do with ingress but whatever as soon as it fix the issue..... Actually my ci is patching the deployment after helm install I. Order to address the issue.

Yes. I was asking if you have selected the correct placement for the variable in the values file. Currently the listen is placed under ingress. If you click on the link you'll see it is placed under the ingress. Shouldn't it be under the image item?

bcdeb63#diff-2105d8e1ded03b7b26c7c794db81f24dd466f6f793fd07a37120863950c2429dR59-R80

[laurb9]

Yes. I was asking if you have selected the correct placement for the variable in the values file. Currently the listen is placed under ingress. If you click on the link you'll see it is placed under the ingress. Shouldn't it be under the image item?

It is actually at root level like masterkey, not under ingress. Github misinterprets where the comment is located based on the last key it saw.

[menardorama]

Yes. I was asking if you have selected the correct placement for the variable in the values file. Currently the listen is placed under ingress. If you click on the link you'll see it is placed under the ingress. Shouldn't it be under the image item?

It is actually at root level like masterkey, not under ingress. Github misinterprets where the comment is located based on the last key it saw.

Exactly, now maybe it can be a good idea to have a dedicated section where masterkey, listen address and future litellm runtime settings are located together ?

[laurb9]

Exactly, now maybe it can be a good idea to have a dedicated section where masterkey, listen address and future litellm runtime settings are located together ?

Something like proxy config ? moving masterkey would be a breaking change and out of scope though.

[menardorama]

Exactly....

Even if the correct way would be that....

What about a section called runtimeConfig ?

If things won't move a lot on the runtime part this can be let on the root level

[krrishdholakia]

Hey @menardorama @laurb9 @qdrddr is this PR ready for review / merge?

If there's additional improvements we can make, would it help to scope that into a separate PR?

[menardorama]

In my opinion having to change where the master key setting is defined will have a huge impact on all deployments.

So for now adding another setting at the root level may be the best solution for now.

Now regarding the default value, is having 0.0.0.0 as default and permit to define it to "::" may be the best choice.

If yes, I can update my PR and you can merge it

Is this plan is ok for you ?

[krrishdholakia]

Hey @menardorama i merged in #5587 which should address the host name issue. Closing this PR as it seemed to address the same issue. If you'd like to submit another PR for improvements on the existing helm chart, that would be great

[menardorama]

Hey @menardorama i merged in #5587 which should address the host name issue. Closing this PR as it seemed to address the same issue. If you'd like to submit another PR for improvements on the existing helm chart, that would be great

Hi,
No problem but new settings needs to be defined in values.yaml even with only comments; this is to avoid having to deep dive in the helm chart to understand and find this setting....