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

cryptnono: declare cpu/mem requests and limits #2451

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Mar 31, 2023

We've seen the cryptnono pod be evicted because it has no resource requests/limits when what needs to be evicted are other pods to resolve the situation. This is not a big deal, but we can set some resource requests and limits to avoid the situation - especially when cryptnono has started up fully and no longer will use much memory and cpu.

For details about requests/limits, see inline comments about cryptnono having an init container and a main container, where the main container is trivial compared to the init container.

Typically, both the init container and the main container are active only for ~4 minutes, following a period of ~0 CPU use.

CPU (leap, 30 last days)

Main container
image

Init container
image

Memory (leap, 30 last days)

Main container
image

Init container
image

@consideRatio consideRatio self-assigned this Mar 31, 2023
@consideRatio consideRatio requested a review from a team April 1, 2023 02:42
# resources configuration configure both containers.
#
# PromQL queries for CPU and memory use:
# - CPU: sum(rate(container_cpu_usage_seconds_total{container="kube-trace-init", namespace="support"}[5m])) by (pod)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This query is just looking for the initcontainer, and not the actively running container (which is called 'trace' https://github.com/yuvipanda/cryptnono/blob/8367fa57835d912524c34fa06698bde6dbbae158/cryptnono/templates/daemonset.yaml#L42). Should account for that too, no, as it is the one that's constantly on?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yepp I checked both containers, the commented commands reflects only on the initcontainer and not both. The PR description includes some screenshots of the findings.

I've tried to come up with a request/limit etc based on both containers are started in sequence using the same resource allocation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figure the queries can get the reader started and together with the text describing that there are two different containers can inspect both.

Copy link
Member

@yuvipanda yuvipanda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can revise if needed

@consideRatio consideRatio merged commit 9f1dd31 into 2i2c-org:master Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants