MON-3304: Add option to specify resource limits for all components

[rexagod]

• I added CHANGELOG entry for this change.
• No user facing changes, so no entry in CHANGELOG was needed.

[openshift-ci-robot]

@rexagod: This pull request references MON-3168 which is a valid jira issue.

In response to this:

• I added CHANGELOG entry for this change.
• No user facing changes, so no entry in CHANGELOG was needed.

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.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[openshift-ci-robot]

@rexagod: This pull request references MON-3304 which is a valid jira issue.

In response to this:

• I added CHANGELOG entry for this change.
• No user facing changes, so no entry in CHANGELOG was needed.

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.

[openshift-ci-robot]

@rexagod: This pull request references MON-3304 which is a valid jira issue.

In response to this:

/jira refresh

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.

[rexagod]

Unused.

[machine424]

Cool, finally :)

but IIUC, these new fields would override the default requests we already set.

Is it really what we want to do? Force users to define both requests and limits even when they only want to set limits?

IIRC, when only limits are defined, Kubernetes will set requests to the same values than limits.

[juzhao]

1. if we don't set requests for the components, it will use the default requests as defined in each deployment/daemonset
2. https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/

Note: If you specify a limit for a resource, but do not specify any request, and no admission-time mechanism has applied a default request for that resource, then Kubernetes copies the limit you specified and uses it as the requested value for the resource.

3. specify request/limit, the component should use the specified settings

[machine424]

Thanks @juzhao for confirming that "if only limits are set, requests are set equal to limits".

Here are my concerns in two scenarios:

• User only sets the limits via this feature field without knowing that the default requests are not taken into account anymore and that the requests will be equal to the limits they provided (they may have some capacity issues) (unexpected behaviour)
• User took the care to copy the default requests in this feature field, but I a change was done that makes a component need less or more requests, we take care of changing the default requests, but the requests in the feature field are not up to date anymore. (maintenance burden)

[juzhao]

Thanks @juzhao for confirming that "if only limits are set, requests are set equal to limits".

Here are my concerns in two scenarios:

* User only sets the limits via this feature field without knowing that the default requests are not taken into account anymore and that the requests will be equal to the limits they provided (they may have some capacity issues) (unexpected behaviour)

I think this is expected behavior, by design of Kubernetes, user should know it

* User took the care to copy the default requests in this feature field, but I a change was done that makes a component need less or more requests, we take care of changing the default requests, but the requests in the feature field are not up to date anymore. (maintenance burden)

example: prometheus-adapter default resources.requests
https://github.com/openshift/cluster-monitoring-operator/blob/release-4.14/assets/prometheus-adapter/deployment.yaml#L73-L76

1. if use does not set resources.requests in cluster-monitoring-config configmap, prometheus-adapter will sue the default resources.requests(if the default value can not let prometheus-adapter pod start up, then it's a bug, should be fixed in CMO)
2. if user only set resources.limits, resources.requests use the same value as resources.limits,

apiVersion: v1
kind: ConfigMap
metadata:
  name: cluster-monitoring-config
  namespace: openshift-monitoring
data:
  config.yaml: |
    k8sPrometheusAdapter:
      resources:
        limits:
          cpu: 200m
          memory: 500Mi

3. set resources.limits and resources.requests, pod should use the specified value

apiVersion: v1
kind: ConfigMap
metadata:
  name: cluster-monitoring-config
  namespace: openshift-monitoring
data:
  config.yaml: |
    k8sPrometheusAdapter:
      resources:
        requests:
          cpu: 20m
          memory: 200Mi
        limits:
          cpu: 200m
          memory: 500Mi

when set resources.limits and resources.requests, user should know clearly of the prometheus-adapter cpu/memory usage, then set resources.limits and resources.requests to a proper value, otherwise, pod would not start up

[jan--f]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jan--f, rexagod

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 [jan--f]

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

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD c46c74a and 2 for PR HEAD ae4369d in total

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD edbb549 and 1 for PR HEAD ae4369d in total

[rexagod]

/retest

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 529349f and 0 for PR HEAD ae4369d in total

[openshift-ci]

@rexagod: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn-single-node	ae4369d	link	false	/test e2e-aws-ovn-single-node

Full PR test history. Your PR dashboard.

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.

[rexagod]

/jira refresh