automated resource request scaling

[dhellmann]

This enhancement describes an approach to allow us to scale the
resource requests for the control plane services to reduce consumption
for constrained environments. This will be especially useful for
single-node production deployments, where the user wants to reserve
most of the CPU resources for their own workloads and needs to
configure OpenShift to run on a fixed number of CPUs within the host.

/cc @smarterclayton @derekwaynecarr @markmc @browsell

[openshift-ci-robot]

@dhellmann: GitHub didn't allow me to request PR reviews from the following users: browsell.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

This enhancement describes an approach to allow us to scale the
resource requests for the control plane services to reduce consumption
for constrained environments. This will be especially useful for
single-node production deployments, where the user wants to reserve
most of the CPU resources for their own workloads and needs to
configure OpenShift to run on a fixed number of CPUs within the host.

/cc @smarterclayton @derekwaynecarr @markmc @browsell

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.

[russellb]

/priority important-soon

[dhellmann]

/retest

[dhellmann]

I have updated the enhancement with more details in the proposal section. Please take another look.

[cynepco3hahue]

We already have some operators for it - https://github.com/openshift/cluster-resource-override-admission-operator, we can improve it and add new API fields.

[dhellmann]

OK, so we can modify this section to say it doesn't need to be in the release payload but that it would need to be installed separately and link to the existing operator.

We'll also need to say that each openshift operator will need to make the appropriate adjustments to ignore changes to the resource requests to avoid contention with the cluster-resource-override-admission web hook. It looks like the way to enable the admission web hook is to add a label to the namespace and then update the code to ignore resource requests on operands when comparing for equality. Is that right?

[dhellmann]

@cynepco3hahue The latest draft elevates the cluster-resource-override-admission-operator to the primary approach being proposed. Let me know what you think.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from dhellmann after the PR has been reviewed.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[dhellmann]

The latest draft of this enhancement reflects the changes to the approach that we came up with today, using https://github.com/openshift/cluster-resource-override-admission. There are still some details to work out, but I think the basics are all here now. All of the old approaches have been folded into the alternatives section at the end of the document.

@derekwaynecarr @browsell @markmc please take a look and let me know if I've missed anything.

[dhellmann]

I forgot to mention that I left the changes in a separate patch in case that made reviewing easier. I will squash the commits before we merge this enhancement.

[browsell]

For a general case where a pod has a limit set, would need to decide the precendence if both cpuRequestPercent and cpuRequestToLimitPercent are set.

[dhellmann]

Good point. It seems like the limit-based scaling (the existing behavior) should take precedence, and the new field should be ignored if the old field is present. What do you think?

[browsell]

Agreed.

[dhellmann]

/cc @rphillips @sjenning

[deads2k]

This enhancement doesn't appear to mention the runlevel restrictions created in the clusterresourceoverride's MutatingWebhookConfiguration which allows us to ensure re-bootstrapping a cluster that has been shut down. We label namespaces in openshift with openshift.io/run-level=0 and openshift.io/run-level=1 to indicate namespaces that are required to have pods before admission webhooks can be called.

This includes 36 out of 61 openshift-* namespaces on our 4.7 CI cluster. Removing this restriction to shrink the pod resource requests/limits would be fraught.

One example flow that will work in a demo and be at risk in a production cluster is

1. install cluster
2. stop skipping runlevel=0 namespaces. this will include the SDN and kube-apiserver namespaces, among others
3. delete pods in these namespaces and they will be restarted successfully
4. shutdown the cluster
5. start the cluster
6. there is no SDN running
7. etcd starts from a static pod
8. the kube-apiserver starts from a static pod
9. the kube-controller-manager starts from a static pod
10. the kube-controller-manager tries to create a daemonset pod for the SDN
11. the kube-apiserver enforces admission webhook configuration, which tries to call the cluster-resource-overrides admission plugin
12. there is no pod running for the cluster-resource-overrides admission plugin and even if there was, it would unreachable because the SDN pod doesn't exist
13. the cluster is live-locked without the ability to run pods

We created the runlevels and skips to ensure that we did not wedge in such a situation. While it may be possible to tune the labeling slightly tighter for a single-node use case (I don't know one way or the other), having an accident at this level can result in situations where clusters cannot be safely restarted.

Knowing that more than half of the namespaces we have today will not be restricted and knowing that we need a way to scale resource requests and limits on pods in these 36 namespaces and on static pods, having a single solution for the additional 25 namespaces doesn't seem so bad.

[dhellmann]

Closing this in favor of #703