kubernetes / ingress-nginx

Ingress NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.47k stars 8.25k forks source link

hpa stopped working with keda disabled #11685

Closed hydrapolic closed 3 months ago

hydrapolic commented 3 months ago

What happened:

After upgrading the controller from 1.10.1 -> 1.11.1 and the chart from 4.10.0 -> 4.11.1, hpa stopped working.

This hpa config worked before:

    {
      type  = "string"
      name  = "controller.autoscaling.enabled"
      value = "true"
    },
    {
      type  = "auto"
      name  = "controller.autoscaling.minReplicas"
      value = 2
    },
    {
      type  = "auto"
      name  = "controller.autoscaling.maxReplicas"
      value = 4
    },
    {
      type  = "auto"
      name  = "controller.autoscaling.targetCPUUtilizationPercentage"
      value = 90
    },
    {
      type  = "auto"
      name  = "controller.autoscaling.targetMemoryUtilizationPercentage"
      value = 90
    }

Now this error was shown then trying to update the controller/chart:

14:34:44  module.ingress_nginx.helm_release.ingress_nginx[0]: Modifying... [id=ingress-nginx]
14:34:47  ╷
14:34:47  │ Error: template: ingress-nginx/templates/controller-deployment.yaml:22:9: executing "ingress-nginx/templates/controller-deployment.yaml" at <eq .Values.controller.autoscaling.enabled .Values.controller.keda.enabled>: error calling eq: incompatible types for comparison

After adding:

    {
      type  = "string"
      name  = "controller.keda.enabled"
      value = "false"
    },

It applies the config, however HPA is disabled.

What you expected to happen:

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:       v1.11.1
  Build:         7c44f992012555ff7f4e47c08d7c542ca9b4b1f7
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.25.5

-------------------------------------------------------------------------------

Kubernetes version (use kubectl version):

Client Version: v1.29.6 Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3 Server Version: v1.29.6-gke.1038001

Environment:

How to reproduce this issue:

Anything else we need to know:

Maybe related to https://github.com/kubernetes/ingress-nginx/pull/11110

k8s-ci-robot commented 3 months ago

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 3 months ago

can you show helm -n ingress-nginx get values ingress-nginx

longwuyuan commented 3 months ago

@Gacko if I stay on v1.11 and go from default install to enabling hpa, then no problems so looks like a upgrade only scene.

Gacko commented 3 months ago

enabled is not a string, it's a boolean. Please use a boolean. We also have unit tests for that, so I doubt it's broke.

longwuyuan commented 3 months ago

/remove-kind bug

I can NOT reproduce error while having installed v1.11.1 without autoscaling enabled and then making a change via helm to enable autoscaling.

So the config itself is not a problem.

/kind support

Like @Gacko mentioned, it could be the config value not being boolean to begin with.

@hydrapolic please provide details around the issue and state to base some comments on

/triage needs-information

hydrapolic commented 3 months ago

Thank you for the help, it really was the problem with string vs boolean in autoscaling.enabled. It worked before, but seems like the changes made it problematic now.

This now works fine:

helm get values ingress-nginx
USER-SUPPLIED VALUES:
controller:
  autoscaling:
    enabled: true
    maxReplicas: 4
    minReplicas: 2
    targetCPUUtilizationPercentage: 90
    targetMemoryUtilizationPercentage: 90

In terraform:

{
  type  = "string"
  name  = "controller.autoscaling.enabled"
  value = "true"
}

->

{
  type  = "auto"
  name  = "controller.autoscaling.enabled"
  value = true
}

And no keda config is needed at all.

Thanks @longwuyuan and @Gacko