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

Update container insights to account for InitContainers/Pod Overhead #145

Conversation

JayPolanco
Copy link

Description:

Kubernetes currently supports a concept called “init containers" which run to completion before the main container(s) of a pod are started. Starting with EKS 1.29, a new feature "SidecarContainers" will be introduced which allows init containers to run indefinitely, alongside the main application container(s) - thus acting as sidecars. A sidecar container is an init container with a restart policy.

https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/753-sidecar-containers#exposing-pod-resource-requirements.

https://kubernetes.io/docs/concepts/scheduling-eviction/pod-overhead/

The current Container Insights implementation does not take into account initContainers(sidecar or regular) when reporting metrics. This will result in incorrect metrics values and therefore affect customer observability in EKS 1.29 clusters. This issue will only affect customers who choose to use the new sidecar containers feature.

This PR updates container insights to be accurate for EKS 1.29, by implementing resource calculations (cpu/mem limits/requests) in the same way kubernetes does.

Kubernetes code for reference: https://github.com/kubernetes/kubernetes/blob/e2afa175e4077d767745246662170acd86affeaf/pkg/api/v1/resource/helpers.go#L50

TLDR: With EKS 1.29 the formula for computing resource usage for Pods will need to be changed to account for init containers (sidecar/regular) and pod overhead if set.

The old formula was just Sum(Container) when it should have been Max ( Max (Init Containers), Sum(Containers)). As of EKS 1.29 the formula will be:

Max ( Max( each InitContainerUse ) , Sum(Sidecar Containers) + Sum(each ContainerUse) ) + pod overhead

where InitContainerUse(i) = Sum(sidecar containers with index < i) + InitContainer(i)

Testing:

  • Unit tests in contrib repo (aws-cwa-dev)
  • Unit tests passing in agent
  • Tested on EKS cluster

@straussb
Copy link

straussb commented Dec 21, 2023

Suggestions for the PR description / commit message:

Show example sidecar container scenario(s) and what the metrics would look like before and after this change.

The new formula makes sense after you think about it for a while, but it's pretty complicated. You might try explaining it in English (even if Kubernetes docs didn't do this, we can do better 😃 ). But if it doesn't end up being more understandable than just the technical formula, then forget it.

List out the full list of metrics affected (pod_cpu_request, etc.).

After you've been able to test it in a real cluster, describe how you did it and show the results.

Copy link

github-actions bot commented Jan 5, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jan 5, 2024
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants