Skip to content
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

Setting istio sidecar proxy resource request/limit #16126

Closed
sebarys opened this issue Aug 8, 2019 · 31 comments · Fixed by #22395 or istio/api#1346
Closed

Setting istio sidecar proxy resource request/limit #16126

sebarys opened this issue Aug 8, 2019 · 31 comments · Fixed by #22395 or istio/api#1346

Comments

@sebarys
Copy link

sebarys commented Aug 8, 2019

Hello :)

Describe the feature request

Is there an option to set istio proxy sidecar request/limits per application (Pod)? For some of our applications we would like to use Guaranteed QoS (https://kubernetes.io/docs/tasks/configure-pod-container/quality-service-pod/) that require setting limit == request for all Pod containers. Without such functionality we're loosing this feature.

Describe alternatives you've considered

We've considered using global config value (https://istio.io/docs/reference/config/installation-options/#global-options) but if I understood correctly it could be used only during Istio installation, not applied for each Pod separately what don't match described case.
Some of our applications have bigger traffic so need higher requested resource values, other lower. As istio-proxy resources are somehow proportional to application container resources (e.g. one application is handling much bigger traffic than another -> it will require more resources also on istio-proxy sidecar layer) I would like to have an option to align resources assigned to isito-proxy per Pod.

Affected product area (please put an X in all that apply)

[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[X] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

@howardjohn
Copy link
Member

You can set requests per proxy with sidecar.istio.io/proxyCPU and sidecar.istio.io/proxyMemory annotations, but doesn't look like we have one for limits.

@sebarys
Copy link
Author

sebarys commented Aug 9, 2019

Thanks for information @howardjohn !
Is there any chance to add similar annotations for limits or it is already some work toward this way?

@mafarinacci
Copy link

I would also like this feature. I know what needs to be done to get this to work. Should I open a PR?

@howardjohn
Copy link
Member

I remember @ostromart mentioning this, I forget if it was "we should do this" or "we should not do this".

If you do, you should submit the PR against https://github.com/istio/installer

@mafarinacci
Copy link

I think this is absolutely a needed feature. I want to get guaranteed cores on kubernetes nodes so that my low latency applications don't get throttled by kubernetes. It looks like me and another user have the same concern. I think we could talk about it at the next community meeting perhaps?

As far as implementation, that's actually not what I was thinking. this is what I think we should modify to do it on a per pod basis: https://github.com/istio/istio/blob/master/install/kubernetes/helm/istio/files/injection-template.yaml

Please lmk if I am incorrect. I will open a PR as soon as I get confirmation.

@mafarinacci
Copy link

@howardjohn howardjohn ^

@anxolerd
Copy link

This feature (per-pod resource limits for sidecar) would be useful for me as well. My concern is that when I override requests with annotation, sidecar limits are getting lost, which may lead to uncontrolled resource usage by sidecar, which I'd like to avoid.

@ZackButcher
Copy link
Contributor

We would love to have a PR adding this. We should approach this by adding explicit flags to set resource limits, in addition to the current flags that sets resource requests. We should be able to both set a global default limit and a per-pod limit, like we do with resource requests today.

@mafarinacci we'll be happy to review a PR as soon as you've got it ready.

@rakeshjain1213
Copy link

Hi,

Please let me know, if this feature is released in ISTIO 1.3.0 or still its in progress.

@howardjohn
Copy link
Member

It did not make 1.3, you can set requests but not limits

@pavelzhurov
Copy link

pavelzhurov commented Oct 3, 2019

@sebarys As far as I understood this annotation is set in the istio-sidecar-injector template. So, you can add any annotation that you want there. For example, I changed my template in this way:

  containers:
  - name: istio-proxy
…
    resources:
      limits:
        [[ if (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsCPU`) -]]
        cpu: "[[ index .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsCPU` ]]"
        [[ else -]]
        cpu: 200m
        [[ end ]]
        [[ if (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsMemory`) -]]
        memory: "[[ index .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsMemory` ]]"
        [[ else -]]
        memory: 256Mi
        [[ end ]]
      requests:
        [[ if (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyRequestsCPU`) -]]
        cpu: "[[ index .ObjectMeta.Annotations `sidecar.istio.io/proxyRequestsCPU` ]]"
        [[ else -]]
        cpu: 100m
        [[ end ]]
        [[ if (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyRequestsMemory`) -]]
        memory: "[[ index .ObjectMeta.Annotations `sidecar.istio.io/proxyRequestsMemory` ]]"
        [[ else -]]
       memory: 128Mi
        [[ end ]]

Thus, if I want fully custom memory/cpu request limits, I add following annotations to the deployment:

sidecar.istio.io/proxyRequestsCPU: "200m"
sidecar.istio.io/proxyRequestsMemory: "512Mi"
sidecar.istio.io/proxyLimitsCPU: "300m"
sidecar.istio.io/proxyLimitsMemory: "1Gi"

Warning: This example doesn't use global-options!!! Be careful, when you change your side-car template.

@kebe7jun
Copy link
Member

emmm, can we set limits same as requests?

@hobbytp
Copy link

hobbytp commented Nov 29, 2019

I have tried to modify istio-sidecar-injector.yaml as below, and then add annotation in application deployment, but the resource can work, and limits can not be added in istio-proxy, any comment?


    resources:
      {{ if or (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyCPU`) (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyMemory`) -}}
      requests:
        {{ if (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyCPU`) -}}
        cpu: "{{ index .ObjectMeta.Annotations `sidecar.istio.io/proxyCPU` }}"
        {{ end}}
        {{ if (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyMemory`) -}}
        memory: "{{ index .ObjectMeta.Annotations `sidecar.istio.io/proxyMemory` }}"
        {{ end }}
    {{ else -}}
  {{- if .Values.global.proxy.resources }}
      {{ toYaml .Values.global.proxy.resources | indent 4 }}
  {{- end }}
    {{  end -}}
      {{ if or (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsCPU`) (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsMemory`) -}}
      limits:
        {{ if (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsCPU`) -}}
        cpu: "{{ index .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsCPU` }}"
        {{ end}}
        {{ if (isset .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsMemory`) -}}
        memory: "{{ index .ObjectMeta.Annotations `sidecar.istio.io/proxyLimitsMemory` }}"
        {{ end }}
    {{ else -}}
  {{- if .Values.global.proxy.resources }}
      {{ toYaml .Values.global.proxy.resources | indent 4 }}
  {{- end }}
    {{  end -}}

@sjmiller609
Copy link

sjmiller609 commented Dec 12, 2019

I would also love for an annotation that helps to set the limit per-pod, but what about adding resource requests and limits to the Sidecar custom resource? It seems useful to be able to control the requests and limits using a workload selector.

@howardjohn
Copy link
Member

@sjmiller609 that seems like it could be a pretty good UX but would be a pretty invasive change; right now the sidecar injection doesn't know about and CRs, so it doesn't read Sidecar resource. It could and maybe should, but doesn't today

@howardjohn
Copy link
Member

I actually like that idea more as I think about it

@howardjohn
Copy link
Member

Just so this doesn't get buried, I opened #19555 to track that idea. Its unlikely to be done in the short term though

@sjmiller609
Copy link

Just so this doesn't get buried, I opened #19555 to track that idea. Its unlikely to be done in the short term though

That's ok, I really appreciate you taking the time to consider it!

@prageethw
Copy link

to set global restrictions... on sidecars

--set values.global.proxy.resources.limits.memory="300Mi"
--set values.global.proxy.resources.limits.cpu="500m" \

@Arnold1
Copy link

Arnold1 commented Feb 12, 2020

@prageethw are there any default values for memory request/limit with Istio 4.1? if so what are those?
what about cpu?
and what happens if there is no memory limit set?

@InsOpDe
Copy link

InsOpDe commented Feb 12, 2020

I think its actually

--set global.proxy.resources.limits.memory="300Mi"
--set global.proxy.resources.limits.cpu="500m" \

and btw - we do not have the same problem with sidecars whose pods and services have no accompanied virtualservice/gateway.
maybe that helps with debugging.
Edit:
just to clarify, we do have a gateway per (virtual)service - which is as far as i know an antipattern we will get rid of soon.

@sjmiller609
Copy link

@prageethw are there any default values for memory request/limit with Istio 4.1? if so what are those?
what about cpu?
and what happens if there is no memory limit set?

@Arnold1 you can find all defaults here: https://istio.io/docs/reference/config/installation-options/#global-options

@rolandkool
Copy link
Contributor

rolandkool commented Mar 12, 2020

Having the option to set the limits via an annotation would help us as well. If there is a limitrange and quota applied on the namespace and you override the CPU and memory requests, the limits are not set. This will cause pods to not start with errors such as

maximum cpu usage per Container is XXXX. No limit is specified., maximum memory usage per Container is YYYY. No limit is specified.

Usually when not setting requests/limits the defaults are applied, but perhaps there's an ordering issue with all the controllers modifying the spec that is causing this.

Patching the sidecar injector template ourselves is not ideal as this templates can change between versions.

istio-testing pushed a commit that referenced this issue Mar 24, 2020
* Add annotations for setting cpu/memory limits on sidecar

When a limitrange is active, not setting the limits will result
in an error. This patch will allow setting limits for the sidecar.

Fixes #16126

Change-Id: I031d510812a867c2790eaa9af2f51145b4e5f006

* update test

Change-Id: Ic610fc6e9c9a6d814083bd7434704592a5c8f92c
stewartbutler pushed a commit to stewartbutler/istio that referenced this issue Mar 24, 2020
* Add annotations for setting cpu/memory limits on sidecar

When a limitrange is active, not setting the limits will result
in an error. This patch will allow setting limits for the sidecar.

Fixes istio#16126

Change-Id: I031d510812a867c2790eaa9af2f51145b4e5f006

* update test

Change-Id: Ic610fc6e9c9a6d814083bd7434704592a5c8f92c
istio-testing pushed a commit that referenced this issue Apr 7, 2020
* Add annotations for setting cpu/memory limits on sidecar

When a limitrange is active, not setting the limits will result
in an error. This patch will allow setting limits for the sidecar.

Fixes #16126

* add auto generated files from make gen
@nicktrav
Copy link
Contributor

nicktrav commented Apr 15, 2020

@howardjohn - any chance of getting #22395 into 1.4 too?

@webercoder
Copy link

Any idea when PR #23053 might land in 1.4?

@peiniliu
Copy link

@sebarys
Hi Sebarys,
Did you finally have a Guaranteed Pod after injecting istio? I have followed those comments to set annotations for istio-proxy container, but the istio-init initcontainer is still a burstable container which will still cause the whole pod to be a burstable pod.
Is there any way to set the resource requests/limits for the istio-init container?
Best,
Peini

@kumarganesh2814
Copy link

Do we have anything yet for istio-proxy, I tried adding annotation my particular deployment but limits remained same also requests

@RicHincapie
Copy link

As of Istio 1.10, the solution is already implemented and its pretty similar to @pavelzhurov post. The istio-sidecar-inejctor template ConfigMap has the limits and resources parameters that allows us to set these parameters on a per-deployment basis. Those params are annotations at the deployment manifest level and look like this:

.spec.template.metadata.annotations:
sidecar.istio.io/proxyCPU: 600m
sidecar.istio.io/proxyCPULimit: 600m
sidecar.istio.io/proxyMemory: 256Mi
sidecar.istio.io/proxyMemoryLimit: 256Mi

With this config, you may be able to have QoS Guaranteed class for your pods.

@peiniliu
Copy link

peiniliu commented Sep 9, 2022

Hi ricarhincapie,

Thanks for your suggestion, you are right after setting these annotations, the istio-proxy container will have the same cpu/memory request and limit. But note that the pod is still not a QOS Guaranteed pod, because the istio-init has burstable settings which will let this pod finally becomes a QOS Burstable pod.

Also some discussion about the initcontainer here k8s-initcontainer-discussion.

I would like to find a way to set the resource requests/limits for the istio-init container as well :)

Best,
Peini

@howardjohn
Copy link
Member

howardjohn commented Sep 9, 2022 via email

@peiniliu
Copy link

I thought the init container uses identical resources as sidecar?

On Thu, Sep 8, 2022, 11:58 PM Peini Liu @.> wrote: Hi ricarhincapie, Thanks for your suggestion, you are right after setting these annotations, the istio-proxy container will have the same cpu/memory request and limit. But note that the pod is still not a QOS Guaranteed pod, because the istio-init has burstable settings which will let this pod finally becomes a QOS Burstable pod. Also some discussion about the initcontainer here k8s-initcontainer-discussion <kubernetes/kubernetes#93282>. I would like to find a way to set the resource requests/limits for the istio-init container as well :) Best, Peini — Reply to this email directly, view it on GitHub <#16126 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEYGXPXEOI36HWWBZECMILV5LN2LANCNFSM4IKHG5UQ . You are receiving this because you were mentioned.Message ID: @.>

Yes, I confirm with the setting the pod can be QoS Guaranteed. initcontainer has the same resources as sidecar. Thanks all.

SRohitRaj pushed a commit to SRohitRaj/Istio that referenced this issue Jun 7, 2024
When a limitrange is active, not setting the limits will result
in an error. This patch will allow setting limits for the sidecar.

Fixes istio/istio#16126
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet