redpanda-data / helm-charts

Redpanda Helm Chart
http://redpanda.com
Apache License 2.0
75 stars 96 forks source link

Add: appProtocol as an option in listeners admin configurations #1472

Closed lemonprogis closed 1 month ago

lemonprogis commented 1 month ago

Hello 👋

When exposing the admin API using Istio as this port isn't made available since it's not an http default name. image

When I edit the service and add appProtocol: http the port becomes available with in istio. image now it's available. image

Since the listeners section is configurable, I added an additional optional key under listeners.admin, but gives the ability to allow instrumentation to pickup that it's not just raw tcp per K8s Service documentation

Istio Docs around automatic detection

CLAassistant commented 1 month ago

CLA assistant check
All committers have signed the CLA.

CLAassistant commented 1 month ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

RafalKorepta commented 1 month ago

Could you run the following command?

nix develop -c task ci:lint
lemonprogis commented 1 month ago

@chrisseto I'm unsure why those tests are failing, any help would be appreciated thanks!

chrisseto commented 1 month ago

The chart testing failures are due to docker's rate limits. We generally re-run those until they succeed (or get a new runner with a new IP really)

The go tests are due to changes in the chart's output. appProtocol is now included as null in the output due to a quick in gotohelm, which is technically acceptable.

To update the golden files run nix develop -c go test ./charts/redpanda -short -update and commit the changes. Rafal or I can push the changes in if you'd prefer.

lemonprogis commented 1 month ago

Ok cool, thanks for the offer, but I just pushed the change!

RafalKorepta commented 1 month ago

Due to unexpected quick release of Redpanda v24.2.2 I merged changes to template-cases.golden.txtar. Could you rebase and re-run nix develop -c go test ./charts/redpanda -short -update and push changes?

RafalKorepta commented 1 month ago

I couldn't push changes to your fork. Thank you for the contribution. It will be released soon.