Make resources flexible

[evilr00t]

Hi!

With the current implementation it is not possible to set resources dynamically.

Deleted keys (limits, requests) for the resources will be picked from the default values.yaml which means we can't set resources explicitly.

[fad3t]

Hey,

Can't we review the default values as well?
1 CPU and 512Mi of RAM requested sounds crazy to me, for such a small component.
I'm running it on a cluster and it takes 2m CPU and 25Mi of RAM... Also I'd remove the CPU limits.

Here's my suggestion:

  resources:
    limits:
      memory: 512Mi
    requests:
      cpu: 10m
      memory: 32Mi

[terinjokes]

@fad3t the defaults used to be lower but I got several reports of OOMs just starting the controller with no OriginIssuer resources in the cluster, such as with #118.

[fad3t]

@fad3t the defaults used to be lower but I got several reports of OOMs just starting the controller with no OriginIssuer resources in the cluster, such as with #118.

I think it's fine to keep higher limits to avoid OOM (512Mi is OK), but requests (= resources exclusively reserved to the CF origin CA pod) should be lower IMO -- and are usually set to match the average resources consumed by the pod.

[terinjokes]

I don't know what the average will be, that depends on the cluster. That's why you can set values when deploying the chart.

[evilr00t]

Those are the metrics from last 30 days of running origin-ca-issuer

[fad3t]

I don't know what the average will be, that depends on the cluster. That's why you can set values when deploying the chart.

Of course, but my point is we could use better defaults that fit most of the use cases. Like you said, if they need more resources, they can configure them in the values file. But by default there's no reason to "waste" 512Mi for this pod.

[terinjokes]

I'm not sure I'll be able to find a happy answer here. It was previous "only" 100MB but I got a lot of feedback here, and via email, that the first-time user experience was bad due to the low default at the time.

@evilr00t thanks for the PR. I see it's what cert-manager does now. I have some concerns about how it will effect existing users. I'll keep it in mind for the next version.

[fad3t]

I'm not sure I'll be able to find a happy answer here. It was previous "only" 100MB but I got a lot of feedback here, and via email, that the first-time user experience was bad due to the low default at the time.

Memory limits were 100Mi, and that's what caused the OOMKilled events - and the bad user experience. You increased them to 512Mi and that fixed the issue.
My point is about the requests, not the limits. Lowering the requests won't cause such issues.