Jaeger-operator: Cannot use HorizontalPodAutoscaling: injected agent lacks resource defs

Created on 5 Dec 2018 · 13Comments · Source: jaegertracing/jaeger-operator

The kube-controller-manager can't autoscale our deployments with an injected Jaeger agent, as the injected container defines no CPU or Memory requests and/or limits.

kube-controller-manager-ip-10-0-1-4.us-east-2.compute.internal kube-controller-manager E1205 19:17:47.410801       1 horizontal.go:188] failed to compute desired number of replicas based on listed metrics for Deployment/default/podredacted: failed to get cpu utilization: missing request for cpu on container jaeger-agent in pod default/podredacted
kube-controller-manager-ip-10-0-1-4.us-east-2.compute.internal kube-controller-manager I1205 19:17:47.410855       1 event.go:221] Event(v1.ObjectReference{Kind:"HorizontalPodAutoscaler", Namespace:"default", Name:"podredacted", UID:"c393ace0-cbd6-11e7-9783-021516797582", APIVersion:"autoscaling/v2beta1", ResourceVersion:"161801118", FieldPath:""}): type: 'Warning' reason: 'FailedGetResourceMetric' missing request for cpu on container jaeger-agent in pod default/podredacted

enhancement good first issue

Source

sgmiller

👍1

All 13 comments

Good catch!

What would be a suitable solution? Copy the values from the main pod? Inherit possible values from the JaegerSpec?

jpkrohling on 6 Dec 2018

I'm guessing the main pods values would be pretty different, usually
higher. Possibly having a default in a ConfigMap, or as conf on the
operator followed by the ability to override in an annotation on the
deployment?

On Thu, Dec 6, 2018 at 1:15 AM Juraci Paixão Kröhling <
[email protected]> wrote:

Good catch!

What would be a suitable solution? Copy the values from the main pod?
Inherit possible values from the JaegerSpec?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/jaegertracing/jaeger-operator/issues/160#issuecomment-444772042,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABLTrhhAkoLWZwOoGfqcEVJZmPjvINTaks5u2MQQgaJpZM4ZEfGi
.

sgmiller on 6 Dec 2018

Would you be interested in drafting a PR with that? I think the most appropriate would be as a conf in the Operator's YAML (CR), instead of ConfigMap and I do like the idea of letting an application override it via annotations.

jpkrohling on 6 Dec 2018

I'm afraid I'm in no way qualified to do so without learning the crd
system.

On Thu, Dec 6, 2018 at 7:39 AM, Juraci Paixão Kröhling
notifications@github.com wrote:

Would you be interested in draft a PR with that? I think the most
appropriate would be as a conf in the Operator's YAML (CR), instead
of ConfigMap and I do like the idea of letting an application
override it via annotations.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

sgmiller on 6 Dec 2018

I could give this a shot :) @jpkrohling

annanay25 on 10 Dec 2018

🎉1

@annanay25 keep going and you'll soon become a maintainer :-)

jpkrohling on 10 Dec 2018

❤1

Looking forward to this! :D

annanay25 on 10 Dec 2018

@annanay25 do you have any news on this?

jpkrohling on 21 Feb 2019

👍1

I'm closing this, as we have a "resources" node in the injected container now. @sgmiller, please check it out and reopen this issue if you think there's still something to do here.

jpkrohling on 16 Jul 2019

Nope, that should do it.

On Tue, Jul 16, 2019 at 5:52 AM Juraci Paixão Kröhling <
[email protected]> wrote:

I'm closing this, as we have a "resources" node in the injected container
now. @sgmiller https://github.com/sgmiller, please check it out and
reopen this issue if you think there's still something to do here.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/jaegertracing/jaeger-operator/issues/160?email_source=notifications&email_token=AAJNHLTIMGAWZEUXGFVS43LP7WR7RA5CNFSM4GIR6GRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2APFMQ#issuecomment-511767218,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJNHLVR6AU22P7BJYCQTCDP7WR7RANCNFSM4GIR6GRA
.

sgmiller on 16 Jul 2019

I'm trying to configure this in my deployment, but it is not clear to me from the docs where these limits are added. Can you add something to https://www.jaegertracing.io/docs/1.13/operator/#auto-injecting-jaeger-agent-sidecars doc, or a snippet in this issue to help those who may have the same problem?
_Edit_
adding a spec.agent.resources block to the jaeger crd will fix this problem, but existing deployments that are using auto-injected sidecars need to have the sidecard removed from the deployment spec and redeployed in order for the jaeger operator to inject an updated version of the sidecar. The jaeger-agent container definition in a deployment will not automatically update if the jaeger spec is updated.

_snippet_

spec:
  ingress:
    enabled: false
  strategy: allInOne
  allInOne:
    image: jaegertracing/all-in-one:latest
  agent:
    image: jaegertracing/jaeger-agent:1.13.1
    resources:
      limits:
        memory: "128Mi"
        cpu: "250m"
      requests:
        memory: "64Mi"
        cpu: "100m"