search

kubernetes / ingress-nginx

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

Incorrect service name emitted in some metrics #10210

Open adrianmoisey opened 1 year ago

adrianmoisey commented 1 year ago

What happened:

When defining two backends, as such:

     - backend:
          service:
            name: service-a
            port:
              number: 80
        path: /
        pathType: Exact
      - backend:
          service:
            name: service-b
            port:
              number: 80
        path: /
        pathType: Prefix

We only get metrics for the service-a backend.

What you expected to happen:

To be able to get metrics for each backend.

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

NGINX Ingress controller
  Release:       v1.3.0
  Build:         2b7b74854d90ad9b4b96a5011b9e8b67d20bfb8f
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.19.10

Kubernetes version (use kubectl version):

Server Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.8", GitCommit:"0ce7342c984110dfc93657d64df5dc3b2c0d1fe9", GitTreeState:"clean", BuildDate:"2023-03-15T13:33:02Z", GoVersion:"go1.19.7", Compiler:"gc", Platform:"linux/amd64"}

Environment:

How to reproduce this issue: Create an ingress object with multiple / paths, of different types. Send traffic to both matches / and /test. Look at resulting metrics.

Anything else we need to know:

I looked at the resulting configuration and compared them to see if I can find the issue. This seems to highlight the bug:

$ diff -u a b
--- a   2023-07-13 10:56:34
+++ b   2023-07-13 10:56:42
@@ -1,4 +1,4 @@
-       location / {
+       location = / {

            set $namespace      "mynamespace";
            set $ingress_name   "myingress";
@@ -46,7 +46,7 @@
            port_in_redirect off;

            set $balancer_ewma_score -1;
-           set $proxy_upstream_name "mynamespace-service-a-80";
+           set $proxy_upstream_name "mynamespace-service-b-80";
            set $proxy_host          $proxy_upstream_name;
            set $pass_access_scheme  $scheme;

This config is fine, except that set $service_name is the same in both.

It seems that the problem is happening around here:

Which I think comes from https://github.com/kubernetes/ingress-nginx/blob/controller-v1.3.0/internal/ingress/controller/template/template.go#L1061-L1149

I think the expected diff is as such:

$ diff -u a b
--- a   2023-07-18 13:13:39
+++ b   2023-07-13 10:56:42
@@ -1,8 +1,8 @@
-       location / {
+       location = / {

            set $namespace      "mynamespace";
            set $ingress_name   "myingress";
-           set $service_name   "service-a";
+           set $service_name   "service-b";
            set $service_port   "80";
            set $location_path  "/";
            set $global_rate_limit_exceeding n;
@@ -46,7 +46,7 @@
            port_in_redirect off;

            set $balancer_ewma_score -1;
-           set $proxy_upstream_name "mynamespace-service-a-80";
+           set $proxy_upstream_name "mynamespace-service-b-80";
            set $proxy_host          $proxy_upstream_name;
            set $pass_access_scheme  $scheme;
k8s-ci-robot commented 1 year 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/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 1 year ago

/remove-kind bug

Does this seem like your use case https://kubernetes.github.io/ingress-nginx/user-guide/monitoring/#wildcard-ingresses

adrianmoisey commented 1 year ago

I don't think so. We do use the --metrics-per-host=false option The particular tag we're worried about is service, which is the backend service that is bring routed to, not the host

strongjz commented 1 year ago

Can you test with /test as a specific path and see if the issue is the same? There may be an issue with pathType and the same path, /, the controller may not be differianating them.

Also can you try this in the latest relase, 1.8.1?

github-actions[bot] commented 1 year ago

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

adrianmoisey commented 5 months ago

Can you test with /test as a specific path and see if the issue is the same? There may be an issue with pathType and the same path, /, the controller may not be differianating them.

Adding a /test path that is an exact match works as expected. I think the problem is exactly as you say, the controller isn't differentiating between them.

Also can you try this in the latest relase, 1.8.1?

I've tested with 1.10.0 and still get this issue

adrianmoisey commented 5 months ago

I think I understand what is happening here. I plan to submit a PR if that is fine with you

longwuyuan commented 4 weeks ago

/assign