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

ingress-nginx metrics disabled when updating to k8s v1.30.3 #3690

Closed
bitfisher opened this issue Sep 12, 2024 · 3 comments · Fixed by rancher/kontainer-driver-metadata#1506
Closed
Assignees

Comments

@bitfisher
Copy link

RKE version:
v1.6.1

Type/provider of hosts: (VirtualBox/Bare-metal/AWS/GCE/DO)
Bare-metal

cluster.yml file:

...
ingress:
  provider: nginx
  default_backend: false
  nginx_ingress_controller_priority_class_name: system-cluster-critical
  options:
    load-balance: ewma
    enable-vts-status: "true"
    use-forwarded-headers: "true"
  tolerations:
    - key: "node.kubernetes.io/unreachable"
      operator: "Exists"
      effect: "NoExecute"
      tolerationseconds: 300
    - key: "node.kubernetes.io/not-ready"
      operator: "Exists"
      effect: "NoExecute"
      tolerationseconds: 300
...

Steps to Reproduce:
update k8s from v1.29.7 to v1.30.3

Results:
metrics are disabled in ds nginx-ingress-controller

...
      containers:
      - args:
        - /nginx-ingress-controller
        - --election-id=ingress-controller-leader-nginx
        - --controller-class=k8s.io/ingress-nginx
        - --configmap=$(POD_NAMESPACE)/ingress-nginx-controller
        - --tcp-services-configmap=$(POD_NAMESPACE)/tcp-services
        - --udp-services-configmap=$(POD_NAMESPACE)/udp-services
        - --validating-webhook=:8443
        - --validating-webhook-certificate=/usr/local/certificates/cert
        - --validating-webhook-key=/usr/local/certificates/key
        - --enable-metrics=false
        - --watch-ingress-without-class=true
...

before the update metrics didn't get disabled explicitly - enabled by default as expected

...
      containers:
      - args:
        - /nginx-ingress-controller
        - --election-id=ingress-controller-leader-nginx
        - --controller-class=k8s.io/ingress-nginx
        - --configmap=$(POD_NAMESPACE)/ingress-nginx-controller
        - --tcp-services-configmap=$(POD_NAMESPACE)/tcp-services
        - --udp-services-configmap=$(POD_NAMESPACE)/udp-services
        - --validating-webhook=:8443
        - --validating-webhook-certificate=/usr/local/certificates/cert
        - --validating-webhook-key=/usr/local/certificates/key
        - --watch-ingress-without-class=true
...

Workaround:
add enable-metrics: true to extra_args

...
ingress:
  provider: nginx
  default_backend: false
  nginx_ingress_controller_priority_class_name: system-cluster-critical
  options:
    load-balance: ewma
    enable-vts-status: "true"
    use-forwarded-headers: "true"
  extra_args:
    enable-metrics: "true"
  tolerations:
    - key: "node.kubernetes.io/unreachable"
      operator: "Exists"
      effect: "NoExecute"
      tolerationseconds: 300
    - key: "node.kubernetes.io/not-ready"
      operator: "Exists"
      effect: "NoExecute"
      tolerationseconds: 300
...

this results in duplication of --enable-metrics param. not nice, but is working for now.

...
      containers:
      - args:
        - /nginx-ingress-controller
        - --election-id=ingress-controller-leader-nginx
        - --controller-class=k8s.io/ingress-nginx
        - --configmap=$(POD_NAMESPACE)/ingress-nginx-controller
        - --tcp-services-configmap=$(POD_NAMESPACE)/tcp-services
        - --udp-services-configmap=$(POD_NAMESPACE)/udp-services
        - --validating-webhook=:8443
        - --validating-webhook-certificate=/usr/local/certificates/cert
        - --validating-webhook-key=/usr/local/certificates/key
        - --enable-metrics=false
        - --enable-metrics=true
        - --watch-ingress-without-class=true
...
@mitulshah-suse
Copy link
Contributor

Raised an upstream issue, since their chart is disabling it now by default, but the controller code is enabling it
Will take a call on how to proceed based on their reply.
kubernetes/ingress-nginx#12023

@mitulshah-suse
Copy link
Contributor

Quick update based on the upstream issue that was raised.. metrics are now disabled by default in the controller and the chart upstream.. so whenever its adopted in rke, metrics will be disabled by default in rke as well..
kubernetes/ingress-nginx#12023
kubernetes/ingress-nginx#12095

For the time being we are removing the flag and by default the controller will enable the metrics like it used to earlier, to avoid having duplicate flags..

@vivek-shilimkar
Copy link

The issue was tested on rancher v2.9-head.
The flag is not set on the container anymore with new fresh installed k8s version. Also after upgrade to new k8s version the flag is getting removed. Hence the issue can be closed.

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

Successfully merging a pull request may close this issue.

4 participants