knative / serving-operator

DEPRECATED: Development continues in https://github.com/knative/operator/
Apache License 2.0
39 stars 45 forks source link

Resources override #362

Closed jcrossley3 closed 4 years ago

jcrossley3 commented 4 years ago

Fixes #161 Related #302 #329

Proposed Changes

This exposes a "knob" in the KnativeServing CR that would enable users to workaround issues like knative/serving#7195

apiVersion: operator.knative.dev/v1alpha1
kind: KnativeServing
metadata:
  name: knative-serving
spec:
  resources:
  - container: webhook
    requests:
      cpu: 20m
      memory: 20Mi
    limits:
      cpu: 400m
      memory: 800Mi

Release Note

Resource limits for any knative deployment containers can now be configured in the `KnativeServing` custom resource using the new `resources` field.
knative-prow-robot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcrossley3

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](https://github.com/knative/serving-operator/blob/master/OWNERS)~~ [jcrossley3] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
markusthoemmes commented 4 years ago

By default, no resource limits will be specified for any knative deployment containers.

I think this bit should be discussed and implemented upstream first 🤔. It doesn't feel right to have the defaults of both installation modes differ that vastly.

jcrossley3 commented 4 years ago

I'm going to change the logic to not delete the upstream config, by default, so... /hold

knative-metrics-robot commented 4 years ago

The following is the coverage report on the affected files. Say /test pull-knative-serving-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/knativeserving/common/resources.go Do not exist 90.9%
jcrossley3 commented 4 years ago

/hold cancel

houshengbo commented 4 years ago

/lgtm