Skip to content
This repository has been archived by the owner on Jun 24, 2020. It is now read-only.

Respect HA Replicas setting in HPAs. #394

Merged
merged 1 commit into from
Apr 9, 2020

Conversation

markusthoemmes
Copy link
Contributor

Proposed Changes

If the operator has a HighAvailabiliy configuration and the replicas are set > 1, we probably want to adjust all HPAs to have at least as many minReplicas as specified in that HA config. An HPA being present is enough of a signal for us to assume that the component supports a HA configuration.

Release Note

Adjusts all HPAs to have at least the number of replicas specified by a potential HighAvailability configuration.

If the operator has a HighAvailabiliy configuration and the replicas are set > 1, we probably want to adjust all HPAs to have at least as many minReplicas as specified in that HA config. An HPA being present is enough of a signal for us to assume that the component supports a HA configuration.
@googlebot googlebot added the cla: yes Author(s) signed a CLA. label Apr 7, 2020
Copy link
Contributor

@knative-prow-robot knative-prow-robot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markusthoemmes: 0 warnings.

In response to this:

Proposed Changes

If the operator has a HighAvailabiliy configuration and the replicas are set > 1, we probably want to adjust all HPAs to have at least as many minReplicas as specified in that HA config. An HPA being present is enough of a signal for us to assume that the component supports a HA configuration.

Release Note

Adjusts all HPAs to have at least the number of replicas specified by a potential HighAvailability configuration.

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.

@knative-metrics-robot
Copy link

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/ha.go 81.2% 80.0% -1.2

@markusthoemmes
Copy link
Contributor Author

/assign @houshengbo @pmorie

@pmorie
Copy link
Member

pmorie commented Apr 8, 2020

in practice, based on the current resources in serving config directory, i expect this initially to set spec.minReplicas of the activator HPA. Is that accurate?

@pmorie
Copy link
Member

pmorie commented Apr 8, 2020

Discussed w/ @markusthoemmes offline that the behavior is what I expect.

/lgtm

@houshengbo
Copy link

houshengbo commented Apr 8, 2020

@markusthoemmes
This PR looks good.
Would you mind contributing this PR to the new repo at https://github.com/knative-sandbox/operator, since we have set up the consolidated operator?
Thx.

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok, markusthoemmes

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:

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

@knative-prow-robot knative-prow-robot merged commit 940a4ca into knative:master Apr 9, 2020
markusthoemmes added a commit to markusthoemmes/serving-operator that referenced this pull request Apr 9, 2020
If the operator has a HighAvailabiliy configuration and the replicas are set > 1, we probably want to adjust all HPAs to have at least as many minReplicas as specified in that HA config. An HPA being present is enough of a signal for us to assume that the component supports a HA configuration.
markusthoemmes added a commit to openshift-knative/serving-operator that referenced this pull request Apr 9, 2020
If the operator has a HighAvailabiliy configuration and the replicas are set > 1, we probably want to adjust all HPAs to have at least as many minReplicas as specified in that HA config. An HPA being present is enough of a signal for us to assume that the component supports a HA configuration.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants