[tempo-distributed] Added resource limit and request samples for all components

[xogoodnow]

Added commented sample for request and limit for components
Having the comments is quite convenient.

[xogoodnow]

Hi,
For consistency, shouldn't the resource section of Minio, Admin-api and enterprise gateway be commented out like the other components?

[Starefossen]

Please don't add CPU limit as it is impossible to remove the CPU limit entirely then.

[xogoodnow]

Hi @Starefossen,

If you are referring to the enterprise section, for the sake of consistency, since this was the only section on which resource request was set by default I did not comment out the limits as well. Now I have commented the section (since the default value itself was very low as well).
Since they are just comments, I do not see any issues. If you meant something else please let me know.

Cheers

[Starefossen]

Yes, the comments are no problem. It was just regarding any default values. Sorry for the confusion.

[xogoodnow]

Yes, the comments are no problem. It was just regarding any default values. Sorry for the confusion.

Thank you, NP.

[zalegrala]

Thanks of for the PR. I think the comments are good, but not sure about the default values on the two components which add them.

[zalegrala]

I expect this to be too small to be useful, but I don't have an example to check at the moment.

[xogoodnow]

This seems more reasonable

[xogoodnow]

How about commenting out the resource section for Minio as well?!

[zalegrala]

Yep, I agree. I think folks can place their own limits here. Scale will matter, so choosing sane defaults is a bit difficult.

[xogoodnow]

exactly.

[xogoodnow]

@zalegrala, If there is anything that I should do please let me know.

[joe-elliott]

I disagree with adding default requests/limits for minio and admin api.

The only part of this PR i'm in strong agreement with is removing the requests for enterprise gateway. Even the comments just feel like they're cluttering the file.

[xogoodnow]

I disagree with adding default requests/limits for minio and admin api.

The only part of this PR i'm in strong agreement with is removing the requests for enterprise gateway. Even the comments just feel like they're cluttering the file.

Hi @joe-elliott,

IMHO, setting comments for resources is quite convenient and is a very common practice within all mature helm charts.
Setting the comments also prevents confusion and possible mistakes (when setting limits).
The values for each resource is just a suggestion so people know how to set the limits.

p.s: On most managed K8S platforms, users have to set request and limit for every pod so when using this chart they have to manually copy and paste the section throughout the whole chart (just wanted to clarify what I meant by convince).

Cheers