kubernetes / ingress-nginx

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

HorizontalPodAutoscaler api version prevents upgrade to K8s version 1.25+ #9728

Closed plynch-magnolia closed 1 year ago

plynch-magnolia commented 1 year ago

What happened:

Error: unable to build kubernetes objects from release manifest: resource mapping not found for name: "mgnl-ingress-nginx-defaultbackend" namespace: "ingress-nginx" from "": no matches for kind "HorizontalPodAutoscaler" in version "autoscaling/v2beta1"

What you expected to happen:

Expected chart to be able to be created using K8s version 1.25+

NGINX Ingress controller version : Chart version 4.5.2

Kubernetes version (use kubectl version): 1.25

How to reproduce this issue: Try to install the helm chart in a k8s v1.2.5+ environment

Anything else we need to know: Updating the API version to "autoscaling/v2" should resolve the issue. https://kubernetes.io/docs/reference/using-api/deprecation-guide/#horizontalpodautoscaler-v125

longwuyuan commented 1 year ago

@plynch-magnolia this is a concern and needs to be addressed asap if a genuine bug because https://github.com/kubernetes/ingress-nginx/pull/9348 is supposed to have taken care of this.

So kindly answer the questions asked in a new issue template by editing your original post description of the issue

plynch-magnolia commented 1 year ago

I am installing via a terraform helm chart, but my terraform plan fails with the error given above. It seems I will need to specify the value as a parameter. The default value, found here https://github.com/kubernetes/ingress-nginx/blob/main/charts/ingress-nginx/templates/default-backend-hpa.yaml still shows the old value which likely just needs to be updated.

longwuyuan commented 1 year ago

On minikube,

Can you try the same and check.

I think we do need more info because normally multiple reports will come in for such a problem but we don't have multiple reports on this.

longwuyuan commented 1 year ago

/remove-kind bug /kind support

longwuyuan commented 1 year ago

/kind bug /triage accepted /area stabilization /priority important-soon

I did not realize earlier that it was about the default-backend so this is a bug and someone has thankfully submitted PR to fix it https://github.com/kubernetes/ingress-nginx/pull/9731

longwuyuan commented 1 year ago

/remove-kind support

longwuyuan commented 1 year ago

@plynch-magnolia please confirm that the fix works for you now https://github.com/kubernetes/ingress-nginx/pull/9731

longwuyuan commented 1 year ago

/reopen to get confirmation that fix worked

k8s-ci-robot commented 1 year ago

@longwuyuan: Reopened this issue.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/9728#issuecomment-1467947116): >/reopen >to get confirmation that fix worked 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/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
plynch-magnolia commented 1 year ago

The issue was not fixed for me. Perhaps if a new release version is tagged it will work for me as we need to specify versions. I tried it with no version but got the same issue,

longwuyuan commented 1 year ago

Try pointing to project helm directory and test, instead of released chart.

On Tue, 14 Mar, 2023, 6:13 pm plynch-magnolia, @.***> wrote:

The issue was not fixed for me. Perhaps if a new release version is tagged it will work for me as we need to specify versions. I tried it with no version but got the same issue,

— Reply to this email directly, view it on GitHub https://github.com/kubernetes/ingress-nginx/issues/9728#issuecomment-1468036920, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGZVWWZZPRFP2OOR63AYFDW4BRXJANCNFSM6AAAAAAVZIPKZE . You are receiving this because you were mentioned.Message ID: @.***>

plynch-magnolia commented 1 year ago

This is the code I have, which is the minimum I can provide.

resource "helm_release" "ingress" { name = local.ingress_nginx_release_name repository = "https://kubernetes.github.io/ingress-nginx" chart = "ingress-nginx" ...

longwuyuan commented 1 year ago

For testing, you can point that to the charts folder of the project on github. Or wait a few days and we will release new chart

On Tue, 14 Mar, 2023, 6:30 pm plynch-magnolia, @.***> wrote:

This is the code I have, which is the minimum I can provide.

resource "helm_release" "ingress" { name = local.ingress_nginx_release_name repository = " https://kubernetes.github.io/ingress-nginx" chart = "ingress-nginx" ...

— Reply to this email directly, view it on GitHub https://github.com/kubernetes/ingress-nginx/issues/9728#issuecomment-1468062420, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGZVWV2ZCOX564O2ZLUNPLW4BTXZANCNFSM6AAAAAAVZIPKZE . You are receiving this because you were mentioned.Message ID: @.***>

plynch-magnolia commented 1 year ago

I was unable to point directly due to a "not a helm chart directory" error because there is no index.yaml.

longwuyuan commented 1 year ago

tf helm provider has to be told that its not a package but the exploded chart.

in any case, plz wait for a few days as we are trying to release the next iteration of the controller, which will ship the next version of the chart as well

hpAB commented 1 year ago

still failing ComparisonError: unsupported HPA GVK: autoscaling/v2, Kind=HorizontalPodAutoscaler

version: 4.5.2 appVersion: 1.6.4 eks 1.25

longwuyuan commented 1 year ago

@hpAB Try to install from the directory "chart" on the project's github repo, until the next release is out

hpAB commented 1 year ago

i dont think i can do that from helm. Helm does not currently support git repositories in its dependency resolver dependencies:

longwuyuan commented 1 year ago

Can you at least clone the github repo or download the zip of the repo from github and try to install from a local path. At least we can know the result of that merged PR which hardcoded the api version to v2.

plynch-magnolia commented 1 year ago

When using the newest version of the chart via Terraform, I am now getting the following error: Error: HorizontalPodAutoscaler.autoscaling "mgnl-ingress-nginx-defaultbackend" is invalid: [spec.metrics[0].resource.target.type: Required value: must specify a metric target type, spec.metrics[0].resource.target.type: Invalid value: "": must be either Utilization, Value, or AverageValue, spec.metrics[0].resource.target.averageUtilization: Required value: must set either a target raw value or a target utilization, spec.metrics[1].resource.target.type: Required value: must specify a metric target type, spec.metrics[1].resource.target.type: Invalid value: "": must be either Utilization, Value, or AverageValue, spec.metrics[1].resource.target.averageUtilization: Required value: must set either a target raw value or a target utilization]

plynch-magnolia commented 1 year ago

I opened this PR to address: https://github.com/kubernetes/ingress-nginx/pull/9803

longwuyuan commented 1 year ago

Any chance you can install without terraform to report what happens ? Like a install using helm cli or whatever the docs suggest for your provider.

longwuyuan commented 1 year ago

And reminding you that your original post describing the problem has skipped all the questions asked in a new issue template. That does not help any reader coming here and does not help the people who would want to work on this. Please answer the questions asked in the new issue template.

longwuyuan commented 1 year ago

Also try the latest release of the controller because this PR https://github.com/kubernetes/ingress-nginx/pull/9731 change the HPA API Version to v2

plynch-magnolia commented 1 year ago

I am using the latest release and this is the error I got from my Terraform environment, which is the one it was working in until v1.25 was released.

longwuyuan commented 1 year ago

I am using the latest release and this is the error I got from my Terraform environment, which is the one it was working in until v1.25 was released.

Incorrect.

The description of this issue says this ;

NGINX Ingress controller version : Chart version 4.5.2

But there is one more release after 4.5.2

plynch-magnolia commented 1 year ago

It's not a terraform problem, the problem is coming from my EKS environment rejecting the chart because of the errors I have provided here. I don't know how much else I can provide without someone testing adding this to their EKS cluster via Terraform helm. The PR #9731 overcame the first error, but a new error came up to which the PR I submitted, #9803 should overcome.

plynch-magnolia commented 1 year ago

I am using the latest release and this is the error I got from my Terraform environment, which is the one it was working in until v1.25 was released.

Incorrect.

The description of this issue says this ;

NGINX Ingress controller version : Chart version 4.5.2

But there is one more release after 4.5.2

I am now using version 4.6.0 and the error I just provided is what I now get.

longwuyuan commented 1 year ago

Basically there is lack of resources and the information provided as per new issue template and as per testing manually installaing on ELS as per documented procedure helps people who want to work on issue. Without that info, others have to guess and do a lot of blind work to even understand the problem, solution and impact of solution on project

hpAB commented 1 year ago

apiVersion: v2 name: ingress-nginx version: 4.6.0 appVersion: 1.7.0 home: https://kubernetes.github.io/ingress-nginx description: Ingress controller for Kubernetes using NGINX as a reverse proxy and load balancer icon: https://upload.wikimedia.org/wikipedia/commons/thumb/c/c5/Nginx_logo.svg/500px-Nginx_logo.svg.png type: application dependencies:

still gives error unsupported HPA GVK: autoscaling/v2, Kind=HorizontalPodAutoscaler when deploying with argocd

plynch-magnolia commented 1 year ago

Closing this issue which is no longer applicable as error has shifted to something new.