Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] New azure-load-balancer-health-probe-request-path not working with rewrite-target #3621

Open
gionapaolini opened this issue Apr 19, 2023 · 18 comments

Comments

@gionapaolini
Copy link

gionapaolini commented Apr 19, 2023

Describe the bug
After upgrading the cluster to version 1.25.6, our loadbalancer stopped working (EXTERNAL IP not reachable).
After a bit of research it seems that the health probe endpoint has changed for the azure load balancer, so we have added the annotation service.beta.kubernetes.io/azure-load-balancer-health-probe-request-path: "/healthz" to our ingress-nginx deployment.

It seems to work, until we deploy our own ingress definition, which has a rewrite-target annotation nginx.ingress.kubernetes.io/rewrite-target: /$2

This seems to be breaking the url path and is unable to reach the /healthz endpoint.

After some more research, we have found a workaround which is to enable the default backend (https://kubernetes.github.io/ingress-nginx/user-guide/default-backend/) on the nginx controller by setting the following defaultBackend.enabled: true, but it doesn't sound right since it's not reaching the azure health endpoint anymore but just the default backend that nginx deploys now.

To Reproduce
Steps to reproduce the behavior:

  1. Deploy the nginx controller
helm install ingress-nginx ingress-nginx/ingress-nginx \
  --set controller.service.annotations."service\.beta\.kubernetes\.io/azure-load-balancer-health-probe-request-path"=/healthz
  1. Deploy an ingress with the rewrite targets:
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: main-ingress
  annotations:
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/rewrite-target: /$2
spec:
  rules:
  - http:
      paths:
      - path: /service1(/|$)(.*)
        pathType: Prefix
        backend:
          service:
            name: service1
            port: 
              number: 80
      - path: /service2(/|$)(.*)
        pathType: Prefix
        backend:
          service:
            name: service2
            port: 
              number: 80

Expected behavior
We expect to see that the EXTERNAL IP that is provided is reachable, but it is not.

Environment:

  • Kubernetes version: 1.25.6
@ghost ghost added the action-required label May 14, 2023
@ghost
Copy link

ghost commented May 19, 2023

Action required from @Azure/aks-pm

@ghost ghost added the Needs Attention 👋 Issues needs attention/assignee/owner label May 19, 2023
@ghost ghost removed action-required Needs Attention 👋 Issues needs attention/assignee/owner labels Jun 2, 2023
@ghost ghost added the action-required label Jun 28, 2023
@jdthorpe
Copy link

jdthorpe commented Aug 5, 2023

In my case, the Load balancer in the kubernetes RG had the health probe set to / and not /healthz despite the annotation you included in the OP (...=/healthz). Manually updating the probe endpoints to /healthz fixed it for me.

Screenshot 2023-08-05 at 12 40 22 PM

(PS. This is still clearly a bug, just this is a manual work around until it gets fixed...)

@danielforberg
Copy link

@jdthorpe But that means changing this on the local nodeports that ingress controller uses then or ? I have one set already for HTTP. But still problem with timeouts reaching Public IP.

@danielforberg
Copy link

danielforberg commented Aug 11, 2023

@gionapaolini How did you patch the, defaultBackend.enabled: true
Did that but it was known as a parameter som it did not add it. Did like this:
kubectl -n ingress-nginx patch deployment ingress-nginx-controller -p '{"spec": {"defaultBackend": {"enabled": true}}}'

Did you do it in a different way. Can you patch the deployment with this also, controller.service.annotations."service.beta.kubernetes.io/azure-load-balancer-health-probe-request-path"=/healthz".
Or how did you do that? But it comes with sideeffects right?

@LukeTouchFoundry
Copy link

I've stumbled into this issue. I've been using Terraform to create the resources. I was wondering if anyone had a way to use Terraform to perform the switch from / to /healthz?

Otherwise this is literally the only manual step I have to take which is kinda lame.

@zioproto
Copy link

@LukeTouchFoundry How are you creating the resources with Terraform ? Are you using the Helm provider or something else ?

@LukeTouchFoundry
Copy link

LukeTouchFoundry commented Oct 6, 2023

@zioproto I am indeed using the Helm provider

resource "helm_release" "main" {
  depends_on = [
    kubernetes_namespace.main,
  ]

  chart     = "${local.module_name}/${local.module_name}"
  name      = local.module_name
  version   = local.chart_version
  namespace = local.module_name

  # NB!! XXX: note this value MAY NOT APPLY, you may need to manually set it via the console
  # https://github.com/Azure/AKS/issues/3621
  set {
    name  = "controller.service.annotations.service.beta.kubernetes.io/azure-load-balancer-health-probe-request-path"
    value = "/healthz"
  }

  set {
    name  = "controller.service.loadBalancerIP"
    value = azurerm_public_ip.main.ip_address
  }

  set {
    name  = "controller.service.annotations.service.beta.kubernetes.io/azure-load-balancer-resource-group"
    value = var.resource_group
  }
}

What I have found is that even if I set the value in the Azure portal, it's being reset back to / after some time. This is basically a show stopping bug for us at this point. The Application gateway is far too expensive to use, I'm basically running out of options with Azure.

@zioproto
Copy link

zioproto commented Oct 6, 2023

You need to look into the Kubernetes API logs and find the Identity of the API call that is reverting the value to /. This way you can understand where the request that reverts the value to / is coming from.

@zioproto
Copy link

zioproto commented Oct 6, 2023

This could be a useful query for you:

AzureDiagnostics
| project TimeGenerated, Category, log_s
| where log_s contains "controller.service.annotations.service.beta.kubernetes.io/azure-load-balancer-health-probe-request-path"

Make sure you have data in the AzureDiagnostics table configuring the Diagnostic Settings, you need the Audit logs to track the changes.

@LukeTouchFoundry
Copy link

Thank you @zioproto I'll give this a try and revert back.

@CHDR-Nigel
Copy link

CHDR-Nigel commented Oct 10, 2023

Hello LukeTouchFoundry,

I have just recently found the same problem. I found that to make the /healthz annotation stick I edited the YAML file in Azure for the nginx-controller service and saved it. If you redeploy it will be lost of course. But it will not be overwritten every day I have found

annotations:
meta.helm.sh/release-name: nginx-ingress
meta.helm.sh/release-namespace: default
service.beta.kubernetes.io/azure-load-balancer-health-probe-request-path: /healthz

image

@zioproto
Copy link

@LukeTouchFoundry @CHDR-Nigel could you folks please share the exact name and version of the Helm chart you are deploying ?

I can't reproduce with the code shared by @LukeTouchFoundry :

  chart     = "${local.module_name}/${local.module_name}"
  name      = local.module_name
  version   = local.chart_version
  namespace = local.module_name

because I dont have the local values

@CHDR-Nigel
Copy link

CHDR-Nigel commented Oct 10, 2023

This is from my test environment...it's chart version 4.1.0 on Azure K8s 1.27.3

image

Go to the yaml and paste the healthz in and save it...

image

Other than that maybe try the Bitnami chart or Radar version 4.8 (latest)- that is my next move

@CHDR-Nigel
Copy link

CHDR-Nigel commented Oct 10, 2023

There is some info in here...
https://artifacthub.io/packages/helm/ingress-nginx/ingress-nginx

So what I have done is add this to my values file for nginx and it works. The /healthz annotation is deployed...

image

@zioproto
Copy link

@CHDR-Nigel can you please edit and fix the markdown link in your command above?
the link https://artifacthub.io/packages/helm/ingress-nginx/ingress-nginx when you click points to https://github.com/Azure/AKS/issues/url
thanks

@CHDR-Nigel
Copy link

Apologies - I don't post much - Should be fixed now.

@LukeTouchFoundry
Copy link

@LukeTouchFoundry @CHDR-Nigel could you folks please share the exact name and version of the Helm chart you are deploying ?

I can't reproduce with the code shared by @LukeTouchFoundry :

  chart     = "${local.module_name}/${local.module_name}"
  name      = local.module_name
  version   = local.chart_version
  namespace = local.module_name

because I dont have the local values

@zioproto

Here are my locals, sorry I should have provided these for context.

locals {
  module_name         = "ingress-nginx"
  repo_url            = "https://kubernetes.github.io/ingress-nginx"
  chart_version       = "4.8.0"
  helm_tasks_executed = fileexists("${path.module}/helm_tasks_executed.sentinel") ? "1" : "0" # ignore this bad boy
}

@LukeTouchFoundry
Copy link

I've figured out my issue. The way I was passing the Helm values with set {} was incorrect. It was translating to the following annotations:

image

Modifying the way I am setting the helm value is what has fixed it for me.

set {
    name  = "controller.service.annotations.service\\.beta\\.kubernetes\\.io/azure-load-balancer-health-probe-request-path"
    value = "/healthz"
  }

Note the \\.. I feel like a dumbass having not figured out to go and look at the ingress' service on the Azure portal, and to look at the annotations that actually got applied. @CHDR-Nigel your screenshots aided me to figuring out my issue thanks.

Thank you @zioproto for your time <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants