kubernetes / ingress-nginx

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

Not all metrics are deleted after removing ingress object from kubernetes cluster #10825

Open dkhachyan opened 9 months ago

dkhachyan commented 9 months ago

What happened:

After creating ingress object added host metrics with status="404" and with empty ingress and namespace labels. After removing this ingress object such metrics not being deleted

What you expected to happen:

Ingress controller ignore metric event from new host until nginx config reload complete.

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

Kubernetes version (use kubectl version): v1.27.5

Environment:

How to reproduce this issue:

Anything else we need to know:

In very large environments during nginx config reload process ingress controller exposes metrics with status="404" label and empty namespace="" and ingress="" labels what causes that such metrics cat not be deleted.

k8s-ci-robot commented 9 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/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 9 months ago

the timeseries data is retained but new data is not appended. so does not seem like a bug. wait for other comments.

/remove-kind bug

dkhachyan commented 9 months ago

Yes the new data is not appended but this metric will remain after deleting ingress object because of empty ingress and namespace labels

https://github.com/kubernetes/ingress-nginx/blob/1bc745619d91b690c8985bbc16097e9fe804d2d2/internal/ingress/metric/collectors/socket.go#L501

I think the problem is here https://github.com/kubernetes/ingress-nginx/blob/1bc745619d91b690c8985bbc16097e9fe804d2d2/internal/ingress/controller/controller.go#L187

host list was updated before the new host was added to the nginx config

May be I should add something like this https://github.com/kubernetes/ingress-nginx/blob/1bc745619d91b690c8985bbc16097e9fe804d2d2/internal/ingress/metric/collectors/socket.go#L327

if sc.metricsPerHost {
    if !sc.hosts.Has(stats.Host) {
        klog.V(3).InfoS("Skipping metric for host not being served", "host", stats.Host)
        continue
    }
    if stats.Host != "" {
        if stats.Namespace == "" || stats.Ingress == "" {
            continue
        }
}
github-actions[bot] commented 8 months 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.