✨ Add resource requests and limits to kube-rbac-proxy

[fgiloux]

Signed-off-by: Frederic Giloux [email protected]

Description

This change introduces default resource requests and limits for the kube-rbac-proxy container scaffolded by KubeBuilder. It also provides a hint to users that this should get amended to match the specific requirements.

Motivation

This is similar to what is already done for the controller-manager.

• Provides information to the scheduler for better pod placement
• Prevents the pod of getting evicted or the container of getting OOM killed as first on the line when there is memory pressure on the node but the pod/container does not consume more than what it is supposed to
• Allows the pod to play well with Quotas and LimitRanges without relying on LimitRange defaults

Fixes #2428

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Frederic Giloux (d773d63)

[fgiloux]

@camilamacedo86 when you have some time can you have a look at this PR?

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
• Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[camilamacedo86]

/hold

@fgiloux we might want not to go forward with this one. Why?

CPU requests and limits are associated with Containers, but it is useful to think of a Pod as having a CPU request and limit. The CPU request for a Pod is the sum of the CPU requests for all the Containers in the Pod. Likewise, the CPU limit for a Pod is the sum of the CPU limits for all the Containers in the Pod.

In this way, seems like that we have it set only for the operator is enough. Otherwise, we will probably be requesting the double. I think we need to re-check.

[fgiloux]

@camilamacedo86 It is true for scheduling that the aggregated value at the Pod level is taken in consideration. On the Node it depends whether the --cgroups-per-qos flag has been set for the Kubelet. It is the case with OpenShift I am not sure for other distributions. This will determine whether a cgroup level (for aggregated accounting) gets created for Pods or not.

That said it is not neutral in terms of resource consumption to add a process, the kube-rbac-proxy in this case. I would agree that the CPU consumption may well be under the approximation level of the controller-manager. It is not the case for memory. At the end it is not that we would be requesting "double" it is that we would account for the overhead that the kube-rbac-proxy brings.

Last point, you motivated this enhancement first by the rejection of Pods when QoS and LimitRanges are configured without default values. LimitRanges can be defined for Pods but also for Containers. If a constraint is set at the Container level without default values the Pod may still get rejected if it does not provide it.

[fgiloux]

If a constraint is set at the Container level without default values the Pod may still get rejected if it does not provide it.

I need to correct my last sentence. This scenario is not possible: If a limitRange is created with a constraint at the Container level Defaults are always created (automatically)
But if a LimitRange constraint at the Pod level or a Quota is specified all the containers of a pod need to provide resource requests or limits otherwise the Pod gets rejected

[fgiloux]

I signed it

[fgiloux]

I signed it

[camilamacedo86]

/ok-to-test
/approved

[camilamacedo86]

/hold cancel

[fgiloux]

@camilamacedo86 just missing lgtm now. Let me know if this is good to go or anything needs to be changed

[varshaprasad96]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, fgiloux, varshaprasad96

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment