`spec.deployments.replicas` does not take care about HorizontalPodAutoscaler

[nak3]

We can overrides spec.deployments.replicas as:

apiVersion: operator.knative.dev/v1alpha1
kind: KnativeServing
metadata:
  name: ks
  namespace: knative-serving
spec:
  deployments:
  - name: webhook
    replicas: 3

But it does not change the values in HorizontalPodAutoscaler:

https://github.com/knative/operator/blob/f4ce4ff51a7fe3f6941073c7733a2e83f8b7faf1/pkg/reconciler/common/deployments_override.go#L86-L90

Some deployments (e.g. webhook, activator) deploys HPA so we need to change the values.

https://github.com/knative/operator/blob/f4ce4ff51a7fe3f6941073c7733a2e83f8b7faf1/cmd/operator/kodata/knative-serving/0.25.1/2-serving-core.yaml#L5435

[matzew]

@nak3 we have some code updating the HPA minReplica here: https://github.com/knative/operator/blob/main/pkg/reconciler/common/ha.go#L76-L89

I think we should do something similar on the deployments_override.go file, to check/tweak for both: min and max...

[maschmid]

I think we need an explicit option like

spec:
  horizontalPodAutoscalers:
  - name: foo
    minReplicas: ...
    maxReplicas: ...

plus a warning if one attempts to set spec.deployments[].replicas for something the operator does deploy an HPA...

[matzew]

or that as a vehicle to make more options directly configurable, w/o guessing.

Regardless, it would also would relate to the ha.go file

[nak3]

Yeah, maxReplicas has a bug in not only spec.deployments.replicas but also spec.ha (ha.go). I think we can apply the same fix with https://github.com/knative/operator/pull/748 to spec.deployments.replicas.

[houshengbo]

@nak3 Can we close this issue for now?

[nak3]

No, unfortunately https://github.com/knative/operator/pull/748 fixed the issue for spec.ha but spec.deployments.replicas still does not care about maxReplicas :cry:

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

[nak3]

/reopen

[knative-prow[bot]]

@nak3: Reopened this issue.

In response to this:

/reopen

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.

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

[nak3]

/reopen

[knative-prow[bot]]

@nak3: Reopened this issue.

In response to this:

/reopen

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.

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

[dprotaso]

/lifecycle frozen

[richardvflux]

not sure what frozen means, its still not fixed :-(

[dprotaso]

Frozen means the bot won't close this issue.

[ReToCode]

/assign