Skip to content

Conversation

@yellowhat
Copy link
Contributor

What this PR does / why we need it

The MR broke the deployment with kubeRBACProxy.enabled=true, as *Probes:

livenessProbe:
  httpGet:
    ...
    port: http
...
readinessProbe:
  httpGet:
    port: metrics
....

reference a port name that it is not defined for that container.

As alternative we could define:

        - name: kube-state-metrics
          args:
            - --host=127.0.0.1
            - --port=9090
            - --resources=certificatesigningrequests,cronjobs,daemonsets,deployments,endpoints,horizontalpodautoscalers,ingresses,jobs,leases,limitranges,mutatingwebhookconfigurations,namespaces,networkpolicies,nodes,persistentvolumeclaims,persistentvolumes,poddisruptionbudgets,pods,replicasets,replicationcontrollers,resourcequotas,services,statefulsets,storageclasses,validatingwebhookconfigurations,volumeattachments
            - --metric-labels-allowlist=pods=[app,component,environment],deployments=[app,component,environment],daemonsets=[app,component,environment],jobs=[app,component,environment],services=[app,component,environment],statefulsets=[app,component,environment]
            - --metric-annotations-allowlist=pods=[alfaview.com/projectPath],deployments=[alfaview.com/projectPath],daemonsets=[alfaview.com/projectPath],jobs=[alfaview.com/projectPath],services=[alfaview.com/projectPath],statefulsets=[alfaview.com/projectPath]
            - --telemetry-host=127.0.0.1
            - --telemetry-port=9091
          imagePullPolicy: IfNotPresent
          image: registry.k8s.io/kube-state-metrics/kube-state-metrics:v2.17.0
+         ports:
+           - containerPort: 8080
+             name: http
+           - containerPort: 8081
+             name: metrics

even if kubeRBACProxy.enabled=true but we would point to the ports of another container.

I would not do that:

  • Due to warning of duplicated port:

    Warning: spec.template.spec.containers[1].ports[0]: duplicate port definition with spec.template.spec.containers[0].ports[0]
    Warning: spec.template.spec.containers[2].ports[0]: duplicate port definition with spec.template.spec.containers[0].ports[1]
    
  • We are checking the same port 2 times (generating more request [I know they are small]) without getting anything

  • Confusing when reading the manifest (reference the port of another container?)

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

  • fixes broken MR when kubeRBACProxy.enabled=true

Special notes for your reviewer

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

@yellowhat yellowhat force-pushed the main branch 2 times, most recently from 7a132ae to 5d315a0 Compare September 4, 2025 06:15
@HaveFun83
Copy link
Contributor

@yellowhat thanks for the PR. We also run into this issue.

@HaveFun83
Copy link
Contributor

But probes are a useful mechanism for application health
Looks like is haunting me #5858

@yellowhat
Copy link
Contributor Author

Before the change of the port name, it was using the port number from another container, so it is the same

@HaveFun83
Copy link
Contributor

but it was working and before the change there where no ports definition for the kube-state-metrics container

@yellowhat
Copy link
Contributor Author

Exactly because it was using the port number from the other container

@HaveFun83
Copy link
Contributor

HaveFun83 commented Sep 5, 2025

maybe we should focus on the native auth-filter #5858 (comment)

and remove the kube-rbac-proxy containers

@yellowhat
Copy link
Contributor Author

If someone wants to take look do it, I just need a solution now.

@HaveFun83
Copy link
Contributor

HaveFun83 commented Sep 5, 2025

rollback to a previous release and set the image tag to 2.17.0 is not an option?

@yellowhat
Copy link
Contributor Author

But now whoever uses kube-state-metrics with kubeRBACProxy is blocked.

I would suggest to roll forward.

@HaveFun83
Copy link
Contributor

but unfortunately i can't help you here. Someone from the reviewers need to merge this

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants